Revert "[vm/aot] Keep column information when possible for precompiled mode."

This reverts commit 7c34a80c11.

Reason for revert: This change seems to cause >2% code size regression
in flutter-release mode.

Original change's description:
> [vm/aot] Keep column information when possible for precompiled mode.
> 
> Previously, we passed line number information to the stack trace printer
> and to DWARF by changing the non-special positions in the CodeSourceMap
> to line numbers in precompiled mode. However, doing this lost column
> information.
> 
> We get the column information back in the majority of cases by encoding
> the line number and column information when neither is too large to pack
> together into 30 bits. (Here, 20 bits for line and 10 bits for column.)
> Otherwise, we just store the line information as before, though due to
> using a bit to encode whether column info exists, it's reduced to 30
> bits. If the line info is too big for that, we just return kNoSourcePos.
> 
> Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-nnbd-linux-release-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm_x64-try
> Change-Id: Id1c826f10871e2f304fa40a59d8b704404d3a2c9
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151507
> Commit-Queue: Tess Strickland <sstrickl@google.com>
> Reviewed-by: Ryan Macnak <rmacnak@google.com>

TBR=rmacnak@google.com,alexmarkov@google.com,sstrickl@google.com

Change-Id: I5cf97543d1ac2731bb27bdb58ae97af6f22f2cfb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-nnbd-linux-release-x64-try, vm-kernel-precomp-linux-release-x64-try, vm-kernel-precomp-linux-product-x64-try, vm-kernel-precomp-linux-debug-x64-try, vm-kernel-precomp-linux-release-simarm_x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151820
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>

pkg/native_stack_traces/CHANGELOG.md

@@ -1,9 +1,5 @@
 # Changelog
 
-## 0.3.8
-
-- Support columns when present in line number programs.
-
 ## 0.3.7
 
 - Added buildId accessor for retrieving GNU build IDs from DWARF files that

pkg/native_stack_traces/lib/src/dwarf.dart

@@ -313,11 +313,6 @@ class DebugInformationEntry {
 
   int get callLine => this[_AttributeName.callLine] as int;
 
-  // We don't assume that call columns are present for backwards compatibility.
-  int get callColumn => containsKey(_AttributeName.callColumn)
-      ? this[_AttributeName.callColumn] as int
-      : 0;
-
   List<CallInfo> callInfo(
       CompilationUnit unit, LineNumberProgram lineNumberProgram, int address) {
     String callFilename(int index) =>
@@ -338,8 +333,7 @@ class DebugInformationEntry {
               function: unit.nameOfOrigin(abstractOrigin),
               inlined: inlined,
               filename: callFilename(child.callFileIndex),
-              line: child.callLine,
-              column: child.callColumn));
+              line: child.callLine));
       }
     }
 
@@ -347,14 +341,12 @@ class DebugInformationEntry {
 
     final filename = lineNumberProgram.filename(address);
     final line = lineNumberProgram.lineNumber(address);
-    final column = lineNumberProgram.column(address);
     return [
       DartCallInfo(
           function: unit.nameOfOrigin(abstractOrigin),
           inlined: inlined,
           filename: filename,
-          line: line,
-          column: column)
+          line: line)
     ];
   }
 
@@ -988,8 +980,6 @@ class LineNumberProgram {
 
   int lineNumber(int address) => this[address]?.line;
 
-  int column(int address) => this[address]?.column;
-
   void writeToStringBuffer(StringBuffer buffer) {
     header.writeToStringBuffer(buffer);
 
@@ -1064,14 +1054,8 @@ class DartCallInfo extends CallInfo {
   final String function;
   final String filename;
   final int line;
-  final int column;
 
-  DartCallInfo(
-      {this.inlined = false,
-      this.function,
-      this.filename,
-      this.line,
-      this.column});
+  DartCallInfo({this.inlined = false, this.function, this.filename, this.line});
 
   @override
   bool get isInternal => false;
@@ -1079,12 +1063,9 @@ class DartCallInfo extends CallInfo {
   @override
   int get hashCode => _hashFinish(_hashCombine(
       _hashCombine(
-          _hashCombine(
-              _hashCombine(
-                  _hashCombine(0, inlined.hashCode), function.hashCode),
-              filename.hashCode),
-          line.hashCode),
-      column.hashCode));
+          _hashCombine(_hashCombine(0, inlined.hashCode), function.hashCode),
+          filename.hashCode),
+      line.hashCode));
 
   @override
   bool operator ==(Object other) {
@@ -1092,29 +1073,13 @@ class DartCallInfo extends CallInfo {
       return inlined == other.inlined &&
           function == other.function &&
           filename == other.filename &&
-          line == other.line &&
-          column == other.column;
+          line == other.line;
     }
     return false;
   }
 
-  void writeToStringBuffer(StringBuffer buffer) {
-    buffer..write(function)..write(' (')..write(filename);
-    if (line > 0) {
-      buffer..write(':')..write(line);
-      if (column > 0) {
-        buffer..write(':')..write(column);
-      }
-    }
-    buffer.write(')');
-  }
-
   @override
-  String toString() {
-    final buffer = StringBuffer();
-    writeToStringBuffer(buffer);
-    return buffer.toString();
-  }
+  String toString() => "${function} (${filename}${line <= 0 ? '' : ':$line'})";
 }
 
 /// Represents the information for a call site located in a Dart stub.

pkg/native_stack_traces/pubspec.yaml

@@ -1,6 +1,6 @@
 name: native_stack_traces
 description: Utilities for working with non-symbolic stack traces.
-version: 0.3.8
+version: 0.3.7
 
 homepage: https://github.com/dart-lang/sdk/tree/master/pkg/native_stack_traces
 

runtime/vm/code_descriptors.cc

@@ -574,16 +574,16 @@ CodeSourceMapPtr CodeSourceMapBuilder::Finalize() {
 void CodeSourceMapBuilder::WriteChangePosition(TokenPosition pos) {
   stream_.Write<uint8_t>(kChangePosition);
   if (FLAG_precompiled_mode) {
-    int32_t loc = TokenPosition::kNoSourcePos;
+    intptr_t line = -1;
     intptr_t inline_id = buffered_inline_id_stack_.Last();
     if (inline_id < inline_id_to_function_.length()) {
       const Function* function = inline_id_to_function_[inline_id];
       Script& script = Script::Handle(function->script());
-      loc = script.GetTokenLocationUsingLineStarts(pos.SourcePosition());
+      line = script.GetTokenLineUsingLineStarts(pos.SourcePosition());
     }
-    stream_.Write(loc);
+    stream_.Write<int32_t>(static_cast<int32_t>(line));
   } else {
-    stream_.Write<int32_t>(pos.value());
+    stream_.Write<int32_t>(static_cast<int32_t>(pos.value()));
   }
   written_token_pos_stack_.Last() = pos;
 }

runtime/vm/dwarf.cc

@@ -170,6 +170,14 @@ intptr_t Dwarf::AddCode(const Code& orig_code,
   return index;
 }
 
+intptr_t Dwarf::TokenPositionToLine(const TokenPosition& token_pos) {
+  // By the point we're creating the DWARF information, the values of
+  // non-special token positions have been converted to line numbers, so
+  // we just need to handle special (negative) token positions.
+  ASSERT(token_pos.value() > TokenPosition::kLast.value());
+  return token_pos.value() < 0 ? kNoLineInformation : token_pos.value();
+}
+
 intptr_t Dwarf::AddFunction(const Function& function) {
   RELEASE_ASSERT(!function.IsNull());
   FunctionIndexPair* pair = function_to_index_.Lookup(&function);
@@ -249,6 +257,8 @@ void Dwarf::WriteAbbreviations(DwarfWriteStream* stream) {
   stream->uleb128(DW_FORM_string);
   stream->uleb128(DW_AT_decl_file);
   stream->uleb128(DW_FORM_udata);
+  stream->uleb128(DW_AT_decl_line);
+  stream->uleb128(DW_FORM_udata);
   stream->uleb128(DW_AT_inline);
   stream->uleb128(DW_FORM_udata);
   stream->uleb128(0);
@@ -279,8 +289,6 @@ void Dwarf::WriteAbbreviations(DwarfWriteStream* stream) {
   stream->uleb128(DW_FORM_udata);
   stream->uleb128(DW_AT_call_line);
   stream->uleb128(DW_FORM_udata);
-  stream->uleb128(DW_AT_call_column);
-  stream->uleb128(DW_FORM_udata);
   stream->uleb128(0);
   stream->uleb128(0);  // End of attributes.
 
@@ -346,8 +354,8 @@ void Dwarf::WriteAbstractFunctions(DwarfWriteStream* stream) {
   String& name = String::Handle(zone_);
   stream->InitializeAbstractOrigins(functions_.length());
   // By the point we're creating DWARF information, scripts have already lost
-  // their token stream and we can't look up their line number or column
-  // information, hence the lack of DW_AT_decl_line and DW_AT_decl_column.
+  // their token stream, so we can't look up their line number information.
+  auto const line = kNoLineInformation;
   for (intptr_t i = 0; i < functions_.length(); i++) {
     const Function& function = *(functions_[i]);
     name = function.QualifiedUserVisibleName();
@@ -359,6 +367,7 @@ void Dwarf::WriteAbstractFunctions(DwarfWriteStream* stream) {
     stream->uleb128(kAbstractFunction);
     stream->string(name_cstr);        // DW_AT_name
     stream->uleb128(file);            // DW_AT_decl_file
+    stream->uleb128(line);            // DW_AT_decl_line
     stream->uleb128(DW_INL_inlined);  // DW_AT_inline
     stream->uleb128(0);               // End of children.
   }
@@ -504,13 +513,8 @@ void Dwarf::WriteInliningNode(DwarfWriteStream* stream,
   stream->OffsetFromSymbol(root_asm_name, node->end_pc_offset);
   // DW_AT_call_file
   stream->uleb128(file);
-  intptr_t line = kNoLineInformation;
-  intptr_t col = kNoColumnInformation;
-  Script::DecodePrecompiledPosition(token_pos, &line, &col);
   // DW_AT_call_line
-  stream->uleb128(line);
-  // DW_at_call_column
-  stream->uleb128(col);
+  stream->uleb128(TokenPositionToLine(token_pos));
 
   for (InliningNode* child = node->children_head; child != NULL;
        child = child->children_next) {
@@ -578,14 +582,10 @@ void Dwarf::WriteLineNumberProgram(DwarfWriteStream* stream) {
 
   // 6.2.5 The Line Number Program
 
-  // The initial values for the line number program state machine registers
-  // according to the DWARF standard.
-  intptr_t previous_pc_offset = 0;
   intptr_t previous_file = 1;
   intptr_t previous_line = 1;
-  intptr_t previous_column = 0;
-  // Other info not stored in the state machine registers.
   const char* previous_asm_name = nullptr;
+  intptr_t previous_pc_offset = 0;
 
   Function& root_function = Function::Handle(zone_);
   Script& script = Script::Handle(zone_);
@@ -641,20 +641,12 @@ void Dwarf::WriteLineNumberProgram(DwarfWriteStream* stream) {
           }
 
           // 2. Update LNP line.
-          auto const position = token_positions.Last();
-          intptr_t line = kNoLineInformation;
-          intptr_t column = kNoColumnInformation;
-          Script::DecodePrecompiledPosition(position, &line, &column);
+          const auto line = TokenPositionToLine(token_positions.Last());
           if (line != previous_line) {
             stream->u1(DW_LNS_advance_line);
             stream->sleb128(line - previous_line);
             previous_line = line;
           }
-          if (column != previous_column) {
-            stream->u1(DW_LNS_set_column);
-            stream->uleb128(column);
-            previous_column = column;
-          }
 
           // 3. Emit LNP row if the address register has been updated to a
           // non-zero value (dartbug.com/41756).

runtime/vm/dwarf.h

@@ -313,7 +313,6 @@ class Dwarf : public ZoneAllocated {
   static const intptr_t DW_LNS_advance_pc = 0x2;
   static const intptr_t DW_LNS_advance_line = 0x3;
   static const intptr_t DW_LNS_set_file = 0x4;
-  static const intptr_t DW_LNS_set_column = 0x5;
 
   static const intptr_t DW_LNE_end_sequence = 0x01;
   static const intptr_t DW_LNE_set_address = 0x02;
@@ -326,7 +325,10 @@ class Dwarf : public ZoneAllocated {
   };
 
   static constexpr intptr_t kNoLineInformation = 0;
-  static constexpr intptr_t kNoColumnInformation = 0;
+
+  // Returns the line number or kNoLineInformation if there is no line
+  // information available for the given token position.
+  static intptr_t TokenPositionToLine(const TokenPosition& token_pos);
 
   void WriteAbstractFunctions(DwarfWriteStream* stream);
   void WriteConcreteFunctions(DwarfWriteStream* stream);

runtime/vm/kernel.cc

@@ -36,6 +36,26 @@ KernelLineStartsReader::KernelLineStartsReader(
   }
 }
 
+intptr_t KernelLineStartsReader::LineNumberForPosition(
+    intptr_t position) const {
+  intptr_t line_count = line_starts_data_.Length();
+  intptr_t current_start = 0;
+  for (intptr_t i = 0; i < line_count; ++i) {
+    current_start += helper_->At(line_starts_data_, i);
+    if (current_start > position) {
+      // If current_start is greater than the desired position, it means that
+      // it is for the line after |position|. However, since line numbers
+      // start at 1, we just return |i|.
+      return i;
+    }
+
+    if (current_start == position) {
+      return i + 1;
+    }
+  }
+  return line_count;
+}
+
 void KernelLineStartsReader::LocationForPosition(intptr_t position,
                                                  intptr_t* line,
                                                  intptr_t* col) const {

runtime/vm/kernel.h

@@ -140,6 +140,8 @@ class KernelLineStartsReader {
     return helper_->At(line_starts_data_, index);
   }
 
+  intptr_t LineNumberForPosition(intptr_t position) const;
+
   void LocationForPosition(intptr_t position,
                            intptr_t* line,
                            intptr_t* col) const;

runtime/vm/object.cc

@@ -10755,84 +10755,26 @@ void Script::SetLocationOffset(intptr_t line_offset,
   StoreNonPointer(&raw_ptr()->col_offset_, col_offset);
 }
 
-// Whether a precompiled (non-special) position contains column information.
-using PrecompiledPositionContainsColumn = BitField<int32_t, bool, 0, 1>;
-// Can be used if PrecompiledPositionContainsColumn::decode(v) is true.
-using PrecompiledPositionColumn =
-    BitField<int32_t,
-             uint32_t,
-             PrecompiledPositionContainsColumn::kNextBit,
-             10>;
-// Can be used if PrecompiledPositionContainsColumn::decode(v) is true.
-// Does not include the sign bit, which should be 0 for encoded values.
-using PrecompiledPositionLine =
-    BitField<int32_t,
-             uint32_t,
-             PrecompiledPositionColumn::kNextBit,
-             (sizeof(int32_t) * kBitsPerByte - 1) -
-                 (PrecompiledPositionColumn::bitsize() +
-                  PrecompiledPositionContainsColumn::bitsize())>;
-// Can be used if PrecompiledPositionContainsColumn::decode(v) is false.
-// Does not include the sign bit, which should be 0 for encoded values.
-using PrecompiledPositionLineOnly =
-    BitField<int32_t,
-             uint32_t,
-             PrecompiledPositionContainsColumn::kNextBit,
-             (sizeof(int32_t) * kBitsPerByte - 1) -
-                 PrecompiledPositionContainsColumn::bitsize()>;
-
-bool Script::DecodePrecompiledPosition(TokenPosition token_pos,
-                                       intptr_t* line,
-                                       intptr_t* column) {
-  ASSERT(line != nullptr);
-  ASSERT(column != nullptr);
-  auto const value = token_pos.value();
-  if (value < 0) return false;
-  // We encode zero-based offsets from start, so convert back to ordinals.
-  if (PrecompiledPositionContainsColumn::decode(value)) {
-    *line = PrecompiledPositionLine::decode(value) + 1;
-    *column = PrecompiledPositionColumn::decode(value) + 1;
-  } else {
-    *line = PrecompiledPositionLineOnly::decode(value) + 1;
-  }
-  return true;
-}
-
 // Specialized for AOT compilation, which does this lookup for every token
 // position that could be part of a stack trace.
-int32_t Script::GetTokenLocationUsingLineStarts(
+intptr_t Script::GetTokenLineUsingLineStarts(
     TokenPosition target_token_pos) const {
-#if !defined(DART_PRECOMPILED_RUNTIME)
-  // Negative positions denote positions that do not correspond to Dart code.
-  if (target_token_pos.value() < 0) return TokenPosition::kNoSourcePos;
-
+  if (target_token_pos.IsNoSource()) {
+    return 0;
+  }
   Zone* zone = Thread::Current()->zone();
   TypedData& line_starts_data = TypedData::Handle(zone, line_starts());
   // Scripts loaded from bytecode may have null line_starts().
-  if (line_starts_data.IsNull()) return TokenPosition::kNoSourcePos;
-
-  kernel::KernelLineStartsReader line_starts_reader(line_starts_data, zone);
-  intptr_t line = -1;
-  intptr_t col = -1;
-  line_starts_reader.LocationForPosition(target_token_pos.value(), &line, &col);
-  // The line and column numbers returned are ordinals, so we shouldn't get 0.
-  ASSERT(line > 0);
-  ASSERT(col > 0);
-  // Encode the returned line and column numbers as 0-based offsets from start
-  // instead of ordinal numbers for better encoding.
-  line -= 1;
-  col -= 1;
-  if (PrecompiledPositionLine::is_valid(line) &&
-      PrecompiledPositionColumn::is_valid(col)) {
-    return PrecompiledPositionLine::encode(line) |
-           PrecompiledPositionColumn::encode(col) |
-           PrecompiledPositionContainsColumn::encode(true);
-  } else if (PrecompiledPositionLineOnly::is_valid(line)) {
-    return PrecompiledPositionLineOnly::encode(line) |
-           PrecompiledPositionContainsColumn::encode(false);
+  if (line_starts_data.IsNull()) {
+    return 0;
   }
+
+#if !defined(DART_PRECOMPILED_RUNTIME)
+  kernel::KernelLineStartsReader line_starts_reader(line_starts_data, zone);
+  return line_starts_reader.LineNumberForPosition(target_token_pos.value());
+#else
+  return 0;
 #endif  // !defined(DART_PRECOMPILED_RUNTIME)
-  return TokenPosition::kNoSourcePos;
 }
 
 #if !defined(DART_PRECOMPILED_RUNTIME)
@@ -23955,7 +23897,7 @@ static void PrintSymbolicStackFrame(Zone* zone,
   intptr_t line = -1;
   intptr_t column = -1;
   if (FLAG_precompiled_mode) {
-    Script::DecodePrecompiledPosition(token_pos, &line, &column);
+    line = token_pos.value();
   } else if (token_pos.IsSourcePosition()) {
     ASSERT(!script.IsNull());
     script.GetTokenLocation(token_pos.SourcePosition(), &line, &column);

runtime/vm/object.h

@@ -4495,14 +4495,7 @@ class Script : public Object {
 
   void SetLocationOffset(intptr_t line_offset, intptr_t col_offset) const;
 
-  // Decode line number and column information if present. Returns false if
-  // this is a special location and thus undecodable.
-  static bool DecodePrecompiledPosition(TokenPosition token_pos,
-                                        intptr_t* line,
-                                        intptr_t* column);
-  // For positions that have line numbers and columns, returns a non-negative
-  // value. Otherwise, returns -1.
-  int32_t GetTokenLocationUsingLineStarts(TokenPosition token_pos) const;
+  intptr_t GetTokenLineUsingLineStarts(TokenPosition token_pos) const;
   void GetTokenLocation(TokenPosition token_pos,
                         intptr_t* line,
                         intptr_t* column,

tests/standalone/dwarf_stack_trace_obfuscate_test.dart

@@ -1,76 +0,0 @@
-// Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-/// VMOptions=--dwarf-stack-traces --save-debugging-info=dwarf_obfuscate.so --obfuscate
-
-import 'dart:io';
-
-import 'package:native_stack_traces/native_stack_traces.dart';
-import 'package:path/path.dart' as path;
-
-import 'dwarf_stack_trace_test.dart' as base;
-
-@pragma("vm:prefer-inline")
-bar() {
-  // Keep the 'throw' and its argument on separate lines.
-  throw // force linebreak with dartfmt
-      "Hello, Dwarf!";
-}
-
-@pragma("vm:never-inline")
-foo() {
-  bar();
-}
-
-Future<void> main() async {
-  String rawStack = "";
-  try {
-    foo();
-  } catch (e, st) {
-    rawStack = st.toString();
-  }
-
-  if (path.basenameWithoutExtension(Platform.executable) !=
-      "dart_precompiled_runtime") {
-    return; // Not running from an AOT compiled snapshot.
-  }
-
-  if (Platform.isAndroid) {
-    return; // Generated dwarf.so not available on the test device.
-  }
-
-  final dwarf = Dwarf.fromFile("dwarf_obfuscate.so");
-
-  await base.checkStackTrace(rawStack, dwarf, expectedCallsInfo);
-}
-
-final expectedCallsInfo = <List<DartCallInfo>>[
-  // The first frame should correspond to the throw in bar, which was inlined
-  // into foo (so we'll get information for two calls for that PC address).
-  [
-    DartCallInfo(
-        function: "bar",
-        filename: "dwarf_stack_trace_obfuscate_test.dart",
-        line: 17,
-        column: 3,
-        inlined: true),
-    DartCallInfo(
-        function: "foo",
-        filename: "dwarf_stack_trace_obfuscate_test.dart",
-        line: 23,
-        column: 3,
-        inlined: false)
-  ],
-  // The second frame corresponds to call to foo in main.
-  [
-    DartCallInfo(
-        function: "main",
-        filename: "dwarf_stack_trace_obfuscate_test.dart",
-        line: 29,
-        column: 5,
-        inlined: false)
-  ],
-  // Don't assume anything about any of the frames below the call to foo
-  // in main, as this makes the test too brittle.
-];

tests/standalone/dwarf_stack_trace_test.dart

@@ -1,228 +0,0 @@
-// Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file
-// for details. All rights reserved. Use of this source code is governed by a
-// BSD-style license that can be found in the LICENSE file.
-
-/// VMOptions=--dwarf-stack-traces --save-debugging-info=dwarf.so
-
-import 'dart:convert';
-import 'dart:io';
-
-import 'package:native_stack_traces/native_stack_traces.dart';
-import 'package:path/path.dart' as path;
-import 'package:expect/expect.dart';
-
-@pragma("vm:prefer-inline")
-bar() {
-  // Keep the 'throw' and its argument on separate lines.
-  throw // force linebreak with dartfmt
-      "Hello, Dwarf!";
-}
-
-@pragma("vm:never-inline")
-foo() {
-  bar();
-}
-
-Future<void> main() async {
-  String rawStack = "";
-  try {
-    foo();
-  } catch (e, st) {
-    rawStack = st.toString();
-  }
-
-  if (path.basenameWithoutExtension(Platform.executable) !=
-      "dart_precompiled_runtime") {
-    return; // Not running from an AOT compiled snapshot.
-  }
-
-  if (Platform.isAndroid) {
-    return; // Generated dwarf.so not available on the test device.
-  }
-
-  final dwarf = Dwarf.fromFile("dwarf.so");
-
-  await checkStackTrace(rawStack, dwarf, expectedCallsInfo);
-}
-
-Future<void> checkStackTrace(String rawStack, Dwarf dwarf,
-    List<List<DartCallInfo>> expectedCallsInfo) async {
-  print("");
-  print("Raw stack trace:");
-  print(rawStack);
-
-  final rawLines =
-      await Stream.value(rawStack).transform(const LineSplitter()).toList();
-
-  final pcOffsets = collectPCOffsets(rawLines).toList();
-
-  // We should have at least enough PC addresses to cover the frames we'll be
-  // checking.
-  Expect.isTrue(pcOffsets.length >= expectedCallsInfo.length);
-
-  final virtualAddresses =
-      pcOffsets.map((o) => dwarf.virtualAddressOf(o)).toList();
-
-  // Some double-checks using other information in the non-symbolic stack trace.
-  final dsoBase = dsoBaseAddresses(rawLines).single;
-  final absolutes = absoluteAddresses(rawLines);
-  final relocatedAddresses = absolutes.map((a) => a - dsoBase);
-  final explicits = explicitVirtualAddresses(rawLines);
-
-  // Explicits will be empty if not generating ELF snapshots directly, which
-  // means we can't depend on virtual addresses in the snapshot lining up with
-  // those in the separate debugging information.
-  if (explicits.isNotEmpty) {
-    // Direct-to-ELF snapshots should have a build ID.
-    Expect.isNotNull(dwarf.buildId);
-    Expect.deepEquals(relocatedAddresses, virtualAddresses);
-    Expect.deepEquals(explicits, virtualAddresses);
-  }
-
-  final gotCallsInfo = <List<DartCallInfo>>[];
-
-  for (final addr in virtualAddresses) {
-    final externalCallInfo = dwarf.callInfoFor(addr);
-    Expect.isNotNull(externalCallInfo);
-    final allCallInfo = dwarf.callInfoFor(addr, includeInternalFrames: true);
-    Expect.isNotNull(allCallInfo);
-    for (final call in allCallInfo) {
-      Expect.isTrue(call is DartCallInfo, "got non-Dart call info ${call}");
-    }
-    Expect.deepEquals(externalCallInfo, allCallInfo);
-    gotCallsInfo.add(allCallInfo.cast<DartCallInfo>().toList());
-  }
-
-  print("");
-  print("Call information for PC addresses:");
-  for (var i = 0; i < virtualAddresses.length; i++) {
-    print("For PC 0x${virtualAddresses[i].toRadixString(16)}:");
-    print("  Calls corresponding to user or library code:");
-    gotCallsInfo[i].forEach((frame) => print("    ${frame}"));
-  }
-
-  checkFrames(gotCallsInfo, expectedCallsInfo);
-
-  final gotSymbolizedLines = await Stream.fromIterable(rawLines)
-      .transform(DwarfStackTraceDecoder(dwarf, includeInternalFrames: true))
-      .toList();
-
-  final gotSymbolizedCalls =
-      gotSymbolizedLines.where((s) => s.startsWith('#')).toList();
-
-  print("");
-  print("Symbolized stack trace:");
-  gotSymbolizedLines.forEach(print);
-  print("");
-  print("Extracted calls:");
-  gotSymbolizedCalls.forEach(print);
-
-  final expectedStrings = extractCallStrings(expectedCallsInfo);
-  // There are two strings in the list for each line in the output.
-  final expectedCallCount = expectedStrings.length ~/ 2;
-
-  Expect.isTrue(gotSymbolizedCalls.length >= expectedCallCount);
-
-  // Strip off any unexpected lines, so we can also make sure we didn't get
-  // unexpected calls prior to those calls we expect.
-  final gotCallsTrace =
-      gotSymbolizedCalls.sublist(0, expectedCallCount).join('\n');
-
-  Expect.stringContainsInOrder(gotCallsTrace, expectedStrings);
-}
-
-final expectedCallsInfo = <List<DartCallInfo>>[
-  // The first frame should correspond to the throw in bar, which was inlined
-  // into foo (so we'll get information for two calls for that PC address).
-  [
-    DartCallInfo(
-        function: "bar",
-        filename: "dwarf_stack_trace_test.dart",
-        line: 17,
-        column: 3,
-        inlined: true),
-    DartCallInfo(
-        function: "foo",
-        filename: "dwarf_stack_trace_test.dart",
-        line: 23,
-        column: 3,
-        inlined: false)
-  ],
-  // The second frame corresponds to call to foo in main.
-  [
-    DartCallInfo(
-        function: "main",
-        filename: "dwarf_stack_trace_test.dart",
-        line: 29,
-        column: 5,
-        inlined: false)
-  ],
-  // Don't assume anything about any of the frames below the call to foo
-  // in main, as this makes the test too brittle.
-];
-
-void checkFrames(
-    List<List<DartCallInfo>> gotInfo, List<List<DartCallInfo>> expectedInfo) {
-  // There may be frames below those we check.
-  Expect.isTrue(gotInfo.length >= expectedInfo.length);
-
-  // We can't just use deep equality, since we only have the filenames in the
-  // expected version, not the whole path, and we don't really care if
-  // non-positive line numbers match, as long as they're both non-positive.
-  for (var i = 0; i < expectedInfo.length; i++) {
-    for (var j = 0; j < expectedInfo[i].length; j++) {
-      final got = gotInfo[i][j];
-      final expected = expectedInfo[i][j];
-      Expect.equals(expected.function, got.function);
-      Expect.equals(expected.inlined, got.inlined);
-      Expect.equals(expected.filename, path.basename(got.filename));
-      if (expected.isInternal) {
-        Expect.isTrue(got.isInternal);
-      } else {
-        Expect.equals(expected.line, got.line);
-      }
-    }
-  }
-}
-
-List<String> extractCallStrings(List<List<CallInfo>> expectedCalls) {
-  var ret = <String>[];
-  for (final frame in expectedCalls) {
-    for (final call in frame) {
-      if (call is DartCallInfo) {
-        ret.add(call.function);
-        if (call.isInternal) {
-          ret.add("${call.filename}:??");
-        } else {
-          ret.add("${call.filename}:${call.line}");
-        }
-      }
-    }
-  }
-  return ret;
-}
-
-Iterable<int> parseUsingAddressRegExp(RegExp re, Iterable<String> lines) sync* {
-  for (final line in lines) {
-    final match = re.firstMatch(line);
-    if (match == null) continue;
-    final s = match.group(1);
-    if (s == null) continue;
-    yield int.parse(s, radix: 16);
-  }
-}
-
-final _absRE = RegExp(r'abs ([a-f\d]+)');
-
-Iterable<int> absoluteAddresses(Iterable<String> lines) =>
-    parseUsingAddressRegExp(_absRE, lines);
-
-final _virtRE = RegExp(r'virt ([a-f\d]+)');
-
-Iterable<int> explicitVirtualAddresses(Iterable<String> lines) =>
-    parseUsingAddressRegExp(_virtRE, lines);
-
-final _dsoBaseRE = RegExp(r'isolate_dso_base: ([a-f\d]+)');
-
-Iterable<int> dsoBaseAddresses(Iterable<String> lines) =>
-    parseUsingAddressRegExp(_dsoBaseRE, lines);

tests/standalone_2/dwarf_stack_trace_obfuscate_test.dart

@@ -24,7 +24,7 @@ foo() {
 }
 
 Future<void> main() async {
-  String rawStack = "";
+  String rawStack;
   try {
     foo();
   } catch (e, st) {
@@ -45,7 +45,7 @@ Future<void> main() async {
   await base.checkStackTrace(rawStack, dwarf, expectedCallsInfo);
 }
 
-final expectedCallsInfo = <List<DartCallInfo>>[
+final expectedCallsInfo = <List<CallInfo>>[
   // The first frame should correspond to the throw in bar, which was inlined
   // into foo (so we'll get information for two calls for that PC address).
   [
@@ -53,13 +53,11 @@ final expectedCallsInfo = <List<DartCallInfo>>[
         function: "bar",
         filename: "dwarf_stack_trace_obfuscate_test.dart",
         line: 17,
-        column: 3,
         inlined: true),
     DartCallInfo(
         function: "foo",
         filename: "dwarf_stack_trace_obfuscate_test.dart",
         line: 23,
-        column: 3,
         inlined: false)
   ],
   // The second frame corresponds to call to foo in main.
@@ -68,7 +66,6 @@ final expectedCallsInfo = <List<DartCallInfo>>[
         function: "main",
         filename: "dwarf_stack_trace_obfuscate_test.dart",
         line: 29,
-        column: 5,
         inlined: false)
   ],
   // Don't assume anything about any of the frames below the call to foo

tests/standalone_2/dwarf_stack_trace_test.dart

@@ -24,7 +24,7 @@ foo() {
 }
 
 Future<void> main() async {
-  String rawStack = "";
+  String rawStack;
   try {
     foo();
   } catch (e, st) {
@@ -46,7 +46,10 @@ Future<void> main() async {
 }
 
 Future<void> checkStackTrace(String rawStack, Dwarf dwarf,
-    List<List<DartCallInfo>> expectedCallsInfo) async {
+    List<List<CallInfo>> expectedCallsInfo) async {
+  final expectedAllCallsInfo = expectedCallsInfo;
+  final expectedExternalCallInfo = removeInternalCalls(expectedCallsInfo);
+
   print("");
   print("Raw stack trace:");
   print(rawStack);
@@ -58,7 +61,7 @@ Future<void> checkStackTrace(String rawStack, Dwarf dwarf,
 
   // We should have at least enough PC addresses to cover the frames we'll be
   // checking.
-  Expect.isTrue(pcOffsets.length >= expectedCallsInfo.length);
+  Expect.isTrue(pcOffsets.length >= expectedAllCallsInfo.length);
 
   final virtualAddresses =
       pcOffsets.map((o) => dwarf.virtualAddressOf(o)).toList();
@@ -79,18 +82,13 @@ Future<void> checkStackTrace(String rawStack, Dwarf dwarf,
     Expect.deepEquals(explicits, virtualAddresses);
   }
 
-  final gotCallsInfo = <List<DartCallInfo>>[];
+  final externalFramesInfo = <List<CallInfo>>[];
+  final allFramesInfo = <List<CallInfo>>[];
 
   for (final addr in virtualAddresses) {
-    final externalCallInfo = dwarf.callInfoFor(addr);
-    Expect.isNotNull(externalCallInfo);
-    final allCallInfo = dwarf.callInfoFor(addr, includeInternalFrames: true);
-    Expect.isNotNull(allCallInfo);
-    for (final call in allCallInfo) {
-      Expect.isTrue(call is DartCallInfo, "got non-Dart call info ${call}");
-    }
-    Expect.deepEquals(externalCallInfo, allCallInfo);
-    gotCallsInfo.add(allCallInfo.cast<DartCallInfo>().toList());
+    externalFramesInfo.add(dwarf.callInfoFor(addr)?.toList());
+    allFramesInfo
+        .add(dwarf.callInfoFor(addr, includeInternalFrames: true)?.toList());
   }
 
   print("");
@@ -98,40 +96,66 @@ Future<void> checkStackTrace(String rawStack, Dwarf dwarf,
   for (var i = 0; i < virtualAddresses.length; i++) {
     print("For PC 0x${virtualAddresses[i].toRadixString(16)}:");
     print("  Calls corresponding to user or library code:");
-    gotCallsInfo[i].forEach((frame) => print("    ${frame}"));
+    externalFramesInfo[i]?.forEach((frame) => print("    ${frame}"));
+    print("  All calls:");
+    allFramesInfo[i]?.forEach((frame) => print("    ${frame}"));
   }
 
-  checkFrames(gotCallsInfo, expectedCallsInfo);
+  // Check that our results are also consistent.
+  checkConsistency(externalFramesInfo, allFramesInfo);
 
-  final gotSymbolizedLines = await Stream.fromIterable(rawLines)
+  checkFrames(externalFramesInfo, expectedExternalCallInfo);
+  checkFrames(allFramesInfo, expectedAllCallsInfo);
+
+  final externalSymbolizedLines = await Stream.fromIterable(rawLines)
+      .transform(DwarfStackTraceDecoder(dwarf))
+      .toList();
+
+  final externalSymbolizedCalls =
+      externalSymbolizedLines.where((s) => s.startsWith('#')).toList();
+
+  print("");
+  print("Symbolized external-only stack trace:");
+  externalSymbolizedLines.forEach(print);
+  print("");
+  print("Extracted calls:");
+  externalSymbolizedCalls.forEach(print);
+
+  final allSymbolizedLines = await Stream.fromIterable(rawLines)
       .transform(DwarfStackTraceDecoder(dwarf, includeInternalFrames: true))
       .toList();
 
-  final gotSymbolizedCalls =
-      gotSymbolizedLines.where((s) => s.startsWith('#')).toList();
+  final allSymbolizedCalls =
+      allSymbolizedLines.where((s) => s.startsWith('#')).toList();
 
   print("");
-  print("Symbolized stack trace:");
-  gotSymbolizedLines.forEach(print);
+  print("Symbolized full stack trace:");
+  allSymbolizedLines.forEach(print);
   print("");
   print("Extracted calls:");
-  gotSymbolizedCalls.forEach(print);
+  allSymbolizedCalls.forEach(print);
 
-  final expectedStrings = extractCallStrings(expectedCallsInfo);
+  final expectedExternalStrings = extractCallStrings(expectedExternalCallInfo);
   // There are two strings in the list for each line in the output.
+  final expectedExternalCallCount = expectedExternalStrings.length ~/ 2;
+  final expectedStrings = extractCallStrings(expectedAllCallsInfo);
   final expectedCallCount = expectedStrings.length ~/ 2;
 
-  Expect.isTrue(gotSymbolizedCalls.length >= expectedCallCount);
+  Expect.isTrue(externalSymbolizedCalls.length >= expectedExternalCallCount);
+  Expect.isTrue(allSymbolizedCalls.length >= expectedCallCount);
 
   // Strip off any unexpected lines, so we can also make sure we didn't get
   // unexpected calls prior to those calls we expect.
-  final gotCallsTrace =
-      gotSymbolizedCalls.sublist(0, expectedCallCount).join('\n');
+  final externalCallsTrace =
+      externalSymbolizedCalls.sublist(0, expectedExternalCallCount).join('\n');
+  final allCallsTrace =
+      allSymbolizedCalls.sublist(0, expectedCallCount).join('\n');
 
-  Expect.stringContainsInOrder(gotCallsTrace, expectedStrings);
+  Expect.stringContainsInOrder(externalCallsTrace, expectedExternalStrings);
+  Expect.stringContainsInOrder(allCallsTrace, expectedStrings);
 }
 
-final expectedCallsInfo = <List<DartCallInfo>>[
+final expectedCallsInfo = <List<CallInfo>>[
   // The first frame should correspond to the throw in bar, which was inlined
   // into foo (so we'll get information for two calls for that PC address).
   [
@@ -139,13 +163,11 @@ final expectedCallsInfo = <List<DartCallInfo>>[
         function: "bar",
         filename: "dwarf_stack_trace_test.dart",
         line: 17,
-        column: 3,
         inlined: true),
     DartCallInfo(
         function: "foo",
         filename: "dwarf_stack_trace_test.dart",
         line: 23,
-        column: 3,
         inlined: false)
   ],
   // The second frame corresponds to call to foo in main.
@@ -154,25 +176,65 @@ final expectedCallsInfo = <List<DartCallInfo>>[
         function: "main",
         filename: "dwarf_stack_trace_test.dart",
         line: 29,
-        column: 5,
         inlined: false)
   ],
   // Don't assume anything about any of the frames below the call to foo
   // in main, as this makes the test too brittle.
 ];
 
+List<List<CallInfo>> removeInternalCalls(List<List<CallInfo>> original) =>
+    original
+        .map((frame) => frame.where((call) => !call.isInternal).toList())
+        .toList();
+
+void checkConsistency(
+    List<List<CallInfo>> externalFrames, List<List<CallInfo>> allFrames) {
+  // We should have the same number of frames for both external-only
+  // and combined call information.
+  Expect.equals(allFrames.length, externalFrames.length);
+
+  for (var frame in externalFrames) {
+    // There should be no frames in either version where we failed to look up
+    // call information.
+    Expect.isNotNull(frame);
+
+    // External-only call information should only include call information with
+    // positive line numbers.
+    for (var call in frame) {
+      Expect.isTrue(!call.isInternal);
+    }
+  }
+
+  for (var frame in allFrames) {
+    // There should be no frames in either version where we failed to look up
+    // call information.
+    Expect.isNotNull(frame);
+
+    // All frames in the internal-including version should have at least one
+    // piece of call information.
+    Expect.isTrue(frame.isNotEmpty);
+  }
+
+  // The information in the external-only and combined call information should
+  // be consistent for externally visible calls.
+  final allFramesStripped = removeInternalCalls(allFrames);
+  for (var i = 0; i < allFramesStripped.length; i++) {
+    Expect.listEquals(allFramesStripped[i], externalFrames[i]);
+  }
+}
+
 void checkFrames(
-    List<List<DartCallInfo>> gotInfo, List<List<DartCallInfo>> expectedInfo) {
+    List<List<CallInfo>> framesInfo, List<List<CallInfo>> expectedInfo) {
   // There may be frames below those we check.
-  Expect.isTrue(gotInfo.length >= expectedInfo.length);
+  Expect.isTrue(framesInfo.length >= expectedInfo.length);
 
   // We can't just use deep equality, since we only have the filenames in the
   // expected version, not the whole path, and we don't really care if
   // non-positive line numbers match, as long as they're both non-positive.
   for (var i = 0; i < expectedInfo.length; i++) {
     for (var j = 0; j < expectedInfo[i].length; j++) {
-      final got = gotInfo[i][j];
-      final expected = expectedInfo[i][j];
+      final DartCallInfo got = framesInfo[i][j];
+      final DartCallInfo expected = expectedInfo[i][j];
       Expect.equals(expected.function, got.function);
       Expect.equals(expected.inlined, got.inlined);
       Expect.equals(expected.filename, path.basename(got.filename));
@@ -205,10 +267,9 @@ List<String> extractCallStrings(List<List<CallInfo>> expectedCalls) {
 Iterable<int> parseUsingAddressRegExp(RegExp re, Iterable<String> lines) sync* {
   for (final line in lines) {
     final match = re.firstMatch(line);
-    if (match == null) continue;
-    final s = match.group(1);
-    if (s == null) continue;
-    yield int.parse(s, radix: 16);
+    if (match != null) {
+      yield int.parse(match.group(1), radix: 16);
+    }
   }
 }