Clarify the order of frame summaries and rename getters

Document that frame summaries are bottom-to-top, i.e. caller before
callee, rename FrameSummary::GetFirst to FrameSummary::GetBottom and
introduce FrameSummary::GetTop.
For debugged JavaScript frames, it does not really matter which of the
functions we call, so I replaced a few GetFirst by GetTop instead of
GetBottom because it matches the semantics more closely.

This CL also reverts part of http://crrev.com/2621953002 by changing
BreakLocation::FromFrame back to accept a DebugInfo and a
JavaScriptFrame. We don't plan to create BreakLocations for wasm.

R=yangguo@chromium.org
BUG=v8:5822

Review-Url: https://codereview.chromium.org/2647433002
Cr-Commit-Position: refs/heads/master@{#42505}

src/debug/debug-scopes.cc

@@ -61,9 +61,10 @@ ScopeIterator::ScopeIterator(Isolate* isolate, FrameInspector* frame_inspector,
     // inspect the function scope.
     // This can only happen if we set a break point inside right before the
     // return, which requires a debug info to be available.
+    Handle<DebugInfo> debug_info(shared_info->GetDebugInfo());
 
     // Find the break point where execution has stopped.
-    BreakLocation location = BreakLocation::FromFrame(GetFrame());
+    BreakLocation location = BreakLocation::FromFrame(debug_info, GetFrame());
 
     ignore_nested_scopes = location.IsReturn();
   }

src/debug/debug.cc

@@ -59,15 +59,12 @@ Debug::Debug(Isolate* isolate)
   ThreadInit();
 }
 
-BreakLocation BreakLocation::FromFrame(StandardFrame* frame) {
-  // TODO(clemensh): Handle Wasm frames.
-  DCHECK(!frame->is_wasm());
-
-  auto summary = FrameSummary::GetFirst(frame).AsJavaScript();
+BreakLocation BreakLocation::FromFrame(Handle<DebugInfo> debug_info,
+                                       JavaScriptFrame* frame) {
+  auto summary = FrameSummary::GetTop(frame).AsJavaScript();
   int offset = summary.code_offset();
   Handle<AbstractCode> abstract_code = summary.abstract_code();
   if (abstract_code->IsCode()) offset = offset - 1;
-  Handle<DebugInfo> debug_info(summary.function()->shared()->GetDebugInfo());
   auto it = BreakIterator::GetIterator(debug_info, abstract_code);
   it->SkipTo(BreakIndexFromCodeOffset(debug_info, abstract_code, offset));
   return it->GetBreakLocation();
@@ -76,7 +73,7 @@ BreakLocation BreakLocation::FromFrame(StandardFrame* frame) {
 void BreakLocation::AllAtCurrentStatement(Handle<DebugInfo> debug_info,
                                           JavaScriptFrame* frame,
                                           List<BreakLocation>* result_out) {
-  auto summary = FrameSummary::GetFirst(frame).AsJavaScript();
+  auto summary = FrameSummary::GetTop(frame).AsJavaScript();
   int offset = summary.code_offset();
   Handle<AbstractCode> abstract_code = summary.abstract_code();
   if (abstract_code->IsCode()) offset = offset - 1;
@@ -521,18 +518,14 @@ void Debug::Break(JavaScriptFrame* frame) {
   Handle<JSFunction> function(frame->function());
   Handle<SharedFunctionInfo> shared(function->shared());
   if (!EnsureDebugInfo(shared, function)) return;
-
-  BreakLocation location = BreakLocation::FromFrame(frame);
-
-  // Find actual break points, if any, and trigger debug break event.
-  MaybeHandle<FixedArray> break_points_hit;
-  if (break_points_active()) {
-    // Get the debug info, which must exist if we reach here.
   Handle<DebugInfo> debug_info(shared->GetDebugInfo(), isolate_);
 
-    break_points_hit = CheckBreakPoints(debug_info, &location);
-  }
+  // Find the break location where execution has stopped.
+  BreakLocation location = BreakLocation::FromFrame(debug_info, frame);
 
+  // Find actual break points, if any, and trigger debug break event.
+  MaybeHandle<FixedArray> break_points_hit =
+      CheckBreakPoints(debug_info, &location);
   if (!break_points_hit.is_null()) {
     // Clear all current stepping setup.
     ClearStepping();
@@ -565,7 +558,7 @@ void Debug::Break(JavaScriptFrame* frame) {
       step_break = location.IsTailCall();
     // Fall through.
     case StepIn: {
-      FrameSummary summary = FrameSummary::GetFirst(frame);
+      FrameSummary summary = FrameSummary::GetTop(frame);
       step_break = step_break || location.IsReturn() || current_fp != last_fp ||
                    thread_local_.last_statement_position_ !=
                        summary.SourceStatementPosition();
@@ -1014,7 +1007,7 @@ void Debug::PrepareStep(StepAction step_action) {
   }
 
   // Get the debug info (create it if it does not exist).
-  auto summary = FrameSummary::GetFirst(frame).AsJavaScript();
+  auto summary = FrameSummary::GetTop(frame).AsJavaScript();
   Handle<JSFunction> function(summary.function());
   Handle<SharedFunctionInfo> shared(function->shared());
   if (!EnsureDebugInfo(shared, function)) {
@@ -1022,7 +1015,8 @@ void Debug::PrepareStep(StepAction step_action) {
     return;
   }
 
-  BreakLocation location = BreakLocation::FromFrame(frame);
+  Handle<DebugInfo> debug_info(shared->GetDebugInfo());
+  BreakLocation location = BreakLocation::FromFrame(debug_info, frame);
 
   // Any step at a return is a step-out.
   if (location.IsReturn()) step_action = StepOut;
@@ -1589,11 +1583,15 @@ void Debug::SetAfterBreakTarget(JavaScriptFrame* frame) {
 bool Debug::IsBreakAtReturn(JavaScriptFrame* frame) {
   HandleScope scope(isolate_);
 
+  // Get the executing function in which the debug break occurred.
+  Handle<SharedFunctionInfo> shared(frame->function()->shared());
+
   // With no debug info there are no break points, so we can't be at a return.
-  if (!frame->function()->shared()->HasDebugInfo()) return false;
+  if (!shared->HasDebugInfo()) return false;
 
   DCHECK(!frame->is_optimized());
-  BreakLocation location = BreakLocation::FromFrame(frame);
+  Handle<DebugInfo> debug_info(shared->GetDebugInfo());
+  BreakLocation location = BreakLocation::FromFrame(debug_info, frame);
   return location.IsReturn() || location.IsTailCall();
 }
 
@@ -2322,7 +2320,7 @@ void Debug::PrintBreakLocation() {
   JavaScriptFrameIterator iterator(isolate_);
   if (iterator.done()) return;
   JavaScriptFrame* frame = iterator.frame();
-  FrameSummary summary = FrameSummary::GetFirst(frame);
+  FrameSummary summary = FrameSummary::GetTop(frame);
   int source_position = summary.SourcePosition();
   Handle<Object> script_obj = summary.script();
   PrintF("[debug] break in function '");

src/debug/debug.h

@@ -77,7 +77,8 @@ const int kDebugPromiseFirstID = 1;
 
 class BreakLocation {
  public:
-  static BreakLocation FromFrame(StandardFrame* frame);
+  static BreakLocation FromFrame(Handle<DebugInfo> debug_info,
+                                 JavaScriptFrame* frame);
 
   static void AllAtCurrentStatement(Handle<DebugInfo> debug_info,
                                     JavaScriptFrame* frame,

src/frames.cc

@@ -1263,12 +1263,15 @@ FrameSummary::~FrameSummary() {
 #undef FRAME_SUMMARY_DESTR
 }
 
-FrameSummary FrameSummary::Get(const StandardFrame* frame, int index) {
-  DCHECK_LE(0, index);
+FrameSummary FrameSummary::GetTop(const StandardFrame* frame) {
   List<FrameSummary> frames(FLAG_max_inlining_levels + 1);
   frame->Summarize(&frames);
-  DCHECK_GT(frames.length(), index);
-  return frames[index];
+  DCHECK_LT(0, frames.length());
+  return frames.last();
+}
+
+FrameSummary FrameSummary::GetBottom(const StandardFrame* frame) {
+  return Get(frame, 0);
 }
 
 FrameSummary FrameSummary::GetSingle(const StandardFrame* frame) {
@@ -1278,6 +1281,14 @@ FrameSummary FrameSummary::GetSingle(const StandardFrame* frame) {
   return frames.first();
 }
 
+FrameSummary FrameSummary::Get(const StandardFrame* frame, int index) {
+  DCHECK_LE(0, index);
+  List<FrameSummary> frames(FLAG_max_inlining_levels + 1);
+  frame->Summarize(&frames);
+  DCHECK_GT(frames.length(), index);
+  return frames[index];
+}
+
 #define FRAME_SUMMARY_DISPATCH(ret, name)        \
   ret FrameSummary::name() const {               \
     switch (base_.kind()) {                      \
@@ -1781,7 +1792,7 @@ Script* WasmInterpreterEntryFrame::script() const {
 }
 
 int WasmInterpreterEntryFrame::position() const {
-  return FrameSummary::GetFirst(this).AsWasmInterpreted().SourcePosition();
+  return FrameSummary::GetBottom(this).AsWasmInterpreted().SourcePosition();
 }
 
 Address WasmInterpreterEntryFrame::GetCallerStackPointer() const {

src/frames.h

@@ -858,11 +858,10 @@ class FrameSummary BASE_EMBEDDED {
 
   ~FrameSummary();
 
-  static inline FrameSummary GetFirst(const StandardFrame* frame) {
-    return Get(frame, 0);
-  }
-  static FrameSummary Get(const StandardFrame* frame, int index);
+  static FrameSummary GetTop(const StandardFrame* frame);
+  static FrameSummary GetBottom(const StandardFrame* frame);
   static FrameSummary GetSingle(const StandardFrame* frame);
+  static FrameSummary Get(const StandardFrame* frame, int index);
 
   // Dispatched accessors.
   Handle<Object> receiver() const;
@@ -924,6 +923,8 @@ class StandardFrame : public StackFrame {
   virtual bool IsConstructor() const;
 
   // Build a list with summaries for this frame including all inlined frames.
+  // The functions are ordered bottom-to-top (i.e. summaries.last() is the
+  // top-most activation; caller comes before callee).
   virtual void Summarize(
       List<FrameSummary>* frames,
       FrameSummary::Mode mode = FrameSummary::kExactSummary) const;

src/objects.cc

@@ -13043,7 +13043,7 @@ void Script::SetEvalOrigin(Handle<Script> script,
     // position, but store it as negative value for lazy translation.
     StackTraceFrameIterator it(script->GetIsolate());
     if (!it.done() && it.is_javascript()) {
-      FrameSummary summary = FrameSummary::GetFirst(it.javascript_frame());
+      FrameSummary summary = FrameSummary::GetTop(it.javascript_frame());
       script->set_eval_from_shared(summary.AsJavaScript().function()->shared());
       script->set_eval_from_position(-summary.code_offset());
       return;