After an upgrade to glibc 2.27 on a Debian/unstable machine, on a program that does nothing, the AddressSanitizer segfaults with the 32-bit x86 ABI. I have the following gcc-snapshot script: #!/bin/sh LD_LIBRARY_PATH=/usr/lib/gcc-snapshot/lib:$LD_LIBRARY_PATH PATH=/usr/lib/gcc-snapshot/bin:$PATH rpath="" OLD_IFS="$IFS" IFS=: for i in $LD_RUN_PATH do rpath="$rpath -Wl,-rpath -Wl,$i" done IFS="$OLD_IFS" exec gcc -Wl,-rpath -Wl,/usr/lib/gcc-snapshot/lib \ -Wl,-rpath -Wl,/usr/lib/gcc-snapshot/lib32 \ -Wl,-rpath -Wl,/usr/lib/gcc-snapshot/libx32 $rpath "$@" cventin:~> gcc-snapshot --version gcc (Debian 20180216-1) 8.0.1 20180216 (experimental) [trunk revision 257720] [...] cventin:~> cat tst.c int main (void) { return 0; } cventin:~> gcc-snapshot -m32 -fsanitize=address tst.c -o tst cventin:~> ./tst AddressSanitizer:DEADLYSIGNAL ================================================================= ==25032==ERROR: AddressSanitizer: SEGV on unknown address 0xf7fa7e70 (pc 0xf7fa7e84 bp 0xffbf40ac sp 0xffbf406c T16777215) ==25032==The signal is caused by a WRITE memory access. #0 0xf7fa7e83 in _dl_get_tls_static_info (/lib/ld-linux.so.2+0x11e83) #1 0xf7ac147d (/usr/lib/gcc-snapshot/lib32/libasan.so.5+0x10e47d) #2 0xf7aafd27 (/usr/lib/gcc-snapshot/lib32/libasan.so.5+0xfcd27) #3 0xf7fa591a (/lib/ld-linux.so.2+0xf91a) #4 0xf7f96cb9 (/lib/ld-linux.so.2+0xcb9) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV (/lib/ld-linux.so.2+0x11e83) in _dl_get_tls_static_info ==25032==ABORTING My original bug report: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=892096 The explanations given by Aurelien Jarno: ------------------------------------------------------------ The AddressSanitizer is using glibc internal functions though dlsym(), and such functions have the right to change in new major versions: From libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc: | void *get_tls_static_info_ptr = dlsym(RTLD_NEXT, "_dl_get_tls_static_info"); And on the glibc side: | $ readelf -s /lib/ld-linux.so.2 | grep _dl_get_tls_static_info | 4: 00011e70 35 FUNC GLOBAL DEFAULT 12 _dl_get_tls_static_info@@GLIBC_PRIVATE This has been discussed for example there: https://www.sourceware.org/ml/libc-alpha/2018-02/msg00611.html The AddressSanitizer people should discuss for a public API so that it doesn't happen again. Otherwise it might break at every new glibc version. ------------------------------------------------------------ In the mean time, it should at least be made compatible with glibc 2.27.
This bit needs to change for glibc 2.27 and later: #ifdef __i386__ # define DL_INTERNAL_FUNCTION __attribute__((regparm(3), stdcall)) #else # define DL_INTERNAL_FUNCTION #endif We removed the regparm attribute for compatibility with CET. The GCC manual says that regparm (3) is ABI-compatible on i386, but it really is not. We could perhaps add an alias to newer glibcs (including 2.27), so that libasan can detect that glibc is more recent and call the function with the right ABI. Or expose the information via dlinfo (but the interface is problematic because it assumes that the static TLS size is fixed—we could eventually switch to a model where we have a larger reserved region for the initial thread).
On Mon, 19 Mar 2018, fw at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84761 > > --- Comment #1 from Florian Weimer <fw at gcc dot gnu.org> --- > This bit needs to change for glibc 2.27 and later: > > #ifdef __i386__ > # define DL_INTERNAL_FUNCTION __attribute__((regparm(3), stdcall)) > #else > # define DL_INTERNAL_FUNCTION > #endif > > We removed the regparm attribute for compatibility with CET. The GCC manual > says that regparm (3) is ABI-compatible on i386, but it really is not. > > We could perhaps add an alias to newer glibcs (including 2.27), so that libasan > can detect that glibc is more recent and call the function with the right ABI. > Or expose the information via dlinfo (but the interface is problematic because > it assumes that the static TLS size is fixed—we could eventually switch to a > model where we have a larger reserved region for the initial thread). I think as there are already quite some __GLIBC_PREREQ uses in the sanitizer lib changing the above prototype appropriately would be good enough. Of course exposing the desired information without the need to resort to a private interface would be nice. So sth like Index: libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc =================================================================== --- libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc (revision 258641) +++ libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc (working copy) @@ -152,7 +152,14 @@ bool SanitizerGetThreadName(char *name, static uptr g_tls_size; #ifdef __i386__ +#ifndef __GLIBC_PREREQ +#define __GLIBC_PREREQ(x, y) 0 +#endif +#ifdef SANITIZER_LINUX && __GLIBC_PREREQ(2, 27) +# define DL_INTERNAL_FUNCTION +#else # define DL_INTERNAL_FUNCTION __attribute__((regparm(3), stdcall)) +#endif #else # define DL_INTERNAL_FUNCTION #endif
(In reply to rguenther@suse.de from comment #2) > I think as there are already quite some __GLIBC_PREREQ uses in the > sanitizer lib changing the above prototype appropriately would be > good enough. If that's the way to go, then your patch should work. I'd add a comment: /* glibc 2.27 changed internal ABI and removed internal_function. */ And perhaps the condition can be simplified to: #if defined (__i386__) && defined (__GLIBC_PREREQ) && !__GLIBC_PREREQ(2, 27)
That won't really work, if you configure against one glibc version and run against another one, that will still not work properly. So I think we need something like: --- libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc 2017-10-19 13:20:58.972958379 +0200 +++ libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc 2018-03-19 13:50:41.978147828 +0100 @@ -151,25 +151,35 @@ bool SanitizerGetThreadName(char *name, !SANITIZER_NETBSD static uptr g_tls_size; -#ifdef __i386__ -# define DL_INTERNAL_FUNCTION __attribute__((regparm(3), stdcall)) -#else -# define DL_INTERNAL_FUNCTION -#endif - void InitTlsSize() { // all current supported platforms have 16 bytes stack alignment const size_t kStackAlign = 16; - typedef void (*get_tls_func)(size_t*, size_t*) DL_INTERNAL_FUNCTION; - get_tls_func get_tls; - void *get_tls_static_info_ptr = dlsym(RTLD_NEXT, "_dl_get_tls_static_info"); - CHECK_EQ(sizeof(get_tls), sizeof(get_tls_static_info_ptr)); - internal_memcpy(&get_tls, &get_tls_static_info_ptr, - sizeof(get_tls_static_info_ptr)); - CHECK_NE(get_tls, 0); size_t tls_size = 0; size_t tls_align = 0; - get_tls(&tls_size, &tls_align); + void *get_tls_static_info_ptr = dlsym(RTLD_NEXT, "_dl_get_tls_static_info"); +#ifdef __i386__ + /* On i?86, _dl_get_tls_static_info used to be regparm(3) before glibc 2.27 + and is now a normal function. */ + if (!dlvsym(RTLD_NEXT, "glob", "GLIBC_2.27")) { + typedef void (*get_tls_func)(size_t*, size_t*) + __attribute__((regparm(3), stdcall)); + get_tls_func get_tls; + CHECK_EQ(sizeof(get_tls), sizeof(get_tls_static_info_ptr)); + internal_memcpy(&get_tls, &get_tls_static_info_ptr, + sizeof(get_tls_static_info_ptr)); + CHECK_NE(get_tls, 0); + get_tls(&tls_size, &tls_align); + } else +#endif + { + typedef void (*get_tls_func)(size_t*, size_t*); + get_tls_func get_tls; + CHECK_EQ(sizeof(get_tls), sizeof(get_tls_static_info_ptr)); + internal_memcpy(&get_tls, &get_tls_static_info_ptr, + sizeof(get_tls_static_info_ptr)); + CHECK_NE(get_tls, 0); + get_tls(&tls_size, &tls_align); + } if (tls_align < kStackAlign) tls_align = kStackAlign; g_tls_size = RoundUpTo(tls_size, tls_align); instead. If we only support using the glibc with which gcc has been configured or later, perhaps that dlvsym call could be avoided if __GLIBC_PREREQ(2, 27).
Created attachment 43701 [details] gcc8-pr84761.patch Full untested patch, tested so far just with building against glibc 2.26 and running a simple 32-bit -fsanitize=address sanitized program against: 1) unpatched libasan.so.5 and glibc 2.26 2) unpatched libasan.so.5 and glibc 2.27 (this crashes) 3) patched libasan.so.5 and glibc 2.26 4) patched libasan.so.5 and glibc 2.27
Filed https://reviews.llvm.org/D44623 upstream.
Is it possible that a distribution would backport glob() changes together with its symver update (without also backporting the regparm change)? In that case the dlvsym check shown above will be wrong I think. Would the approach with confstr query for glibc version (discussed on irc) be less fragile?
Distributions should never backport symbol additions to symbol versioned libraries, unless they backport all the corresponding changes. So no, I don't think this is an issue.
Author: jakub Date: Mon Mar 19 20:47:29 2018 New Revision: 258663 URL: https://gcc.gnu.org/viewcvs?rev=258663&root=gcc&view=rev Log: PR sanitizer/84761 * sanitizer_common/sanitizer_linux_libcdep.cc (__GLIBC_PREREQ): Define if not defined. (DL_INTERNAL_FUNCTION): Don't define. (InitTlsSize): For __i386__ if not compiled against glibc 2.27+ determine at runtime whether to use regparm(3), stdcall calling convention for older glibcs or normal calling convention for newer glibcs for call to _dl_get_tls_static_info. Modified: trunk/libsanitizer/ChangeLog trunk/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
Fixed for 8+ so far.
*** Bug 85477 has been marked as a duplicate of this bug. ***
I was not aware of this report or existing patches. The issue is reported in upstream compiler-rt here: https://github.com/google/sanitizers/issues/954 Jakub, according to upstream LLVM the compiler-rt fixes should first enter LLVM and then be merged downstream in GCC, is there any reason to deviate from this process? Upstream does not seem happy about it. Also please see the LLVM review and linked bugs for comments (I think that using confstr is better than dlvsym, nobody uses pre-release glibc for production right?).
Author: rguenth Date: Thu Jun 7 11:56:25 2018 New Revision: 261272 URL: https://gcc.gnu.org/viewcvs?rev=261272&root=gcc&view=rev Log: 2018-06-07 Richard Biener <rguenther@suse.de> Backport from mainline 2018-03-19 Jakub Jelinek <jakub@redhat.com> PR sanitizer/84761 * sanitizer_common/sanitizer_linux_libcdep.cc (__GLIBC_PREREQ): Define if not defined. (DL_INTERNAL_FUNCTION): Don't define. (InitTlsSize): For __i386__ if not compiled against glibc 2.27+ determine at runtime whether to use regparm(3), stdcall calling convention for older glibcs or normal calling convention for newer glibcs for call to _dl_get_tls_static_info. Modified: branches/gcc-7-branch/libsanitizer/ChangeLog branches/gcc-7-branch/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
Fixed on the GCC 7 branch as well.
Author: rguenth Date: Mon Oct 15 13:43:09 2018 New Revision: 265164 URL: https://gcc.gnu.org/viewcvs?rev=265164&root=gcc&view=rev Log: 2018-10-15 Richard Biener <rguenther@suse.de> Backport from mainline 2018-03-19 Jakub Jelinek <jakub@redhat.com> PR sanitizer/84761 * sanitizer_common/sanitizer_linux_libcdep.cc (__GLIBC_PREREQ): Define if not defined. (DL_INTERNAL_FUNCTION): Don't define. (InitTlsSize): For __i386__ if not compiled against glibc 2.27+ determine at runtime whether to use regparm(3), stdcall calling convention for older glibcs or normal calling convention for newer glibcs for call to _dl_get_tls_static_info. Modified: branches/gcc-6-branch/libsanitizer/ChangeLog branches/gcc-6-branch/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc
Jakub: Can the bug be marked as resolved?
Fixed.