[PATCH] D12883: [llvm-symbolizer] Make --relative-address work with DWARF contexts
Alexey Samsonov via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 17 18:02:09 PDT 2015
samsonov added a comment.
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.
================
Comment at: lib/Object/COFFObjectFile.cpp:177
@@ -176,6 +176,3 @@
// return virtual addresses.
- if (PE32Header)
- Result += PE32Header->ImageBase;
- else if (PE32PlusHeader)
- Result += PE32PlusHeader->ImageBase;
+ Result += getImageBase().get();
----------------
See another comments. Also, this can go in as a separate change.
================
Comment at: lib/Object/COFFObjectFile.cpp:274
@@ -276,6 +273,3 @@
// return virtual addresses.
- if (PE32Header)
- Result += PE32Header->ImageBase;
- else if (PE32PlusHeader)
- Result += PE32PlusHeader->ImageBase;
+ Result += getImageBase().get();
return Result;
----------------
What should you do if getImageBase() failed? Maybe, just add 0 in this case? Do we want to crash/assert in this case?
================
Comment at: lib/Object/COFFObjectFile.cpp:431
@@ -428,4 +430,3 @@
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;
----------------
Do you need to propagate `object_error::parse_failed` if getImageBase() returned it?
================
Comment at: test/tools/llvm-symbolizer/coff-dwarf.test:2
@@ +1,3 @@
+RUN: llvm-symbolizer --inlining --relative-address -obj="%p/Inputs/coff-dwarf.exe" \
+RUN: < %p/Inputs/coff-dwarf.input | FileCheck %s
+
----------------
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?
================
Comment at: test/tools/llvm-symbolizer/pdb/pdb.test:3
@@ -2,1 +2,3 @@
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
----------------
Why do you need this copy here?
http://reviews.llvm.org/D12883
More information about the llvm-commits
mailing list