Fix crash in SafeStackFrameIterator related to native frames entry/exit

Fix crash in SafeStackFrameIterator related to native frames entry/exit

There might be several ExternalCallbackScope's created
during the native callback. Remove the assert that is not
aligned with that.

Moreover this iterator must work for any kind of
stacks including corrupted ones.

BUG=v8:4705
LOG=N

Committed: https://crrev.com/271f68ba026d252753ca9b2c947f4807b473cd08
Cr-Commit-Position: refs/heads/master@{#33751}

[alph, 2016-02-03 22:25:20]

alph@chromium.org changed reviewers:
+ bmeurer@chromium.org, titzer@chromium.org, yurys@chromium.org

[alph, 2016-02-03 22:25:20]

ptal

[Benedikt Meurer, 2016-02-04 04:54:34]

Description was changed from

==========
Fix crash in SafeStackFrameIterator related to native frames entry/exit

There might be several ExternalCallbackScope's created
during the native callback. Remove the assert that is not
aligned with that.

Moreover this iterator must work for any kind of
stacks including corrupted ones.

BUG=4705
LOG=N
==========

to

==========
Fix crash in SafeStackFrameIterator related to native frames entry/exit

There might be several ExternalCallbackScope's created
during the native callback. Remove the assert that is not
aligned with that.

Moreover this iterator must work for any kind of
stacks including corrupted ones.

BUG=v8:4705
LOG=N
==========

[Benedikt Meurer, 2016-02-04 04:55:42]

Thanks, I ran into this issue with C++ builtins recently, but I wasn't sure what
the exact issue is. LGTM.

[Benedikt Meurer, 2016-02-04 04:55:46]

The CQ bit was checked by bmeurer@chromium.org

[commit-bot: I haz the power, 2016-02-04 04:55:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1663193003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1663193003/1

[commit-bot: I haz the power, 2016-02-04 05:13:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-04 05:13:02]

Try jobs failed on following builders:
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/1...)

[titzer, 2016-02-04 09:24:27]

lgtm

[alph, 2016-02-04 18:55:00]

The CQ bit was checked by alph@chromium.org

[commit-bot: I haz the power, 2016-02-04 18:55:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1663193003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1663193003/1

[commit-bot: I haz the power, 2016-02-04 19:13:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-04 19:13:14]

Try jobs failed on following builders:
  v8_linux_nodcheck_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel/build...)

[alph, 2016-02-04 19:21:29]

Looks like --ignition flags breaks most of cpu profiler tests. :-(
Unfortunately they happen to be disabled at the moment because of an unrelated
flakes.

[alph, 2016-02-04 19:39:51]

The CQ bit was checked by alph@chromium.org

[alph, 2016-02-04 19:39:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/1663193003/#ps20001
(title: "Mark the test as failing under --ignition")

[commit-bot: I haz the power, 2016-02-04 19:40:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1663193003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1663193003/20001

[commit-bot: I haz the power, 2016-02-04 20:00:32]

Description was changed from

==========
Fix crash in SafeStackFrameIterator related to native frames entry/exit

There might be several ExternalCallbackScope's created
during the native callback. Remove the assert that is not
aligned with that.

Moreover this iterator must work for any kind of
stacks including corrupted ones.

BUG=v8:4705
LOG=N
==========

to

==========
Fix crash in SafeStackFrameIterator related to native frames entry/exit

There might be several ExternalCallbackScope's created
during the native callback. Remove the assert that is not
aligned with that.

Moreover this iterator must work for any kind of
stacks including corrupted ones.

BUG=v8:4705
LOG=N
==========

[commit-bot: I haz the power, 2016-02-04 20:00:33]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-04 20:00:56]

Description was changed from

==========
Fix crash in SafeStackFrameIterator related to native frames entry/exit

There might be several ExternalCallbackScope's created
during the native callback. Remove the assert that is not
aligned with that.

Moreover this iterator must work for any kind of
stacks including corrupted ones.

BUG=v8:4705
LOG=N
==========

to

==========
Fix crash in SafeStackFrameIterator related to native frames entry/exit

There might be several ExternalCallbackScope's created
during the native callback. Remove the assert that is not
aligned with that.

Moreover this iterator must work for any kind of
stacks including corrupted ones.

BUG=v8:4705
LOG=N

Committed: https://crrev.com/271f68ba026d252753ca9b2c947f4807b473cd08
Cr-Commit-Position: refs/heads/master@{#33751}
==========

[commit-bot: I haz the power, 2016-02-04 20:00:57]

Patchset 2 (id:??) landed as
https://crrev.com/271f68ba026d252753ca9b2c947f4807b473cd08
Cr-Commit-Position: refs/heads/master@{#33751}

[louderspace, 2016-03-25 20:31:09]

I was able to integrate the latest v8 and verify the fix on the mac.  But, I'm
getting a crash on windows :(

>	v8.dll!v8::internal::ExitFrame::ComputeStackPointer(unsigned char * fp) Line
614	C++
 	v8.dll!v8::internal::ExitFrame::GetStateForFramePointer(unsigned char * fp,
v8::internal::StackFrame::State * state) Line 606	C++

	v8.dll!v8::internal::EntryFrame::GetCallerState(v8::internal::StackFrame::State
* state) Line 554	C++

	v8.dll!v8::internal::EntryFrame::ComputeCallerState(v8::internal::StackFrame::State
* state) Line 541	C++

	v8.dll!v8::internal::SafeStackFrameIterator::IsValidCaller(v8::internal::StackFrame
* frame) Line 304	C++
 	v8.dll!v8::internal::SafeStackFrameIterator::AdvanceOneFrame() Line 256	C++
 	v8.dll!v8::internal::SafeStackFrameIterator::Advance() Line 321	C++
 	v8.dll!v8::internal::TickSample::GetStackSample(v8::internal::Isolate *
isolate, const v8::RegisterState & regs,
v8::internal::TickSample::RecordCEntryFrame record_c_entry_frame, void * *
frames, unsigned __int64 frames_limit, v8::SampleInfo * sample_info) Line
766	C++
 	v8.dll!v8::internal::TickSample::Init(v8::internal::Isolate * isolate, const
v8::RegisterState & regs, v8::internal::TickSample::RecordCEntryFrame
record_c_entry_frame, bool update_stats) Line 724	C++
 	v8.dll!v8::internal::Sampler::SampleStack(const v8::RegisterState & state)
Line 838	C++
 	v8.dll!v8::internal::Sampler::DoSample() Line 891	C++

As for this function:

Address ExitFrame::ComputeStackPointer(Address fp) {
  return Memory::Address_at(fp + ExitFrameConstants::kSPOffset);
}

 0xC0000005: Access violation reading location 0x00000016CBCFFFF0.

fp = 0x00000016cbd00000

Should I create a new bug, or should this be re-opened?  Any thoughts on why
windows would be have differently?

I'm using 5.1.215.  Thanks.  -Jim

[titzer, 2016-03-29 08:37:20]

On 2016/03/25 20:31:09, louderspace wrote:
> I was able to integrate the latest v8 and verify the fix on the mac.  But, I'm
> getting a crash on windows :(
> 
> >	v8.dll!v8::internal::ExitFrame::ComputeStackPointer(unsigned char * fp) Line
> 614	C++
>  	v8.dll!v8::internal::ExitFrame::GetStateForFramePointer(unsigned char * fp,
> v8::internal::StackFrame::State * state) Line 606	C++
> 
>
	v8.dll!v8::internal::EntryFrame::GetCallerState(v8::internal::StackFrame::State
> * state) Line 554	C++
> 
>
	v8.dll!v8::internal::EntryFrame::ComputeCallerState(v8::internal::StackFrame::State
> * state) Line 541	C++
> 
>
	v8.dll!v8::internal::SafeStackFrameIterator::IsValidCaller(v8::internal::StackFrame
> * frame) Line 304	C++
>  	v8.dll!v8::internal::SafeStackFrameIterator::AdvanceOneFrame() Line 256	C++
>  	v8.dll!v8::internal::SafeStackFrameIterator::Advance() Line 321	C++
>  	v8.dll!v8::internal::TickSample::GetStackSample(v8::internal::Isolate *
> isolate, const v8::RegisterState & regs,
> v8::internal::TickSample::RecordCEntryFrame record_c_entry_frame, void * *
> frames, unsigned __int64 frames_limit, v8::SampleInfo * sample_info) Line
> 766	C++
>  	v8.dll!v8::internal::TickSample::Init(v8::internal::Isolate * isolate, const
> v8::RegisterState & regs, v8::internal::TickSample::RecordCEntryFrame
> record_c_entry_frame, bool update_stats) Line 724	C++
>  	v8.dll!v8::internal::Sampler::SampleStack(const v8::RegisterState & state)
> Line 838	C++
>  	v8.dll!v8::internal::Sampler::DoSample() Line 891	C++
> 
> As for this function:
> 
> Address ExitFrame::ComputeStackPointer(Address fp) {
>   return Memory::Address_at(fp + ExitFrameConstants::kSPOffset);
> }
> 
>  0xC0000005: Access violation reading location 0x00000016CBCFFFF0.
> 
> fp = 0x00000016cbd00000
> 
> Should I create a new bug, or should this be re-opened?  Any thoughts on why
> windows would be have differently?
> 
> I'm using 5.1.215.  Thanks.  -Jim

Please create a new bug and reference this issue. Thanks!