Don't try to read return address from the stack of another thread on Win64.

Don't try to read return address from the stack of another thread on Win64.

This is not safe unless we are inside the Dart code.

Remove incorrect code that was trying to guard against STATUS_GUARD_PAGE_VIOLATION by checking FP.

BUG=http://dartbug.com/20421
R=johnmccutchan@google.com

Committed: https://code.google.com/p/dart/source/detail?r=39035

[Vyacheslav Egorov (Google), 2014-08-08 12:47:55]

PTAL

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

https://codereview.chromium.org/451033002/diff/1/runtime/vm/profiler.cc#oldco...
runtime/vm/profiler.cc:1679: #if defined(TARGET_OS_WINDOWS)
I don't think this code was doing anything.

Stack is growing downwards hence GUARD page can only be reasoned to be below
last accessed SP (and thus FP --- if FP is really FP not some random stack
pointer).

If FP is at the beggining of the page FP+8 is on the same page not on the next
page. Hence if FP is really FP then FP+8 can't be on the guard page (it was
already written to).

Overall logic behind this check is unclear to me.

I would understand this guard if we were reading [FP-8] here. In any case this
is not sufficient to protect us from hitting guard page given how freely Win64
compiler uses RBP.

[Cutch, 2014-08-08 13:24:53]

lgtm

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

https://codereview.chromium.org/451033002/diff/1/runtime/vm/profiler.cc#oldco...
runtime/vm/profiler.cc:1679: #if defined(TARGET_OS_WINDOWS)
On 2014/08/08 12:47:54, Vyacheslav Egorov (Google wrote:
> I don't think this code was doing anything.
> 
> Stack is growing downwards hence GUARD page can only be reasoned to be below
> last accessed SP (and thus FP --- if FP is really FP not some random stack
> pointer).
> 
> If FP is at the beggining of the page FP+8 is on the same page not on the next
> page. Hence if FP is really FP then FP+8 can't be on the guard page (it was
> already written to).
> 
> Overall logic behind this check is unclear to me.
> 
> I would understand this guard if we were reading [FP-8] here. In any case this
> is not sufficient to protect us from hitting guard page given how freely Win64
> compiler uses RBP.

Your analysis is correct, this logic isn't entirely correct- but, this code was
successfully avoiding a similar flaky crash on Win32.

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

https://codereview.chromium.org/451033002/diff/1/runtime/vm/profiler.cc#newco...
runtime/vm/profiler.cc:2044: SetPCMarkerIfSafe(sample);
This should either be:

#if !defined(TARGET_OS_WINDOWS)
#endif

or keep the check in SetPCMarkerIfSafe

[Cutch, 2014-08-08 13:42:32]

patch set 2 lgtm

[Vyacheslav Egorov (Google), 2014-08-08 13:43:14]

Committed patchset #2 manually as 39035 (presubmit successful).