[PATCH] D120660: [llvm-symbolizer] Add --approximate-missing-line-numbers Command Line Option

[James Henderson via Phabricator via llvm-commits]

jhenderson added a reviewer: dblaikie.
jhenderson added a subscriber: dblaikie.
jhenderson added a comment.
Herald added a project: All.

I've already expressed my doubts offline about this change potentially causing people to end up with misleading output. However, in and of itself, I don't think that's a reason to reject it, since the user has to opt-in, although I'd like to hear from some other opinions (@dblaikie?). However, I do think it is important for a user to be able to distinguish between an approximated address and a "definitely right" address, by the output being annotated somehow with something like "(approximate)".



================
Comment at: llvm/docs/CommandGuide/llvm-symbolizer.rst:209-211
+  example when the compiler has given no line number entry due to it
+  being nebulous due to optimization. The line number of the previous
+  address will instead be output.
----------------



================
Comment at: llvm/include/llvm/DebugInfo/Symbolize/Symbolize.h:147
+      DILineInfo *LineInfo);
+
   /// Returns a SymbolizableModule or an error if loading debug info failed.
----------------
Nit: no blank lines at start of functions.


================
Comment at: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp:126-127
     }
   }
+  if (Opts.ApproximateLineNumbers)
+    ApproximateMissingLineNumber(Info, ModuleOffset,
----------------
This is unrelated to the demangling block above, so split it up.


================
Comment at: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp:154
+// compiler optimization, look at the previous address.
+void LLVMSymbolizer::ApproximateMissingLineNumber(
+    SymbolizableModule *Info, object::SectionedAddress ModuleOffset,
----------------
Wrong function name style.


================
Comment at: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp:160-164
+  --ModuleOffset.Address;
+  DILineInfo ApproxLineInfo = Info->symbolizeCode(
+      ModuleOffset, DILineInfoSpecifier(Opts.PathStyle, Opts.PrintFunctions),
+      Opts.UseSymbolTable);
+  LineInfo->Line = ApproxLineInfo.Line;
----------------
If I'm following this correctly, this ends up in a recursive loop until you find a non-zero line number or address zero? It sounds honestly rather inefficient to me, in the event of a compiler emitting a large amount of code with line 0. That being said, I don't know how often such a situation could occur. I would at least like to see a test case for multiple consecutive line 0 entries.


================
Comment at: llvm/test/tools/llvm-symbolizer/approximate-missing-line-numbers-inline.s:7-8
+# RUN: llvm-symbolizer --obj=%t.o 0x0000000000000000 --approximate-missing-line-numbers | FileCheck %s -DLINE=0
+# RUN: llvm-symbolizer --obj=%t.o 0x0000000000000006 | FileCheck %s -DLINE=4
+# RUN: llvm-symbolizer --obj=%t.o 0x0000000000000006 --approximate-missing-line-numbers | FileCheck %s -DLINE=4
+# RUN: llvm-symbolizer --obj=%t.o 0x0000000000000010 | FileCheck %s -DLINE1=0 -DLINE2=8 --check-prefix=INLINED
----------------
I might be being dumb: is this test case supposed to be showing that --approximate-missing-line-numbers does nothing if the address is non-zero? That's fine (we should have that case), but if so, I a) don't see a non-inlined case where the option does anything (the address 16 case looks like it is the same essential case as this address 6 case).


================
Comment at: llvm/test/tools/llvm-symbolizer/approximate-missing-line-numbers.s:1
+# REQUIRES: x86-registered-target
+
----------------
Is there any real difference between inlined and non-inline cases? The code change is under the `symbolizeCodeCommon` function after all.


================
Comment at: llvm/test/tools/llvm-symbolizer/approximate-missing-line-numbers.s:8-12
+# RUN: llvm-symbolizer --obj=%t.o 0x0000000000000006 --output-style=GNU --no-inlines | FileCheck %s -DLINE=0
+# RUN: llvm-symbolizer --approximate-missing-line-numbers --obj=%t.o 0x0000000000000006 --output-style=GNU --no-inlines | FileCheck %s -DLINE=4
+
+# RUN: llvm-symbolizer --obj=%t.o 0x0000000000000006 --output-style=JSON --no-inlines | FileCheck %s --check-prefix=JSON -DLINE=0
+# RUN: llvm-symbolizer --approximate-missing-line-numbers --obj=%t.o 0x0000000000000006 --output-style=JSON --no-inlines | FileCheck %s --check-prefix=JSON -DLINE=4
----------------
I'm not sure output style is at all relevant to this option?


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:371
   parseIntArg(Args, OPT_adjust_vma_EQ, AdjustVMA);
+  Opts.ApproximateLineNumbers = Args.hasArg(OPT_approximate_missing_line_numbers);
   if (const opt::Arg *A = Args.getLastArg(OPT_basenames, OPT_relativenames)) {
----------------
Nit: clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120660