[llvm-branch-commits] [lld] 8b8088a - [lld] Use -1 as tombstone value for discarded code ranges

Derek Schuff via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Dec 1 17:12:00 PST 2020


Author: Eric Leese
Date: 2020-12-01T17:06:32-08:00
New Revision: 8b8088ac6ca977fbce52abd50ac88ecbe08d9cb2

URL: https://github.com/llvm/llvm-project/commit/8b8088ac6ca977fbce52abd50ac88ecbe08d9cb2
DIFF: https://github.com/llvm/llvm-project/commit/8b8088ac6ca977fbce52abd50ac88ecbe08d9cb2.diff

LOG: [lld] Use -1 as tombstone value for discarded code ranges

Under existing behavior discarded functions are relocated to have the start pc
0. This causes problems when debugging as they typically overlap the first
function and lldb symbol resolution frequently chooses a discarded function
instead of the correct one. Using the value -1 or -2 (depending on which DWARF
section we are writing) is sufficient to prevent lldb from resolving to these
symbols.

Reviewed By: MaskRay, yurydelendik, sbc100

Differential Revision: https://reviews.llvm.org/D91803

Added: 
    

Modified: 
    lld/test/wasm/debug-removed-fn.ll
    lld/test/wasm/debuginfo.test
    lld/wasm/InputChunks.cpp
    lld/wasm/InputChunks.h
    lld/wasm/InputFiles.cpp
    lld/wasm/InputFiles.h

Removed: 
    


################################################################################
diff  --git a/lld/test/wasm/debug-removed-fn.ll b/lld/test/wasm/debug-removed-fn.ll
index 89a55a3d4187..9d2c5dc107de 100644
--- a/lld/test/wasm/debug-removed-fn.ll
+++ b/lld/test/wasm/debug-removed-fn.ll
@@ -4,11 +4,12 @@
 
 ; CHECK: Address
 ; CHECK: 0x0000000000000005
-; CHECK: 0x0000000000000000
+; CHECK-NEXT: 0x0000000000000006
+; CHECK-EMPTY:
 
 ; CHECK: .debug_ranges contents:
 ; CHECK: 00000000 {{[0-9]+}} {{[0-9]+}}
-; CHECK: 00000000 {{[0-9]+}} {{[0-9]+}}
+; CHECK: 00000000 fffffffe fffffffe
 ; CHECK: 00000000 <End of list>
 
 ; ModuleID = 't.bc'

diff  --git a/lld/test/wasm/debuginfo.test b/lld/test/wasm/debuginfo.test
index 2566b74d93bf..94d183a3c1d4 100644
--- a/lld/test/wasm/debuginfo.test
+++ b/lld/test/wasm/debuginfo.test
@@ -71,7 +71,7 @@ CHECK-NEXT:                DW_AT_type	(0x000000a7 "int[2]")
 CHECK-NEXT:                DW_AT_external	(true)
 CHECK-NEXT:                DW_AT_decl_file	("{{.*}}hi_foo.c")
 CHECK-NEXT:                DW_AT_decl_line	(8)
-CHECK-NEXT:                DW_AT_location	(DW_OP_addr 0x0)
+CHECK-NEXT:                DW_AT_location	(DW_OP_addr 0xffffffff)
 
 CHECK:   DW_TAG_subprogram
 CHECK-NEXT:                DW_AT_low_pc

diff  --git a/lld/wasm/InputChunks.cpp b/lld/wasm/InputChunks.cpp
index 06a734b69a97..52d19e6ddc10 100644
--- a/lld/wasm/InputChunks.cpp
+++ b/lld/wasm/InputChunks.cpp
@@ -134,10 +134,11 @@ void InputChunk::writeTo(uint8_t *buf) const {
   LLVM_DEBUG(dbgs() << "applying relocations: " << toString(this)
                     << " count=" << relocations.size() << "\n");
   int32_t off = outputOffset - getInputSectionOffset();
+  auto tombstone = getTombstone();
 
   for (const WasmRelocation &rel : relocations) {
     uint8_t *loc = buf + rel.Offset + off;
-    auto value = file->calcNewValue(rel);
+    auto value = file->calcNewValue(rel, tombstone);
     LLVM_DEBUG(dbgs() << "apply reloc: type=" << relocTypeToString(rel.Type));
     if (rel.Type != R_WASM_TYPE_INDEX_LEB)
       LLVM_DEBUG(dbgs() << " sym=" << file->getSymbols()[rel.Index]->getName());
@@ -291,11 +292,13 @@ void InputFunction::calculateSize() {
   uint32_t start = getInputSectionOffset();
   uint32_t end = start + function->Size;
 
+  auto tombstone = getTombstone();
+
   uint32_t lastRelocEnd = start + functionSizeLength;
   for (const WasmRelocation &rel : relocations) {
     LLVM_DEBUG(dbgs() << "  region: " << (rel.Offset - lastRelocEnd) << "\n");
     compressedFuncSize += rel.Offset - lastRelocEnd;
-    compressedFuncSize += getRelocWidth(rel, file->calcNewValue(rel));
+    compressedFuncSize += getRelocWidth(rel, file->calcNewValue(rel, tombstone));
     lastRelocEnd = rel.Offset + getRelocWidthPadded(rel);
   }
   LLVM_DEBUG(dbgs() << "  final region: " << (end - lastRelocEnd) << "\n");
@@ -323,6 +326,7 @@ void InputFunction::writeTo(uint8_t *buf) const {
   const uint8_t *secStart = file->codeSection->Content.data();
   const uint8_t *funcStart = secStart + getInputSectionOffset();
   const uint8_t *end = funcStart + function->Size;
+  auto tombstone = getTombstone();
   uint32_t count;
   decodeULEB128(funcStart, &count);
   funcStart += count;
@@ -335,7 +339,7 @@ void InputFunction::writeTo(uint8_t *buf) const {
     LLVM_DEBUG(dbgs() << "  write chunk: " << chunkSize << "\n");
     memcpy(buf, lastRelocEnd, chunkSize);
     buf += chunkSize;
-    buf += writeCompressedReloc(buf, rel, file->calcNewValue(rel));
+    buf += writeCompressedReloc(buf, rel, file->calcNewValue(rel, tombstone));
     lastRelocEnd = secStart + rel.Offset + getRelocWidthPadded(rel);
   }
 
@@ -359,6 +363,7 @@ void InputSegment::generateRelocationCode(raw_ostream &os) const {
                                 ? WASM_OPCODE_I64_ADD
                                 : WASM_OPCODE_I32_ADD;
 
+  auto tombstone = getTombstone();
   // TODO(sbc): Encode the relocations in the data section and write a loop
   // here to apply them.
   uint64_t segmentVA = outputSeg->startVA + outputSegmentOffset;
@@ -405,7 +410,7 @@ void InputSegment::generateRelocationCode(raw_ostream &os) const {
       writeU8(os, WASM_OPCODE_GLOBAL_GET, "GLOBAL_GET");
       writeUleb128(os, baseSymbol->getGlobalIndex(), "base");
       writeU8(os, opcode_reloc_const, "CONST");
-      writeSleb128(os, file->calcNewValue(rel), "offset");
+      writeSleb128(os, file->calcNewValue(rel, tombstone), "offset");
       writeU8(os, opcode_reloc_add, "ADD");
     }
 
@@ -416,5 +421,20 @@ void InputSegment::generateRelocationCode(raw_ostream &os) const {
   }
 }
 
+uint64_t InputSection::getTombstoneForSection(StringRef name) {
+  // When a function is not live we need to update relocations referring to it.
+  // If they occur in DWARF debug symbols, we want to change the pc of the
+  // function to -1 to avoid overlapping with a valid range. However for the
+  // debug_ranges and debug_loc sections that would conflict with the existing
+  // meaning of -1 so we use -2.
+  // Returning 0 means there is no tombstone value for this section, and relocation
+  // will just use the addend.
+  if (!name.startswith(".debug_"))
+    return 0;
+  if (name.equals(".debug_ranges") || name.equals(".debug_loc"))
+    return UINT64_C(-2);
+  return UINT64_C(-1);
+}
+
 } // namespace wasm
 } // namespace lld

diff  --git a/lld/wasm/InputChunks.h b/lld/wasm/InputChunks.h
index e5671fb89237..ba4e2a591074 100644
--- a/lld/wasm/InputChunks.h
+++ b/lld/wasm/InputChunks.h
@@ -74,6 +74,7 @@ class InputChunk {
       : file(f), live(!config->gcSections), discarded(false), sectionKind(k) {}
   virtual ~InputChunk() = default;
   virtual ArrayRef<uint8_t> data() const = 0;
+  virtual uint64_t getTombstone() const { return 0; }
 
   // Verifies the existing data at relocation targets matches our expectations.
   // This is performed only debug builds as an extra sanity check.
@@ -214,7 +215,7 @@ class SyntheticFunction : public InputFunction {
 class InputSection : public InputChunk {
 public:
   InputSection(const WasmSection &s, ObjFile *f)
-      : InputChunk(f, InputChunk::Section), section(s) {
+      : InputChunk(f, InputChunk::Section), section(s), tombstoneValue(getTombstoneForSection(s.Name)) {
     assert(section.Type == llvm::wasm::WASM_SEC_CUSTOM);
   }
 
@@ -228,8 +229,11 @@ class InputSection : public InputChunk {
   // Offset within the input section.  This is only zero since this chunk
   // type represents an entire input section, not part of one.
   uint32_t getInputSectionOffset() const override { return 0; }
+  uint64_t getTombstone() const override { return tombstoneValue; }
+  static uint64_t getTombstoneForSection(StringRef name);
 
   const WasmSection §ion;
+  const uint64_t tombstoneValue;
 };
 
 } // namespace wasm

diff  --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp
index 57db39dd76fd..2b3c2591058c 100644
--- a/lld/wasm/InputFiles.cpp
+++ b/lld/wasm/InputFiles.cpp
@@ -197,17 +197,18 @@ uint64_t ObjFile::calcExpectedValue(const WasmRelocation &reloc) const {
 }
 
 // Translate from the relocation's index into the final linked output value.
-uint64_t ObjFile::calcNewValue(const WasmRelocation &reloc) const {
+uint64_t ObjFile::calcNewValue(const WasmRelocation &reloc, uint64_t tombstone) const {
   const Symbol* sym = nullptr;
   if (reloc.Type != R_WASM_TYPE_INDEX_LEB) {
     sym = symbols[reloc.Index];
 
     // We can end up with relocations against non-live symbols.  For example
-    // in debug sections. We return reloc.Addend because always returning zero
-    // causes the generation of spurious range-list terminators in the
-    // .debug_ranges section.
+    // in debug sections. We return a tombstone value in debug symbol sections
+    // so this will not produce a valid range conflicting with ranges of actual
+    // code. In other sections we return reloc.Addend.
+
     if ((isa<FunctionSymbol>(sym) || isa<DataSymbol>(sym)) && !sym->isLive())
-      return reloc.Addend;
+      return tombstone ? tombstone : reloc.Addend;
   }
 
   switch (reloc.Type) {

diff  --git a/lld/wasm/InputFiles.h b/lld/wasm/InputFiles.h
index 253224a011c7..18b4f8de9bb5 100644
--- a/lld/wasm/InputFiles.h
+++ b/lld/wasm/InputFiles.h
@@ -118,7 +118,7 @@ class ObjFile : public InputFile {
   void dumpInfo() const;
 
   uint32_t calcNewIndex(const WasmRelocation &reloc) const;
-  uint64_t calcNewValue(const WasmRelocation &reloc) const;
+  uint64_t calcNewValue(const WasmRelocation &reloc, uint64_t tombstone) const;
   uint64_t calcNewAddend(const WasmRelocation &reloc) const;
   uint64_t calcExpectedValue(const WasmRelocation &reloc) const;
   Symbol *getSymbol(const WasmRelocation &reloc) const {


        


More information about the llvm-branch-commits mailing list