Re: [PATCH] Add support for exceptions to tsan (PR sanitizer/64265)
Hi Jakub,
Do you plan to move forward with this patch? Is it waiting to be reviewed?
People keep asking about exception support for tsan (I guess it is
critical for a significant potion of C++ out there). I am trying to
sketch support in llvm. And I am leaning towards an approach similar
to yours, that is, add cleanup blocks in middle-end. I think it is
more portable and reliable then interception of __personality/_Unwind
routines.
On Mon, Dec 15, 2014 at 7:50 PM, Jakub Jelinek ja...@redhat.com wrote:
Hi!
As discussed in the PR, to support exceptions in -fsanitize=thread code,
it is desirable to call __tsan_func_exit also when leaving functions by
means of exceptions.
Adding EH too late sounds too hard to me, so this patch instead adds an
internal call during gimplification, makes sure the inliner removes the
internal calls from the inline functions
(we don't care about inlines, only about functions we emit), and
for functions that didn't go through gimplify_function_tree (e.g. omp/tm
etc. functions) just keeps using the old __tsan_func_exit additions.
Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk?
On:
#include pthread.h
int v;
int
foo (int x)
{
if (x 99)
throw x;
v++;
return x;
}
void *
tf (void *)
{
for (int i = 0; i 100; i++)
try { foo (i); } catch (int) {}
return NULL;
}
int
main ()
{
pthread_t th;
if (pthread_create (th, NULL, tf, NULL))
return 0;
v++;
pthread_join (th, NULL);
return 0;
}
I used to get without this patch:
==
WARNING: ThreadSanitizer: data race (pid=20449)
Read of size 4 at 0x006020e0 by thread T1:
#0 foo(int) /usr/src/gcc/obj/gcc/ts.C:10 (ts+0x00400cb9)
#1 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#2 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#3 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#4 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#5 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#6 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#7 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#8 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#9 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#10 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#11 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#12 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#13 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#14 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#15 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#16 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#17 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#18 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#19 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#20 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#21 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#22 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#23 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#24 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#25 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#26 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#27 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#28 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#29 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#30 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#31 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#32 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#33 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#34 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#35 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#36 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#37 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#38 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#39 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#40 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#41 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#42 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#43 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#44 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#45 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#46 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#47 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#48 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18
Re: [PATCH] Add support for exceptions to tsan (PR sanitizer/64265)
On Sat, Jun 27, 2015 at 04:53:22PM +0200, Dmitry Vyukov wrote: Do you plan to move forward with this patch? Is it waiting to be reviewed? People keep asking about exception support for tsan (I guess it is critical for a significant potion of C++ out there). I am trying to sketch support in llvm. And I am leaning towards an approach similar to yours, that is, add cleanup blocks in middle-end. I think it is more portable and reliable then interception of __personality/_Unwind routines. It is already in GCC 5.1 and on the trunk too. Jakub
Re: [PATCH] Add support for exceptions to tsan (PR sanitizer/64265)
On Sat, Jun 27, 2015 at 7:12 PM, Jakub Jelinek ja...@redhat.com wrote: On Sat, Jun 27, 2015 at 04:53:22PM +0200, Dmitry Vyukov wrote: Do you plan to move forward with this patch? Is it waiting to be reviewed? People keep asking about exception support for tsan (I guess it is critical for a significant potion of C++ out there). I am trying to sketch support in llvm. And I am leaning towards an approach similar to yours, that is, add cleanup blocks in middle-end. I think it is more portable and reliable then interception of __personality/_Unwind routines. It is already in GCC 5.1 and on the trunk too. Great! Thanks I've grepped excep.*tsan in ChangeLog, but found nothing. Nevermind
Re: [PATCH] Add support for exceptions to tsan (PR sanitizer/64265)
I am not qualified to review the actual code changes, but from the
description it looks good to me.
It adds a EH frame to every function, right? In 64-bit mode there is
no runtime penalty, right? Do you have any idea about binary size
increase? Does gcc build in C++ mode nowadays? It can be a good test.
I am somewhat worried about potential corner cases that can lead to
compiler crashes or missed/excessive func_entry/exit calls. But I
guess there is no way to make it working w/o getting some code in
first. Rigorous testing would require running a large C++ app that
actually throws and catches exception and precisely verifying stacks
in reports.
On Mon, Dec 15, 2014 at 9:50 PM, Jakub Jelinek ja...@redhat.com wrote:
Hi!
As discussed in the PR, to support exceptions in -fsanitize=thread code,
it is desirable to call __tsan_func_exit also when leaving functions by
means of exceptions.
Adding EH too late sounds too hard to me, so this patch instead adds an
internal call during gimplification, makes sure the inliner removes the
internal calls from the inline functions
(we don't care about inlines, only about functions we emit), and
for functions that didn't go through gimplify_function_tree (e.g. omp/tm
etc. functions) just keeps using the old __tsan_func_exit additions.
Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk?
On:
#include pthread.h
int v;
int
foo (int x)
{
if (x 99)
throw x;
v++;
return x;
}
void *
tf (void *)
{
for (int i = 0; i 100; i++)
try { foo (i); } catch (int) {}
return NULL;
}
int
main ()
{
pthread_t th;
if (pthread_create (th, NULL, tf, NULL))
return 0;
v++;
pthread_join (th, NULL);
return 0;
}
I used to get without this patch:
==
WARNING: ThreadSanitizer: data race (pid=20449)
Read of size 4 at 0x006020e0 by thread T1:
#0 foo(int) /usr/src/gcc/obj/gcc/ts.C:10 (ts+0x00400cb9)
#1 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#2 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#3 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#4 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#5 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#6 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#7 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#8 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#9 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#10 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#11 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#12 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#13 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#14 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#15 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#16 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#17 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#18 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#19 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#20 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#21 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#22 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#23 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#24 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#25 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#26 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#27 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#28 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#29 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#30 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#31 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#32 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#33 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#34 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#35 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#36 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#37 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#38 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#39 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#40 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#41 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#42 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#43 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#44 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#45 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18
Re: [PATCH] Add support for exceptions to tsan (PR sanitizer/64265)
Cross referenced this patch from
https://code.google.com/p/thread-sanitizer/issues/detail?id=78
On Tue, Dec 16, 2014 at 12:25 PM, Dmitry Vyukov dvyu...@google.com wrote:
I am not qualified to review the actual code changes, but from the
description it looks good to me.
It adds a EH frame to every function, right? In 64-bit mode there is
no runtime penalty, right? Do you have any idea about binary size
increase? Does gcc build in C++ mode nowadays? It can be a good test.
I am somewhat worried about potential corner cases that can lead to
compiler crashes or missed/excessive func_entry/exit calls. But I
guess there is no way to make it working w/o getting some code in
first. Rigorous testing would require running a large C++ app that
actually throws and catches exception and precisely verifying stacks
in reports.
On Mon, Dec 15, 2014 at 9:50 PM, Jakub Jelinek ja...@redhat.com wrote:
Hi!
As discussed in the PR, to support exceptions in -fsanitize=thread code,
it is desirable to call __tsan_func_exit also when leaving functions by
means of exceptions.
Adding EH too late sounds too hard to me, so this patch instead adds an
internal call during gimplification, makes sure the inliner removes the
internal calls from the inline functions
(we don't care about inlines, only about functions we emit), and
for functions that didn't go through gimplify_function_tree (e.g. omp/tm
etc. functions) just keeps using the old __tsan_func_exit additions.
Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk?
On:
#include pthread.h
int v;
int
foo (int x)
{
if (x 99)
throw x;
v++;
return x;
}
void *
tf (void *)
{
for (int i = 0; i 100; i++)
try { foo (i); } catch (int) {}
return NULL;
}
int
main ()
{
pthread_t th;
if (pthread_create (th, NULL, tf, NULL))
return 0;
v++;
pthread_join (th, NULL);
return 0;
}
I used to get without this patch:
==
WARNING: ThreadSanitizer: data race (pid=20449)
Read of size 4 at 0x006020e0 by thread T1:
#0 foo(int) /usr/src/gcc/obj/gcc/ts.C:10 (ts+0x00400cb9)
#1 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#2 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#3 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#4 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#5 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#6 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#7 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#8 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#9 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#10 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#11 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#12 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#13 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#14 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#15 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#16 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#17 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#18 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#19 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#20 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#21 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#22 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#23 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#24 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#25 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#26 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#27 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#28 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#29 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#30 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#31 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#32 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#33 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#34 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#35 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#36 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#37 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#38 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#39 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#40 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#41 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#42 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
Re: [PATCH] Add support for exceptions to tsan (PR sanitizer/64265)
On Mon, Dec 15, 2014 at 7:50 PM, Jakub Jelinek ja...@redhat.com wrote:
Hi!
As discussed in the PR, to support exceptions in -fsanitize=thread code,
it is desirable to call __tsan_func_exit also when leaving functions by
means of exceptions.
Adding EH too late sounds too hard to me, so this patch instead adds an
internal call during gimplification, makes sure the inliner removes the
internal calls from the inline functions
(we don't care about inlines, only about functions we emit), and
for functions that didn't go through gimplify_function_tree (e.g. omp/tm
etc. functions) just keeps using the old __tsan_func_exit additions.
Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk?
So the issue is externally throwing EH, right? I wonder if we can teach
the unwinder to do __tsan_func_exit instead, or have hooks in it that can
be used by tsan? At least the issue seems generic enough for
code instrumentation to consider a more general solution? How does
-finstrument-functions work with externally throwing EH?
Thanks,
Richard.
On:
#include pthread.h
int v;
int
foo (int x)
{
if (x 99)
throw x;
v++;
return x;
}
void *
tf (void *)
{
for (int i = 0; i 100; i++)
try { foo (i); } catch (int) {}
return NULL;
}
int
main ()
{
pthread_t th;
if (pthread_create (th, NULL, tf, NULL))
return 0;
v++;
pthread_join (th, NULL);
return 0;
}
I used to get without this patch:
==
WARNING: ThreadSanitizer: data race (pid=20449)
Read of size 4 at 0x006020e0 by thread T1:
#0 foo(int) /usr/src/gcc/obj/gcc/ts.C:10 (ts+0x00400cb9)
#1 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#2 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#3 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#4 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#5 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#6 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#7 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#8 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#9 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#10 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#11 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#12 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#13 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#14 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#15 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#16 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#17 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#18 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#19 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#20 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#21 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#22 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#23 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#24 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#25 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#26 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#27 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#28 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#29 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#30 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#31 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#32 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#33 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#34 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#35 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#36 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#37 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#38 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#39 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#40 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#41 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#42 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#43 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#44 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#45 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#46 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#47 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#48 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#49 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#50
Re: [PATCH] Add support for exceptions to tsan (PR sanitizer/64265)
On Tue, Dec 16, 2014 at 10:47:06AM +0100, Richard Biener wrote: On Mon, Dec 15, 2014 at 7:50 PM, Jakub Jelinek ja...@redhat.com wrote: Hi! As discussed in the PR, to support exceptions in -fsanitize=thread code, it is desirable to call __tsan_func_exit also when leaving functions by means of exceptions. Adding EH too late sounds too hard to me, so this patch instead adds an internal call during gimplification, makes sure the inliner removes the internal calls from the inline functions (we don't care about inlines, only about functions we emit), and for functions that didn't go through gimplify_function_tree (e.g. omp/tm etc. functions) just keeps using the old __tsan_func_exit additions. Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk? So the issue is externally throwing EH, right? I wonder if we can teach the unwinder to do __tsan_func_exit instead, or have hooks in it that can be used by tsan? At least the issue seems generic enough for code instrumentation to consider a more general solution? How does -finstrument-functions work with externally throwing EH? -finstrument-functions works the way I wrote the patch, in fact the gimplify_function_tree bits I've added were right after the -finstrument-functions handling of the same. Anyway, the alternative would be to wrap the various personality functions like __gcc_personality_v0 __gxx_personality_v0 __gcj_personality_v0 __gccgo_personality_v0 __gnu_objc_personality_v0 call the dlsym (, RTLD_NEXT) version from there and if it returns _URC_INSTALL_CONTEXT , try to figure out what frame it will be in. We can query the IP (_Unwind_GetIP), or the CFA (_Unwind_GetCFA), but then map it through supposedly target dependent code to the actual frame pointers __tsan_func_* store (do they?). The problem with wrapping those personality functions is that I'm not sure how could it work with the various -static* options. Say if you -static-libstdc++ (and link libtsan dynamically), then the __gxx_personality_v0 in the binary will not be wrapped by what is in libtsan.so. If you -static-libstdc++ -static-libtsan, then __gxx_personality_v0 linked in will be very likely from libstdc++ and thus again not overloaded, or if -ltsan would come first (right now it doesn't), then you still couldn't use dlsym to get at the overloaded symbol. So, while the __tsan_func_exit in cleanup is more expensive at runtime, it will work with all the wierdo options people are using. Jakub
Re: [PATCH] Add support for exceptions to tsan (PR sanitizer/64265)
On Tue, Dec 16, 2014 at 01:25:54PM +0400, Dmitry Vyukov wrote: I am not qualified to review the actual code changes, but from the description it looks good to me. It adds a EH frame to every function, right? In 64-bit mode there is no runtime penalty, right? Do you have any idea about binary size No, not to every function. Only to function that throws or could throw something externally, and only if exceptions are on. increase? Does gcc build in C++ mode nowadays? It can be a good test. GCC builds with C++, but with -fno-exceptions, so it is not a good example, but e.g. libstdc++, as it generally supports exceptions, could be an example. Jakub
Re: [PATCH] Add support for exceptions to tsan (PR sanitizer/64265)
On Tue, Dec 16, 2014 at 11:23 AM, Jakub Jelinek ja...@redhat.com wrote: On Tue, Dec 16, 2014 at 10:47:06AM +0100, Richard Biener wrote: On Mon, Dec 15, 2014 at 7:50 PM, Jakub Jelinek ja...@redhat.com wrote: Hi! As discussed in the PR, to support exceptions in -fsanitize=thread code, it is desirable to call __tsan_func_exit also when leaving functions by means of exceptions. Adding EH too late sounds too hard to me, so this patch instead adds an internal call during gimplification, makes sure the inliner removes the internal calls from the inline functions (we don't care about inlines, only about functions we emit), and for functions that didn't go through gimplify_function_tree (e.g. omp/tm etc. functions) just keeps using the old __tsan_func_exit additions. Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk? So the issue is externally throwing EH, right? I wonder if we can teach the unwinder to do __tsan_func_exit instead, or have hooks in it that can be used by tsan? At least the issue seems generic enough for code instrumentation to consider a more general solution? How does -finstrument-functions work with externally throwing EH? -finstrument-functions works the way I wrote the patch, in fact the gimplify_function_tree bits I've added were right after the -finstrument-functions handling of the same. Anyway, the alternative would be to wrap the various personality functions like __gcc_personality_v0 __gxx_personality_v0 __gcj_personality_v0 __gccgo_personality_v0 __gnu_objc_personality_v0 call the dlsym (, RTLD_NEXT) version from there and if it returns _URC_INSTALL_CONTEXT , try to figure out what frame it will be in. We can query the IP (_Unwind_GetIP), or the CFA (_Unwind_GetCFA), but then map it through supposedly target dependent code to the actual frame pointers __tsan_func_* store (do they?). I suppose we could annotate the CFA with appropriate information? The problem with wrapping those personality functions is that I'm not sure how could it work with the various -static* options. Say if you -static-libstdc++ (and link libtsan dynamically), then the __gxx_personality_v0 in the binary will not be wrapped by what is in libtsan.so. If you -static-libstdc++ -static-libtsan, then __gxx_personality_v0 linked in will be very likely from libstdc++ and thus again not overloaded, or if -ltsan would come first (right now it doesn't), then you still couldn't use dlsym to get at the overloaded symbol. Yeah, which is why I suggested that one might want to have a generic (list of) callbacks that can be registered (like malloc hooks). It would be all extensions but GCC controls the unwinding ABI, right? So, while the __tsan_func_exit in cleanup is more expensive at runtime, it will work with all the wierdo options people are using. True. As it follows existing practice with -finstrument-functions the patch is probably the way to go for GCC 5. I was just wondering of a less heavy-weight solution piggy-backing on the unwinder. I suppose that idea wouldn't work for SJLJ exceptions so you'd need both approaches anyway. Richard. Jakub
[PATCH] Add support for exceptions to tsan (PR sanitizer/64265)
Hi!
As discussed in the PR, to support exceptions in -fsanitize=thread code,
it is desirable to call __tsan_func_exit also when leaving functions by
means of exceptions.
Adding EH too late sounds too hard to me, so this patch instead adds an
internal call during gimplification, makes sure the inliner removes the
internal calls from the inline functions
(we don't care about inlines, only about functions we emit), and
for functions that didn't go through gimplify_function_tree (e.g. omp/tm
etc. functions) just keeps using the old __tsan_func_exit additions.
Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk?
On:
#include pthread.h
int v;
int
foo (int x)
{
if (x 99)
throw x;
v++;
return x;
}
void *
tf (void *)
{
for (int i = 0; i 100; i++)
try { foo (i); } catch (int) {}
return NULL;
}
int
main ()
{
pthread_t th;
if (pthread_create (th, NULL, tf, NULL))
return 0;
v++;
pthread_join (th, NULL);
return 0;
}
I used to get without this patch:
==
WARNING: ThreadSanitizer: data race (pid=20449)
Read of size 4 at 0x006020e0 by thread T1:
#0 foo(int) /usr/src/gcc/obj/gcc/ts.C:10 (ts+0x00400cb9)
#1 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#2 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#3 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#4 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#5 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#6 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#7 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#8 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#9 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#10 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#11 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#12 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#13 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#14 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#15 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#16 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#17 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#18 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#19 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#20 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#21 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#22 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#23 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#24 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#25 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#26 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#27 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#28 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#29 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#30 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#31 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#32 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#33 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#34 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#35 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#36 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#37 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#38 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#39 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#40 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#41 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#42 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#43 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#44 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#45 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#46 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#47 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#48 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#49 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#50 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#51 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#52 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#53 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#54 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#55 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#56 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x00400d13)
#57 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18
