[PATCH] D70720: [llvm-objdump] Display locations of variables alongside disassembly

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Minor request, but could you please remember to mark my comments as "done" once you've uploaded a patch to address them. It helps me with my follow-ups.



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:1106
+    } else {
+      consumeError(Loc.takeError());
+    }
----------------
A comment explaining why we just throw away the error here woul be good.


================
Comment at: llvm/test/tools/llvm-objdump/ARM/debug-vars-dwarf4-sections.s:57
+.Lfunc_begin0:
+	.file	1 "/work" "llvm/src/llvm/test/tools/llvm-objdump/ARM/Inputs/debug.c"
+	.loc	1 1 0
----------------
jhenderson wrote:
> Is this string important to the test? If it is, I don't think it'll work on others' machines.
> 
> You may need to use sed to replace the input string at test time. There are examples in the source-interleave tests in the X86 folder of llvm-objdump.
Ping? You have addressed this in this test?


================
Comment at: llvm/test/tools/llvm-objdump/ARM/debug-vars-dwarf4.s:3-4
+## using DWARF4 debug info, with multiple functions in one section. Check that
+## the live-range lines are rendered correctly when using the -no-show-raw-insn,
+## -line-numbers and -source options.
+
----------------
-no-show-raw-isn -> --no-show-raw-insn etc


================
Comment at: llvm/test/tools/llvm-objdump/ARM/debug-vars-dwarf4.s:14
+# RUN: llvm-mc -triple armv8a--none-eabi < %s -filetype=obj | \
+# RUN:     llvm-objdump - -d --debug-vars --debug-vars-indent=30 | \
+# RUN:     FileCheck %s --check-prefix=INDENT --strict-whitespace
----------------
You might want to consider another test case for --debug-vars-indent with the value equal to what it would be without the switch, testing against the same check-prefix. That will show what the default value of --debug-vars-indent is more clearly.


================
Comment at: llvm/test/tools/llvm-objdump/ARM/debug-vars-dwarf5-sections.s:1
+## Check that the -debug-vars option works for simple register locations, when
+## using DWARF4 debug info, with functions in multiple sections.
----------------
--debug-vars


================
Comment at: llvm/test/tools/llvm-objdump/ARM/debug-vars-dwarf5.s:1
+## Check that the -debug-vars option works for simple register locations, when
+## using DWARF5 debug info, with multiple functions in one section.
----------------
--debug-vars


================
Comment at: llvm/test/tools/llvm-objdump/PowerPC/debug-vars.s:1
+## Check that the -debug-vars option works for simple register locations, when
+## using DWARF4 debug info, with multiple functions in one section.
----------------
--debug-vars


================
Comment at: llvm/test/tools/llvm-symbolizer/frame-loclistx.s:1
+// Test dwarf-5 DW_AT_location [DW_FORM_loclistx].
+// REQUIRES: aarch64-registered-target
----------------
This test seems unrelated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70720