[runtime] Unify and simplify how frames are marked

src/frames.cc

Index: src/frames.cc
diff --git a/src/frames.cc b/src/frames.cc
index 50a2e21a055d7a9d8f11442f6cfa024d555c71dc..fc6e91c64e716940b3458044f68d8047eddd88fa 100644
--- a/src/frames.cc
+++ b/src/frames.cc
@@ -216,9 +216,9 @@ SafeStackFrameIterator::SafeStackFrameIterator(
     // we check only that kMarkerOffset is within the stack bounds and do
     // compile time check that kContextOffset slot is pushed on the stack before
     // kMarkerOffset.
-    STATIC_ASSERT(StandardFrameConstants::kMarkerOffset <
+    STATIC_ASSERT(StandardFrameConstants::kFunctionOffset <
                   StandardFrameConstants::kContextOffset);
-    Address frame_marker = fp + StandardFrameConstants::kMarkerOffset;
+    Address frame_marker = fp + StandardFrameConstants::kFunctionOffset;
     if (IsValidStackAddress(frame_marker)) {
       type = StackFrame::ComputeType(this, &state);
       top_frame_type_ = type;
@@ -408,6 +408,34 @@ StackFrame::Type StackFrame::ComputeType(const StackFrameIteratorBase* iterator,
                                          State* state) {
   DCHECK(state->fp != NULL);
 
+  Object* marker = Memory::Object_at(
+      state->fp + CommonFrameConstants::kContextOrFrameTypeOffset);
+  if (marker->IsSmi()) {
+    StackFrame::Type candidate =
+        static_cast<StackFrame::Type>(Smi::cast(marker)->value());
+    switch (candidate) {
+      case ENTRY:
+      case ENTRY_CONSTRUCT:
+      case EXIT:
+      case STUB:
+      case STUB_FAILURE_TRAMPOLINE:
+      case INTERNAL:
+      case CONSTRUCT:
+      case ARGUMENTS_ADAPTOR:
+        return candidate;
+      case JAVA_SCRIPT:
+      case OPTIMIZED:
+      case INTERPRETED:
+      default:
+        // Unoptimized and optimized JavaScript frames, including interpreted
+        // frames, should never have a StackFrame::Type marker. If we find one,
+        // we're likely being called from the profiler in a bogus stack frame.
+        return NONE;
+    }
+  }
+
+  Object* maybe_function =
+      Memory::Object_at(state->fp + StandardFrameConstants::kFunctionOffset);
   if (!iterator->can_access_heap_objects_) {
     // TODO(titzer): "can_access_heap_objects" is kind of bogus. It really
     // means that we are being called from the profiler, which can interrupt
@@ -420,15 +448,8 @@ StackFrame::Type StackFrame::ComputeType(const StackFrameIteratorBase* iterator,
     MSAN_MEMORY_IS_INITIALIZED(
         state->fp + StandardFrameConstants::kMarkerOffset, kPointerSize);
 #endif
-    if (StandardFrame::IsArgumentsAdaptorFrame(state->fp)) {
-      // An adapter frame has a special SMI constant for the context and
-      // is not distinguished through the marker.
-      return ARGUMENTS_ADAPTOR;
-    }
-    Object* marker =
-        Memory::Object_at(state->fp + StandardFrameConstants::kMarkerOffset);
-    if (marker->IsSmi()) {
-      return static_cast<StackFrame::Type>(Smi::cast(marker)->value());
+    if (maybe_function->IsSmi()) {
+      return NONE;
     } else {
       return JAVA_SCRIPT;
     }
@@ -436,9 +457,6 @@ StackFrame::Type StackFrame::ComputeType(const StackFrameIteratorBase* iterator,
 
   // Look up the code object to figure out the type of the stack frame.
   Code* code_obj = GetContainingCode(iterator->isolate(), *(state->pc_address));
-
-  Object* marker =
-      Memory::Object_at(state->fp + StandardFrameConstants::kMarkerOffset);
   if (code_obj != nullptr) {
     switch (code_obj->kind()) {
       case Code::FUNCTION:
@@ -448,34 +466,18 @@ StackFrame::Type StackFrame::ComputeType(const StackFrameIteratorBase* iterator,
       case Code::WASM_FUNCTION:
         return STUB;
       case Code::BUILTIN:
-        if (!marker->IsSmi()) {
-          if (StandardFrame::IsArgumentsAdaptorFrame(state->fp)) {
-            // An adapter frame has a special SMI constant for the context and
-            // is not distinguished through the marker.
-            return ARGUMENTS_ADAPTOR;
-          } else {
-            // The interpreter entry trampoline has a non-SMI marker.
-            DCHECK(code_obj->is_interpreter_entry_trampoline() ||
-                   code_obj->is_interpreter_enter_bytecode_dispatch());
-            return INTERPRETED;
-          }
-        }
-        break;  // Marker encodes the frame type.
-      case Code::HANDLER:
-        if (!marker->IsSmi()) {
-          // Only hydrogen code stub handlers can have a non-SMI marker.
-          DCHECK(code_obj->is_hydrogen_stub());
-          return OPTIMIZED;
-        }
-        break;  // Marker encodes the frame type.
+        DCHECK(!maybe_function->IsSmi());
+        DCHECK(code_obj->is_interpreter_entry_trampoline() ||
+               code_obj->is_interpreter_enter_bytecode_dispatch());
+        return INTERPRETED;
       default:
-        break;  // Marker encodes the frame type.
+        // All other types should have an explicit marker
+        UNREACHABLE();
+        break;
     }
   }
 
-  // Didn't find a code object, or the code kind wasn't specific enough.
-  // The marker should encode the frame type.
-  return static_cast<StackFrame::Type>(Smi::cast(marker)->value());
+  return NONE;
 }
 
 
@@ -648,13 +650,46 @@ void StandardFrame::IterateCompiledFrame(ObjectVisitor* v) const {
   SafepointEntry safepoint_entry;
   Code* code = StackFrame::GetSafepointData(
       isolate(), pc(), &safepoint_entry, &stack_slots);
-  unsigned slot_space =
-      stack_slots * kPointerSize - StandardFrameConstants::kFixedFrameSize;
+  unsigned slot_space = stack_slots * kPointerSize;
 
   // Visit the outgoing parameters.

[Michael Starzinger, 2016/02/23 10:57:33]

nit: Comment is outdated.

[danno, 2016/03/07 09:33:38]

Done.

+  int frame_header_size = StandardFrameConstants::kFixedFrameSizeFromFp;
+  Object* marker = Memory::Object_at(fp() - kPointerSize);
+  if (marker->IsSmi()) {
+    StackFrame::Type candidate =
+        static_cast<StackFrame::Type>(Smi::cast(marker)->value());
+    switch (candidate) {
+      case ENTRY:
+      case ENTRY_CONSTRUCT:
+      case EXIT:
+      case STUB_FAILURE_TRAMPOLINE:
+      case ARGUMENTS_ADAPTOR:
+      case STUB:
+      case INTERNAL:
+      case CONSTRUCT:
+        frame_header_size = TypedFrameConstants::kFixedFrameSizeFromFp;
+        break;
+      case JAVA_SCRIPT:
+      case OPTIMIZED:
+      case INTERPRETED:
+        // These frame types have a context, but they are actually stored
+        // in the place on the stack that one finds the frame type.
+        UNREACHABLE();
+        break;
+      case NONE:
+      case NUMBER_OF_TYPES:
+      case MANUAL:
+        UNREACHABLE();
+        break;
+    }
+  }
+  slot_space -=
+      (frame_header_size + StandardFrameConstants::kFixedFrameSizeAboveFp);
+
+  Object** frame_header_base = &Memory::Object_at(fp() - frame_header_size);
+  Object** frame_header_limit = &Memory::Object_at(fp());
   Object** parameters_base = &Memory::Object_at(sp());
-  Object** parameters_limit = &Memory::Object_at(
-      fp() + JavaScriptFrameConstants::kFunctionOffset - slot_space);
+  Object** parameters_limit = frame_header_base - slot_space / kPointerSize;
 
   // Visit the parameters that may be on top of the saved registers.
   if (safepoint_entry.argument_count() > 0) {
@@ -706,10 +741,7 @@ void StandardFrame::IterateCompiledFrame(ObjectVisitor* v) const {
 
   // Visit the context in stub frame and JavaScript frame.
   // Visit the function in JavaScript frame.
-  Object** fixed_base = &Memory::Object_at(
-      fp() + StandardFrameConstants::kMarkerOffset);
-  Object** fixed_limit = &Memory::Object_at(fp());
-  v->VisitPointers(fixed_base, fixed_limit);
+  v->VisitPointers(frame_header_base, frame_header_limit);
 }
 
 
@@ -1437,10 +1469,10 @@ void InternalFrame::Iterate(ObjectVisitor* v) const {
 
 void StubFailureTrampolineFrame::Iterate(ObjectVisitor* v) const {
   Object** base = &Memory::Object_at(sp());
-  Object** limit = &Memory::Object_at(fp() +
-                                      kFirstRegisterParameterFrameOffset);
+  Object** limit = &Memory::Object_at(
+      fp() + StubFailureTrampolineFrameConstants::kFixedHeaderBottomOffset);
   v->VisitPointers(base, limit);
-  base = &Memory::Object_at(fp() + StandardFrameConstants::kMarkerOffset);
+  base = &Memory::Object_at(fp() + StandardFrameConstants::kFunctionOffset);
   const int offset = StandardFrameConstants::kLastObjectOffset;
   limit = &Memory::Object_at(fp() + offset) + 1;
   v->VisitPointers(base, limit);