svn commit: r338486 - in head/libexec/rtld-elf: . aarch64 arm mips powerpc powerpc64 riscv

[Andreas Tobler]

On 23.09.18 22:41, Andreas Tobler wrote:
> Hi Brooks,
> 
> On 06.09.18 01:23, Brooks Davis wrote:
>> Author: brooks
>> Date: Wed Sep  5 23:23:16 2018
>> New Revision: 338486
>> URL: https://svnweb.freebsd.org/changeset/base/338486
>>
>> Log:
>>     Rework rtld's TLS Variant I implementation to match r326794
>>     
>>     The above commit fixed handling overaligned TLS segments in libc's
>>     TLS Variant I implementation, but rtld provides its own implementation
>>     for dynamically-linked executables which lacks these fixes.  Thus,
>>     port these changes to rtld.
>>     
>>     This was previously commited as r337978 and reverted in r338149 due to
>>     exposing a bug the ARM rtld.  This bug was fixed in r338317 by mmel.
>>     
>>     Submitted by:	James Clarke
>>     Approved by:	re (kib)
>>     Reviewed by:	kbowling
>>     Testing by:	kbowling (powerpc64), br (riscv), kevans (armv7)
> 
> And no testing on powerpc?
> 
> This patch breaks ppc-32. It makes the system unusable, one can not even
> login. Neither on the console nor via ssh. After entering the user I get
> flooded with backslashes and the login shell dumps core. (Only visible
> if I netboot)
> 
> I'm testing now a fix and see if it survives world and kernel build.
> 
>> Modified: head/libexec/rtld-elf/powerpc/rtld_machdep.h
>> ==============================================================================
>> --- head/libexec/rtld-elf/powerpc/rtld_machdep.h	Wed Sep  5 21:47:22 2018	(r338485)
>> +++ head/libexec/rtld-elf/powerpc/rtld_machdep.h	Wed Sep  5 23:23:16 2018	(r338486)
>> @@ -74,10 +74,11 @@ void _rtld_powerpc_pltcall(void);
>>    #define round(size, align) \
>>        (((size) + (align) - 1) & ~((align) - 1))
>>    #define calculate_first_tls_offset(size, align) \
>> -    round(8, align)
>> +    TLS_TCB_SIZE
> 
> Here, if I revert to 'round(8, align)', I can login again. But the fix
> I'm testing now, is to increase the TLS_TCB_SIZE to 16 as suggested by
> Justin.
> 
> I'll let you know how it goes.

So, here the feedback.

Additional testing showed that increasing the TLS_TCB_SIZE is not 
enough. I have the below snippet which lets me build world/kernel and do 
some portmaster updates.

If I use TLS_TCB_SIZE in calculate_first_tls_offset I get a system where 
I can login, but when I try to run 'portmaster' with arguments I get 
'operator expected' on a option which works on other archs.

Putting in round(8, align) instead of TLS_TCB_SIZE makes portmaster work 
again.

And if I leave TLS_TCB_SIZE with 8, and only bring back round(8, align) 
is also not sufficient.

I rebuild a few things again and then I propose the below for -CURRENT 
to re at . Unless someone else has a better idea.

Regards,
Andreas

Index: libexec/rtld-elf/powerpc/rtld_machdep.h
===================================================================
--- libexec/rtld-elf/powerpc/rtld_machdep.h	(revision 338919)
+++ libexec/rtld-elf/powerpc/rtld_machdep.h	(working copy)
@@ -69,12 +69,12 @@

  #define TLS_TP_OFFSET	0x7000
  #define TLS_DTV_OFFSET	0x8000
-#define TLS_TCB_SIZE	8
+#define TLS_TCB_SIZE	16

  #define round(size, align) \
      (((size) + (align) - 1) & ~((align) - 1))
  #define calculate_first_tls_offset(size, align) \
-    TLS_TCB_SIZE
+    round(8, align)
  #define calculate_tls_offset(prev_offset, prev_size, size, align) \
      round(prev_offset + prev_size, align)
  #define calculate_tls_end(off, size)    ((off) + (size))