[RFC,readelf] Fix end_seq entry in -wL

Message ID 8a71b750-7ba3-ce9e-ba57-a2823d1eb8f5@suse.de
State New
Headers show
Series
  • [RFC,readelf] Fix end_seq entry in -wL
Related show

Commit Message

Tom de Vries July 3, 2020, 3:23 p.m.
Hi,

this patch is missing updates for the test-suite, without those we have:
...
FAIL: objdump -WL
FAIL: readelf -wiaoRlL dw5
...
but I'd rather update those once the new format for the end_seq lines
has been agreed upon.

Any comments?

Thanks,
- Tom

Comments

David Faust via Binutils July 9, 2020, 1:31 p.m. | #1
Hi Tom,

> Any comments?


The new patch looks fine to me, but I think that it would be useful 
to include a comment in the code itself explaining why the line number 
is not displayed.

Cheers
  Nick

Patch

[readelf] Fix end_seq entry in -wL

For a hello world with debug info, we have with readelf -wl:
...
  [0x000002a3]  Extended opcode 2: set Address to 0x400598
  [0x000002ae]  Advance Line by 43 to 44
  [0x000002b0]  Copy
  [0x000002b1]  Special opcode 62: advance Address by 4 to 0x40059c \
    and Line by 1 to 45
  [0x000002b2]  Advance PC by 1 to 0x40059d
  [0x000002b4]  Extended opcode 1: End of Sequence
...
and with readelf -wL (dropping the File name part):
...
Line number    Starting address    View    Stmt
         44            0x400598               x
         45            0x40059c               x
         45            0x40059d               x
...

The "End of Sequence" is DW_LNE_end_sequence, which:
...
sets the end_sequence register of the state machine to “true” and appends a
row to the matrix using the current values of the state-machine registers.
...
where the end_sequence register has the semantics:
...
A boolean indicating that the current address is that of the first byte after
the end of a sequence of target machine instructions. end_sequence
terminates a sequence of lines; therefore other information in the same
row is not meaningful.
...

So, the last entry in the -wL bit shows us a line number which is not
meaningful, which is confusing.

Fix this by instead emitting:
...
Line number    Starting address    View    Stmt
         44            0x400598               x
         45            0x40059c               x
          -            0x40059d
...

binutils/ChangeLog:

2020-07-03  Tom de Vries  <tdevries@suse.de>

	* dwarf.c (display_debug_lines_decoded): Don't emit meaningless
	information in the end_sequence row.

---
 binutils/dwarf.c | 62 +++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 46 insertions(+), 16 deletions(-)

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index bdabd9c4ba..189f0f5f71 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -4991,34 +4991,64 @@  display_debug_lines_decoded (struct dwarf_section *  section,
 	      if (!do_wide || (fileNameLength <= MAX_FILENAME_LENGTH))
 		{
 		  if (linfo.li_max_ops_per_insn == 1)
-		    printf ("%-35s  %11d  %#18" DWARF_VMA_FMT "x",
-			    newFileName, state_machine_regs.line,
-			    state_machine_regs.address);
+		    {
+		      if (xop == -DW_LNE_end_sequence)
+			printf ("%-35s  %11s  %#18" DWARF_VMA_FMT "x",
+				newFileName, "-",
+				state_machine_regs.address);
+		      else
+			printf ("%-35s  %11d  %#18" DWARF_VMA_FMT "x",
+				newFileName, state_machine_regs.line,
+				state_machine_regs.address);
+		    }
 		  else
-		    printf ("%-35s  %11d  %#18" DWARF_VMA_FMT "x[%d]",
-			    newFileName, state_machine_regs.line,
-			    state_machine_regs.address,
-			    state_machine_regs.op_index);
+		    {
+		      if (xop == -DW_LNE_end_sequence)
+			printf ("%-35s  %11s  %#18" DWARF_VMA_FMT "x[%d]",
+				newFileName, "-",
+				state_machine_regs.address,
+				state_machine_regs.op_index);
+		      else
+			printf ("%-35s  %11d  %#18" DWARF_VMA_FMT "x[%d]",
+				newFileName, state_machine_regs.line,
+				state_machine_regs.address,
+				state_machine_regs.op_index);
+		    }
 		}
 	      else
 		{
 		  if (linfo.li_max_ops_per_insn == 1)
-		    printf ("%s  %11d  %#18" DWARF_VMA_FMT "x",
-			    newFileName, state_machine_regs.line,
-			    state_machine_regs.address);
+		    {
+		      if (xop == -DW_LNE_end_sequence)
+			printf ("%s  %11s  %#18" DWARF_VMA_FMT "x",
+				newFileName, "-",
+				state_machine_regs.address);
+		      else
+			printf ("%s  %11d  %#18" DWARF_VMA_FMT "x",
+				newFileName, state_machine_regs.line,
+				state_machine_regs.address);
+		    }			
 		  else
-		    printf ("%s  %11d  %#18" DWARF_VMA_FMT "x[%d]",
-			    newFileName, state_machine_regs.line,
-			    state_machine_regs.address,
-			    state_machine_regs.op_index);
+		    {
+		      if (xop == -DW_LNE_end_sequence)
+			printf ("%s  %11s  %#18" DWARF_VMA_FMT "x[%d]",
+				newFileName, "-",
+				state_machine_regs.address,
+				state_machine_regs.op_index);
+		      else
+			printf ("%s  %11d  %#18" DWARF_VMA_FMT "x[%d]",
+				newFileName, state_machine_regs.line,
+				state_machine_regs.address,
+				state_machine_regs.op_index);
+		    }
 		}
 
-	      if (state_machine_regs.view)
+	      if (state_machine_regs.view && (xop != -DW_LNE_end_sequence))
 		printf ("  %6u", state_machine_regs.view);
 	      else
 		printf ("        ");
 
-	      if (state_machine_regs.is_stmt)
+	      if (state_machine_regs.is_stmt && (xop != -DW_LNE_end_sequence))
 		printf ("       x");
 
 	      putchar ('\n');