Commit 920796b3 authored by Leszek Swirski's avatar Leszek Swirski Committed by Commit Bot

Revert "[frames] Make interpreted frame detection stricter (reland)"

This reverts commit b7a036a6.

Reason for revert: We don't want to ever access the heap when walking the stack

Original change's description:
> [frames] Make interpreted frame detection stricter (reland)
> 
> When iterating over stack frames, make the interpreted frame detection
> require that the frame header contains the bytecode array.
> 
> Currently, the stack frame iterator supports bytecode handlers that
> don't create stack frames by checking if the top of the stack (i.e. the
> return address) is the interpreter entry trampoline. However, optimized
> code tail called from the interpreter entry trampoline can move the
> stack pointer without clearing the stack, which means it can end up with
> a pointer into the interpreter entry trampoline on the top of its stack
> (in an uninitialized value), and be interpreted as an interpreted frame.
> 
> To avoid such optimized code frames being interpreted as interpreted
> frames, we now additionally test the frame header, to see if it contains
> a valid pointer to a BytecodeArray.
> 
> Reland of https://chromium-review.googlesource.com/c/535646/
> 
> Change-Id: Iefbf305c9e4b43bebd2fc111663671d2b675e64a
> Reviewed-on: https://chromium-review.googlesource.com/536935
> Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#45959}

TBR=kozyatinskiy@chromium.org,leszeks@chromium.org

Change-Id: I52a62c8e11af4d1565af92f10113b955f8c2c2f2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/536938Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45960}
parent b7a036a6
......@@ -230,23 +230,14 @@ SafeStackFrameIterator::SafeStackFrameIterator(
reinterpret_cast<Address*>(StandardFrame::ComputePCAddress(fp)));
// If the top of stack is a return address to the interpreter trampoline,
// then we are likely in a bytecode handler with elided frame. Check if
// there is a bytecode array in the frame header, and if there is, case, set
// the PC properly and make sure we do not drop the frame.
// then we are likely in a bytecode handler with elided frame. In that
// case, set the PC properly and make sure we do not drop the frame.
if (IsValidStackAddress(sp)) {
MSAN_MEMORY_IS_INITIALIZED(sp, kPointerSize);
Address tos = ReadMemoryAt(reinterpret_cast<Address>(sp));
if (IsInterpreterFramePc(isolate, tos)) {
Address bytecode_array_pointer =
fp + InterpreterFrameConstants::kBytecodeArrayFromFp;
if (IsValidStackAddress(bytecode_array_pointer)) {
Address bytecode_array = Memory::Address_at(bytecode_array_pointer);
if (IsValidHeapAddress(bytecode_array, isolate) &&
reinterpret_cast<Object*>(bytecode_array)->IsBytecodeArray()) {
state.pc_address = reinterpret_cast<Address*>(sp);
advance_frame = false;
}
}
state.pc_address = reinterpret_cast<Address*>(sp);
advance_frame = false;
}
}
......@@ -312,21 +303,12 @@ void SafeStackFrameIterator::AdvanceOneFrame() {
}
}
bool SafeStackFrameIterator::IsValidHeapAddress(Address addr,
Isolate* isolate) const {
// We check HasHeapObjectTag to avoid failing DCHECKs when the input is not a
// tagged value (neither a valid Smi nor a valid HeapObject). We only do a
// range check on the address to avoid iterating through pages -- this might
// mean that the address is in some random or even unallocated page in the
// heap, but at least it won't segfault if we try to dereference it.
return Internals::HasHeapObjectTag(reinterpret_cast<Object*>(addr)) &&
!isolate->heap()->memory_allocator()->IsOutsideAllocatedSpace(addr);
}
bool SafeStackFrameIterator::IsValidFrame(StackFrame* frame) const {
return IsValidStackAddress(frame->sp()) && IsValidStackAddress(frame->fp());
}
bool SafeStackFrameIterator::IsValidCaller(StackFrame* frame) {
StackFrame::State state;
if (frame->is_entry() || frame->is_entry_construct()) {
......
......@@ -1632,7 +1632,6 @@ class SafeStackFrameIterator: public StackFrameIteratorBase {
bool IsValidStackAddress(Address addr) const {
return low_bound_ <= addr && addr <= high_bound_;
}
bool IsValidHeapAddress(Address addr, Isolate* isolate) const;
bool IsValidFrame(StackFrame* frame) const;
bool IsValidCaller(StackFrame* frame);
bool IsValidExitFrame(Address fp) const;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment