| Summary: | [5/6/7 Regression] -fsanitize=thread broken by ignoring __attribute__((tls_model("initial-exec"))) | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Markus Trippelsdorf <trippels> |
| Component: | middle-end | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | amodra, dodji, jakub, kcc |
| Priority: | P3 | Keywords: | wrong-code |
| Version: | 7.0 | ||
| Target Milestone: | 5.5 | ||
| Host: | Target: | ||
| Build: | Known to work: | 4.9.3 | |
| Known to fail: | 5.4.0, 6.2.0, 7.0 | Last reconfirmed: | 2016-11-15 00:00:00 |
| Attachments: | unreduced testcase | ||
Humm... This is puzzling. Just in case, you set LD_LIBRARY_PATH to point to the new libtsan.so? ScopedInterceptor already called cur_thread here: #4 0x00007ffff6f43b90 in __tsan::ScopedInterceptor::ScopedInterceptor and it did not crash. But then #2 0x00007ffff6f8b00a in __tsan::ScopedIgnoreInterceptors::ScopedIgnoreInterceptors calls cur_thread again and it ends up in __tls_get_addr, which crashes... I've just built gcc version 7.0.0 20161111 (experimental) (GCC), and your repro works. Although, I have a different kernel and glibc. However, __tsan::Initialize does not contain a call to __tls_get_addr to get the address of cur_thread: 000000000006da60 <__tsan::Initialize(__tsan::ThreadState*)>: 6da60: 80 3d 25 73 47 00 00 cmpb $0x0,0x477325(%rip) # 4e4d8c <__tsan::Initialize(__tsan::ThreadState*)::is_initialized> 6da67: 74 07 je 6da70 <__tsan::Initialize(__tsan::ThreadState*)+0x10> 6da69: f3 c3 repz retq 6da6b: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) 6da70: 41 57 push %r15 6da72: 41 56 push %r14 6da74: 41 55 push %r13 6da76: 41 54 push %r12 6da78: 55 push %rbp 6da79: 53 push %rbx 6da7a: 48 83 ec 48 sub $0x48,%rsp 6da7e: c6 05 07 73 47 00 01 movb $0x1,0x477307(%rip) # 4e4d8c <__tsan::Initialize(__tsan::ThreadState*)::is_initialized> 6da85: 48 89 7c 24 18 mov %rdi,0x18(%rsp) 6da8a: 64 48 8b 04 25 00 00 mov %fs:0x0,%rax 6da91: 00 00 6da93: 48 03 05 fe 9e 26 00 add 0x269efe(%rip),%rax # 2d7998 <_DYNAMIC+0xa10> 6da9a: 48 8d 3d 6f ce 00 00 lea 0xce6f(%rip),%rdi # 7a910 <__tsan::TsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long)> 6daa1: 48 8d 0d 1c 09 04 00 lea 0x4091c(%rip),%rcx # ae3c4 <__tsan::MutexSet::kMaxSize+0x1024> 6daa8: 83 80 98 02 02 00 01 addl $0x1,0x20298(%rax) The instructions are: 6da8a: 64 48 8b 04 25 00 00 mov %fs:0x0,%rax 6da93: 48 03 05 fe 9e 26 00 add 0x269efe(%rip),%rax 6daa8: 83 80 98 02 02 00 01 addl $0x1,0x20298(%rax) which is reasonable for initial_exec tls. Please post disassembly of these frames and point to the exact call sites: #2 0x00007ffff6f8b00a in __tsan::ScopedIgnoreInterceptors::ScopedIgnoreInterceptors (this=<synthetic pointer>) at ../../../../gcc/libsanitizer/tsan/tsan_rtl.h:549 #3 __tsan::Initialize (thr=thr@entry=0x7ffff6277780) at ../../../../gcc/libsanitizer/tsan/tsan_rtl.cc:331 The testcase also works for me with Glibc 2.24 and dynamic libtsan:
max@max:/tmp$ echo "int main(){}" | ~/install/master/bin/g++ -fsanitize=thread -L/home/max/install/glibc/usr/lib -L/home/max/install/glibc/lib -I/home/max/install/glibc/include -Wl,-rpath=/home/max/install/glibc/lib -Wl,--dynamic-linker=/home/max/install/glibc/lib/ld-2.24.90.so -Wl,-rpath=/home/max//install/master/lib64 -x c++ - && ./a.out
max@max:/tmp$ echo $?
0
max@max:/tmp$ ldd a.out
linux-vdso.so.1 => (0x00007ffe0edb0000)
libtsan.so.0 => /home/max//install/master/lib64/libtsan.so.0 (0x00007f6d97505000)
libstdc++.so.6 => /home/max//install/master/lib64/libstdc++.so.6 (0x00007f6d97183000)
libm.so.6 => /home/max/install/glibc/lib/libm.so.6 (0x00007f6d96e7f000)
libgcc_s.so.1 => /home/max/install/glibc/lib/libgcc_s.so.1 (0x00007f6d96c69000)
libc.so.6 => /home/max/install/glibc/lib/libc.so.6 (0x00007f6d968d0000)
libdl.so.2 => /home/max/install/glibc/lib/libdl.so.2 (0x00007f6d966cc000)
librt.so.1 => /home/max/install/glibc/lib/librt.so.1 (0x00007f6d964c4000)
libpthread.so.0 => /home/max/install/glibc/lib/libpthread.so.0 (0x00007f6d962a7000)
/home/max/install/glibc/lib/ld-2.24.90.so => /lib64/ld-linux-x86-64.so.2 (0x00007f6d985bc000)
(gdb) up #2 0x00007ffff6f8b00a in __tsan::ScopedIgnoreInterceptors::ScopedIgnoreInterceptors (this=<synthetic pointer>) at ../../../../gcc/libsanitizer/tsan/tsan_rtl.h:549 549 cur_thread()->ignore_interceptors++; (gdb) disass Dump of assembler code for function __tsan::Initialize(__tsan::ThreadState*): 0x00007ffff6f8afd0 <+0>: cmpb $0x0,0x278ff5(%rip) # 0x7ffff7203fcc <_ZZN6__tsan10InitializeEPNS_11ThreadStateEE14is_initialized> 0x00007ffff6f8afd7 <+7>: je 0x7ffff6f8afe0 <__tsan::Initialize(__tsan::ThreadState*)+16> 0x00007ffff6f8afd9 <+9>: repz retq 0x00007ffff6f8afdb <+11>: nopl 0x0(%rax,%rax,1) 0x00007ffff6f8afe0 <+16>: push %r15 0x00007ffff6f8afe2 <+18>: push %r14 0x00007ffff6f8afe4 <+20>: push %r13 0x00007ffff6f8afe6 <+22>: push %r12 0x00007ffff6f8afe8 <+24>: push %rbp 0x00007ffff6f8afe9 <+25>: push %rbx 0x00007ffff6f8afea <+26>: sub $0x48,%rsp 0x00007ffff6f8afee <+30>: movb $0x1,0x278fd7(%rip) # 0x7ffff7203fcc <_ZZN6__tsan10InitializeEPNS_11ThreadStateEE14is_initialized> 0x00007ffff6f8aff5 <+37>: mov %rdi,0x18(%rsp) 0x00007ffff6f8affa <+42>: data16 lea 0x6cf66(%rip),%rdi # 0x7ffff6ff7f68 0x00007ffff6f8b002 <+50>: data16 data16 callq 0x7ffff6f3ed20 <__tls_get_addr@plt> => 0x00007ffff6f8b00a <+58>: lea 0xce6f(%rip),%rdi # 0x7ffff6f97e80 <__tsan::TsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long)> 0x00007ffff6f8b011 <+65>: lea 0x3df3b(%rip),%rcx # 0x7ffff6fc8f53 0x00007ffff6f8b018 <+72>: addl $0x1,0x20298(%rax) 0x00007ffff6f8b01f <+79>: lea 0x6f4b2(%rip),%rax # 0x7ffff6ffa4d8 <_ZN11__sanitizer17SanitizerToolNameE> 0x00007ffff6f8b026 <+86>: mov %rcx,(%rax) 0x00007ffff6f8b029 <+89>: callq 0x7ffff6fb4760 <__sanitizer::SetCheckFailedCallback(void (*)(char const*, int, char const*, unsigned long long, unsigned long long))> 0x00007ffff6f8b02e <+94>: lea 0x27908b(%rip),%rdi # 0x7ffff72040c0 <_ZN6__tsanL15ctx_placeholderE> 0x00007ffff6f8b035 <+101>: callq 0x7ffff6f8a960 <__tsan::Context::Context()> (gdb) up #3 __tsan::Initialize (thr=thr@entry=0x7ffff6277780) at ../../../../gcc/libsanitizer/tsan/tsan_rtl.cc:331 331 ScopedIgnoreInterceptors ignore; (gdb) disass Dump of assembler code for function __tsan::Initialize(__tsan::ThreadState*): 0x00007ffff6f8afd0 <+0>: cmpb $0x0,0x278ff5(%rip) # 0x7ffff7203fcc <_ZZN6__tsan10InitializeEPNS_11ThreadStateEE14is_initialized> 0x00007ffff6f8afd7 <+7>: je 0x7ffff6f8afe0 <__tsan::Initialize(__tsan::ThreadState*)+16> 0x00007ffff6f8afd9 <+9>: repz retq 0x00007ffff6f8afdb <+11>: nopl 0x0(%rax,%rax,1) 0x00007ffff6f8afe0 <+16>: push %r15 0x00007ffff6f8afe2 <+18>: push %r14 0x00007ffff6f8afe4 <+20>: push %r13 0x00007ffff6f8afe6 <+22>: push %r12 0x00007ffff6f8afe8 <+24>: push %rbp 0x00007ffff6f8afe9 <+25>: push %rbx 0x00007ffff6f8afea <+26>: sub $0x48,%rsp 0x00007ffff6f8afee <+30>: movb $0x1,0x278fd7(%rip) # 0x7ffff7203fcc <_ZZN6__tsan10InitializeEPNS_11ThreadStateEE14is_initialized> 0x00007ffff6f8aff5 <+37>: mov %rdi,0x18(%rsp) 0x00007ffff6f8affa <+42>: data16 lea 0x6cf66(%rip),%rdi # 0x7ffff6ff7f68 0x00007ffff6f8b002 <+50>: data16 data16 callq 0x7ffff6f3ed20 <__tls_get_addr@plt> => 0x00007ffff6f8b00a <+58>: lea 0xce6f(%rip),%rdi # 0x7ffff6f97e80 <__tsan::TsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long)> 0x00007ffff6f8b011 <+65>: lea 0x3df3b(%rip),%rcx # 0x7ffff6fc8f53 0x00007ffff6f8b018 <+72>: addl $0x1,0x20298(%rax) 0x00007ffff6f8b01f <+79>: lea 0x6f4b2(%rip),%rax # 0x7ffff6ffa4d8 <_ZN11__sanitizer17SanitizerToolNameE> 0x00007ffff6f8b026 <+86>: mov %rcx,(%rax) Markus, how do you configure and build gcc? Is there anything special? Our tls accesses should not go through __tls_get_addr because we use initial-exec attribute. __tls_get_addr vs indirect access through got is chosen by compiler, right? So it's not linker. The question is why does your compiler chooses to use the call... If I am not mistaken, __tsan::ScopedInterceptor::ScopedInterceptor should have been already accesses cur_thread, and that did not crash, which suggests that there is no __tls_get_addr call. OK, it looks like a ld.gold bug. I normally always use ld.gold on my system. However if I build gcc with ld.bfd then everything works fine. Disassembly of libtsan.so.0.0.0:
000000000006efd0 <_ZN6__tsan10InitializeEPNS_11ThreadStateE>:
if (is_initialized)
6efd0: 80 3d f5 8f 27 00 00 cmpb $0x0,0x278ff5(%rip) # 2e7fcc <_ZZN6__tsan10InitializeEPNS_11ThreadStateEE14is_initialized>
6efd7: 74 07 je 6efe0 <_ZN6__tsan10InitializeEPNS_11ThreadStateE+0x10>
6efd9: f3 c3 repz retq
6efdb: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
void Initialize(ThreadState *thr) {
6efe0: 41 57 push %r15
6efe2: 41 56 push %r14
6efe4: 41 55 push %r13
6efe6: 41 54 push %r12
6efe8: 55 push %rbp
6efe9: 53 push %rbx
6efea: 48 83 ec 48 sub $0x48,%rsp
is_initialized = true;
6efee: c6 05 d7 8f 27 00 01 movb $0x1,0x278fd7(%rip) # 2e7fcc <_ZZN6__tsan10InitializeEPNS_11ThreadStateEE14is_initialized>
6eff5: 48 89 7c 24 18 mov %rdi,0x18(%rsp)
6effa: 66 48 8d 3d 66 cf 06 data16 lea 0x6cf66(%rip),%rdi # dbf68 <_ZN6__tsan22cur_thread_placeholderE@@Base+0xdbf28>
6f001: 00
cur_thread()->ignore_interceptors++;
6f002: 66 66 48 e8 16 3d fb data16 data16 callq 22d20 <__tls_get_addr@plt>
6f009: ff
SetCheckFailedCallback(TsanCheckFailed);
6f00a: 48 8d 3d 6f ce 00 00 lea 0xce6f(%rip),%rdi # 7be80 <_ZN6__tsan15TsanCheckFailedEPKciS1_yy>
SanitizerToolName = "ThreadSanitizer";
6f011: 48 8d 0d 3b df 03 00 lea 0x3df3b(%rip),%rcx # acf53 <_fini+0x35ab>
vs.
000000000006d3e0 <_ZN6__tsan10InitializeEPNS_11ThreadStateE>:
if (is_initialized)
6d3e0: 80 3d 65 7a 47 00 00 cmpb $0x0,0x477a65(%rip) # 4e4e4c <_ZZN6__tsan10InitializeEPNS_11ThreadStateEE14is_initialized>
6d3e7: 74 07 je 6d3f0 <_ZN6__tsan10InitializeEPNS_11ThreadStateE+0x10>
6d3e9: f3 c3 repz retq
6d3eb: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
void Initialize(ThreadState *thr) {
6d3f0: 41 57 push %r15
6d3f2: 41 56 push %r14
6d3f4: 41 55 push %r13
6d3f6: 41 54 push %r12
6d3f8: 55 push %rbp
6d3f9: 53 push %rbx
6d3fa: 48 83 ec 48 sub $0x48,%rsp
is_initialized = true;
6d3fe: c6 05 47 7a 47 00 01 movb $0x1,0x477a47(%rip) # 4e4e4c <_ZZN6__tsan10InitializeEPNS_11ThreadStateEE14is_initialized>
6d405: 48 89 7c 24 18 mov %rdi,0x18(%rsp)
6d40a: 64 48 8b 04 25 00 00 mov %fs:0x0,%rax
6d411: 00 00
cur_thread()->ignore_interceptors++;
6d413: 48 03 05 be a7 26 00 add 0x26a7be(%rip),%rax # 2d7bd8 <.got+0x7c0>
SetCheckFailedCallback(TsanCheckFailed);
6d41a: 48 8d 3d 6f ce 00 00 lea 0xce6f(%rip),%rdi # 7a290 <_ZN6__tsan15TsanCheckFailedEPKciS1_yy>
SanitizerToolName = "ThreadSanitizer";
6d421: 48 8d 0d 64 09 04 00 lea 0x40964(%rip),%rcx # add8c <_ZN6__tsan8MutexSet8kMaxSizeE+0x1034>
6d428: 83 80 98 02 02 00 01 addl $0x1,0x20298(%rax)
6d42f: 48 8d 05 22 df 26 00 lea 0x26df22(%rip),%rax # 2db358 <_ZN11__sanitizer17SanitizerToolNameE>
6d436: 48 89 08 mov %rcx,(%rax)
SetCheckFailedCallback(TsanCheckFailed);
6d439: e8 32 97 02 00 callq 96b70 <_ZN11__sanitizer22SetCheckFailedCallbackEPFvPKciS1_yyE>
ctx = new(ctx_placeholder) Context;
6d43e: 48 8d 3d fb 7a 47 00 lea 0x477afb(%rip),%rdi # 4e4f40 <_ZN6__tsanL15ctx_placeholderE>
6d445: e8 26 f9 ff ff callq 6cd70 <_ZN6__tsan7ContextC1Ev>
const char *options = GetEnv(SANITIZER_GO ? "GORACE" : "TSAN_OPTIONS");
I've opened https://sourceware.org/bugzilla/show_bug.cgi?id=20805. It would be good if someone could come up with a small testcase. Not a gcc bug, so lets close it. It might make sense to add -fuse-ld=bfd to the AM_CXXFLAGS for now. (lld is also broken.) As Cary pointed out in the sourceware gold bug, it only worked by
luck, because ld.bfd made an invalid optimization.
Apparently the __attribute__((tls_model("initial-exec"))) doesn't get
propagated correctly by the inliner.
Created attachment 40050 [details]
unreduced testcase
Indeed it look like gcc simply ignores the attribute:
g++ -fPIC -O2 -g -c tsan_rtl.ii
vs.
clang++ -fPIC -O2 -g -c tsan_rtl.ii
gcc:
cur_thread()->ignore_interceptors++;
.byte 0x66
leaq _ZN6__tsan22cur_thread_placeholderE@tlsgd(%rip), %rdi #
.value 0x6666
rex64
call __tls_get_addr@PLT #
clang:
movq _ZN6__tsan22cur_thread_placeholderE@GOTTPOFF(%rip), %rax
incl %fs:131736(%rax)
movb $1, _ZZN6__tsan10InitializeEPNS_11ThreadStateEE14is_initialized(%rip)
leaq .L.str.8(%rip), %rax
And indeed if I compile only tsan_rtl.cc with clang and the rest with gcc,
threadsanitzer works fine.
Started with r216361 (bizarrely committed by me, but it wasn't my patch). From the comment added in that revision: + /* This isn't quite correct for something like + int __thread x attribute ((tls_model ("local-exec"))); + extern int __thread x; + as we'll lose the "local-exec" model. */ Here is a small testcase:
markus@x4 tsan % cat tsan_rtl.ii
__attribute__((tls_model("initial-exec"))) extern __thread char b[];
__thread char b[1];
int c;
void fn1() { c = *b; }
markus@x4 tsan % clang++ -fPIC -O2 -c tsan_rtl.ii -S -o -
...
movq b@GOTTPOFF(%rip), %rax
movsbl %fs:(%rax), %eax
movq c@GOTPCREL(%rip), %rcx
movl %eax, (%rcx)
retq
...
markus@x4 tsan % g++ -fPIC -O2 -c tsan_rtl.ii -S -o -
...
subq $8, %rsp
.cfi_def_cfa_offset 16
.byte 0x66
leaq b@tlsgd(%rip), %rdi
.value 0x6666
rex64
call __tls_get_addr@PLT
movsbl (%rax), %edx
movq c@GOTPCREL(%rip), %rax
movl %edx, (%rax)
addq $8, %rsp
...
Easiest fix:
diff --git a/libsanitizer/tsan/tsan_rtl.cc b/libsanitizer/tsan/tsan_rtl.cc
index 07fa165e939c..0a40e3c9a2e1 100644
--- a/libsanitizer/tsan/tsan_rtl.cc
+++ b/libsanitizer/tsan/tsan_rtl.cc
@@ -43,6 +43,7 @@ extern "C" void __tsan_resume() {
namespace __tsan {
#if !SANITIZER_GO && !SANITIZER_MAC
+__attribute__((tls_model("initial-exec")))
THREADLOCAL char cur_thread_placeholder[sizeof(ThreadState)] ALIGNED(64);
#endif
static char ctx_placeholder[sizeof(Context)] ALIGNED(64);
So is it a bug in gcc or in tsan runtime? I thought that an attribute attached to a definition must affect declaration as well. (In reply to Dmitry Vyukov from comment #16) > So is it a bug in gcc or in tsan runtime? > I thought that an attribute attached to a definition must affect declaration > as well. Yes, I would say it is a bug in gcc. (The tsan_rtl.cc patch just allows one to build tsan with gold). Well, GCC has always required that the tls_model attribute is specified on all the declarations/definitions (since the introduction of the extension) rather than just one of them, and matches in between them. Apparently LLVM implemented the GNU extension differently. The question is if we want to change this or not and in what way. Do we want to error out if there are multiple tls_model attributes with different arguments, or silently pick up something from those (both gcc and llvm is quiet on the mismatches right now)? What do we want to do when merging the decls? The decl without tls_model attribute gets the default tls model, the one with the attribute gets the chosen one. Shall we pick the more optimized one of those two (LE wins over IE, which wins over LD which wins over GD)? Or shall we keep the tls model from the decl that has tls_model attribute if any (and again, what to do if there are mismatches)? As for the ld.bfd optimization that makes linking with ld.bfd work, that is an optimization if there are any IE relocations then other GD/LD relocations are turned into them too, because once you have any IE relocations, all of the TLS has to be static anyway, so the TLS GD/LD calls are just a waste of time. Author: trippels Date: Wed Nov 16 11:21:42 2016 New Revision: 242480 URL: https://gcc.gnu.org/viewcvs?rev=242480&root=gcc&view=rev Log: Fix PR78294 - thread sanitizer broken when using ld.gold When one uses ld.gold to build gcc, the thread sanitizer doesn't work, because gold is more conservative when applying TLS relaxations than ld.bfd. In this case a missing initial-exec attribute on a declaration causes gcc to assume the general dynamic model. With ld.bfd this gets relaxed to initial exec when linking the shared library, so the missing attribute doesn't matter. But ld.gold doesn't perform this optimization and this leads to crashes on tsan instrumented binaries. See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78294 and: https://sourceware.org/bugzilla/show_bug.cgi?id=20805 The fix is easy, just add the missing attribute. PR sanitizer/78294 * tsan/tsan_rtl.cc: Add missing attribute. Modified: trunk/libsanitizer/ChangeLog trunk/libsanitizer/tsan/tsan_rtl.cc Author: trippels Date: Wed Nov 16 11:28:57 2016 New Revision: 242482 URL: https://gcc.gnu.org/viewcvs?rev=242482&root=gcc&view=rev Log: Fix PR78294 - thread sanitizer broken when using ld.gold When one uses ld.gold to build gcc, the thread sanitizer doesn't work, because gold is more conservative when applying TLS relaxations than ld.bfd. In this case a missing initial-exec attribute on a declaration causes gcc to assume the general dynamic model. With ld.bfd this gets relaxed to initial exec when linking the shared library, so the missing attribute doesn't matter. But ld.gold doesn't perform this optimization and this leads to crashes on tsan instrumented binaries. See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78294 and: https://sourceware.org/bugzilla/show_bug.cgi?id=20805 The fix is easy, just add the missing attribute. PR sanitizer/78294 * tsan/tsan_rtl.cc: Add missing attribute. Modified: branches/gcc-6-branch/libsanitizer/ChangeLog branches/gcc-6-branch/libsanitizer/tsan/tsan_rtl.cc Author: trippels Date: Wed Nov 16 11:31:18 2016 New Revision: 242483 URL: https://gcc.gnu.org/viewcvs?rev=242483&root=gcc&view=rev Log: Fix PR78294 - thread sanitizer broken when using ld.gold When one uses ld.gold to build gcc, the thread sanitizer doesn't work, because gold is more conservative when applying TLS relaxations than ld.bfd. In this case a missing initial-exec attribute on a declaration causes gcc to assume the general dynamic model. With ld.bfd this gets relaxed to initial exec when linking the shared library, so the missing attribute doesn't matter. But ld.gold doesn't perform this optimization and this leads to crashes on tsan instrumented binaries. See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78294 and: https://sourceware.org/bugzilla/show_bug.cgi?id=20805 The fix is easy, just add the missing attribute. PR sanitizer/78294 * tsan/tsan_rtl.cc: Add missing attribute. Modified: branches/gcc-5-branch/libsanitizer/ChangeLog branches/gcc-5-branch/libsanitizer/tsan/tsan_rtl.cc Markus, Changes to sanitizer runtimes are not committed into gcc tree. The upstream is in llvm tree. Changes must go there first, then they are backported to gcc tree. Your change will be overwritten on next integrate. I've committed a similar change upstream: http://llvm.org/viewvc/llvm-project?view=revision&revision=287629 So for now we are safe. (In reply to Dmitry Vyukov from comment #23) > Markus, > > Changes to sanitizer runtimes are not committed into gcc tree. The upstream > is in llvm tree. Changes must go there first, then they are backported to > gcc tree. Your change will be overwritten on next integrate. > > I've committed a similar change upstream: > http://llvm.org/viewvc/llvm-project?view=revision&revision=287629 > So for now we are safe. Thanks. I was under the impression that upstream wouldn't be interested in this patch, because llvm uses static compiler-rt libs and clang doesn't run into this issue. > The question is if we want to change this or not and in what way.
I would say that we need to change something, because current behavior is counter-intuitive.
In tsan runtime we have a declaration with the attribute and definition without the attribute. For most of other things (e.g. static) attributes/qualifiers are propagated from declaration to definition.
I would do the obvious propagation automatically, and then maybe warn about all other cases (e.g. conflicting tls models, or addition of initial-exec after the variable was already used).
> I was under the impression that upstream wouldn't be interested in this patch,
because llvm uses static compiler-rt libs and clang doesn't run into this issue.
Upstream is not interested from this point of view. But we are interested very-very much in keeping code in sync to keep ourselves sane. As far as I know, runtime code is copied verbatim so far.
The regression is fixed. Discussion about what to do in duplicate decl handling for tls_model can be held in a non-regression PR. No that the fix is upstream, should r242480 be removed from libsanitizer/LOCAL_PATCHES? Sure. |
markus@x4 ~ % echo "int main(){}" | g++ -fsanitize=thread -x c++ - && ./a.out [1] 4123 segmentation fault ./a.out Program received signal SIGSEGV, Segmentation fault. 0x0000000000000000 in ?? () (gdb) bt #0 0x0000000000000000 in ?? () #1 0x00007ffff6f4312d in __interceptor___tls_get_addr (arg=0x7ffff6ff7f68) at ../../../../gcc/libsanitizer/tsan/tsan_interceptors.cc:2421 #2 0x00007ffff6f8b00a in __tsan::ScopedIgnoreInterceptors::ScopedIgnoreInterceptors (this=<synthetic pointer>) at ../../../../gcc/libsanitizer/tsan/tsan_rtl.h:549 #3 __tsan::Initialize (thr=thr@entry=0x7ffff6277780) at ../../../../gcc/libsanitizer/tsan/tsan_rtl.cc:331 #4 0x00007ffff6f43b90 in __tsan::ScopedInterceptor::ScopedInterceptor (this=this@entry=0x7fffffffe490, thr=0x7ffff6277780, pc=140737353072294, fname=0x7ffff6fc5eea "__cxa_atexit") at ../../../../gcc/libsanitizer/tsan/tsan_interceptors.cc:256 #5 0x00007ffff6f44e67 in __interceptor___cxa_atexit (f=f@entry=0x7ffff7efd480 <std::(anonymous namespace)::generic_error_category::~generic_error_category()>, arg=arg@entry=0x7ffff7ff2410 <std::(anonymous namespace)::generic_category_instance>, dso=0x7ffff7ff2280) at ../../../../gcc/libsanitizer/tsan/tsan_interceptors.cc:399 #6 0x00007ffff7efbea6 in __static_initialization_and_destruction_0 (__initialize_p=1, __priority=65535) at ../../../../gcc/libstdc++-v3/src/c++11/compatibility-c++0x.cc:214 #7 _GLOBAL__sub_I_compatibility_c__0x.cc(void) () at ../../../../gcc/libstdc++-v3/src/c++11/compatibility-c++0x.cc:257 #8 0x00007ffff7de73a3 in call_init (env=0x7fffffffe568, argv=0x7fffffffe558, argc=1, l=<optimized out>) at dl-init.c:72 #9 _dl_init (main_map=0x7ffff7ffe130, argc=1, argv=0x7fffffffe558, env=0x7fffffffe568) at dl-init.c:120 #10 0x00007ffff7dd6c5a in _dl_start_user () from /lib64/ld-linux-x86-64.so.2 #11 0x0000000000000001 in ?? () #12 0x00007fffffffe88a in ?? () #13 0x0000000000000000 in ?? () Using the static version libtsan.a works fine. markus@x4 ~ % uname -snrvmio Linux x4 4.9.0-rc3-00450-gffbcbfca846e-dirty #79 SMP Sat Nov 5 20:22:52 CET 2016 x86_64 AuthenticAMD GNU/Linux markus@x4 ~ % /lib64/libc-2.24.90.so GNU C Library (GNU libc) development release version 2.24.90, by Roland McGrath et al. Copyright (C) 2016 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Compiled by GNU CC version 7.0.0 20161031 (experimental). Available extensions: crypt add-on version 2.1 by Michael Glad and others GNU Libidn by Simon Josefsson Native POSIX Threads Library by Ulrich Drepper et al BIND-8.2.3-T5B libc ABIs: UNIQUE IFUNC For bug reporting instructions, please see: <http://www.gnu.org/software/libc/bugs.html>.