Paths

Table of Contentst

-
head/lib/libc/gen/
-
lib/
-
libc/
-
gen/
-
tls.c

Rework alignment handling in __libc_allocate_tls() for Variant I of TLS layout.
ClosedPublic
Actions

Authored by mmel on Dec 5 2017, 4:46 PM.

Details

Reviewers

kib
brooks
mizhka
jhibbits
andrew
br

Commits

rS326794: Rework alignment handling in __libc_allocate_tls() for Variant I of TLS layout.

Summary

Rework alignment handling in __libc_allocate_tls() for Variant I of TLS layout.

There are two versions of variant I of TLS

ARM and aarch64 uses original version of variant I here TP points to start of TCB followed by aligned TLS segment. Both TCB and TLS must be aligned to alignment of TLS section. The TCB[0] points to DTV vector and DTV values are real addresses (without bias).

MIPS, PowerPC and RISC-V uses modified, incomatible version of variant I, where TP points (with bias) to TLS and TCB immediately precedes TLS without any alignment gap[4]. Only TLS should be aligned. The TCB[0] points to DTV vector and DTV values are biased by constant value (0x8000) from real addresses.

Take this in account when allocating memory for TLS structures.

MFC after: 1 month

There are at least one oustanding issue: I cannot find any evidence
if a DTV vector is used in Local Exec TLS Model and if so, whether
is expected that it contains biased TLS values (addresses).

Test Plan

I made a simple test, so it would be nice if you can run it on the
affected platforms: https://people.freebsd.org/~mmel/test_tls.tgz
Thanks.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

mmel created this revision.Dec 5 2017, 4:46 PM

Herald added a subscriber: imp. · View Herald TranscriptDec 5 2017, 4:46 PM

mizhka mentioned this in D13134: [mips32/tls] change TCB size from 8 to 16 to be aligned with r324938 & r325364.Dec 5 2017, 7:19 PM

landonf added a subscriber: landonf.Dec 5 2017, 7:43 PM

MIPS is fine:

[RT-N53:/tmp]$ ./test_tls 
OK.

Tested on Asus RT-N53 with jemalloc correction. jemalloc issue (incorrect debug assertion) has been discussed with David Goldblatt in https://gitter.im/jemalloc/jemalloc. It's accepted as bug to be fixed before 5.1 planned for early next year.

This revision is now accepted and ready to land.Dec 5 2017, 9:21 PM

I did not found anything obviously wrong (i.e. an easy to see contradiction between the comment and code).

lib/libc/gen/tls.c
143 ↗	(On Diff #36245)	Perhaps commit the malloc_aligned() changes separately. Am I right that this is needed only for Cheri ?
172 ↗	(On Diff #36245)	use
205 ↗	(On Diff #36245)	Why int and not size_t ? Also, order the variables in declaration alphabetically.
207 ↗	(On Diff #36245)	fragments
240 ↗	(On Diff #36245)	the following
261 ↗	(On Diff #36245)	Same note for the type and ordering.
269 ↗	(On Diff #36245)	fragments
287 ↗	(On Diff #36245)	Space between Elf_addr and '**'.

This code generally seem like an improvement for code where there are hidden alignment requirements in the binary, but it isn't complete. If you do something like:

extern __thread char array_16k[16384];
__attribute__((__aligned__(16384)))
__thread char array_16k[16384];

Then it will only reliably be aligned to 8k. On at least mips64 this will be a regression.

Kib,
thanks. I will incorporate all your comments into final commit.

Brooks,
I'm sorry for my ignorance, but why? I don't see single reason for this. Can you be more verbose, please.

Closed by commit rS326794: Rework alignment handling in __libc_allocate_tls() for Variant I of TLS layout. (authored by mmel). · Explain WhyDec 12 2017, 11:25 AM

This revision was automatically updated to reflect the committed changes.

First the overall alignment is computed:

maxalign = MAX(tcbalign, tls_init_align);

and then tls_block_size is computed with all the assorted padding and without relation to tcbalign.
The whole allocation is then aligned to maxalign with:

tls_block = malloc_aligned(tls_block_size, maxalign);

However, if tcbalign is > tls_block_size the TLS segment will be aligned to tls_block_size not maxalign. As a practical matter I'm not sure this can happen, but this code is confusing enough that I really don't want to have to read through multiple codepaths and traverser weak symbols to convince myself it won't. We should either honor all reasonable values for tcbalign is document our assumptions about them clearly.

Revision Contents
Changeset List

Path

Size

head/

lib/

libc/

gen/

tls.c

138 lines

Diff 36499

View Options

head/lib/libc/gen/tls.c

Show First 20 Lines • Show All 83 Lines • ▼ Show 20 Lines
#if defined(__aarch64__) \|\| defined(__arm__) \|\| defined(__mips__) \|\| \		#if defined(__aarch64__) \|\| defined(__arm__) \|\| defined(__mips__) \|\| \
defined(__powerpc__) \|\| defined(__riscv)		defined(__powerpc__) \|\| defined(__riscv)
#define TLS_VARIANT_I		#define TLS_VARIANT_I
#endif		#endif
#if defined(__i386__) \|\| defined(__amd64__) \|\| defined(__sparc64__)		#if defined(__i386__) \|\| defined(__amd64__) \|\| defined(__sparc64__)
#define TLS_VARIANT_II		#define TLS_VARIANT_II
#endif		#endif

		#if defined(__mips__) \|\| defined(__powerpc__) \|\| defined(__riscv)
		#define DTV_OFFSET 0x8000
		#else
		#define DTV_OFFSET 0
		#endif

#ifndef PIC		#ifndef PIC

static size_t tls_static_space;		static size_t tls_static_space;
static size_t tls_init_size;		static size_t tls_init_size;
static size_t tls_init_align;		static size_t tls_init_align;
static void *tls_init;		static void *tls_init;
#endif		#endif

▲ Show 20 Lines • Show All 44 Lines • ▼ Show 20 Lines	free_aligned(void *ptr)
x = (uintptr_t)ptr;		x = (uintptr_t)ptr;
x -= sizeof(void *);		x -= sizeof(void *);
mem = (void *)x;		mem = (void *)x;
__je_bootstrap_free(mem);		__je_bootstrap_free(mem);
}		}

#ifdef TLS_VARIANT_I		#ifdef TLS_VARIANT_I

		/*
		* There are two versions of variant I of TLS
		*
		* - ARM and aarch64 uses original variant I as is described in [1] and [2],
		* where TP points to start of TCB followed by aligned TLS segment.
		* Both TCB and TLS must be aligned to alignment of TLS section. The TCB[0]
		* points to DTV vector and DTV values are real addresses (without bias).
		* Note: for Local Exec TLS Model, the offsets from TP (TCB in this case) to
		* TLS variables are computed by linker, so we cannot overalign TLS section.
		*
		* - MIPS, PowerPC and RISC-V use modified version of variant I,
		* described in [3] where TP points (with bias) to TLS and TCB immediately
		* precedes TLS without any alignment gap[4]. Only TLS should be aligned.
		* The TCB[0] points to DTV vector and DTV values are biased by constant
		* value (0x8000) from real addresses[5].
		*
		* [1] Ulrich Drepper: ELF Handling for Thread-Local Storage
		* www.akkadia.org/drepper/tls.pdf
		*
		* [2] ARM IHI 0045E: Addenda to, and Errata in, the ABI for the ARM(r)
		* Architecture
		* infocenter.arm.com/help/topic/com.arm.doc.ihi0045e/IHI0045E_ABI_addenda.pdf
		*
		* [3] OpenPOWER: Power Architecture 64-Bit ELF V2 ABI Specification
		* https://members.openpowerfoundation.org/document/dl/576
		*
		* [4] Its unclear if "without any alignment gap" is hard ABI requirement,
		* but we must follow this rule due to suboptimal _set_tp()
		* (aka <ARCH>_SET_TP) implementation. This function doesn't expect TP but
		* TCB as argument.
		*
		* [5] I'm not able to validate "values are biased" assertions.
		*/

#define TLS_TCB_SIZE (2 * sizeof(void *))		#define TLS_TCB_SIZE (2 * sizeof(void *))

/*		/*
* Free Static TLS using the Variant I method.		* Return pointer to allocated TLS block
*/		*/
		static void *
		get_tls_block_ptr(void *tcb, size_t tcbsize)
		{
		size_t extra_size, post_size, pre_size, tls_block_size;

		/* Compute fragments sizes. */
		extra_size = tcbsize - TLS_TCB_SIZE;
		#if defined(__aarch64__) \|\| defined(__arm__)
		post_size = roundup2(TLS_TCB_SIZE, tls_init_align) - TLS_TCB_SIZE;
		#else
		post_size = 0;
		#endif
		tls_block_size = tcbsize + post_size;
		pre_size = roundup2(tls_block_size, tls_init_align) - tls_block_size;

		return ((char *)tcb - pre_size - extra_size);
		}

		/*
		* Free Static TLS using the Variant I method. The tcbsize
		* and tcbalign parameters must be the same as those used to allocate
		* the block.
		*/
void		void
__libc_free_tls(void *tcb, size_t tcbsize __unused, size_t tcbalign __unused)		__libc_free_tls(void *tcb, size_t tcbsize, size_t tcbalign __unused)
{		{
Elf_Addr *dtv;		Elf_Addr *dtv;
Elf_Addr **tls;		Elf_Addr **tls;

tls = (Elf_Addr **)tcb;		tls = (Elf_Addr **)tcb;
dtv = tls[0];		dtv = tls[0];
__je_bootstrap_free(dtv);		__je_bootstrap_free(dtv);
free_aligned(tls);		free_aligned(get_tls_block_ptr(tcb, tcbsize));
}		}

/*		/*
* Allocate Static TLS using the Variant I method.		* Allocate Static TLS using the Variant I method.
		*
		* To handle all above requirements, we setup the following layout for
		* TLS block:
		* (whole memory block is aligned with MAX(TLS_TCB_ALIGN, tls_init_align))
		*
		* +----------+--------------+--------------+-----------+------------------+
		* \| pre gap \| extended TCB \| TCB \| post gap \| TLS segment \|
		* \| pre_size \| extra_size \| TLS_TCB_SIZE \| post_size \| tls_static_space \|
		* +----------+--------------+--------------+-----------+------------------+
		*
		* where:
		* extra_size is tcbsize - TLS_TCB_SIZE
		* post_size is used to adjust TCB to TLS aligment for first version of TLS
		* layout and is always 0 for second version.
		* pre_size is used to adjust TCB aligment for first version and to adjust
		* TLS alignment for second version.
		*
*/		*/
void *		void *
__libc_allocate_tls(void *oldtcb, size_t tcbsize, size_t tcbalign)		__libc_allocate_tls(void *oldtcb, size_t tcbsize, size_t tcbalign)
{		{
Elf_Addr *dtv;		Elf_Addr dtv, *tcb;
Elf_Addr **tls;		char tls_block, tls;
		size_t extra_size, maxalign, post_size, pre_size, tls_block_size;

if (oldtcb != NULL && tcbsize == TLS_TCB_SIZE)		if (oldtcb != NULL && tcbsize == TLS_TCB_SIZE)
return (oldtcb);		return (oldtcb);

tls_assert(tcbalign >= TLS_TCB_ALIGN);		tls_assert(tcbalign >= TLS_TCB_ALIGN);
tls_assert(tcbsize == TLS_TCB_SIZE);		maxalign = MAX(tcbalign, tls_init_align);

tcbsize = roundup2(tcbsize, tcbalign);		/* Compute fragmets sizes. */
tls = malloc_aligned(tcbsize + tls_static_space, tcbalign);		extra_size = tcbsize - TLS_TCB_SIZE;
if (tls == NULL) {		#if defined(__aarch64__) \|\| defined(__arm__)
		post_size = roundup2(TLS_TCB_SIZE, tls_init_align) - TLS_TCB_SIZE;
		#else
		post_size = 0;
		#endif
		tls_block_size = tcbsize + post_size;
		pre_size = roundup2(tls_block_size, tls_init_align) - tls_block_size;
		tls_block_size += pre_size + tls_static_space;

		/* Allocate whole TLS block */
		tls_block = malloc_aligned(tls_block_size, maxalign);
		if (tls_block == NULL) {
tls_msg("__libc_allocate_tls: Out of memory.\n");		tls_msg("__libc_allocate_tls: Out of memory.\n");
abort();		abort();
}		}
memset(tls, 0, tcbsize + tls_static_space);		memset(tls_block, 0, tls_block_size);
		tcb = (Elf_Addr **)(tls_block + pre_size + extra_size);
		tls = (char *)tcb + TLS_TCB_SIZE + post_size;

if (oldtcb != NULL) {		if (oldtcb != NULL) {
memcpy(tls, oldtcb, tcbsize + tls_static_space);		memcpy(tls_block, get_tls_block_ptr(oldtcb, tcbsize),
__je_bootstrap_free(oldtcb);		tls_block_size);
		free_aligned(oldtcb);

/* Adjust the DTV. */		/* Adjust the DTV. */
dtv = tls[0];		dtv = tcb[0];
dtv[2] = (Elf_Addr)tls + tcbsize;		dtv[2] = (Elf_Addr)(tls + DTV_OFFSET);
} else {		} else {
dtv = __je_bootstrap_malloc(3 * sizeof(Elf_Addr));		dtv = __je_bootstrap_malloc(3 * sizeof(Elf_Addr));
if (dtv == NULL) {		if (dtv == NULL) {
tls_msg("__libc_allocate_tls: Out of memory.\n");		tls_msg("__libc_allocate_tls: Out of memory.\n");
abort();		abort();
}		}
tls[0] = dtv;		/* Build the DTV. */
		tcb[0] = dtv;
dtv[0] = 1; /* Generation. */		dtv[0] = 1; /* Generation. */
dtv[1] = 1; /* Segments count. */		dtv[1] = 1; /* Segments count. */
dtv[2] = (Elf_Addr)tls + tcbsize;		dtv[2] = (Elf_Addr)(tls + DTV_OFFSET);

if (tls_init_size > 0)		if (tls_init_size > 0)
memcpy((void*)dtv[2], tls_init, tls_init_size);		memcpy(tls, tls_init, tls_init_size);
}		}

return (tls);		return (tcb);
}		}

#endif		#endif

#ifdef TLS_VARIANT_II		#ifdef TLS_VARIANT_II

#define TLS_TCB_SIZE (3 * sizeof(Elf_Addr))		#define TLS_TCB_SIZE (3 * sizeof(Elf_Addr))

/*		/*
* Free Static TLS using the Variant II method.		* Free Static TLS using the Variant II method.
*/		*/
void		void
__libc_free_tls(void *tcb, size_t tcbsize __unused, size_t tcbalign)		__libc_free_tls(void *tcb, size_t tcbsize __unused, size_t tcbalign)
{		{
size_t size;		size_t size;
Elf_Addr* dtv;		Elf_Addr* dtv;
Elf_Addr tlsstart, tlsend;		Elf_Addr tlsstart, tlsend;

/*		/*
* Figure out the size of the initial TLS block so that we can		* Figure out the size of the initial TLS block so that we can
* find stuff which ___tls_get_addr() allocated dynamically.		* find stuff which ___tls_get_addr() allocated dynamically.
*/		*/
		tcbalign = MAX(tcbalign, tls_init_align);
size = roundup2(tls_static_space, tcbalign);		size = roundup2(tls_static_space, tcbalign);

dtv = ((Elf_Addr**)tcb)[1];		dtv = ((Elf_Addr**)tcb)[1];
tlsend = (Elf_Addr) tcb;		tlsend = (Elf_Addr) tcb;
tlsstart = tlsend - size;		tlsstart = tlsend - size;
free_aligned((void*)tlsstart);		free_aligned((void*)tlsstart);
__je_bootstrap_free(dtv);		__je_bootstrap_free(dtv);
}		}

/*		/*
* Allocate Static TLS using the Variant II method.		* Allocate Static TLS using the Variant II method.
*/		*/
void *		void *
__libc_allocate_tls(void *oldtls, size_t tcbsize, size_t tcbalign)		__libc_allocate_tls(void *oldtls, size_t tcbsize, size_t tcbalign)
{		{
size_t size;		size_t size;
char *tls;		char *tls;
Elf_Addr *dtv;		Elf_Addr *dtv;
Elf_Addr segbase, oldsegbase;		Elf_Addr segbase, oldsegbase;

		tcbalign = MAX(tcbalign, tls_init_align);
size = roundup2(tls_static_space, tcbalign);		size = roundup2(tls_static_space, tcbalign);

if (tcbsize < 2 * sizeof(Elf_Addr))		if (tcbsize < 2 * sizeof(Elf_Addr))
tcbsize = 2 * sizeof(Elf_Addr);		tcbsize = 2 * sizeof(Elf_Addr);
tls = malloc_aligned(size + tcbsize, tcbalign);		tls = malloc_aligned(size + tcbsize, tcbalign);
if (tls == NULL) {		if (tls == NULL) {
tls_msg("__libc_allocate_tls: Out of memory.\n");		tls_msg("__libc_allocate_tls: Out of memory.\n");
abort();		abort();
▲ Show 20 Lines • Show All 98 Lines • ▼ Show 20 Lines	if (phdr[i].p_type == PT_TLS) {
tls_static_space = roundup2(phdr[i].p_memsz,		tls_static_space = roundup2(phdr[i].p_memsz,
phdr[i].p_align);		phdr[i].p_align);
tls_init_size = phdr[i].p_filesz;		tls_init_size = phdr[i].p_filesz;
tls_init_align = phdr[i].p_align;		tls_init_align = phdr[i].p_align;
tls_init = (void*) phdr[i].p_vaddr;		tls_init = (void*) phdr[i].p_vaddr;
break;		break;
}		}
}		}
		tls = _rtld_allocate_tls(NULL, TLS_TCB_SIZE, TLS_TCB_ALIGN);
tls = _rtld_allocate_tls(NULL, TLS_TCB_SIZE,
MAX(TLS_TCB_ALIGN, tls_init_align));

_set_tp(tls);		_set_tp(tls);
#endif		#endif
}		}

Rework alignment handling in __libc_allocate_tls() for Variant I of TLS layout.ClosedPublicActions