[PATCH] D139859: [symbolizer] Support symbol+offset lookup

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff marked 4 inline comments as done.
sepavloff added inline comments.


================
Comment at: llvm/test/tools/llvm-symbolizer/symbol-search.test:44
+RUN: llvm-addr2line -e %p/Inputs/symbols.so func_01+0 | FileCheck --check-prefix=A2L0 %s
+RUN: llvm-addr2line -e %p/Inputs/symbols.so func_01 | FileCheck --check-prefix=A2L0 %s
+#A2L0: /tmp/dbginfo{{[/\]+}}symbols.part1.cpp:12
----------------
jhenderson wrote:
> It's slightly offputting that the same set of arguments don't exist for both the llvm-symbolizer and llvm-addr2line test cases. Could you rationalise them, please, so that they both test the same set?
Actually they are the same, the test is fixed accordingly.


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:162
                           std::string &ModuleName, object::BuildID &BuildID,
-                          StringRef &Symbol, uint64_t &ModuleOffset) {
+                          StringRef &Symbol, uint64_t &Displacement) {
   ModuleName = BinaryName;
----------------
jhenderson wrote:
> Why has this name changed?
Now this parameter may be assigned either a module offset, or a displacement relative to a symbol, so the old name is not precise. More suitable name is `Offset` but it requires to rename the variable `Offset` used in this function body. Now the names and comments must be more consistent.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139859/new/

https://reviews.llvm.org/D139859