Do not iterate stack handlers in SafeStackFrameIterator

Do not iterate stack handlers in SafeStackFrameIterator

CPU profiler doesn't use stack handlers so there is no need to iterate through them while traversing stack. This change SafeStackFrameIterator always iterate only frames and removes checks corresponding to the handlers iteration.

The problem described in the bug occurred because of a false assumption in SafeStackFrameIterator that if Isolate::c_entry_fp is not NULL then the top frame on the stack is always a C++ frame. It is false because we may have entered JS code again, in which case JS_ENTRY code stub generated by JSEntryStub::GenerateBody() will save current c_entry_fp value but not reset it to NULL and after that it will create ENTRY stack frame and JS_ENTRY handler on the stack and put the latter into Isolate::handler(top). This means that if we start iterating from c_entry_fp frame and try to compare the frame's sp with Isolate::handler()->address() it will turn out that frame->sp() > handler->address() and the condition in SafeStackFrameIterator::CanIterateHandles is not held.


BUG=252097
R=loislo@chromium.org, svenpanne@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=15348

[loislo, 2013-06-25 13:42:43]

lgtm

https://codereview.chromium.org/17589022/diff/1/test/cctest/test-cpu-profiler.cc
File test/cctest/test-cpu-profiler.cc (right):

https://codereview.chromium.org/17589022/diff/1/test/cctest/test-cpu-profiler...
test/cctest/test-cpu-profiler.cc:962: CheckChildrenNames(root, names);
Do you have a guarantee that profiler ever hit into foo?

[yurys, 2013-06-26 05:18:11]

https://codereview.chromium.org/17589022/diff/1/test/cctest/test-cpu-profiler.cc
File test/cctest/test-cpu-profiler.cc (right):

https://codereview.chromium.org/17589022/diff/1/test/cctest/test-cpu-profiler...
test/cctest/test-cpu-profiler.cc:962: CheckChildrenNames(root, names);
On 2013/06/25 13:42:43, loislo wrote:
> Do you have a guarantee that profiler ever hit into foo?

Yes, most of the time is spent inside the callback. I added a check that there
is |foo| node under |start|.

[Sven Panne, 2013-06-27 09:19:29]

lgtm

[yurys, 2013-06-27 09:28:47]

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