[llvm-objdump] Add inlined function display support #142246
Conversation
|
@llvm/pr-subscribers-llvm-binary-utilities Author: None (gulfemsavrun) ChangesThis patch adds the support for displaying inlined functions into llvm-objdump.
Patch is 62.68 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/142246.diff 10 Files Affected:
diff --git a/llvm/docs/CommandGuide/llvm-objdump.rst b/llvm/docs/CommandGuide/llvm-objdump.rst
index ab9f583e96ec6..65136d5ba22eb 100644
--- a/llvm/docs/CommandGuide/llvm-objdump.rst
+++ b/llvm/docs/CommandGuide/llvm-objdump.rst
@@ -153,10 +153,16 @@ OPTIONS
alongside disassembly. ``format`` may be ``unicode`` or ``ascii``, defaulting
to ``unicode`` if omitted.
-.. option:: --debug-vars-indent=<width>
+.. option:: --debug-indent=<width>
- Distance to indent the source-level variable display, relative to the start
- of the disassembly. Defaults to 52 characters.
+ Distance to indent the source-level variable or inlined function display,
+ relative to the start of the disassembly. Defaults to 52 characters.
+
+.. option:: --debug-inlined-funcs=<format>
+
+ Print the locations of inlined functions alongside disassembly.
+ ``format`` may be ``unicode``, ``ascii`` or ``line`` defaulting to
+ ``unicode`` if omitted.
.. option:: -j, --section=<section1[,section2,...]>
diff --git a/llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s b/llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s
index 69b7489e7e62e..085f258edfa57 100644
--- a/llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s
+++ b/llvm/test/tools/llvm-objdump/ELF/ARM/debug-vars-dwarf4.s
@@ -15,10 +15,10 @@
## Check that passing the default value for --debug-vars-indent (52) makes no
## change to the output.
-# RUN: llvm-objdump %t.o -d --debug-vars --debug-vars-indent=52 | \
+# RUN: llvm-objdump %t.o -d --debug-vars --debug-indent=52 | \
# RUN: FileCheck %s --check-prefix=RAW --strict-whitespace
-# RUN: llvm-objdump %t.o -d --debug-vars --debug-vars-indent=30 | \
+# RUN: llvm-objdump %t.o -d --debug-vars --debug-indent=30 | \
# RUN: FileCheck %s --check-prefix=INDENT --strict-whitespace
# RUN: llvm-objdump %t.o -d --debug-vars --no-show-raw-insn | \
diff --git a/llvm/test/tools/llvm-objdump/X86/Inputs/debug-inlined-functions.c b/llvm/test/tools/llvm-objdump/X86/Inputs/debug-inlined-functions.c
new file mode 100644
index 0000000000000..a708bc0cae604
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/X86/Inputs/debug-inlined-functions.c
@@ -0,0 +1,10 @@
+int bar(int x, int y) {
+ int sum = x + y;
+ int mul = x * y;
+ return sum + mul;
+}
+
+int foo(int a, int b) {
+ int result = bar(a, b);
+ return result;
+}
diff --git a/llvm/test/tools/llvm-objdump/X86/debug-inlined-function.s b/llvm/test/tools/llvm-objdump/X86/debug-inlined-function.s
new file mode 100644
index 0000000000000..c98cc7ba06359
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/X86/debug-inlined-function.s
@@ -0,0 +1,632 @@
+## Generated with this compile command, with the source code in Inputs/debug-inlined-functions.c:
+## clang -g -c debug-inlined-function.c -O0 -S -o -
+
+# RUN: llvm-mc -triple=x86_64 %s -filetype=obj -o %t.o
+
+# RUN: llvm-objdump %t.o -d --debug-inlined-funcs=line | \
+# RUN: FileCheck %s --check-prefix=LINE
+
+# RUN: llvm-objdump %t.o -d --debug-inlined-funcs | \
+# RUN: FileCheck %s --check-prefix=UNICODE --strict-whitespace
+
+# RUN: llvm-objdump %t.o -d --debug-inlined-funcs --debug-indent=30 | \
+# RUN: FileCheck %s --check-prefix=INDENT --strict-whitespace
+
+# RUN: llvm-objdump %t.o -d --debug-inlined-funcs=ascii | \
+# RUN: FileCheck %s --check-prefix=ASCII --strict-whitespace
+
+# LINE: 0000000000000000 <bar>:
+# LINE-NEXT: 0: 8d 04 3e leal (%rsi,%rdi), %eax
+# LINE-NEXT: 3: 0f af f7 imull %edi, %esi
+# LINE-NEXT: 6: 01 f0 addl %esi, %eax
+# LINE-NEXT: 8: c3 retq
+# LINE-NEXT: 9: 0f 1f 80 00 00 00 00 nopl (%rax)
+# LINE-EMPTY:
+# LINE-NEXT: 0000000000000010 <foo>:
+# LINE-NEXT: debug-inlined-functions.c:8:16: bar inlined into foo
+# LINE-NEXT: 10: 8d 04 3e leal (%rsi,%rdi), %eax
+# LINE-NEXT: 13: 0f af f7 imull %edi, %esi
+# LINE-NEXT: 16: 01 f0 addl %esi, %eax
+# LINE-NEXT: debug-inlined-functions.c:8:16: end of bar inlined into foo
+# LINE-NEXT: 18: c3 retq
+
+# UNICODE: 0000000000000000 <bar>:
+# UNICODE-NEXT: 0: 8d 04 3e leal (%rsi,%rdi), %eax
+# UNICODE-NEXT: 3: 0f af f7 imull %edi, %esi
+# UNICODE-NEXT: 6: 01 f0 addl %esi, %eax
+# UNICODE-NEXT: 8: c3 retq
+# UNICODE-NEXT: 9: 0f 1f 80 00 00 00 00 nopl (%rax)
+# UNICODE-EMPTY:
+# UNICODE-NEXT: 0000000000000010 <foo>:
+# UNICODE-NEXT: ┠─ bar = inlined into foo
+# UNICODE-NEXT: 10: 8d 04 3e leal (%rsi,%rdi), %eax ┃
+# UNICODE-NEXT: 13: 0f af f7 imull %edi, %esi ┃
+# UNICODE-NEXT: 16: 01 f0 addl %esi, %eax ┻
+# UNICODE-NEXT: 18: c3 retq
+
+# INDENT: 0000000000000000 <bar>:
+# INDENT-NEXT: 0: 8d 04 3e leal (%rsi,%rdi), %eax
+# INDENT-NEXT: 3: 0f af f7 imull %edi, %esi
+# INDENT-NEXT: 6: 01 f0 addl %esi, %eax
+# INDENT-NEXT: 8: c3 retq
+# INDENT-NEXT: 9: 0f 1f 80 00 00 00 00 nopl (%rax)
+# INDENT-EMPTY:
+# INDENT-NEXT: 0000000000000010 <foo>:
+# INDENT-NEXT: ┠─ bar = inlined into foo
+# INDENT-NEXT: 10: 8d 04 3e leal (%rsi,%rdi), %eax ┃
+# INDENT-NEXT: 13: 0f af f7 imull %edi, %esi ┃
+# INDENT-NEXT: 16: 01 f0 addl %esi, %eax ┻
+# INDENT-NEXT: 18: c3 retq
+
+# ASCII: 0000000000000000 <bar>:
+# ASCII-NEXT: 0: 8d 04 3e leal (%rsi,%rdi), %eax
+# ASCII-NEXT: 3: 0f af f7 imull %edi, %esi
+# ASCII-NEXT: 6: 01 f0 addl %esi, %eax
+# ASCII-NEXT: 8: c3 retq
+# ASCII-NEXT: 9: 0f 1f 80 00 00 00 00 nopl (%rax)
+# ASCII-EMPTY:
+# ASCII-NEXT: 0000000000000010 <foo>:
+# ASCII-NEXT: |- bar = inlined into foo
+# ASCII-NEXT: 10: 8d 04 3e leal (%rsi,%rdi), %eax |
+# ASCII-NEXT: 13: 0f af f7 imull %edi, %esi |
+# ASCII-NEXT: 16: 01 f0 addl %esi, %eax v
+# ASCII-NEXT: 18: c3 retq
+
+ .file "debug-inlined-functions.c"
+ .text
+ .globl bar # -- Begin function bar
+ .p2align 4
+ .type bar,@function
+bar: # @bar
+.Lfunc_begin0:
+ .file 0 "" "debug-inlined-function.c"
+ .cfi_startproc
+# %bb.0:
+ #DEBUG_VALUE: bar:x <- $edi
+ #DEBUG_VALUE: bar:y <- $esi
+ # kill: def $esi killed $esi def $rsi
+ # kill: def $edi killed $edi def $rdi
+ .file 1 "" "debug-inlined-functions.c"
+ .loc 1 2 15 prologue_end
+ leal (%rsi,%rdi), %eax
+.Ltmp0:
+ #DEBUG_VALUE: bar:sum <- $eax
+ .loc 1 3 15
+ imull %edi, %esi
+.Ltmp1:
+ #DEBUG_VALUE: bar:y <- [DW_OP_LLVM_entry_value 1] $esi
+ #DEBUG_VALUE: bar:mul <- $esi
+ .loc 1 4 14
+ addl %esi, %eax
+.Ltmp2:
+ .loc 1 4 3 is_stmt 0
+ retq
+.Ltmp3:
+.Lfunc_end0:
+ .size bar, .Lfunc_end0-bar
+ .cfi_endproc
+ # -- End function
+ .globl foo # -- Begin function foo
+ .p2align 4
+ .type foo,@function
+foo: # @foo
+.Lfunc_begin1:
+ .cfi_startproc
+# %bb.0:
+ #DEBUG_VALUE: foo:a <- $edi
+ #DEBUG_VALUE: foo:b <- $esi
+ #DEBUG_VALUE: bar:x <- $edi
+ #DEBUG_VALUE: bar:y <- $esi
+ # kill: def $esi killed $esi def $rsi
+ # kill: def $edi killed $edi def $rdi
+ .loc 1 2 15 prologue_end is_stmt 1
+ leal (%rsi,%rdi), %eax
+.Ltmp4:
+ #DEBUG_VALUE: bar:sum <- $eax
+ .loc 1 3 15
+ imull %edi, %esi
+.Ltmp5:
+ #DEBUG_VALUE: foo:b <- [DW_OP_LLVM_entry_value 1] $esi
+ #DEBUG_VALUE: bar:mul <- $esi
+ .loc 1 4 14
+ addl %esi, %eax
+.Ltmp6:
+ #DEBUG_VALUE: foo:result <- $eax
+ .loc 1 9 3
+ retq
+.Ltmp7:
+.Lfunc_end1:
+ .size foo, .Lfunc_end1-foo
+ .cfi_endproc
+ # -- End function
+ .section .debug_loclists,"",@progbits
+ .long .Ldebug_list_header_end0-.Ldebug_list_header_start0 # Length
+.Ldebug_list_header_start0:
+ .short 5 # Version
+ .byte 8 # Address size
+ .byte 0 # Segment selector size
+ .long 8 # Offset entry count
+.Lloclists_table_base0:
+ .long .Ldebug_loc0-.Lloclists_table_base0
+ .long .Ldebug_loc1-.Lloclists_table_base0
+ .long .Ldebug_loc2-.Lloclists_table_base0
+ .long .Ldebug_loc3-.Lloclists_table_base0
+ .long .Ldebug_loc4-.Lloclists_table_base0
+ .long .Ldebug_loc5-.Lloclists_table_base0
+ .long .Ldebug_loc6-.Lloclists_table_base0
+ .long .Ldebug_loc7-.Lloclists_table_base0
+.Ldebug_loc0:
+ .byte 4 # DW_LLE_offset_pair
+ .uleb128 .Lfunc_begin0-.Lfunc_begin0 # starting offset
+ .uleb128 .Ltmp1-.Lfunc_begin0 # ending offset
+ .byte 1 # Loc expr size
+ .byte 84 # super-register DW_OP_reg4
+ .byte 4 # DW_LLE_offset_pair
+ .uleb128 .Ltmp1-.Lfunc_begin0 # starting offset
+ .uleb128 .Lfunc_end0-.Lfunc_begin0 # ending offset
+ .byte 4 # Loc expr size
+ .byte 163 # DW_OP_entry_value
+ .byte 1 # 1
+ .byte 84 # super-register DW_OP_reg4
+ .byte 159 # DW_OP_stack_value
+ .byte 0 # DW_LLE_end_of_list
+.Ldebug_loc1:
+ .byte 4 # DW_LLE_offset_pair
+ .uleb128 .Ltmp0-.Lfunc_begin0 # starting offset
+ .uleb128 .Ltmp2-.Lfunc_begin0 # ending offset
+ .byte 1 # Loc expr size
+ .byte 80 # super-register DW_OP_reg0
+ .byte 0 # DW_LLE_end_of_list
+.Ldebug_loc2:
+ .byte 4 # DW_LLE_offset_pair
+ .uleb128 .Ltmp1-.Lfunc_begin0 # starting offset
+ .uleb128 .Lfunc_end0-.Lfunc_begin0 # ending offset
+ .byte 1 # Loc expr size
+ .byte 84 # super-register DW_OP_reg4
+ .byte 0 # DW_LLE_end_of_list
+.Ldebug_loc3:
+ .byte 4 # DW_LLE_offset_pair
+ .uleb128 .Lfunc_begin1-.Lfunc_begin0 # starting offset
+ .uleb128 .Ltmp5-.Lfunc_begin0 # ending offset
+ .byte 1 # Loc expr size
+ .byte 84 # super-register DW_OP_reg4
+ .byte 4 # DW_LLE_offset_pair
+ .uleb128 .Ltmp5-.Lfunc_begin0 # starting offset
+ .uleb128 .Lfunc_end1-.Lfunc_begin0 # ending offset
+ .byte 4 # Loc expr size
+ .byte 163 # DW_OP_entry_value
+ .byte 1 # 1
+ .byte 84 # super-register DW_OP_reg4
+ .byte 159 # DW_OP_stack_value
+ .byte 0 # DW_LLE_end_of_list
+.Ldebug_loc4:
+ .byte 4 # DW_LLE_offset_pair
+ .uleb128 .Lfunc_begin1-.Lfunc_begin0 # starting offset
+ .uleb128 .Ltmp5-.Lfunc_begin0 # ending offset
+ .byte 1 # Loc expr size
+ .byte 84 # super-register DW_OP_reg4
+ .byte 0 # DW_LLE_end_of_list
+.Ldebug_loc5:
+ .byte 4 # DW_LLE_offset_pair
+ .uleb128 .Ltmp4-.Lfunc_begin0 # starting offset
+ .uleb128 .Ltmp6-.Lfunc_begin0 # ending offset
+ .byte 1 # Loc expr size
+ .byte 80 # super-register DW_OP_reg0
+ .byte 0 # DW_LLE_end_of_list
+.Ldebug_loc6:
+ .byte 4 # DW_LLE_offset_pair
+ .uleb128 .Ltmp5-.Lfunc_begin0 # starting offset
+ .uleb128 .Lfunc_end1-.Lfunc_begin0 # ending offset
+ .byte 1 # Loc expr size
+ .byte 84 # super-register DW_OP_reg4
+ .byte 0 # DW_LLE_end_of_list
+.Ldebug_loc7:
+ .byte 4 # DW_LLE_offset_pair
+ .uleb128 .Ltmp6-.Lfunc_begin0 # starting offset
+ .uleb128 .Lfunc_end1-.Lfunc_begin0 # ending offset
+ .byte 1 # Loc expr size
+ .byte 80 # super-register DW_OP_reg0
+ .byte 0 # DW_LLE_end_of_list
+.Ldebug_list_header_end0:
+ .section .debug_abbrev,"",@progbits
+ .byte 1 # Abbreviation Code
+ .byte 17 # DW_TAG_compile_unit
+ .byte 1 # DW_CHILDREN_yes
+ .byte 37 # DW_AT_producer
+ .byte 37 # DW_FORM_strx1
+ .byte 19 # DW_AT_language
+ .byte 5 # DW_FORM_data2
+ .byte 3 # DW_AT_name
+ .byte 37 # DW_FORM_strx1
+ .byte 114 # DW_AT_str_offsets_base
+ .byte 23 # DW_FORM_sec_offset
+ .byte 16 # DW_AT_stmt_list
+ .byte 23 # DW_FORM_sec_offset
+ .byte 27 # DW_AT_comp_dir
+ .byte 37 # DW_FORM_strx1
+ .byte 17 # DW_AT_low_pc
+ .byte 27 # DW_FORM_addrx
+ .byte 18 # DW_AT_high_pc
+ .byte 6 # DW_FORM_data4
+ .byte 115 # DW_AT_addr_base
+ .byte 23 # DW_FORM_sec_offset
+ .ascii "\214\001" # DW_AT_loclists_base
+ .byte 23 # DW_FORM_sec_offset
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 2 # Abbreviation Code
+ .byte 46 # DW_TAG_subprogram
+ .byte 1 # DW_CHILDREN_yes
+ .byte 17 # DW_AT_low_pc
+ .byte 27 # DW_FORM_addrx
+ .byte 18 # DW_AT_high_pc
+ .byte 6 # DW_FORM_data4
+ .byte 64 # DW_AT_frame_base
+ .byte 24 # DW_FORM_exprloc
+ .byte 122 # DW_AT_call_all_calls
+ .byte 25 # DW_FORM_flag_present
+ .byte 49 # DW_AT_abstract_origin
+ .byte 19 # DW_FORM_ref4
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 3 # Abbreviation Code
+ .byte 5 # DW_TAG_formal_parameter
+ .byte 0 # DW_CHILDREN_no
+ .byte 2 # DW_AT_location
+ .byte 24 # DW_FORM_exprloc
+ .byte 49 # DW_AT_abstract_origin
+ .byte 19 # DW_FORM_ref4
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 4 # Abbreviation Code
+ .byte 5 # DW_TAG_formal_parameter
+ .byte 0 # DW_CHILDREN_no
+ .byte 2 # DW_AT_location
+ .byte 34 # DW_FORM_loclistx
+ .byte 49 # DW_AT_abstract_origin
+ .byte 19 # DW_FORM_ref4
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 5 # Abbreviation Code
+ .byte 52 # DW_TAG_variable
+ .byte 0 # DW_CHILDREN_no
+ .byte 2 # DW_AT_location
+ .byte 34 # DW_FORM_loclistx
+ .byte 49 # DW_AT_abstract_origin
+ .byte 19 # DW_FORM_ref4
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 6 # Abbreviation Code
+ .byte 46 # DW_TAG_subprogram
+ .byte 1 # DW_CHILDREN_yes
+ .byte 3 # DW_AT_name
+ .byte 37 # DW_FORM_strx1
+ .byte 58 # DW_AT_decl_file
+ .byte 11 # DW_FORM_data1
+ .byte 59 # DW_AT_decl_line
+ .byte 11 # DW_FORM_data1
+ .byte 39 # DW_AT_prototyped
+ .byte 25 # DW_FORM_flag_present
+ .byte 73 # DW_AT_type
+ .byte 19 # DW_FORM_ref4
+ .byte 63 # DW_AT_external
+ .byte 25 # DW_FORM_flag_present
+ .byte 32 # DW_AT_inline
+ .byte 33 # DW_FORM_implicit_const
+ .byte 1
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 7 # Abbreviation Code
+ .byte 5 # DW_TAG_formal_parameter
+ .byte 0 # DW_CHILDREN_no
+ .byte 3 # DW_AT_name
+ .byte 37 # DW_FORM_strx1
+ .byte 58 # DW_AT_decl_file
+ .byte 11 # DW_FORM_data1
+ .byte 59 # DW_AT_decl_line
+ .byte 11 # DW_FORM_data1
+ .byte 73 # DW_AT_type
+ .byte 19 # DW_FORM_ref4
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 8 # Abbreviation Code
+ .byte 52 # DW_TAG_variable
+ .byte 0 # DW_CHILDREN_no
+ .byte 3 # DW_AT_name
+ .byte 37 # DW_FORM_strx1
+ .byte 58 # DW_AT_decl_file
+ .byte 11 # DW_FORM_data1
+ .byte 59 # DW_AT_decl_line
+ .byte 11 # DW_FORM_data1
+ .byte 73 # DW_AT_type
+ .byte 19 # DW_FORM_ref4
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 9 # Abbreviation Code
+ .byte 36 # DW_TAG_base_type
+ .byte 0 # DW_CHILDREN_no
+ .byte 3 # DW_AT_name
+ .byte 37 # DW_FORM_strx1
+ .byte 62 # DW_AT_encoding
+ .byte 11 # DW_FORM_data1
+ .byte 11 ...
[truncated]
|
gulfemsavrun
force-pushed
the
llvm-objdump
branch
from
a6851c7 to
2781748
Compare
gulfemsavrun
force-pushed
the
llvm-objdump
branch
2 times, most recently
from
8bc4966 to
0c29a77
Compare
There was a problem hiding this comment.
This deserves a release note, I think, including highlighting the renamed indent option.
Also, I think it would be useful to have a test showing the interaction between the variable and inlined function options, so how a variable is displayed inside an inlined function when both are enabled.
On that note, does it even make sense for the two to have different values? E.g. one having unicode and the other ASCII? I wonder if we need to revisit how the option works. I'm not sure what would be the best approach, but perhaps there could be a "format" option and a separate option (or options) to determine what to enable. Alternatively, we just go with it and accept that ASCII + unicode is allowed.
| @@ -153,10 +153,16 @@ OPTIONS | |||
| alongside disassembly. ``format`` may be ``unicode`` or ``ascii``, defaulting | |||
| to ``unicode`` if omitted. | |||
|
|
|||
| .. option:: --debug-vars-indent=<width> | |||
| .. option:: --debug-indent=<width> | |||
There was a problem hiding this comment.
Options in this file should ideally be in alphabetical order, so please adjust accordingly.
There was a problem hiding this comment.
If we ignore hyphens, --debug-indent should appear before --debuginfod (and if we don't, --debug-file-directory is in the wrong place relative to --debuginfod).
| FuncDie.getName(llvm::DINameKind::LinkageName); | ||
| if (!MangledCallerName) | ||
| return; | ||
| std::string CallerName = llvm::demangle(MangledCallerName); |
There was a problem hiding this comment.
I think the call to demangle needs to be conditional on the --demangle option. Same below. This should also be tested.
There was a problem hiding this comment.
I added Demangle check.
| # RUN: llvm-objdump %t.o -d --debug-inlined-funcs=line | \ | ||
| # RUN: FileCheck %s --check-prefix=LINE | ||
|
|
||
| # RUN: llvm-objdump %t.o -d --debug-inlined-funcs | \ |
There was a problem hiding this comment.
You should also have a test for explicitly specifying unicode.
There was a problem hiding this comment.
I explicitly added unicode into the test.
There was a problem hiding this comment.
Not quite what I wanted: I'd like a test that shows that --debug-inlined-funcs (without argument) and --debug-inlined-funcs=unicode are identical, i.e. that unicode is the default value.
gulfemsavrun
force-pushed
the
llvm-objdump
branch
from
0c29a77 to
91c4418
Compare
|
Thanks for all the feedback @jh7370!
I added this into the release notes.
I extended the test case to show how it looks like when
I think we can add a new option like Note: I'll be OOO for a few weeks, so my responses might be delayed. |
gulfemsavrun
force-pushed
the
llvm-objdump
branch
from
91c4418 to
c2d8564
Compare
This patch adds the support for displaying inlined functions into llvm-objdump. 1) It extends the source variable display support for inlined functions both for ascii and unicode formats. 2) It also introduces a new format called line that only prints a line for the start and end of an inlined function without line-drawing characters.
gulfemsavrun
force-pushed
the
llvm-objdump
branch
from
c2d8564 to
8ea0679
Compare
There was a problem hiding this comment.
Please don't force push to update PR branches once they have active reviewers, in line with the LLVM GitHub policy (see https://llvm.org/docs/GitHub.html#updating-pull-requests and the following section): it impairs the reviewing experience, making it harder and slower for me to review your changes. Also, please don't resolve conversations that I initiated, as I use the "resolved" status to track whether I'm happy with the response you've made (it's not unusual for people to miss/misunderstand/etc comments and do the wrong thing). I've gone into more detail on Discourse in the past on this topic.
I think we can add a new option like --debug-format for the format. It can be shared between --debug-vars and --debug--inlined-funcs like --debug-indent option.
Having given it a bit more thought, I don't think we want an extra option and it's fine to leave the behaviour as you've implemented. If users want to specify different formats for the two, that's on them.
What are your thoughts on the line format that I added? This was a feature requested by some of our project developers who do not want to look at line-drawing characters.
I think the format looks reasonable. I'm not certain about the naming though, as line doesn't really convey anything to me. I'm thinking something more like limits-only or something like that, but am also open to other ideas.
@MaskRay, @ostannard, @eugenis, paging you since you implemented or reviewed the original --debug-vars option, which this is closely tied to.
| @@ -153,10 +153,16 @@ OPTIONS | |||
| alongside disassembly. ``format`` may be ``unicode`` or ``ascii``, defaulting | |||
| to ``unicode`` if omitted. | |||
|
|
|||
| .. option:: --debug-vars-indent=<width> | |||
| .. option:: --debug-indent=<width> | |||
There was a problem hiding this comment.
If we ignore hyphens, --debug-indent should appear before --debuginfod (and if we don't, --debug-file-directory is in the wrong place relative to --debuginfod).
| * llvm-objdump now supports the `--debug-inlined-funcs` flag that prints the | ||
| locations of inlined functions alongside disassembly. It also renames | ||
| `--debug-vars-indent` flag to `--debug-indent`. |
There was a problem hiding this comment.
| * llvm-objdump now supports the `--debug-inlined-funcs` flag that prints the | |
| locations of inlined functions alongside disassembly. It also renames | |
| `--debug-vars-indent` flag to `--debug-indent`. | |
| * llvm-objdump now supports the `--debug-inlined-funcs` flag, which prints the | |
| locations of inlined functions alongside disassembly. The | |
| `--debug-vars-indent` flag has also been renamed to `--debug-indent`. |
I'd reword these slightly. The first sentence feels more natural this way and the second was using the incorrect phrasing for a release note (what did "it" refer to?)
| * llvm-objdump now supports the `--debug-inlined-funcs` flag that prints the | ||
| locations of inlined functions alongside disassembly. It also renames | ||
| `--debug-vars-indent` flag to `--debug-indent`. | ||
|
|
There was a problem hiding this comment.
Nit: no need for the extra blank line.
| Values<"unicode,ascii,line">; | ||
| def : Flag<["--"], "debug-inlined-funcs">, Alias<debug_inlined_funcs_EQ>, AliasArgs<["unicode"]>; | ||
|
|
||
| def debug_indent_EQ : Joined<["--"], "debug-indent=">, | ||
| HelpText<"Distance to indent the source-level variable display, " |
There was a problem hiding this comment.
The help text here needs tweaking now. Also, these options should be in alphabetical order too.
| Child.getTag() == dwarf::DW_TAG_formal_parameter) | ||
| if (DbgVariables != DVDisabled && | ||
| (Child.getTag() == dwarf::DW_TAG_variable || | ||
| Child.getTag() == dwarf::DW_TAG_formal_parameter)) |
There was a problem hiding this comment.
Please add braces on each of the if/else clauses here. The coding standards say that if one part of an if/else chain needs braces, all parts should have it.
| # RUN: llvm-objdump %t.o -d --debug-inlined-funcs=line | \ | ||
| # RUN: FileCheck %s --check-prefix=LINE | ||
|
|
||
| # RUN: llvm-objdump %t.o -d --debug-inlined-funcs | \ |
There was a problem hiding this comment.
Not quite what I wanted: I'd like a test that shows that --debug-inlined-funcs (without argument) and --debug-inlined-funcs=unicode are identical, i.e. that unicode is the default value.
This patch adds the support for displaying inlined functions into llvm-objdump.
It extends the source variable display
support for inlined functions both for ascii and unicode formats.
It also introduces a new format called line that only prints a line for the start and end of an inlined function without line-drawing characters.