Fix tsan failure

Fix tsan failure
We have a racy read of the object header in the stack frame walker
called from the profiler (in this case the gc marker is running at
the same time).

It is appropriate to avoid this even though we think the access is
safe as the objects are in old space and never forwarded and only
the mark bits are manipulated while marking
BUG=
R=johnmccutchan@google.com

Committed: https://github.com/dart-lang/sdk/commit/159a5ab3667b95a74d2be06d880bb75d74a895df

[siva, 2016-11-18 17:28:39]

asiva@google.com changed reviewers:
+ johnmccutchan@google.com, rmacnak@google.com

[Cutch, 2016-11-18 17:59:08]

LGTM wC

https://codereview.chromium.org/2517473002/diff/1/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/2517473002/diff/1/runtime/vm/profiler.cc#newc...
runtime/vm/profiler.cc:771:
AtomicOperations::IncrementInt64By(&counters->stack_walker_dart_exit, 1);
these two branches could be merged or maybe we want the counters to be distinct
still.

https://codereview.chromium.org/2517473002/diff/1/runtime/vm/profiler.cc#newc...
runtime/vm/profiler.cc:934: void Profiler::DumpStackTrace(bool
native_stack_trace) {
maybe remove the parameter?

https://codereview.chromium.org/2517473002/diff/1/runtime/vm/profiler_test.cc
File runtime/vm/profiler_test.cc (left):

https://codereview.chromium.org/2517473002/diff/1/runtime/vm/profiler_test.cc...
runtime/vm/profiler_test.cc:604: EXPECT_EQ(3, walker.CurrentExclusiveTicks());
why was this expect removed?

[siva, 2016-11-18 22:08:47]

https://codereview.chromium.org/2517473002/diff/1/runtime/vm/profiler.cc
File runtime/vm/profiler.cc (right):

https://codereview.chromium.org/2517473002/diff/1/runtime/vm/profiler.cc#newc...
runtime/vm/profiler.cc:771:
AtomicOperations::IncrementInt64By(&counters->stack_walker_dart_exit, 1);
On 2016/11/18 17:59:08, Cutch wrote:
> these two branches could be merged or maybe we want the counters to be
distinct
> still.

We would still have to change the condition to
else if (StubCode::HasBeenInitialized() &&
         (exited_dart_code || in_dart_code))
so much of a merge, we could leave the counters as is.

https://codereview.chromium.org/2517473002/diff/1/runtime/vm/profiler.cc#newc...
runtime/vm/profiler.cc:934: void Profiler::DumpStackTrace(bool
native_stack_trace) {
On 2016/11/18 17:59:08, Cutch wrote:
> maybe remove the parameter?

Done.

https://codereview.chromium.org/2517473002/diff/1/runtime/vm/profiler_test.cc
File runtime/vm/profiler_test.cc (left):

https://codereview.chromium.org/2517473002/diff/1/runtime/vm/profiler_test.cc...
runtime/vm/profiler_test.cc:604: EXPECT_EQ(3, walker.CurrentExclusiveTicks());
On 2016/11/18 17:59:08, Cutch wrote:
> why was this expect removed?

The expect was not removed, it was moved up to the Allocate A stub
frame because that is where it is counted.

[siva, 2016-11-18 22:09:06]

Description was changed from

==========
Fix tsan failure
We have a racy read of the object header in the stack frame walker
called from the profiler (in this case the gc marker is running at
the same time).

It is appropriate to avoid this even though we think the access is
safe as the objects are in old space and never forwarded and only
the mark bits are manipulated while marking
BUG=
==========

to

==========
Fix tsan failure
We have a racy read of the object header in the stack frame walker
called from the profiler (in this case the gc marker is running at
the same time).

It is appropriate to avoid this even though we think the access is
safe as the objects are in old space and never forwarded and only
the mark bits are manipulated while marking
BUG=
R=johnmccutchan@google.com

Committed:
https://github.com/dart-lang/sdk/commit/159a5ab3667b95a74d2be06d880bb75d74a895df
==========

[siva, 2016-11-18 22:09:08]

Committed patchset #2 (id:20001) manually as
159a5ab3667b95a74d2be06d880bb75d74a895df (presubmit successful).