The recent libsanitizer import broke bootstrap with the native Solaris/sparc as: /usr/ccs/bin/as: ".libs/sanitizer_errno.s", line 4247: error: symbol "__sanitizer_internal_memset" is used but not defined /usr/ccs/bin/as: ".libs/sanitizer_errno.s", line 4247: error: symbol "__sanitizer_internal_memcpy" is used but not defined /usr/ccs/bin/as: ".libs/sanitizer_errno.s", line 4247: error: symbol "__sanitizer_internal_memmove" is used but not defined make[4]: *** [Makefile:623: sanitizer_errno.lo] Error 1 The code in question is memcpy = __sanitizer_internal_memcpy memmove = __sanitizer_internal_memmove memset = __sanitizer_internal_memset which is from libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h. I'm currently continuing the build with that hack disabled on Solaris/sparc (it isn't easy to distinguish between as and gas toolchains here).
So, what is the Solaris/sparc native as syntax for something similar? Strangely, non-weak aliases in config/sparc/sysv4.h use the x = y syntax rather than .set x, y that is used on most targets. Would using extern "C" void *memcpy (void *, const void *, decltype (sizeof 0)) __asm ("__sanitizer_internal_memcpy"); extern "C" void *memmove (void *, const void *, decltype (sizeof 0)) __asm ("__sanitizer_internal_memmove"); extern "C" void *memset (void *, int, decltype (sizeof 0)) __asm ("__sanitizer_internal_memset"); extern "C" void *__builtin_memcpy (void *, const void *, decltype (sizeof 0)) __asm ("__sanitizer_internal_memcpy"); extern "C" void *__builtin_memmove (void *, const void *, decltype (sizeof 0)) __asm ("__sanitizer_internal_memmove"); extern "C" void *__builtin_memset (void *, int, decltype (sizeof 0)) __asm ("__sanitizer_internal_memset"); instead work there? At least on x86_64-linux, I get the same effects when using e.g. void foo (void *r, void *s, void *t, void *u) { memcpy (r, s, 512); __builtin_memcpy (t, u, 512); } with -O2 -mstringop-strategy=libcall
Note, following patch --- libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h.jj 2023-11-15 17:19:11.512783752 +0100 +++ libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h 2023-11-19 22:52:14.659301189 +0100 @@ -17,9 +17,24 @@ // The asm hack only works with GCC and Clang. # if !defined(_WIN32) +# if !defined(__GNUC__) || defined(__clang__) asm("memcpy = __sanitizer_internal_memcpy"); asm("memmove = __sanitizer_internal_memmove"); asm("memset = __sanitizer_internal_memset"); +# else +# if defined(__cplusplus) +extern "C" { +# endif +extern void *(memcpy) (void *, const void *, __SIZE_TYPE__) __asm ("__sanitizer_internal_memcpy"); +extern void *(memmove) (void *, const void *, __SIZE_TYPE__) __asm ("__sanitizer_internal_memmove"); +extern void *(memset) (void *, int, __SIZE_TYPE__) __asm ("__sanitizer_internal_memset"); +extern void *__builtin_memcpy (void *, const void *, __SIZE_TYPE__) __asm ("__sanitizer_internal_memcpy"); +extern void *__builtin_memmove (void *, const void *, __SIZE_TYPE__) __asm ("__sanitizer_internal_memmove"); +extern void *__builtin_memset (void *, int, __SIZE_TYPE__) __asm ("__sanitizer_internal_memset"); +# if defined(__cplusplus) +} +# endif +# endif # if defined(__cplusplus) && \ !defined(SANITIZER_COMMON_REDEFINE_BUILTINS_IN_STD) passed bootstrap/regtest for me on x86_64-linux and i686-linux and didn't create any new memset/memcpy/memmove calls in objdump -dr libasan.so.* (we have 2 memset calls from inside of libbacktrace, guess we ought to deal with that eventually). Does this help on Solaris/sparc?
> --- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > Note, following patch [...] > passed bootstrap/regtest for me on x86_64-linux and i686-linux and didn't > create any new memset/memcpy/memmove calls in objdump -dr libasan.so.* (we have > 2 memset calls > from inside of libbacktrace, guess we ought to deal with that eventually). > Does this help on Solaris/sparc? It's almost there, but not quite: I've started with a Solaris/sparc build of unpatched libsanitizer which uses gas. In that case, libsanitizer.so contains 000fc594 <__asan_backtrace_dwarf_add>: fd220: 40 00 9b f2 call 1241e8 <memcpy@plt> 000fd8dc <__asan_backtrace_create_state>: fd91c: 40 00 9a 33 call 1241e8 <memcpy@plt> 0010b0b4 <elf_zstd_read_fse>: 10b384: 40 00 63 e7 call 124320 <memset@plt> 10b4a0: 40 00 63 a0 call 124320 <memset@plt> With your patch and using as, there are a few more: 0005ab90 <__asan_register_globals.part.0>: 5afdc: 40 03 30 d2 call 127324 <memcpy@plt> 00091c94 <printf_common(void*, char const*, void*)>: 92134: 40 02 54 7c call 127324 <memcpy@plt> 926a8: 40 02 53 1f call 127324 <memcpy@plt> 000c4154 <__asan::ReportDeadlySignal(__sanitizer::SignalContext const&)>: c4218: 40 01 8c 43 call 127324 <memcpy@plt> 000c76b0 <__asan::ReportODRViolation(__asan_global const*, unsigned int, __asan_global const*, unsigned int)>: c77a8: 40 01 7e df call 127324 <memcpy@plt> c77b8: 40 01 7e db call 127324 <memcpy@plt> 000e2140 <__sanitizer::VSNPrintf(char*, int, char const*, void*)>: e2554: 40 01 13 74 call 127324 <memcpy@plt> e2610: 40 01 13 45 call 127324 <memcpy@plt> gcc warns: /vol/gcc/src/hg/master/local/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h:31:104: warning: ‘asm’ declaration ignored due to conflict with previous rename [-Wpragmas] 31 | extern void *__builtin_memcpy (void *, const void *, __SIZE_TYPE__) __asm ("__sanitizer_internal_memcpy"); | ^ /vol/gcc/src/hg/master/local/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h:32:106: warning: ‘asm’ declaration ignored due to conflict with previous rename [-Wpragmas] 32 | extern void *__builtin_memmove (void *, const void *, __SIZE_TYPE__) __asm ("__sanitizer_internal_memmove"); | ^ /vol/gcc/src/hg/master/local/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h:33:95: warning: ‘asm’ declaration ignored due to conflict with previous rename [-Wpragmas] 33 | extern void *__builtin_memset (void *, int, __SIZE_TYPE__) __asm ("__sanitizer_internal_memset"); | ^ Though I don't see what previous rename that would be: compiling e.g asan_globals.cpp with -g3 -save-temps shows no rename (that would be #pragma redefine_extname, I guess) at all.
The __builtin_mem* redirections actually don't seem to be necessary (at least for me on x86_64-linux), though I certainly don't see any messages about ignoring something.
Ah, I see, the problem is that the sanitizers use (both in GCC and Clang) -fno-builtin option, which makes it not work unless user uses just memcpy. Short testcase: extern "C" void *memcpy (void *, const void *, decltype (sizeof 0)) __asm ("__sanitizer_internal_memcpy"); extern "C" void *memmove (void *, const void *, decltype (sizeof 0)) __asm ("__sanitizer_internal_memmove"); extern "C" void *memset (void *, int, decltype (sizeof 0)) __asm ("__sanitizer_internal_memset"); struct S { char s[512]; }; void foo (S *p, S *q) { *p = *q; } void bar (int, void *r, void *s) { memcpy (r, s, 512); } void baz (int, int, void *t, void *u) { __builtin_memcpy (t, u, 512); } On x86_64 -O2 -mstringop-strategy=libcall vs. that + -fno-builtin, on sparc without -mstringop-strategy=libcall.
Maybe extern "C" { static void *memcpy (void *, const void *, decltype (sizeof 0)) __attribute__ ((weakref ("__sanitizer_internal_memcpy"))); static void *memmove (void *, const void *, decltype (sizeof 0)) __attribute__ ((weakref ("__sanitizer_internal_memmove"))); static void *memset (void *, int, decltype (sizeof 0)) __attribute__ ((weakref ("__sanitizer_internal_memset"))); } struct S { char s[512]; }; void foo (S *p, S *q) { *p = *q; } void bar (int, void *r, void *s) { memcpy (r, s, 512); } void baz (int, int, void *t, void *u) { __builtin_memcpy (t, u, 512); } works instead. But in any case, I'd use it only when what is in doesn't really work.
It doesn't work after the string.h declarations of those functions.
So, shall we go with --- libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h.jj 2023-11-15 12:45:17.359586776 +0100 +++ libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h 2023-11-21 18:29:52.401817763 +0100 @@ -15,7 +15,8 @@ # define SANITIZER_REDEFINE_BUILTINS_H // The asm hack only works with GCC and Clang. -# if !defined(_WIN32) +// It doesn't work when using Solaris as either. +# if !defined(_WIN32) && !SANITIZER_SOLARIS asm("memcpy = __sanitizer_internal_memcpy"); asm("memmove = __sanitizer_internal_memmove"); @@ -50,7 +51,7 @@ using vector = Define_SANITIZER_COMMON_N } // namespace std # endif // __cpluplus -# endif // !_WIN32 +# endif // !_WIN32 && !SANITIZER_SOLARIS # endif // SANITIZER_REDEFINE_BUILTINS_H #endif // SANITIZER_COMMON_NO_REDEFINE_BUILTINS then (either as local patch or try to push it upstream)?
> --- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > So, shall we go with > --- libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h.jj > 2023-11-15 12:45:17.359586776 +0100 > +++ libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h 2023-11-21 > 18:29:52.401817763 +0100 > @@ -15,7 +15,8 @@ > # define SANITIZER_REDEFINE_BUILTINS_H > > // The asm hack only works with GCC and Clang. > -# if !defined(_WIN32) > +// It doesn't work when using Solaris as either. > +# if !defined(_WIN32) && !SANITIZER_SOLARIS > > asm("memcpy = __sanitizer_internal_memcpy"); > asm("memmove = __sanitizer_internal_memmove"); > @@ -50,7 +51,7 @@ using vector = Define_SANITIZER_COMMON_N > } // namespace std > > # endif // __cpluplus > -# endif // !_WIN32 > +# endif // !_WIN32 && !SANITIZER_SOLARIS > > # endif // SANITIZER_REDEFINE_BUILTINS_H > #endif // SANITIZER_COMMON_NO_REDEFINE_BUILTINS > > then (either as local patch or try to push it upstream)? That's way to heavy IMO: it punishes the Solaris/x86 as which isn't affected and also Solaris/SPARC with gas. I've now come up with an alternative. It's a bit ugly, but it gets the work done: diff --git a/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h b/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h --- a/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h +++ b/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h @@ -17,6 +17,17 @@ // The asm hack only works with GCC and Clang. # if !defined(_WIN32) +// FIXME: Explain. +# if defined(__sparc__) +# define ASM_MEM_DEF(FUNC) \ + __asm__(".global " #FUNC "\n" \ + ".type " #FUNC ",function\n" \ + ".weak " #FUNC "\n" \ + #FUNC ":\n"); +ASM_MEM_DEF(__sanitizer_internal_memcpy) +ASM_MEM_DEF(__sanitizer_internal_memmove) +ASM_MEM_DEF(__sanitizer_internal_memset) +# endif asm("memcpy = __sanitizer_internal_memcpy"); asm("memmove = __sanitizer_internal_memmove"); asm("memset = __sanitizer_internal_memset"); I've run libsanitizer builds on sparc without this patch (gas only since as fails) and with it (as and gas). It fixes the as build failure and leaves the same number of calls to mem* functions in libasan.so as an unpatched tree with gas.
(In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #9) > > --- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > > So, shall we go with > > --- libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h.jj > > 2023-11-15 12:45:17.359586776 +0100 > > +++ libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h 2023-11-21 > > 18:29:52.401817763 +0100 > > @@ -15,7 +15,8 @@ > > # define SANITIZER_REDEFINE_BUILTINS_H > > > > // The asm hack only works with GCC and Clang. > > -# if !defined(_WIN32) > > +// It doesn't work when using Solaris as either. > > +# if !defined(_WIN32) && !SANITIZER_SOLARIS > > > > asm("memcpy = __sanitizer_internal_memcpy"); > > asm("memmove = __sanitizer_internal_memmove"); > > @@ -50,7 +51,7 @@ using vector = Define_SANITIZER_COMMON_N > > } // namespace std > > > > # endif // __cpluplus > > -# endif // !_WIN32 > > +# endif // !_WIN32 && !SANITIZER_SOLARIS > > > > # endif // SANITIZER_REDEFINE_BUILTINS_H > > #endif // SANITIZER_COMMON_NO_REDEFINE_BUILTINS > > > > then (either as local patch or try to push it upstream)? > > That's way to heavy IMO: it punishes the Solaris/x86 as which isn't > affected and also Solaris/SPARC with gas. > > I've now come up with an alternative. It's a bit ugly, but it gets the > work done: > > diff --git a/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h > b/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h > --- a/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h > +++ b/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h > @@ -17,6 +17,17 @@ > // The asm hack only works with GCC and Clang. > # if !defined(_WIN32) > > +// FIXME: Explain. > +# if defined(__sparc__) > +# define ASM_MEM_DEF(FUNC) \ > + __asm__(".global " #FUNC "\n" \ > + ".type " #FUNC ",function\n" \ Not @function ? > + ".weak " #FUNC "\n" \ > + #FUNC ":\n"); > +ASM_MEM_DEF(__sanitizer_internal_memcpy) > +ASM_MEM_DEF(__sanitizer_internal_memmove) > +ASM_MEM_DEF(__sanitizer_internal_memset) > +# endif > asm("memcpy = __sanitizer_internal_memcpy"); > asm("memmove = __sanitizer_internal_memmove"); > asm("memset = __sanitizer_internal_memset"); > > I've run libsanitizer builds on sparc without this patch (gas only since > as fails) and with it (as and gas). It fixes the as build failure and > leaves the same number of calls to mem* functions in libasan.so as an > unpatched tree with gas. If it works, nice. Can you file it on github.com/llvm/llvm-project as an issue and see if upstream is willing to accept it? I think they'll want some indentation changes (if defined(__sparc__) is below the _WIN32 #if, so they probably want it indented more and the define even more. And dunno if defined(__sparc__) or SANITIZER_SPARC should be used.
> --- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > (In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #9) [...] >> I've now come up with an alternative. It's a bit ugly, but it gets the >> work done: >> >> diff --git a/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h >> b/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h >> --- a/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h >> +++ b/libsanitizer/sanitizer_common/sanitizer_redefine_builtins.h >> @@ -17,6 +17,17 @@ >> // The asm hack only works with GCC and Clang. >> # if !defined(_WIN32) >> >> +// FIXME: Explain. >> +# if defined(__sparc__) >> +# define ASM_MEM_DEF(FUNC) \ >> + __asm__(".global " #FUNC "\n" \ >> + ".type " #FUNC ",function\n" \ > > Not @function ? No, this should be #function: that's the only variant sparc as understands, and gas does for compatibility. >> + ".weak " #FUNC "\n" \ >> + #FUNC ":\n"); >> +ASM_MEM_DEF(__sanitizer_internal_memcpy) >> +ASM_MEM_DEF(__sanitizer_internal_memmove) >> +ASM_MEM_DEF(__sanitizer_internal_memset) >> +# endif >> asm("memcpy = __sanitizer_internal_memcpy"); >> asm("memmove = __sanitizer_internal_memmove"); >> asm("memset = __sanitizer_internal_memset"); >> >> I've run libsanitizer builds on sparc without this patch (gas only since >> as fails) and with it (as and gas). It fixes the as build failure and >> leaves the same number of calls to mem* functions in libasan.so as an >> unpatched tree with gas. > > If it works, nice. Can you file it on github.com/llvm/llvm-project as an issue > and see if upstream is willing to accept it? I think they'll want some Can do, either as an issue or directly as a pull request. I'll run it through a full llvm build, too, first. > indentation changes (if defined(__sparc__) is below the _WIN32 #if, so they > probably want it > indented more and the define even more. And dunno if defined(__sparc__) or > SANITIZER_SPARC should be used. I know: LLVM has clang/tools/clang-format/clang-format-diff.py to handle this. I usually run my patches through that first, unlike it messes up the existing formatting as was the case for pull request #72973. The patches also needs an explanatory comment; this was just a proof of concept. It might be even better to restrict the hack to __sparc__ && __sun__ && __svr4__ to avoid interfering with Linux/sparc64.
> --- Comment #11 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot > Uni-Bielefeld.DE> --- >> --- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> --- >> (In reply to ro@CeBiTec.Uni-Bielefeld.DE from comment #9) [...] >> If it works, nice. Can you file it on github.com/llvm/llvm-project as an issue >> and see if upstream is willing to accept it? I think they'll want some > > Can do, either as an issue or directly as a pull request. I'll run it > through a full llvm build, too, first. Unfortunately, this didn't work out at all. Initially, I had only rebuilt libsanitizer with the patch, checking that there were only the exepected number of calls to mem* functions in libasan.so. The LLVM build on Solaris/sparcv9, while successful, doesn't tell you much, unfortunately: due to a clang bug on sparc, the asan tests are never run there. When I ran a full Solaris/SPARC bootstrap, all asan tests FAILed with AddressSanitizer: CHECK failed: asan_rtl.cpp:397 "((!AsanInitIsRunning() && "ASan init calls itself!")) != (0)" (0x0, 0x0) (tid=1) I've once had the misfortune of debugging such a initializion order issue in LLVM, and that was an long and painful experience which I'd rather not repeat here. The fix found in that case (build both libasan.so and all asan tests with -z now) didn't work here, either: this just turned the assertion failures into SEGVs. This reliance on init order is a total nightmare: the details are completely platform-specific and not portable at all. Given all this, I've instead developed a patch that checks for assembler a = b support in the libsanitizer configury and uses the result in sanitizer_redefine_builtins.h. This way, only the one configuration that has an issue here is punished. However, this has to stay local to libsanitizer for obvious reasons.
Patch posted.
The master branch has been updated by Rainer Orth <ro@gcc.gnu.org>: https://gcc.gnu.org/g:088d3cbc5f906444a7dee98bc9a6f4b724ddfc21 commit r14-5927-g088d3cbc5f906444a7dee98bc9a6f4b724ddfc21 Author: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> Date: Tue Nov 28 14:54:34 2023 +0100 libsanitizer: Check assembler support for symbol assignment [PR112563] The recent libsanitizer import broke the build on Solaris/SPARC with the native as: /usr/ccs/bin/as: ".libs/sanitizer_errno.s", line 4247: error: symbol "__sanitizer_internal_memset" is used but not defined /usr/ccs/bin/as: ".libs/sanitizer_errno.s", line 4247: error: symbol "__sanitizer_internal_memcpy" is used but not defined /usr/ccs/bin/as: ".libs/sanitizer_errno.s", line 4247: error: symbol "__sanitizer_internal_memmove" is used but not defined Since none of the alternatives considered in the PR worked out, this patch checks if the assembler does support symbol assignment, disabling the code otherwise. This returns the code to the way it was up to LLVM 16. Bootstrapped without regressions on sparc-sun-solaris2.11 (as and gas) and i386-pc-solaris2.11 (as and gas). 2023-11-23 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> libsanitizer: PR sanitizer/112563 * configure.ac (libsanitizer_cv_as_sym_assign): Check for assembler symbol assignment support. * configure: Regenerate. * asan/Makefile.am (DEFS): Add @AS_SYM_ASSIGN_DEFS@. * Makefile.in, asan/Makefile.in, hwasan/Makefile.in, interception/Makefile.in, libbacktrace/Makefile.in, lsan/Makefile.in, sanitizer_common/Makefile.in, tsan/Makefile.in, ubsan/Makefile.in: Regenerate.
The master branch has been updated by Rainer Orth <ro@gcc.gnu.org>: https://gcc.gnu.org/g:33b6ce99b54917a910b59dbd643fd223fbba834c commit r14-5928-g33b6ce99b54917a910b59dbd643fd223fbba834c Author: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> Date: Tue Nov 28 14:55:52 2023 +0100 libsanitizer: Only use assembler symbol assignment if supported [PR112563] This patch only enables symbol assignment if the configure test determined it's supported. Bootstrapped without regressions on sparc-sun-solaris2.11 (as and gas) and i386-pc-solaris2.11 (as and gas). 2023-11-23 Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> libsanitizer: PR sanitizer/112563 * sanitizer_common/sanitizer_redefine_builtins.h: Check HAVE_AS_SYM_ASSIGN.
Fixed for GCC 14.