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

Issue 1708042: A lot of stuff.

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 7 months ago by Evgeniy Stepanov
Modified:
15 years, 7 months ago
Reviewers:
glider, kcc
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : Restored the DCHECK that in phb mode intersection of LSs is empty. #

Patch Set 3 : z #

Total comments: 14

Patch Set 4 : z #

Patch Set 5 : z #

Total comments: 8

Patch Set 6 : z #

Patch Set 7 : z #

Unified diffs Side-by-side diffs Delta from patch set Stats (+355 lines, -188 lines) Patch
M dynamic_annotations/dynamic_annotations.h View 1 2 3 4 5 13 chunks +142 lines, -102 lines 0 comments Download
M dynamic_annotations/dynamic_annotations.c View 1 2 3 4 1 chunk +73 lines, -67 lines 0 comments Download
M tsan/thread_sanitizer.h View 1 2 3 4 5 6 2 chunks +10 lines, -0 lines 0 comments Download
M tsan/thread_sanitizer.cc View 1 2 3 4 5 6 12 chunks +60 lines, -10 lines 0 comments Download
M tsan/ts_events.h View 1 chunk +1 line, -0 lines 0 comments Download
M tsan/ts_pin.cc View 2 chunks +7 lines, -0 lines 0 comments Download
M tsan/ts_valgrind.cc View 1 2 3 4 5 6 5 chunks +24 lines, -8 lines 0 comments Download
M tsan/ts_valgrind_client_requests.h View 1 chunk +2 lines, -1 line 0 comments Download
M tsan/ts_valgrind_intercepts.c View 1 chunk +7 lines, -0 lines 0 comments Download
M unittest/racecheck_unittest.cc View 3 4 5 1 chunk +29 lines, -0 lines 0 comments Download

Messages

Total messages: 6
Evgeniy Stepanov
This CL does 3 things: - Adds a new annotation that tells TSan that a ...
15 years, 7 months ago (2010-06-16 12:48:26 UTC) #1
kcc
http://codereview.appspot.com/1708042/diff/5001/4009 File dynamic_annotations/dynamic_annotations.c (right): http://codereview.appspot.com/1708042/diff/5001/4009#newcode59 dynamic_annotations/dynamic_annotations.c:59: void DYNAMIC_ANNOTATIONS_NAME(AnnotateRWLockReleased)(const char *file, int line, 80 chars per ...
15 years, 7 months ago (2010-06-18 11:28:45 UTC) #2
Evgeniy Stepanov
http://codereview.appspot.com/1708042/diff/5001/4009 File dynamic_annotations/dynamic_annotations.c (right): http://codereview.appspot.com/1708042/diff/5001/4009#newcode59 dynamic_annotations/dynamic_annotations.c:59: void DYNAMIC_ANNOTATIONS_NAME(AnnotateRWLockReleased)(const char *file, int line, On 2010/06/18 11:28:46, ...
15 years, 7 months ago (2010-06-18 11:50:15 UTC) #3
kcc
http://codereview.appspot.com/1708042/diff/14001/15002 File dynamic_annotations/dynamic_annotations.h (right): http://codereview.appspot.com/1708042/diff/14001/15002#newcode172 dynamic_annotations/dynamic_annotations.h:172: #define ANNOTATE_NON_PURE_HAPPENS_BEFORE_MUTEX(mu) \ Bad name, imho. Naming is hard... ...
15 years, 7 months ago (2010-06-18 12:05:05 UTC) #4
kcc
LGTM Please make sure that 32-bit build does not have warnings
15 years, 7 months ago (2010-06-18 12:41:53 UTC) #5
Evgeniy Stepanov
15 years, 7 months ago (2010-06-18 12:53:22 UTC) #6
http://codereview.appspot.com/1708042/diff/14001/15002
File dynamic_annotations/dynamic_annotations.h (right):

http://codereview.appspot.com/1708042/diff/14001/15002#newcode172
dynamic_annotations/dynamic_annotations.h:172: #define
ANNOTATE_NON_PURE_HAPPENS_BEFORE_MUTEX(mu) \
On 2010/06/18 12:05:05, kcc wrote:
> Bad name, imho. Naming is hard... 
> ANNOTATE_PURE_LOCKSET_MUTEX? 
> ANNOTATE_NO_HAPPENS_BEFORE_MUTEX? 

ANNOTATE_NOT_HAPPENS_BEFORE_MUTEX

http://codereview.appspot.com/1708042/diff/14001/15003
File tsan/thread_sanitizer.cc (right):

http://codereview.appspot.com/1708042/diff/14001/15003#newcode5576
tsan/thread_sanitizer.cc:5576: } else if (DEBUG_MODE) {
On 2010/06/18 12:05:05, kcc wrote:
> why need DEBUG_MODE given the DHECK? 

Indeed.

http://codereview.appspot.com/1708042/diff/14001/15003#newcode6696
tsan/thread_sanitizer.cc:6696: return addr < g_nacl_mem_start || addr >=
g_nacl_mem_start + 0x100000000ULL;
On 2010/06/18 12:05:05, kcc wrote:
> What happens if g_nacl_mem_start is still -1? 
> Please make a function like AddrIsInNaclUntrustedRegion and call it from here.

> Also, name the magic const something like kFourGig

Done.

http://codereview.appspot.com/1708042/diff/14001/15007
File tsan/ts_valgrind.cc (right):

http://codereview.appspot.com/1708042/diff/14001/15007#newcode562
tsan/ts_valgrind.cc:562: // Whether we are currently ignoring sync events for
the given thread at the given
On 2010/06/18 12:05:05, kcc wrote:
> 80 chars

Done.
Sign in to reply to this message.

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