[vm/elf] Reorder non-NOBITS sections before NOBITS sections.

This ensures that the relocated addresses of sections with file contents
are contained in the loaded segment.

Fixes https://github.com/dart-lang/sdk/issues/47289

TEST=vm/dart{,_2}/use_save_debugging_info_flag

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-release-x64-try
Change-Id: I6f7f94900ab1f9f0cb5ead4b0dd63bd2402e5a19
Fixed: 47289
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214800
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>

pkg/native_stack_traces/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.4.4
+
+- Added handling of dynamic tables for testing.
+
 ## 0.4.3
 
 - Exported some more of the ELF utilities for use in Dart tests.

pkg/native_stack_traces/lib/elf.dart

@@ -2,7 +2,7 @@
 // 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.
 
-export 'src/elf.dart' show Elf, Section, Symbol;
+export 'src/elf.dart' show DynamicTable, DynamicTableTag, Elf, Section, Symbol;
 export 'src/constants.dart'
     show
         isolateDataSymbolName,

pkg/native_stack_traces/lib/src/elf.dart

@@ -214,7 +214,10 @@ class ElfHeader {
   static const _ELFDATA2MSB = 0x02;
 
   void writeToStringBuffer(StringBuffer buffer) {
-    buffer..write('Format is ')..write(wordSize * 8)..write(' bits');
+    buffer
+      ..write('Format is ')
+      ..write(wordSize * 8)
+      ..write(' bits');
     switch (endian) {
       case Endian.little:
         buffer..writeln(' and little-endian');
@@ -342,6 +345,15 @@ class ProgramHeader {
   int get length => _entries.length;
   ProgramHeaderEntry operator [](int index) => _entries[index];
 
+  ProgramHeaderEntry? loadSegmentFor(int address) {
+    for (final entry in _entries) {
+      if (entry.vaddr <= address && address <= entry.vaddr + entry.memsz) {
+        return entry;
+      }
+    }
+    return null;
+  }
+
   static ProgramHeader fromReader(Reader reader, ElfHeader header) {
     final programReader = reader.refocusedCopy(
         header.programHeaderOffset, header.programHeaderSize);
@@ -352,7 +364,10 @@ class ProgramHeader {
 
   void writeToStringBuffer(StringBuffer buffer) {
     for (var i = 0; i < length; i++) {
-      if (i != 0) buffer..writeln()..writeln();
+      if (i != 0)
+        buffer
+          ..writeln()
+          ..writeln();
       buffer
         ..write('Entry ')
         ..write(i)
@@ -422,6 +437,17 @@ class SectionHeaderEntry {
   static const _SHT_NOBITS = 8;
   static const _SHT_DYNSYM = 11;
 
+  // sh_flags constants from ELF specification.
+  static const _SHF_WRITE = 0x1;
+  static const _SHF_ALLOC = 0x2;
+  static const _SHF_EXECINSTR = 0x4;
+
+  bool get isWritable => flags & _SHF_WRITE != 0;
+  bool get isAllocated => flags & _SHF_ALLOC != 0;
+  bool get isExecutable => flags & _SHF_EXECINSTR != 0;
+
+  bool get hasBits => type != _SHT_NOBITS;
+
   void setName(StringTable nameTable) {
     name = nameTable[nameIndex]!;
   }
@@ -495,7 +521,10 @@ class SectionHeader {
 
   void writeToStringBuffer(StringBuffer buffer) {
     for (var i = 0; i < entries.length; i++) {
-      if (i != 0) buffer..writeln()..writeln();
+      if (i != 0)
+        buffer
+          ..writeln()
+          ..writeln();
       buffer
         ..write('Entry ')
         ..write(i)
@@ -536,6 +565,8 @@ class Section {
         return SymbolTable.fromReader(reader, entry);
       case SectionHeaderEntry._SHT_NOTE:
         return Note.fromReader(reader, entry);
+      case SectionHeaderEntry._SHT_DYNAMIC:
+        return DynamicTable.fromReader(reader, entry);
       default:
         return Section._(entry);
     }
@@ -713,7 +744,10 @@ class Symbol {
   SymbolVisibility get visibility => SymbolVisibility.values[other & 0x03];
 
   void writeToStringBuffer(StringBuffer buffer) {
-    buffer..write('"')..write(name)..write('" =>');
+    buffer
+      ..write('"')
+      ..write(name)
+      ..write('" =>');
     switch (bind) {
       case SymbolBinding.STB_GLOBAL:
         buffer..write(' a global');
@@ -794,6 +828,109 @@ class SymbolTable extends Section {
   }
 }
 
+/// Represents d_tag constants from ELF specification.
+enum DynamicTableTag {
+  DT_NULL,
+  DT_NEEDED,
+  DT_PLTRELSZ,
+  DT_PLTGOT,
+  DT_HASH,
+  DT_STRTAB,
+  DT_SYMTAB,
+  DT_RELA,
+  DT_RELASZ,
+  DT_RELAENT,
+  DT_STRSZ,
+  DT_SYMENT,
+  // Later d_tag values are not currently used in Dart ELF files.
+}
+
+/// The dynamic table, which contains entries pointing to various relocated
+/// addresses.
+class DynamicTable extends Section {
+  // We don't use DynamicTableTag for the key so that we can handle ELF files
+  // that may use unknown (to us) tags.
+  final Map<int, int> _entries;
+  final _wordSize;
+
+  DynamicTable._(SectionHeaderEntry entry, this._entries, this._wordSize)
+      : super._(entry);
+
+  static DynamicTable fromReader(Reader reader, SectionHeaderEntry entry) {
+    final sectionReader = reader.refocusedCopy(entry.offset, entry.size);
+    final entries = <int, int>{};
+    while (true) {
+      // Each entry is a tag and a value, both native word sized.
+      final tag = _readElfNative(sectionReader);
+      final value = _readElfNative(sectionReader);
+      // A DT_NULL entry signfies the end of entries.
+      if (tag == DynamicTableTag.DT_NULL.index) break;
+      entries[tag] = value;
+    }
+    return DynamicTable._(entry, entries, sectionReader.wordSize);
+  }
+
+  int? operator [](DynamicTableTag tag) => _entries[tag.index];
+  bool containsKey(DynamicTableTag tag) => _entries.containsKey(tag.index);
+
+  // To avoid depending on EnumName.name from 2.15.
+  static const _tagStrings = {
+    DynamicTableTag.DT_NULL: 'DT_NULL',
+    DynamicTableTag.DT_NEEDED: 'DT_NEEDED',
+    DynamicTableTag.DT_PLTRELSZ: 'DT_PLTRELSZ',
+    DynamicTableTag.DT_PLTGOT: 'DT_PLTGOT',
+    DynamicTableTag.DT_HASH: 'DT_HASH',
+    DynamicTableTag.DT_STRTAB: 'DT_STRTAB',
+    DynamicTableTag.DT_SYMTAB: 'DT_SYMTAB',
+    DynamicTableTag.DT_RELA: 'DT_RELA',
+    DynamicTableTag.DT_RELASZ: 'DT_RELASZ',
+    DynamicTableTag.DT_STRSZ: 'DT_STRSZ',
+    DynamicTableTag.DT_SYMENT: 'DT_SYMENT',
+  };
+  static final _maxTagStringLength = (_tagStrings.values.toList()
+        ..sort((s1, s2) => s2.length - s1.length))
+      .first
+      .length;
+
+  @override
+  void writeToStringBuffer(StringBuffer buffer) {
+    buffer
+      ..write('Section "')
+      ..write(headerEntry.name)
+      ..writeln('" is a dynamic table:');
+    for (var kv in _entries.entries) {
+      buffer.write(' ');
+      if (kv.key < DynamicTableTag.values.length) {
+        final tag = DynamicTableTag.values[kv.key];
+        buffer
+          ..write(_tagStrings[tag]?.padRight(_maxTagStringLength))
+          ..write(' => ');
+        switch (tag) {
+          // These are relocated addresses.
+          case DynamicTableTag.DT_HASH:
+          case DynamicTableTag.DT_PLTGOT:
+          case DynamicTableTag.DT_SYMTAB:
+          case DynamicTableTag.DT_STRTAB:
+          case DynamicTableTag.DT_RELA:
+            buffer
+              ..write('0x')
+              ..writeln(paddedHex(kv.value, _wordSize));
+            break;
+          // Other entries are just values or offsets.
+          default:
+            buffer.writeln(kv.value);
+        }
+      } else {
+        buffer
+          ..write("Unknown tag ")
+          ..write(kv.key)
+          ..write(' => ')
+          ..writeln(kv.value);
+      }
+    }
+  }
+}
+
 /// Information parsed from an Executable and Linking Format (ELF) file.
 class Elf {
   final ElfHeader _header;
@@ -819,6 +956,21 @@ class Elf {
   Iterable<Section> namedSections(String name) =>
       _sectionsByName[name] ?? <Section>[];
 
+  /// Checks that the contents of a given section have valid addresses when the
+  /// file contents for the corresponding segment is loaded into memory.
+  ///
+  /// Returns false for sections that are not allocated or where the address
+  /// does not correspond to file contents (i.e., NOBITS sections).
+  bool sectionHasValidSegmentAddresses(Section section) {
+    final headerEntry = section.headerEntry;
+    if (!headerEntry.isAllocated || !headerEntry.hasBits) return false;
+    final segment = _programHeader.loadSegmentFor(headerEntry.addr);
+    if (segment == null) return false;
+    return (headerEntry.addr < (segment.vaddr + segment.filesz)) &&
+        (headerEntry.addr + headerEntry.size) <=
+            (segment.vaddr + segment.filesz);
+  }
+
   /// Lookup of a dynamic symbol by name.
   ///
   /// Returns -1 if there is no dynamic symbol that matches [name].

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.4.3
+version: 0.4.4
 
 homepage: https://github.com/dart-lang/sdk/tree/master/pkg/native_stack_traces
 

runtime/tests/vm/dart/use_save_debugging_info_flag_test.dart

@@ -13,6 +13,7 @@ import "dart:math";
 import "dart:typed_data";
 
 import 'package:expect/expect.dart';
+import 'package:native_stack_traces/elf.dart';
 import 'package:native_stack_traces/native_stack_traces.dart';
 import 'package:path/path.dart' as path;
 
@@ -63,6 +64,7 @@ main(List<String> args) async {
       '--elf=$scriptWholeSnapshot',
       scriptDill,
     ]);
+    checkElf(scriptWholeSnapshot);
 
     final scriptStrippedOnlySnapshot = path.join(tempDir, 'stripped_only.so');
     await run(genSnapshot, <String>[
@@ -72,6 +74,7 @@ main(List<String> args) async {
       '--strip',
       scriptDill,
     ]);
+    checkElf(scriptStrippedOnlySnapshot);
 
     final scriptStrippedSnapshot = path.join(tempDir, 'stripped.so');
     final scriptDebuggingInfo = path.join(tempDir, 'debug.so');
@@ -83,6 +86,8 @@ main(List<String> args) async {
       '--save-debugging-info=$scriptDebuggingInfo',
       scriptDill,
     ]);
+    checkElf(scriptStrippedSnapshot);
+    checkElf(scriptDebuggingInfo);
 
     // Run the resulting scripts, saving the stack traces.
     final wholeTrace = await runError(aotRuntime, <String>[
@@ -184,3 +189,46 @@ void printDiff(Map<int, List<int>> map, [int maxOutput = 100]) {
     }
   }
 }
+
+void checkElf(String filename) {
+  print("Checking ELF file $filename:");
+  final elf = Elf.fromFile(filename);
+  Expect.isNotNull(elf);
+  final dynamic = elf!.namedSections(".dynamic").single as DynamicTable;
+
+  // Check the dynamic string table information.
+  Expect.isTrue(
+      dynamic.containsKey(DynamicTableTag.DT_STRTAB), "no string table entry");
+  final dynstr = elf.namedSections(".dynstr").single;
+  print(".dynstr address = ${dynamic[DynamicTableTag.DT_STRTAB]}");
+  Expect.isTrue(elf.sectionHasValidSegmentAddresses(dynstr),
+      "string table addresses are invalid");
+  Expect.equals(dynamic[DynamicTableTag.DT_STRTAB], dynstr.headerEntry.addr);
+  Expect.isTrue(dynamic.containsKey(DynamicTableTag.DT_STRSZ),
+      "no string table size entry");
+  print(".dynstr size = ${dynamic[DynamicTableTag.DT_STRSZ]}");
+  Expect.equals(dynamic[DynamicTableTag.DT_STRSZ], dynstr.headerEntry.size);
+
+  // Check the dynamic symbol table information.
+  Expect.isTrue(
+      dynamic.containsKey(DynamicTableTag.DT_SYMTAB), "no symbol table entry");
+  print(".dynsym address = ${dynamic[DynamicTableTag.DT_SYMTAB]}");
+  final dynsym = elf.namedSections(".dynsym").single;
+  Expect.isTrue(elf.sectionHasValidSegmentAddresses(dynsym),
+      "string table addresses are invalid");
+  Expect.equals(dynamic[DynamicTableTag.DT_SYMTAB], dynsym.headerEntry.addr);
+  Expect.isTrue(dynamic.containsKey(DynamicTableTag.DT_SYMENT),
+      "no symbol table entry size entry");
+  print(".dynsym entry size = ${dynamic[DynamicTableTag.DT_SYMENT]}");
+  Expect.equals(
+      dynamic[DynamicTableTag.DT_SYMENT], dynsym.headerEntry.entrySize);
+
+  // Check the hash table information.
+  Expect.isTrue(
+      dynamic.containsKey(DynamicTableTag.DT_HASH), "no hash table entry");
+  print(".hash address = ${dynamic[DynamicTableTag.DT_HASH]}");
+  final hash = elf.namedSections(".hash").single;
+  Expect.isTrue(elf.sectionHasValidSegmentAddresses(hash),
+      "hash table addresses are invalid");
+  Expect.equals(dynamic[DynamicTableTag.DT_HASH], hash.headerEntry.addr);
+}

runtime/tests/vm/dart_2/use_save_debugging_info_flag_test.dart

@@ -15,6 +15,7 @@ import "dart:math";
 import "dart:typed_data";
 
 import 'package:expect/expect.dart';
+import 'package:native_stack_traces/elf.dart';
 import 'package:native_stack_traces/native_stack_traces.dart';
 import 'package:path/path.dart' as path;
 
@@ -65,6 +66,7 @@ main(List<String> args) async {
       '--elf=$scriptWholeSnapshot',
       scriptDill,
     ]);
+    checkElf(scriptWholeSnapshot);
 
     final scriptStrippedOnlySnapshot = path.join(tempDir, 'stripped_only.so');
     await run(genSnapshot, <String>[
@@ -74,6 +76,7 @@ main(List<String> args) async {
       '--strip',
       scriptDill,
     ]);
+    checkElf(scriptStrippedOnlySnapshot);
 
     final scriptStrippedSnapshot = path.join(tempDir, 'stripped.so');
     final scriptDebuggingInfo = path.join(tempDir, 'debug.so');
@@ -85,6 +88,8 @@ main(List<String> args) async {
       '--save-debugging-info=$scriptDebuggingInfo',
       scriptDill,
     ]);
+    checkElf(scriptStrippedSnapshot);
+    checkElf(scriptDebuggingInfo);
 
     // Run the resulting scripts, saving the stack traces.
     final wholeTrace = await runError(aotRuntime, <String>[
@@ -186,3 +191,46 @@ void printDiff(Map<int, List<int>> map, [int maxOutput = 100]) {
     }
   }
 }
+
+void checkElf(String filename) {
+  print("Checking ELF file $filename:");
+  final elf = Elf.fromFile(filename);
+  Expect.isNotNull(elf);
+  final dynamic = elf.namedSections(".dynamic").single as DynamicTable;
+
+  // Check the dynamic string table information.
+  Expect.isTrue(
+      dynamic.containsKey(DynamicTableTag.DT_STRTAB), "no string table entry");
+  final dynstr = elf.namedSections(".dynstr").single;
+  print(".dynstr address = ${dynamic[DynamicTableTag.DT_STRTAB]}");
+  Expect.isTrue(elf.sectionHasValidSegmentAddresses(dynstr),
+      "string table addresses are invalid");
+  Expect.equals(dynamic[DynamicTableTag.DT_STRTAB], dynstr.headerEntry.addr);
+  Expect.isTrue(dynamic.containsKey(DynamicTableTag.DT_STRSZ),
+      "no string table size entry");
+  print(".dynstr size = ${dynamic[DynamicTableTag.DT_STRSZ]}");
+  Expect.equals(dynamic[DynamicTableTag.DT_STRSZ], dynstr.headerEntry.size);
+
+  // Check the dynamic symbol table information.
+  Expect.isTrue(
+      dynamic.containsKey(DynamicTableTag.DT_SYMTAB), "no symbol table entry");
+  print(".dynsym address = ${dynamic[DynamicTableTag.DT_SYMTAB]}");
+  final dynsym = elf.namedSections(".dynsym").single;
+  Expect.isTrue(elf.sectionHasValidSegmentAddresses(dynsym),
+      "string table addresses are invalid");
+  Expect.equals(dynamic[DynamicTableTag.DT_SYMTAB], dynsym.headerEntry.addr);
+  Expect.isTrue(dynamic.containsKey(DynamicTableTag.DT_SYMENT),
+      "no symbol table entry size entry");
+  print(".dynsym entry size = ${dynamic[DynamicTableTag.DT_SYMENT]}");
+  Expect.equals(
+      dynamic[DynamicTableTag.DT_SYMENT], dynsym.headerEntry.entrySize);
+
+  // Check the hash table information.
+  Expect.isTrue(
+      dynamic.containsKey(DynamicTableTag.DT_HASH), "no hash table entry");
+  print(".hash address = ${dynamic[DynamicTableTag.DT_HASH]}");
+  final hash = elf.namedSections(".hash").single;
+  Expect.isTrue(elf.sectionHasValidSegmentAddresses(hash),
+      "hash table addresses are invalid");
+  Expect.equals(dynamic[DynamicTableTag.DT_HASH], hash.headerEntry.addr);
+}

runtime/vm/elf.cc

@@ -173,6 +173,8 @@ class Section : public ZoneAllocated {
   }
   bool IsWritable() const { return (flags & elf::SHF_WRITE) == elf::SHF_WRITE; }
 
+  bool HasBits() const { return type != elf::SectionHeaderType::SHT_NOBITS; }
+
   // Returns whether the size of a section can change.
   bool HasBeenFinalized() const {
     // Sections can grow or shrink up until Elf::ComputeOffsets has been run,
@@ -1559,8 +1561,27 @@ ProgramTable* SectionTable::CreateProgramTable(SymbolTable* symtab) {
   // We now do several passes over the collected sections to reorder them in
   // a way that minimizes segments (and thus padding) in the resulting snapshot.
 
-  // If a build ID was created, we put it after the program table so it can
-  // be read with a minimum number of bytes from the ELF file.
+  auto add_sections_matching =
+      [&](const std::function<bool(Section*)>& should_add) {
+        // We order the sections in a segment so all non-NOBITS sections come
+        // before NOBITS sections, since the former sections correspond to the
+        // file contents for the segment.
+        for (auto* const section : sections_) {
+          if (!section->HasBits()) continue;
+          if (should_add(section)) {
+            add_to_reordered_sections(section);
+          }
+        }
+        for (auto* const section : sections_) {
+          if (section->HasBits()) continue;
+          if (should_add(section)) {
+            add_to_reordered_sections(section);
+          }
+        }
+      };
+
+  // If a build ID was created, we put it right after the program table so it
+  // can be read with a minimum number of bytes from the ELF file.
   auto* const build_id = Find(Elf::kBuildIdNoteName);
   if (build_id != nullptr) {
     ASSERT(build_id->type == elf::SectionHeaderType::SHT_NOTE);
@@ -1568,38 +1589,31 @@ ProgramTable* SectionTable::CreateProgramTable(SymbolTable* symtab) {
   }
 
   // Now add the other non-writable, non-executable allocated sections.
-  for (auto* const section : sections_) {
-    if (section == build_id) continue;  // Already added.
-    if (section->IsAllocated() && !section->IsWritable() &&
-        !section->IsExecutable()) {
-      add_to_reordered_sections(section);
-    }
-  }
+  add_sections_matching([&](Section* section) -> bool {
+    if (section == build_id) return false;  // Already added.
+    return section->IsAllocated() && !section->IsWritable() &&
+           !section->IsExecutable();
+  });
 
   // Now add the executable sections in a new segment.
-  for (auto* const section : sections_) {
-    if (section->IsExecutable()) {  // Implies IsAllocated() && !IsWritable()
-      add_to_reordered_sections(section);
-    }
-  }
+  add_sections_matching([](Section* section) -> bool {
+    return section->IsExecutable();  // Implies IsAllocated() && !IsWritable()
+  });
 
   // Now add all the writable sections.
-  for (auto* const section : sections_) {
-    if (section->IsWritable()) {  // Implies IsAllocated() && !IsExecutable()
-      add_to_reordered_sections(section);
-    }
-  }
+  add_sections_matching([](Section* section) -> bool {
+    return section->IsWritable();  // Implies IsAllocated() && !IsExecutable()
+  });
 
   // We put all non-reserved unallocated sections last. Otherwise, they would
   // affect the file offset but not the memory offset of any following allocated
   // sections. Doing it in this order makes it easier to keep file and memory
   // offsets page-aligned with respect to each other, which is required for
   // some loaders.
-  for (intptr_t i = 1; i < num_sections; i++) {
-    auto* const section = sections_[i];
-    if (section->IsAllocated()) continue;
-    add_to_reordered_sections(section);
-  }
+  add_sections_matching([](Section* section) -> bool {
+    // Don't re-add the initial reserved section.
+    return !section->IsReservedSection() && !section->IsAllocated();
+  });
 
   // All sections should have been accounted for in the loops above.
   ASSERT_EQUAL(sections_.length(), reordered_sections.length());