Fix source position for stack traces with optimized top function.

runtime/vm/stack_frame.cc

Index: runtime/vm/stack_frame.cc
===================================================================
--- runtime/vm/stack_frame.cc	(revision 17448)
+++ runtime/vm/stack_frame.cc	(working copy)
@@ -285,6 +285,7 @@
                          frame_(frame),
                          func_(Function::Handle()),
                          deopt_info_(DeoptInfo::Handle()),
+                         deopt_instructions_(),
                          object_table_(Array::Handle()) {
   ASSERT(frame_ != NULL);
   const Code& code = Code::Handle(frame_->LookupDartCode());
@@ -292,11 +293,18 @@
   func_ = code.function();
   intptr_t deopt_reason = kDeoptUnknown;
   deopt_info_ = code.GetDeoptInfoAtPc(frame_->pc(), &deopt_reason);
+
+  // Unpack deopt info into instructions (translate away suffixes).
+  const Array& deopt_table = Array::Handle(code.deopt_info_array());
+  ASSERT(!deopt_table.IsNull());
+  deopt_info_.ToInstructions(deopt_table, &deopt_instructions_);

[siva, 2013/01/28 22:46:55]

Is this some kind of code hoisting optimization you are trying to do in order to
avoid potential repeated
calls to deopt_info_.Instruction(..) as there are many inlined frames?

In that case it seems like a premature optimization to me, the trade off is
always between memory consumption and speed and I think in this case speed is
not critical.

[Florian Schneider, 2013/01/29 12:19:07]

No, this is not for optimization purposes: The raw deopt info contains
DeoptSuffixInstr instructions to compact redundant deopt infos. ToInstruction is
the helper to uncompress the DeoptSuffixInstr into individual deopt
instructions. It would be possible to do this while iterating, but this seems
unnecessarily complicated and duplicating the code we already have.

+
   object_table_ = code.object_table();
 }
 
 
-RawFunction* InlinedFunctionsInDartFrameIterator::GetNextFunction(uword* pc) {
+RawFunction* InlinedFunctionsInDartFrameIterator::GetNextFunction(uword* pc,
+                                                                  Code* code) {

[siva, 2013/01/28 22:46:55]

I try and avoid functions which have two return values one as a regular return
value and the other being returned as a parameter. Maybe we could change this to
always return the code object and then we get the function from it.

RawCode* InlinedFunctionsInDartFrameIterator::GetNextCode(uword* pc);

What do you think?

[Florian Schneider, 2013/01/29 12:19:07]

I agree. I'll get rid of one output parameter and make the iterator return the
function object as before, and extract the code via func.unoptimized_code() from
it.

   if (index_ == -1) {
     return Function::null();
   }
@@ -305,21 +313,22 @@
     // inlined functions to iterate over, we return the function.
     index_ = -1;  // No more functions.
     *pc = frame_->pc();
+    *code = func_.CurrentCode();
     return func_.raw();
   }
   // Iterate over the deopt instructions and determine the inlined
   // functions if any and iterate over them.
-  ASSERT(deopt_info_.Length() != 0);
-  while (index_ < deopt_info_.Length()) {
+  ASSERT(deopt_instructions_.length() != 0);
+  while (index_ < deopt_instructions_.length()) {
     intptr_t cur_index = index_;
     index_ += 1;
-    intptr_t deopt_instr = deopt_info_.Instruction(cur_index);
-    ASSERT(deopt_instr != DeoptInstr::kRetBeforeAddress);
-    if (deopt_instr == DeoptInstr::kRetAfterAddress) {
-      intptr_t deopt_from_index = deopt_info_.FromIndex(cur_index);
-      *pc = DeoptInstr::GetRetAfterAddress(deopt_from_index,
+    DeoptInstr* deopt_instr = deopt_instructions_[cur_index];
+    ASSERT(deopt_instr->kind() != DeoptInstr::kRetBeforeAddress);
+    if (deopt_instr->kind() == DeoptInstr::kRetAfterAddress) {
+      *pc = DeoptInstr::GetRetAfterAddress(deopt_instr,
                                            object_table_,
                                            &func_);
+      *code = func_.unoptimized_code();
       return func_.raw();
     }
   }