Use modern @got syntax in tsan assembly, instead of old style non_lazy_ptr's. NFC

[pete]

Summary

For Apple platforms, the TSan assembly used the old style syntax to reference symbols via a non-lazy pointer slot.

Using the @gotpageoff and @gotpageoff relocations is a better way to achieve the same result, as the linker will syntheize a GOT as needed.

[kledzik]

compiler-rt/lib/tsan/rtl/tsan_rtl_aarch64.S

106–107

The original had two setjmp pointers. One to _setjmp and one to __setjmp. I don't know if that was intentional or needed, but this changes this case to the single underscore version.

[yln]

compiler-rt/lib/tsan/rtl/tsan_rtl_aarch64.S

106–107

Good catch, Nick!

Looks like this file defines 4 interceptors for:

• setjmp
• _setjmp
• sigsetjmp
• __sigsetjmp (using @page and @pageoff)

So I think it's definitely intentional (not sure about needed, but no reason to change it either?).

Is @page and @pageoff just a different spelling or different in semantics? Can we change all 4 occurrences to use the same spelling/mechanism for consistency?

[pete]

Updated to fix calling the correct setjmp

[yln]

compiler-rt/lib/tsan/rtl/tsan_rtl_aarch64.S

210–211

Can we also change this for consistency?

[pete]

compiler-rt/lib/tsan/rtl/tsan_rtl_aarch64.S

106–107

Good catch!

The use of ASM_SYMBOL was confusing me, as it was also adding an _. The original code didn't use it in this location, so i've removed it and just use the symbols directly.

106–107

Same semantics if you step through the code, ie, you'll see a load from a GOT. The only difference is that using the @got style syntax allows the linker to synthesize it for you, so it may choose to re-use a GOT slot for the same symbol, eg, if another function called the symbol too.

I wanted to change sigsetjmp too, but that code is actually unused on Apple. It has the #if around the use just like the others, but actually the whole ASM_SYMBOL_INTERCEPTOR(sigsetjmp) is disabled for Apple.

I can change it anyway, if you prefer, but i have no way to test it, other than perhaps trying to allow that function to build for Apple and make sure i get no assembler errors, or perhaps even that we succeed in linking the dylib. Let me know if you prefer i change it or not.

210–211

Happy to, if you like. Please see previous comments that this code is actually unused on Apple platforms right now.

[yln]

compiler-rt/lib/tsan/rtl/tsan_rtl_aarch64.S

210–211

Ah, I missed that we have a #if !defined(__APPLE__) around the whole thing. Removing it altogether or making it an #error (so we notice in case the enclosing !APPLE is removed) are alternatives then also. I leave it up to you; it's not something that we need cleanup here.

LGTM, thanks!

I asked @wrotki or @rsundahl for a quick local sanity check (build & run tests/example). Please wait for their OK before landing.

[wrotki]

LGTM. I verified using debugger and the longjmp3 Tsan test case that he setjmp Tsan interceptor is being correctly called with tis change.

[dvyukov]

kubamracek added a reviewer: dvyukov.

Do you want me to look at something in particular? Frankly, I don't understand the details here and can't meaningfully review.
If you think it's a good change and tests pass, I would say go on and merge.

[kubamracek]

In D143959#4128779, @dvyukov wrote:

kubamracek added a reviewer: dvyukov.

Do you want me to look at something in particular? Frankly, I don't understand the details here and can't meaningfully review.
If you think it's a good change and tests pass, I would say go on and merge.

That's all I wanted :) Thanks!

compiler-rt/lib/tsan/rtl/tsan_rtl_aarch64.S

 // The content of this file is AArch64-only:
 #if defined(__aarch64__)
 
 #include "sanitizer_common/sanitizer_asm.h"
 
-#if defined(__APPLE__)
-.align  2
-
-.section  __DATA,__nl_symbol_ptr,non_lazy_symbol_pointers
-.long _setjmp$non_lazy_ptr
-_setjmp$non_lazy_ptr:
-.indirect_symbol _setjmp
-.long 0
-
-.section  __DATA,__nl_symbol_ptr,non_lazy_symbol_pointers
-.long __setjmp$non_lazy_ptr
-__setjmp$non_lazy_ptr:
-.indirect_symbol __setjmp
-.long 0
-
-.section  __DATA,__nl_symbol_ptr,non_lazy_symbol_pointers
-.long _sigsetjmp$non_lazy_ptr
-_sigsetjmp$non_lazy_ptr:
-.indirect_symbol _sigsetjmp
-.long 0
-#endif
-
 #if !defined(__APPLE__)
 .section .text
 #else
 .section __TEXT,__text
 .align 3
 #endif
 
 ASM_HIDDEN(__tsan_setjmp)
ASM_SYMBOL_INTERCEPTOR(setjmp):
   CFI_DEF_CFA (31, 0)
 
   // tail jump to libc setjmp
 #if !defined(__APPLE__)
   adrp    x1, :got:_ZN14__interception11real_setjmpE
   ldr     x1, [x1, #:got_lo12:_ZN14__interception11real_setjmpE]
   ldr     x1, [x1]
 #else
-  adrp    x1, _setjmp$non_lazy_ptr@page
-  add     x1, x1, _setjmp$non_lazy_ptr@pageoff
-  ldr     x1, [x1]
+  adrp    x1, _setjmp@GOTPAGE
+  ldr     x1, [x1, _setjmp@GOTPAGEOFF]
 #endif
   br      x1
 
   CFI_ENDPROC
 ASM_SIZE(ASM_SYMBOL_INTERCEPTOR(setjmp))
 
 .comm _ZN14__interception12real__setjmpE,8,8
 .globl ASM_SYMBOL_INTERCEPTOR(_setjmp)
ASM_SYMBOL_INTERCEPTOR(_setjmp):
   CFI_DEF_CFA (31, 0)
 
   // tail jump to libc setjmp
 #if !defined(__APPLE__)
   adrp    x1, :got:_ZN14__interception12real__setjmpE
   ldr     x1, [x1, #:got_lo12:_ZN14__interception12real__setjmpE]
   ldr     x1, [x1]
 #else
-  adrp    x1, __setjmp$non_lazy_ptr@page
-  add     x1, x1, __setjmp$non_lazy_ptr@pageoff
-  ldr     x1, [x1]
+  adrp    x1, __setjmp@GOTPAGE
+  ldr     x1, [x1, __setjmp@GOTPAGEOFF]
 #endif
   br      x1
 
   CFI_ENDPROC
 ASM_SIZE(ASM_SYMBOL_INTERCEPTOR(_setjmp))
 
 .comm _ZN14__interception14real_sigsetjmpE,8,8
 .globl ASM_SYMBOL_INTERCEPTOR(sigsetjmp)
ASM_SYMBOL_INTERCEPTOR(sigsetjmp):
   CFI_DEF_CFA (31, 0)
 
   // tail jump to libc sigsetjmp
 #if !defined(__APPLE__)
   adrp    x2, :got:_ZN14__interception14real_sigsetjmpE
   ldr     x2, [x2, #:got_lo12:_ZN14__interception14real_sigsetjmpE]
   ldr     x2, [x2]
 #else
-  adrp    x2, _sigsetjmp$non_lazy_ptr@page
-  add     x2, x2, _sigsetjmp$non_lazy_ptr@pageoff
-  ldr     x2, [x2]
+  adrp    x2, _sigsetjmp@GOTPAGE
+  ldr     x2, [x2, _sigsetjmp@GOTPAGEOFF]
 #endif
   br      x2
   CFI_ENDPROC
 ASM_SIZE(ASM_SYMBOL_INTERCEPTOR(sigsetjmp))
 
 #if !defined(__APPLE__)
 .comm _ZN14__interception16real___sigsetjmpE,8,8
 .globl ASM_SYMBOL_INTERCEPTOR(__sigsetjmp)
ASM_SYMBOL_INTERCEPTOR(__sigsetjmp):
   CFI_DEF_CFA (31, 0)
 
   // tail jump to libc __sigsetjmp
 #if !defined(__APPLE__)
   adrp    x2, :got:_ZN14__interception16real___sigsetjmpE
   ldr     x2, [x2, #:got_lo12:_ZN14__interception16real___sigsetjmpE]
   ldr     x2, [x2]
 #else
   adrp    x2, ASM_SYMBOL(__sigsetjmp)@page
   add     x2, x2, ASM_SYMBOL(__sigsetjmp)@pageoff
 #endif
   br      x2
   CFI_ENDPROC
 ASM_SIZE(ASM_SYMBOL_INTERCEPTOR(__sigsetjmp))
 #endif
 
 NO_EXEC_STACK_DIRECTIVE
 
 #endif