Changeset c880421 in mainline

Timestamp:

2025-01-25T21:22:04Z (4 weeks ago)

Author:

Matěj Volf <git@…>

Parents:

4ef27eb

Message:

fix comments based on improved understanding

Location:

uspace/lib/c/generic

Files:

• rtld/module.c (modified) (1 diff)
• thread/tls.c (modified) (2 diffs)

uspace/lib/c/generic/rtld/module.c

@@ -398 +398 @@
          * be correct, "zero" offset (i.e. the total size) must be aligned
          * to the strictest alignment present.
-         * Note that the padding is actually in front of the TLS data,
-         * not after it.
          */
         rtld->tls_size = ALIGN_UP(rtld->tls_size, rtld->tls_align);
 
-        /* Space for the TCB.
+        /*
+         * Space for the TCB.
          * Later, the TLS zero offset is equal to the pointer to tcb_t, so
          * adding the sizeof(tcb_t) block AFTER we calculated the alignment

uspace/lib/c/generic/thread/tls.c

@@ -104 +104 @@
         assert(tls_align <= PAGE_SIZE);
 
+        /*
+         * FIXME: the calculation of alloc_size shouldn't include the alignment
+         * of tcb_t (at least in Variant II)
+         * See https://github.com/HelenOS/helenos/pull/240/files/4ef27ebf98a0656e09889b7d00efdec03343f1aa#r1929592924
+         * (you will also need to fix _tcb_data_offset)
+         */
+
 #ifdef CONFIG_TLS_VARIANT_1
         size_t alloc_size =
@@ -132 +139 @@
         uintptr_t bias = elf_get_bias(elf);
 
-        /*
-         * FIXME: I couldn't convince myself this is correct, but I couldn't
-         * find a case where it breaks either: if the actual alloc_size is
-         * bigger than the requested size (tls->p_memsz), the alignment padding
-         * will be placed at the beginning (because TLS is (at least sometimes?)
-         * indexed with negative offsets from the TCB pointer).
-         *
-         * Now we will copy the initialization data to a position at the start of
-         * the allocation, so if the padding has nonzero size, if think the initialization
-         * data is now incorrectly offset by its size?
-         *
-         * Maybe a diagram helps explaining this?
-         * |    allocation   |     |
-         * | paddding | data | tcb |
-         *   ^
-         *   +--- we will copy the initialization data here
-         *              ^
-         *              +--- but the data should be actually here?
-         */
-
         /* Copy thread local data from the initialization image. */
         memcpy(data, (void *)(tls->p_vaddr + bias), tls->p_filesz);