[llvm-symbolizer] Make --relative-address work with DWARF contexts

[rnk]

Summary

Previously the relative address flag only affected PDB debug info. Now
both DIContext implementations always expect to be passed virtual
addresses. llvm-symbolizer is now responsible for adding ImageBase to
module offsets when --relative-offset is passed.

[zturner]

Originally the plan was to remove --relative-address and update the callers to always use absolute addresses. Enhancing the functionality of --relative-address seems contrary to this goal. +samsonov in case he has some thoughts on this direction.

[rnk]

I want to go the other direction now. Producing absolute virtual addresses from a stack dumper is tricky, but it is very easy to produce object offsets. See the code in compiler-rt / sanitizer_common that takes a runtime virtual address and turns it into an "virtual" address. I didn't want to have to rewrite this code in LLVM's stack dumper.

[rnk]

Given that it's more convenient for both known clients of llvm-symbolizer to produce module offsets, the next logical step is to make --relative-offsets the default, and put in a --virtual-addresses flag.

[samsonov]

I agree with the direction of the patch - it's indeed tricky to figure out virtual addresses when we use llvm-symbolizer as an online tool (bleh, parsing PECOFF headers :(), so we might stick with passing --relative-address in this case. However, per our offline discussion, this flag should still be off by default, so that we will expect the same PCs as those stored in the debug info, or those printed in objdump output.

lib/Object/COFFObjectFile.cpp

177 ↗

(On Diff #34815)

See another comments. Also, this can go in as a separate change.

274 ↗

(On Diff #34815)

What should you do if getImageBase() failed? Maybe, just add 0 in this case? Do we want to crash/assert in this case?

431 ↗

(On Diff #34815)

Do you need to propagate object_error::parse_failed if getImageBase() returned it?

test/tools/llvm-symbolizer/coff-dwarf.test

2 ↗

(On Diff #34815)

Is there no way to create the input file on the fly with "echo" commands on Windows, instead of storing the input in a separate file?

test/tools/llvm-symbolizer/pdb/pdb.test

3 ↗

(On Diff #34815)

Why do you need this copy here?

[rnk]

• Address review comments, remove error case and return 0 instead as required by tests

lib/Object/COFFObjectFile.cpp

274 ↗

(On Diff #34815)

Hm, the test case says I need to return 0. =/ Awkward. I guess I'll preserve the existing behavior then.

test/tools/llvm-symbolizer/coff-dwarf.test

2 ↗

(On Diff #34815)

I don't think \n works in strings in lit's shell. I can use grep and sed though to cut down on files.

test/tools/llvm-symbolizer/pdb/pdb.test

3 ↗

(On Diff #34815)

gone

[rnk]

Woops, I accidentally committed this. Let me know if there are issues.

[samsonov]

LGTM. Thank you!

llvm/trunk/include/llvm/DebugInfo/PDB/PDBContext.h

 /// access can be had by using the PDB interfaces directly.
 class PDBContext : public DIContext {
 
   PDBContext(PDBContext &) = delete;
   PDBContext &operator=(PDBContext &) = delete;
 
 public:
   PDBContext(const object::COFFObjectFile &Object,
-             std::unique_ptr<IPDBSession> PDBSession,
-             bool RelativeAddress);
+             std::unique_ptr<IPDBSession> PDBSession);
 
   static bool classof(const DIContext *DICtx) {
     return DICtx->getKind() == CK_PDB;
   }
 
   void dump(raw_ostream &OS, DIDumpType DumpType = DIDT_All) override;
 
   DILineInfo getLineInfoForAddress(

llvm/trunk/include/llvm/Object/COFF.h

public:
   iterator_range<const coff_relocation *>
   getRelocations(const coff_section *Sec) const;
 
   std::error_code getSectionName(const coff_section *Sec, StringRef &Res) const;
   uint64_t getSectionSize(const coff_section *Sec) const;
   std::error_code getSectionContents(const coff_section *Sec,
                                      ArrayRef<uint8_t> &Res) const;
 
+  ErrorOr<uint64_t> getImageBase() const;
   std::error_code getVaPtr(uint64_t VA, uintptr_t &Res) const;
   std::error_code getRvaPtr(uint32_t Rva, uintptr_t &Res) const;
   std::error_code getHintName(uint32_t Rva, uint16_t &Hint,
                               StringRef &Name) const;
 
   bool isRelocatableObject() const override;
   bool is64() const { return PE32PlusHeader; }
 

llvm/trunk/lib/DebugInfo/PDB/PDBContext.cpp

 #include "llvm/DebugInfo/PDB/PDBSymbolData.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolPublicSymbol.h"
 #include "llvm/Object/COFF.h"
 
 using namespace llvm;
 using namespace llvm::object;
 
 PDBContext::PDBContext(const COFFObjectFile &Object,
-                       std::unique_ptr<IPDBSession> PDBSession,
-                       bool RelativeAddress)
+                       std::unique_ptr<IPDBSession> PDBSession)
     : DIContext(CK_PDB), Session(std::move(PDBSession)) {
-  if (!RelativeAddress) {
-    uint64_t ImageBase = 0;
-    if (Object.is64()) {
-      const pe32plus_header *Header = nullptr;
-      Object.getPE32PlusHeader(Header);
-      if (Header)
-        ImageBase = Header->ImageBase;
-    } else {
-      const pe32_header *Header = nullptr;
-      Object.getPE32Header(Header);
-      if (Header)
-        ImageBase = static_cast<uint64_t>(Header->ImageBase);
-    }
-    Session->setLoadAddress(ImageBase);
-  }
+  ErrorOr<uint64_t> ImageBase = Object.getImageBase();
+  if (ImageBase)
+    Session->setLoadAddress(ImageBase.get());
 }
 
 void PDBContext::dump(raw_ostream &OS, DIDumpType DumpType) {}
 
 DILineInfo PDBContext::getLineInfoForAddress(uint64_t Address,
                                              DILineInfoSpecifier Specifier) {
   DILineInfo Result;
   Result.FunctionName = getFunctionName(Address, Specifier.FNKind);

llvm/trunk/lib/Object/COFFObjectFile.cpp

ErrorOr<uint64_t> COFFObjectFile::getSymbolAddress(DataRefImpl Ref) const {
 
   const coff_section *Section = nullptr;
   if (std::error_code EC = getSection(SectionNumber, Section))
     return EC;
   Result += Section->VirtualAddress;
 
   // The section VirtualAddress does not include ImageBase, and we want to
   // return virtual addresses.
-  if (PE32Header)
-    Result += PE32Header->ImageBase;
-  else if (PE32PlusHeader)
-    Result += PE32PlusHeader->ImageBase;
+  Result += getImageBase().get();
 
   return Result;
 }
 
 SymbolRef::Type COFFObjectFile::getSymbolType(DataRefImpl Ref) const {
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
   int32_t SectionNumber = Symb.getSectionNumber();
 

 }
 
 uint64_t COFFObjectFile::getSectionAddress(DataRefImpl Ref) const {
   const coff_section *Sec = toSec(Ref);
   uint64_t Result = Sec->VirtualAddress;
 
   // The section VirtualAddress does not include ImageBase, and we want to
   // return virtual addresses.
-  if (PE32Header)
-    Result += PE32Header->ImageBase;
-  else if (PE32PlusHeader)
-    Result += PE32PlusHeader->ImageBase;
+  Result += getImageBase().get();
   return Result;
 }
 
 uint64_t COFFObjectFile::getSectionSize(DataRefImpl Ref) const {
   return getSectionSize(toSec(Ref));
 }
 
 std::error_code COFFObjectFile::getSectionContents(DataRefImpl Ref,
if (StringTableSize < 4)
       StringTableSize = 4;
 
   // Check that the string table is null terminated if has any in it.
   if (StringTableSize > 4 && StringTable[StringTableSize - 1] != 0)
     return  object_error::parse_failed;
   return std::error_code();
 }
 
+ErrorOr<uint64_t> COFFObjectFile::getImageBase() const {
+  if (PE32Header)
+    return uint64_t(PE32Header->ImageBase);
+  else if (PE32PlusHeader)
+    return uint64_t(PE32PlusHeader->ImageBase);
+  return object_error::parse_failed;
+}
+
 // Returns the file offset for the given VA.
 std::error_code COFFObjectFile::getVaPtr(uint64_t Addr, uintptr_t &Res) const {
-  uint64_t ImageBase = PE32Header ? (uint64_t)PE32Header->ImageBase
-                                  : (uint64_t)PE32PlusHeader->ImageBase;
+  uint64_t ImageBase = getImageBase().get();
   uint64_t Rva = Addr - ImageBase;
   assert(Rva <= UINT32_MAX);
   return getRvaPtr((uint32_t)Rva, Res);
 }
 
 // Returns the file offset for the given RVA.
 std::error_code COFFObjectFile::getRvaPtr(uint32_t Addr, uintptr_t &Res) const {
   for (const SectionRef &S : sections()) {

llvm/trunk/test/tools/llvm-symbolizer/Inputs/coff-dwarf.cpp

+// To generate the corresponding EXE, run:
+// clang-cl -O2 -gdwarf -c coff-dwarf.cpp && lld-link -debug coff-dwarf.obj
+
+extern "C" int puts(const char *str);
+
+void __declspec(noinline) foo() {
+  puts("foo1");
+  puts("foo2");
+}
+
+// LLVM should inline this into main.
+static void bar() {
+  foo();
+}
+
+int main() {
+  bar();
+  return 0;
+}

llvm/trunk/test/tools/llvm-symbolizer/Inputs/coff-dwarf.input

+0x5009
+0x5038

llvm/trunk/test/tools/llvm-symbolizer/coff-dwarf.test

+RUN: llvm-symbolizer --inlining --relative-address -obj="%p/Inputs/coff-dwarf.exe" \
+RUN:      < %p/Inputs/coff-dwarf.input | FileCheck %s
+
+CHECK: foo(void)
+CHECK: coff-dwarf.cpp:7
+CHECK: bar(void)
+CHECK: coff-dwarf.cpp:13
+CHECK: main
+CHECK: coff-dwarf.cpp:17

llvm/trunk/test/tools/llvm-symbolizer/pdb/pdb.test

 RUN: llvm-symbolizer -obj="%p/Inputs/test.exe" < "%p/Inputs/test.exe.input" | \
 RUN:    FileCheck %s --check-prefix=CHECK
+RUN: llvm-symbolizer -obj="%p/Inputs/test.exe" < "%p/Inputs/test.exe.input" | \
+RUN:    FileCheck %s --check-prefix=CHECK
 RUN: llvm-symbolizer -obj="%p/Inputs/test.exe" -demangle=false < \
 RUN:    "%p/Inputs/test.exe.input" | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
 
+Subtract ImageBase from all the offsets and run the test again with
+--relative-address.
+
+RUN: python -c 'import sys;print "\n".join([hex(int(x, 16) - 0x400000) for x in sys.stdin])' \
+RUN:  < %p/Inputs/test.exe.input \
+RUN:  | llvm-symbolizer -obj="%p/Inputs/test.exe" -demangle=false --relative-address \
+RUN:    | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
+
 CHECK: foo(void)
 CHECK-NEXT: test.cpp:10
 CHECK: main
 CHECK-NEXT: test.cpp:13:0
 CHECK: NS::Foo::bar(void)
 CHECK-NEXT: test.cpp:6:0
 CHECK: {{^foo_cdecl$}}
 CHECK: {{^foo_stdcall$}}

llvm/trunk/tools/llvm-symbolizer/LLVMSymbolize.h

public:
   DIInliningInfo symbolizeInlinedCode(
       uint64_t ModuleOffset, const LLVMSymbolizer::Options &Opts) const;
   bool symbolizeData(uint64_t ModuleOffset, std::string &Name, uint64_t &Start,
                      uint64_t &Size) const;
 
   // Return true if this is a 32-bit x86 PE COFF module.
   bool isWin32Module() const;
 
+  // Returns the preferred base of the module, i.e. where the loader would place
+  // it in memory assuming there were no conflicts.
+  uint64_t getModulePreferredBase() const;
+
 private:
   bool getNameFromSymbolTable(SymbolRef::Type Type, uint64_t Address,
                               std::string &Name, uint64_t &Addr,
                               uint64_t &Size) const;
   // For big-endian PowerPC64 ELF, OpdAddress is the address of the .opd
   // (function descriptor) section and OpdExtractor refers to its contents.
   void addSymbol(const SymbolRef &Symbol, uint64_t SymbolSize,
                  DataExtractor *OpdExtractor = nullptr,

llvm/trunk/tools/llvm-symbolizer/LLVMSymbolize.cpp

 }
 
 // Return true if this is a 32-bit x86 PE COFF module.
 bool ModuleInfo::isWin32Module() const {
   auto *CoffObject = dyn_cast<COFFObjectFile>(Module);
   return CoffObject && CoffObject->getMachine() == COFF::IMAGE_FILE_MACHINE_I386;
 }
 
+uint64_t ModuleInfo::getModulePreferredBase() const {
+  if (auto *CoffObject = dyn_cast<COFFObjectFile>(Module))
+    if (auto Base = CoffObject->getImageBase())
+      return Base.get();
+  return 0;
+}
+
 bool ModuleInfo::getNameFromSymbolTable(SymbolRef::Type Type, uint64_t Address,
                                         std::string &Name, uint64_t &Addr,
                                         uint64_t &Size) const {
   const auto &SymbolMap = Type == SymbolRef::ST_Function ? Functions : Objects;
   if (SymbolMap.empty())
     return false;
   SymbolDesc SD = { Address, Address };
   auto SymbolIterator = SymbolMap.upper_bound(SD);

 
 const char LLVMSymbolizer::kBadString[] = "??";
 
 std::string LLVMSymbolizer::symbolizeCode(const std::string &ModuleName,
                                           uint64_t ModuleOffset) {
   ModuleInfo *Info = getOrCreateModuleInfo(ModuleName);
   if (!Info)
     return printDILineInfo(DILineInfo(), Info);
+
+  // If the user is giving us relative addresses, add the preferred base of the
+  // object to the offset before we do the query. It's what DIContext expects.
+  if (Opts.RelativeAddresses)
+    ModuleOffset += Info->getModulePreferredBase();
+
   if (Opts.PrintInlining) {
     DIInliningInfo InlinedContext =
         Info->symbolizeInlinedCode(ModuleOffset, Opts);
     uint32_t FramesNum = InlinedContext.getNumberOfFrames();
     assert(FramesNum > 0);
     std::string Result;
     for (uint32_t i = 0; i < FramesNum; i++) {
       DILineInfo LineInfo = InlinedContext.getFrame(i);
       Result += printDILineInfo(LineInfo, Info);
     }
     return Result;
   }
   DILineInfo LineInfo = Info->symbolizeCode(ModuleOffset, Opts);
   return printDILineInfo(LineInfo, Info);
 }
 
 std::string LLVMSymbolizer::symbolizeData(const std::string &ModuleName,
                                           uint64_t ModuleOffset) {
   std::string Name = kBadString;
   uint64_t Start = 0;
   uint64_t Size = 0;
   if (Opts.UseSymbolTable) {
     if (ModuleInfo *Info = getOrCreateModuleInfo(ModuleName)) {
+      // If the user is giving us relative addresses, add the preferred base of the
+      // object to the offset before we do the query. It's what DIContext expects.
+      if (Opts.RelativeAddresses)
+        ModuleOffset += Info->getModulePreferredBase();
       if (Info->symbolizeData(ModuleOffset, Name, Start, Size) && Opts.Demangle)
         Name = DemangleName(Name, Info);
     }
   }
   std::stringstream ss;
   ss << Name << "\n" << Start << " " << Size << "\n";
   return ss.str();
 }
LLVMSymbolizer::getOrCreateModuleInfo(const std::string &ModuleName) {
   DIContext *Context = nullptr;
   if (auto CoffObject = dyn_cast<COFFObjectFile>(Objects.first)) {
     // If this is a COFF object, assume it contains PDB debug information.  If
     // we don't find any we will fall back to the DWARF case.
     std::unique_ptr<IPDBSession> Session;
     PDB_ErrorCode Error = loadDataForEXE(PDB_ReaderType::DIA,
                                          Objects.first->getFileName(), Session);
     if (Error == PDB_ErrorCode::Success) {
-      Context = new PDBContext(*CoffObject, std::move(Session),
-                               Opts.RelativeAddresses);
+      Context = new PDBContext(*CoffObject, std::move(Session));
     }
   }
   if (!Context)
     Context = new DWARFContextInMemory(*Objects.second);
   assert(Context);
   ModuleInfo *Info = new ModuleInfo(Objects.first, Context);
   Modules.insert(make_pair(ModuleName, Info));
   return Info;