[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/536935Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45959}

src/frames.cc

@@ -230,14 +230,23 @@ 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. In that
-    // 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. 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.
     if (IsValidStackAddress(sp)) {
       MSAN_MEMORY_IS_INITIALIZED(sp, kPointerSize);
       Address tos = ReadMemoryAt(reinterpret_cast<Address>(sp));
       if (IsInterpreterFramePc(isolate, tos)) {
-        state.pc_address = reinterpret_cast<Address*>(sp);
-        advance_frame = false;
+        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;
+          }
+        }
       }
     }
 
@@ -303,12 +312,21 @@ 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()) {

src/frames.h

@@ -1632,6 +1632,7 @@ 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;