Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1045)

Issue 5303083: [google] ThreadSanitizer instrumentation pass (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 6 months ago by dvyukov
Modified:
12 years, 4 months ago
Reviewers:
Diego Novillo, davidxl, mjambor, gcc-patches
CC:
kcc1, gcc-patches_gcc.gnu.org
Base URL:
svn://gcc.gnu.org/svn/gcc/branches/google/main/
Visibility:
Public.

Patch Set 1 #

Total comments: 49

Patch Set 2 : fixes after the review #

Total comments: 3

Patch Set 3 : fix review comments #

Patch Set 4 : fix crashes #

Patch Set 5 : remove commented out code #

Total comments: 1

Patch Set 6 : use cgraph_node_for_asm + add per-translation-unit constructors + synch repo #

Patch Set 7 : Add ChangeLog #

Patch Set 8 : Delete garbage from ChangeLog #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1486 lines, -3 lines) Patch
M gcc/ChangeLog.google-main View 1 2 3 4 5 6 7 1 chunk +20 lines, -0 lines 0 comments Download
M gcc/Makefile.in View 1 2 3 4 5 6 3 chunks +8 lines, -1 line 0 comments Download
M gcc/common.opt View 1 2 3 4 5 6 1 chunk +8 lines, -0 lines 0 comments Download
M gcc/doc/invoke.texi View 1 2 3 4 5 6 4 chunks +14 lines, -2 lines 0 comments Download
M gcc/passes.c View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
A gcc/testsuite/gcc.dg/tsan.h View 1 2 3 4 5 1 chunk +101 lines, -0 lines 0 comments Download
A gcc/testsuite/gcc.dg/tsan-ignore.h View 1 chunk +5 lines, -0 lines 0 comments Download
A gcc/testsuite/gcc.dg/tsan-ignore.c View 1 2 1 chunk +49 lines, -0 lines 0 comments Download
A gcc/testsuite/gcc.dg/tsan-ignore.ignore View 1 chunk +7 lines, -0 lines 0 comments Download
A gcc/testsuite/gcc.dg/tsan-mop.c View 1 2 1 chunk +93 lines, -0 lines 0 comments Download
A gcc/testsuite/gcc.dg/tsan-stack.c View 1 2 1 chunk +34 lines, -0 lines 0 comments Download
M gcc/toplev.c View 1 2 3 4 5 6 2 chunks +5 lines, -0 lines 0 comments Download
M gcc/tree-pass.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
A gcc/tree-tsan.h View 1 2 3 4 5 1 chunk +26 lines, -0 lines 0 comments Download
A gcc/tree-tsan.c View 1 2 3 4 5 1 chunk +1114 lines, -0 lines 0 comments Download

Messages

Total messages: 42
dvyukov
The patch is for google/main branch. ThreadSanitizer is a data race detector for C/C++ programs. ...
12 years, 6 months ago (2011-10-28 13:25:14 UTC) #1
davidxl
Dmitriy, I will review it next week. Do you have some runtime overhead data ? ...
12 years, 6 months ago (2011-10-28 15:53:42 UTC) #2
Diego Novillo
[ Removing closed list from CCs, please trim CC in future replies. Thanks. ] On ...
12 years, 6 months ago (2011-10-28 15:55:45 UTC) #3
dvyukov
On Fri, Oct 28, 2011 at 7:53 PM, Xinliang David Li <davidxl@google.com>wrote: > Dmitriy, I ...
12 years, 6 months ago (2011-10-28 16:09:28 UTC) #4
davidxl
Have not done with reviewing. This is the first batch. David http://codereview.appspot.com/5303083/diff/1/gcc/passes.c File gcc/passes.c (right): ...
12 years, 6 months ago (2011-10-31 06:08:34 UTC) #5
dvyukov
Fixes after davidxl review. The patch is for google/main branch. 2011-10-31 Dmitriy Vyukov <dvyukov@google.com> * ...
12 years, 6 months ago (2011-10-31 12:43:50 UTC) #6
dvyukov
On Mon, Oct 31, 2011 at 10:08 AM, <davidxl@google.com> wrote: > Have not done with ...
12 years, 6 months ago (2011-10-31 12:54:35 UTC) #7
dvyukov
http://codereview.appspot.com/5303083/diff/3002/gcc/tree-tsan.c File gcc/tree-tsan.c (right): http://codereview.appspot.com/5303083/diff/3002/gcc/tree-tsan.c#newcode1082 gcc/tree-tsan.c:1082: gsi_insert_seq_before (&gsi, post_func_seq, GSI_SAME_STMT); Do I need to make ...
12 years, 6 months ago (2011-11-01 11:39:49 UTC) #8
dvyukov
On 2011/10/31 06:08:34, davidxl wrote: > http://codereview.appspot.com/5303083/diff/1/gcc/passes.c#newcode1423 > gcc/passes.c:1423: NEXT_PASS (pass_tsan); > Move this to ...
12 years, 6 months ago (2011-11-01 12:26:22 UTC) #9
davidxl
On Mon, Oct 31, 2011 at 5:54 AM, Dmitry Vyukov <dvyukov@google.com> wrote: > On Mon, ...
12 years, 6 months ago (2011-11-01 16:55:01 UTC) #10
davidxl
that means some existing bugs get exposed. Your previous version simply skipped the target mem ...
12 years, 6 months ago (2011-11-01 16:59:04 UTC) #11
davidxl
http://codereview.appspot.com/5303083/diff/3002/gcc/tree-tsan.c File gcc/tree-tsan.c (right): http://codereview.appspot.com/5303083/diff/3002/gcc/tree-tsan.c#newcode1075 gcc/tree-tsan.c:1075: for (eidx = 0; VEC_iterate (edge, exit_bb->preds, eidx, e); ...
12 years, 6 months ago (2011-11-01 17:20:11 UTC) #12
davidxl
http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c File gcc/tree-tsan.c (right): http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode432 gcc/tree-tsan.c:432: /* Builds either (__tsan_shadow_stack += 1) or (__tsan_shadow_stack -= ...
12 years, 6 months ago (2011-11-01 18:05:46 UTC) #13
mjambor_suse.cz
Hi, sorry that I'm not using the fancy web tool but I do not want ...
12 years, 6 months ago (2011-11-01 19:26:50 UTC) #14
Diego Novillo
On 11-11-01 15:26 , Martin Jambor wrote: > Hi, > > sorry that I'm not ...
12 years, 6 months ago (2011-11-01 19:29:54 UTC) #15
davidxl
On Tue, Nov 1, 2011 at 12:26 PM, Martin Jambor <mjambor@suse.cz> wrote: > Hi, > ...
12 years, 6 months ago (2011-11-01 19:37:32 UTC) #16
dvyukov
Fixes after davidxl review. The patch is for google/main branch. 2011-11-03 Dmitriy Vyukov <dvyukov@google.com> * ...
12 years, 6 months ago (2011-11-03 13:13:53 UTC) #17
dvyukov
On 2011/11/01 16:55:01, davidxl wrote: > > gcc/tree-tsan.c:161: tree __attribute__((weak)) > >> Explain this. > ...
12 years, 6 months ago (2011-11-03 13:18:03 UTC) #18
dvyukov
On 2011/11/01 16:59:04, davidxl wrote: > that means some existing bugs get exposed. It is ...
12 years, 6 months ago (2011-11-03 13:31:51 UTC) #19
dvyukov
On 2011/11/01 17:20:11, davidxl wrote: > http://codereview.appspot.com/5303083/diff/3002/gcc/tree-tsan.c > File gcc/tree-tsan.c (right): > > http://codereview.appspot.com/5303083/diff/3002/gcc/tree-tsan.c#newcode1075 > ...
12 years, 6 months ago (2011-11-03 13:32:49 UTC) #20
dvyukov
On 2011/11/01 18:05:46, davidxl wrote: > http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c > File gcc/tree-tsan.c (right): > > http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode432 > ...
12 years, 6 months ago (2011-11-03 13:38:01 UTC) #21
dvyukov
On 2011/11/01 19:26:50, mjambor_suse.cz wrote: > Hi, > > sorry that I'm not using the ...
12 years, 6 months ago (2011-11-03 13:39:00 UTC) #22
davidxl
> http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode911 >> >> gcc/tree-tsan.c:911: bbd->has_sb = 0; >> Where is the code that instrument ...
12 years, 6 months ago (2011-11-04 05:38:33 UTC) #23
davidxl
On Thu, Nov 3, 2011 at 6:31 AM, <dvyukov@google.com> wrote: > On 2011/11/01 16:59:04, davidxl ...
12 years, 6 months ago (2011-11-04 05:48:24 UTC) #24
dvyukov
On 2011/11/04 05:38:33, davidxl wrote: > > http://codereview.appspot.com/5303083/diff/1/gcc/tree-tsan.c#newcode911 > >> > >> gcc/tree-tsan.c:911: bbd->has_sb = ...
12 years, 5 months ago (2011-11-07 06:52:06 UTC) #25
dvyukov
On 2011/11/04 05:48:24, davidxl wrote: > On Thu, Nov 3, 2011 at 6:31 AM, <mailto:dvyukov@google.com> ...
12 years, 5 months ago (2011-11-07 06:53:23 UTC) #26
dvyukov
Fix crashes during instrumentation. The patch is for google/main branch. 2011-11-08 Dmitriy Vyukov <dvyukov@google.com> * ...
12 years, 5 months ago (2011-11-08 13:59:54 UTC) #27
dvyukov
Remove commented out code. The patch is for google/main branch. 2011-11-08 Dmitriy Vyukov <dvyukov@google.com> * ...
12 years, 5 months ago (2011-11-08 14:01:37 UTC) #28
dvyukov
On 2011/11/01 16:59:04, davidxl wrote: > that means some existing bugs get exposed. Your previous ...
12 years, 5 months ago (2011-11-08 14:03:18 UTC) #29
davidxl
http://codereview.appspot.com/5303083/diff/28001/gcc/tree-tsan.c File gcc/tree-tsan.c (right): http://codereview.appspot.com/5303083/diff/28001/gcc/tree-tsan.c#newcode227 gcc/tree-tsan.c:227: var = varpool_node_for_asm (id); Use cgraph_node_for_asm instead.
12 years, 5 months ago (2011-11-10 23:56:59 UTC) #30
davidxl
Have you run through SPEC, and SPEC06 with this change? What is the instrumentation overhead ...
12 years, 5 months ago (2011-11-11 00:00:35 UTC) #31
kcc1
On Thu, Nov 10, 2011 at 4:00 PM, <davidxl@google.com> wrote: > > Have you run ...
12 years, 5 months ago (2011-11-11 00:24:59 UTC) #32
davidxl
On Thu, Nov 10, 2011 at 4:24 PM, Kostya Serebryany <kcc@google.com> wrote: > > > ...
12 years, 5 months ago (2011-11-11 00:30:44 UTC) #33
kcc1
On Thu, Nov 10, 2011 at 4:30 PM, Xinliang David Li <davidxl@google.com>wrote: > On Thu, ...
12 years, 5 months ago (2011-11-11 00:41:18 UTC) #34
dvyukov
The patch is for google/main branch. ThreadSanitizer is a data race detector for C/C++ programs. ...
12 years, 5 months ago (2011-11-14 07:53:49 UTC) #35
dvyukov
On 2011/11/10 23:56:59, davidxl wrote: > http://codereview.appspot.com/5303083/diff/28001/gcc/tree-tsan.c > File gcc/tree-tsan.c (right): > > http://codereview.appspot.com/5303083/diff/28001/gcc/tree-tsan.c#newcode227 > ...
12 years, 5 months ago (2011-11-14 07:55:52 UTC) #36
dvyukov
On 2011/11/11 00:00:35, davidxl wrote: > Have you run through SPEC, and SPEC06 with this ...
12 years, 5 months ago (2011-11-14 07:59:54 UTC) #37
davidxl
Ok for google/main after compiler bootstrap and regression test (without ftsan), and some large tests ...
12 years, 5 months ago (2011-11-14 16:48:45 UTC) #38
dvyukov
On 2011/11/14 16:48:45, davidxl wrote: > Ok for google/main after compiler bootstrap and regression test ...
12 years, 5 months ago (2011-11-30 08:53:18 UTC) #39
Diego Novillo
On 11-11-30 03:53 , dvyukov@google.com wrote: > On 2011/11/14 16:48:45, davidxl wrote: >> Ok for ...
12 years, 5 months ago (2011-11-30 13:17:04 UTC) #40
dvyukov
On 2011/11/30 13:17:04, Diego Novillo wrote: > On 11-11-30 03:53 , mailto:dvyukov@google.com wrote: > > ...
12 years, 5 months ago (2011-11-30 14:09:53 UTC) #41
davidxl
12 years, 5 months ago (2011-11-30 17:44:56 UTC) #42
ok for google branches.

David

On Wed, Nov 30, 2011 at 6:09 AM,  <dvyukov@google.com> wrote:
> On 2011/11/30 13:17:04, Diego Novillo wrote:
>>
>> On 11-11-30 03:53 , mailto:dvyukov@google.com wrote:
>> > On 2011/11/14 16:48:45, davidxl wrote:
>> >> Ok for google/main after compiler bootstrap and regression test
>> >> (without ftsan), and some large tests with tsan turned on (as many
>
> as
>>
>> >> you can but at your discretion).
>> >
>> > Hi David,
>> >
>> > Perhaps a bit late question... but better late than never.
>> > I've added some unit tests, and I only tested them on Linux/x86 (and
>
> it
>>
>> > is the only platform that we currently care about). AFAIR I've seen
>
> some
>>
>> > sort of annotations to restrict tests to particular platforms. Does
>
> it
>>
>> > make sense to add them to the tests?
>
>> Yes, please.  See http://gcc.gnu.org/onlinedocs/gccint/Directives.html
>
>> for documentation on the directives you can use.  The testsuite/
>> directory is also full of examples you can cut-n-paste from.
>
> Here it is
> http://codereview.appspot.com/5437087/
>
>
> http://codereview.appspot.com/5303083/
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b