From db8a105de7ef704daf93c6767ac4c77caeb48a63 Mon Sep 17 00:00:00 2001 From: Martin Kustermann Date: Thu, 18 Jun 2020 19:57:44 +0000 Subject: [PATCH] Revert "[vm/aot] Keep column information when possible for precompiled mode." This reverts commit 7c34a80c11cb882244c685b04e12a132598360c5. 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 > Reviewed-by: Ryan Macnak 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 Commit-Queue: Martin Kustermann --- pkg/native_stack_traces/CHANGELOG.md | 4 - pkg/native_stack_traces/lib/src/dwarf.dart | 51 +--- pkg/native_stack_traces/pubspec.yaml | 2 +- runtime/vm/code_descriptors.cc | 8 +- runtime/vm/dwarf.cc | 40 ++- runtime/vm/dwarf.h | 6 +- runtime/vm/kernel.cc | 20 ++ runtime/vm/kernel.h | 2 + runtime/vm/object.cc | 84 +------ runtime/vm/object.h | 9 +- .../dwarf_stack_trace_obfuscate_test.dart | 76 ------ tests/standalone/dwarf_stack_trace_test.dart | 228 ------------------ .../dwarf_stack_trace_obfuscate_test.dart | 7 +- .../standalone_2/dwarf_stack_trace_test.dart | 137 ++++++++--- 14 files changed, 170 insertions(+), 504 deletions(-) delete mode 100644 tests/standalone/dwarf_stack_trace_obfuscate_test.dart delete mode 100644 tests/standalone/dwarf_stack_trace_test.dart diff --git a/pkg/native_stack_traces/CHANGELOG.md b/pkg/native_stack_traces/CHANGELOG.md index 71ca8ba3c85..10e8638a56b 100644 --- a/pkg/native_stack_traces/CHANGELOG.md +++ b/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 diff --git a/pkg/native_stack_traces/lib/src/dwarf.dart b/pkg/native_stack_traces/lib/src/dwarf.dart index fd0911bb293..72f762394d5 100644 --- a/pkg/native_stack_traces/lib/src/dwarf.dart +++ b/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( 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. diff --git a/pkg/native_stack_traces/pubspec.yaml b/pkg/native_stack_traces/pubspec.yaml index cc0d2144073..b086b607a01 100644 --- a/pkg/native_stack_traces/pubspec.yaml +++ b/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 diff --git a/runtime/vm/code_descriptors.cc b/runtime/vm/code_descriptors.cc index f236a583cb7..1a5a5f2a146 100644 --- a/runtime/vm/code_descriptors.cc +++ b/runtime/vm/code_descriptors.cc @@ -574,16 +574,16 @@ CodeSourceMapPtr CodeSourceMapBuilder::Finalize() { void CodeSourceMapBuilder::WriteChangePosition(TokenPosition pos) { stream_.Write(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(static_cast(line)); } else { - stream_.Write(pos.value()); + stream_.Write(static_cast(pos.value())); } written_token_pos_stack_.Last() = pos; } diff --git a/runtime/vm/dwarf.cc b/runtime/vm/dwarf.cc index 0cdb092e3c0..cdee3d38956 100644 --- a/runtime/vm/dwarf.cc +++ b/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). diff --git a/runtime/vm/dwarf.h b/runtime/vm/dwarf.h index 115b888426c..dc1692c5c20 100644 --- a/runtime/vm/dwarf.h +++ b/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); diff --git a/runtime/vm/kernel.cc b/runtime/vm/kernel.cc index 84f456c95d9..d6663e33334 100644 --- a/runtime/vm/kernel.cc +++ b/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 { diff --git a/runtime/vm/kernel.h b/runtime/vm/kernel.h index 2ac42a1b311..f612540941c 100644 --- a/runtime/vm/kernel.h +++ b/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; diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 928a69f84b6..8e58e0fe9b8 100644 --- a/runtime/vm/object.cc +++ b/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; -// Can be used if PrecompiledPositionContainsColumn::decode(v) is true. -using PrecompiledPositionColumn = - BitField; -// 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; -// 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; - -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); diff --git a/runtime/vm/object.h b/runtime/vm/object.h index 11c218cf772..d87f4116232 100644 --- a/runtime/vm/object.h +++ b/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, diff --git a/tests/standalone/dwarf_stack_trace_obfuscate_test.dart b/tests/standalone/dwarf_stack_trace_obfuscate_test.dart deleted file mode 100644 index 5750ab5e964..00000000000 --- a/tests/standalone/dwarf_stack_trace_obfuscate_test.dart +++ /dev/null @@ -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 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 = >[ - // 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. -]; diff --git a/tests/standalone/dwarf_stack_trace_test.dart b/tests/standalone/dwarf_stack_trace_test.dart deleted file mode 100644 index 9cf2cb07383..00000000000 --- a/tests/standalone/dwarf_stack_trace_test.dart +++ /dev/null @@ -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 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 checkStackTrace(String rawStack, Dwarf dwarf, - List> 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 = >[]; - - 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().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 = >[ - // 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> gotInfo, List> 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 extractCallStrings(List> expectedCalls) { - var ret = []; - 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 parseUsingAddressRegExp(RegExp re, Iterable 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 absoluteAddresses(Iterable lines) => - parseUsingAddressRegExp(_absRE, lines); - -final _virtRE = RegExp(r'virt ([a-f\d]+)'); - -Iterable explicitVirtualAddresses(Iterable lines) => - parseUsingAddressRegExp(_virtRE, lines); - -final _dsoBaseRE = RegExp(r'isolate_dso_base: ([a-f\d]+)'); - -Iterable dsoBaseAddresses(Iterable lines) => - parseUsingAddressRegExp(_dsoBaseRE, lines); diff --git a/tests/standalone_2/dwarf_stack_trace_obfuscate_test.dart b/tests/standalone_2/dwarf_stack_trace_obfuscate_test.dart index 5750ab5e964..37fd3059eaa 100644 --- a/tests/standalone_2/dwarf_stack_trace_obfuscate_test.dart +++ b/tests/standalone_2/dwarf_stack_trace_obfuscate_test.dart @@ -24,7 +24,7 @@ foo() { } Future main() async { - String rawStack = ""; + String rawStack; try { foo(); } catch (e, st) { @@ -45,7 +45,7 @@ Future main() async { await base.checkStackTrace(rawStack, dwarf, expectedCallsInfo); } -final expectedCallsInfo = >[ +final expectedCallsInfo = >[ // 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 = >[ 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 = >[ 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 diff --git a/tests/standalone_2/dwarf_stack_trace_test.dart b/tests/standalone_2/dwarf_stack_trace_test.dart index 9cf2cb07383..dac3e580c36 100644 --- a/tests/standalone_2/dwarf_stack_trace_test.dart +++ b/tests/standalone_2/dwarf_stack_trace_test.dart @@ -24,7 +24,7 @@ foo() { } Future main() async { - String rawStack = ""; + String rawStack; try { foo(); } catch (e, st) { @@ -46,7 +46,10 @@ Future main() async { } Future checkStackTrace(String rawStack, Dwarf dwarf, - List> expectedCallsInfo) async { + List> expectedCallsInfo) async { + final expectedAllCallsInfo = expectedCallsInfo; + final expectedExternalCallInfo = removeInternalCalls(expectedCallsInfo); + print(""); print("Raw stack trace:"); print(rawStack); @@ -58,7 +61,7 @@ Future 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 checkStackTrace(String rawStack, Dwarf dwarf, Expect.deepEquals(explicits, virtualAddresses); } - final gotCallsInfo = >[]; + final externalFramesInfo = >[]; + final allFramesInfo = >[]; 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().toList()); + externalFramesInfo.add(dwarf.callInfoFor(addr)?.toList()); + allFramesInfo + .add(dwarf.callInfoFor(addr, includeInternalFrames: true)?.toList()); } print(""); @@ -98,40 +96,66 @@ Future 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 = >[ +final expectedCallsInfo = >[ // 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 = >[ 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 = >[ 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> removeInternalCalls(List> original) => + original + .map((frame) => frame.where((call) => !call.isInternal).toList()) + .toList(); + +void checkConsistency( + List> externalFrames, List> 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> gotInfo, List> expectedInfo) { + List> framesInfo, List> 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 extractCallStrings(List> expectedCalls) { Iterable parseUsingAddressRegExp(RegExp re, Iterable 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); + } } }