[PATCH v2 1/1] rtld: add glibc.rtld.optional_static_tls_alignment

[DJ Delorie]

Florian Weimer <fweimer@redhat.com> writes:
>> +  tst-tlsalign-tunable-a \
>
> We usually call those auditmods, so tst-tlsalign-tunable-auditmod.

Changed.

>> +tst-tlsalign-tunable-ENV = LD_AUDIT=$(objpfx)tst-tlsalign-tunable-a.so
>> +#tst-tlsalign-tunable-ENV = LD_DEBUG=all
>
> Leftover comment?

Yup.

>> +static size_t optional_static_tls_alignment = 0;
>
> Maybe add a comment where this value comes from?

Added.

>> +  optional_static_tls_alignment = TUNABLE_GET (optional_static_tls_alignment, size_t, NULL);
>
> I think this should ignore the value if it's not a power of two.

Done.  No warning messages?

>> +__thread int b[4] __attribute__((tls_model("initial-exec"))) __attribute__((aligned(0x1000)));
>
> This line is too long, and there are missing spaces before '('.  There
> are more such missing spaces below.

Fixed.

> Please also add the usual file header.

Added.

>> +/* These exist just to make the above variables "used".  */
>> +int *
>> +func_x(void)
>> +{
>> +  return &x;
>> +}
>
> I guess this is needed because static TLS allocation can in theory be
> reference-based.

I wanted a way to make sure the compiler didn't omit the tls variable,
but I didn't need it to *use* the tls variable.

>> +  return tdata1 ? EXIT_FAILURE : 43;
>> +}
>
> I think you need to make tdata1 volatile, otherwise this is still
> optimized out (because tdata1 is static and does not have its address
> taken).

Done.

>> +@end deftp
>
> It's not an alternative, both can be needed.  It's also not just about
> the auditor, but anything loaded in audit mode counts (whether by the
> auditor or anything else).

I made the audit case a "for example" and took out the last bit.  It's
hard to describe with simple language how it can be used to optimize
memory needs.