public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [review] gdb: Include end of sequence markers in the line table
@ 2019-11-08  0:23 Andrew Burgess (Code Review)
  2019-11-08 15:03 ` [review v2] " Andrew Burgess (Code Review)
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Andrew Burgess (Code Review) @ 2019-11-08  0:23 UTC (permalink / raw)
  To: gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/525
......................................................................

gdb: Include end of sequence markers in the line table

In this commit:

  commit d9b3de22f33e400f7f409cce3acf6c7dab07dd79
  Date:   Wed May 27 14:44:29 2015 -0700

      Add struct to record dwarf line number state machine.

I believe an unintended change was made to how we store the DWARF line
table, the end of sequence markers between sequences of lines were
lost from the line table.

This commit fixes this small oversight and restores the end of
sequence markers.

Given that we've survived this long without noticing is clearly an
indication that this isn't that serious, however, a later patch that I
am developing would benefit from having the markers in place, so I'd
like to restore them.

Having the markers also means that the output of 'maintenance info
line-table' now more closely reflects the DWARF line table.

I've taken this opportunity to improve how 'maintenance info
line-table' displays the end of sequence markers - it now uses the END
keyword, rather than just printing an entry with line number 0.  So we
see this:

  INDEX    LINE ADDRESS
  0          12 0x00000000004003b0
  1          17 0x00000000004003b0
  2          18 0x00000000004003b0
  3         END 0x00000000004003b7
  4           5 0x00000000004004a0
  5           6 0x00000000004004a0
  6         END 0x00000000004004a7

Instead of what we would have seen, which was this:

  INDEX    LINE ADDRESS
  0          12 0x00000000004003b0
  1          17 0x00000000004003b0
  2          18 0x00000000004003b0
  3           0 0x00000000004003b7
  4           5 0x00000000004004a0
  5           6 0x00000000004004a0
  6           0 0x00000000004004a7

I've added a small test that uses 'maintenance info line-table' to
ensure that we don't regress this again.

gdb/ChangeLog:

	* dwarf2read.c (lnp_state_machine::record_line): Include
	end_sequence parameter in debug print out.  Record the line if we
	are at an end_sequence marker even if it's not the start of a
	statement.
	* symmisc.c (maintenance_print_one_line_table): Print end of
	sequence markers with 'END' not '0'.

gdb/testsuite/ChangeLog:

	* gdb.base/maint.exp: Update line table parsing test.
	* gdb.dwarf2/dw2-ranges-base.exp: Add new line table parsing test.

Change-Id: I002f872248db82a1d4fefdc6b51ff5dbf932d8a8
---
M gdb/ChangeLog
M gdb/dwarf2read.c
M gdb/symmisc.c
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.base/maint.exp
M gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
6 files changed, 61 insertions(+), 11 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f43d3a5..e7f7976 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2019-11-08  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* dwarf2read.c (lnp_state_machine::record_line): Include
+	end_sequence parameter in debug print out.  Record the line if we
+	are at an end_sequence marker even if it's not the start of a
+	statement.
+	* symmisc.c (maintenance_print_one_line_table): Print end of
+	sequence markers with 'END' not '0'.
+
 2019-11-06  Christian Biesinger  <cbiesinger@google.com>
 
 	* linux-tdep.c (linux_info_proc): Use strtok_r instead of strtok.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0a7a033..1070fc3 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -21072,10 +21072,11 @@
     {
       fprintf_unfiltered (gdb_stdlog,
 			  "Processing actual line %u: file %u,"
-			  " address %s, is_stmt %u, discrim %u\n",
+			  " address %s, is_stmt %u, discrim %u%s\n",
 			  m_line, m_file,
 			  paddress (m_gdbarch, m_address),
-			  m_is_stmt, m_discriminator);
+			  m_is_stmt, m_discriminator,
+			  (end_sequence ? "\t(end sequence)" : ""));
     }
 
   file_entry *fe = current_file ();
@@ -21088,7 +21089,8 @@
   else if (m_op_index == 0 || end_sequence)
     {
       fe->included_p = 1;
-      if (m_record_lines_p && (producer_is_codewarrior (m_cu) || m_is_stmt))
+      if (m_record_lines_p
+	  && (producer_is_codewarrior (m_cu) || m_is_stmt || end_sequence))
 	{
 	  if (m_last_subfile != m_cu->get_builder ()->get_current_subfile ()
 	      || end_sequence)
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 24ee255..5ec5cd0 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -1000,8 +1000,12 @@
 	  struct linetable_entry *item;
 
 	  item = &linetable->item [i];
-	  printf_filtered (_("%-6d %6d %s\n"), i, item->line,
-			   core_addr_to_string (item->pc));
+	  printf_filtered ("%-6d ", i);
+	  if (item->line > 0)
+	    printf_filtered ("%6d ", item->line);
+	  else
+	    printf_filtered ("%6s ", _("END"));
+	  printf_filtered ("%s\n", core_addr_to_string (item->pc));
 	}
     }
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 9b7bcf6..c31809c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-11-08  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.base/maint.exp: Update line table parsing test.
+	* gdb.dwarf2/dw2-ranges-base.exp: Add new line table parsing test.
+
 2019-11-02  Tom de Vries  <tdevries@suse.de>
 
 	* gdb.base/advance.exp: Drop superfluous 3rd argument to gdb_test.
diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 15988c7..9022063 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -508,8 +508,8 @@
 # Test that "main info line-table" w/o a file name shows the symtab for
 # $srcfile.
 set saw_srcfile 0
-set test "maint info line-table w/o a file name"
-gdb_test_multiple "maint info line-table" $test {
+gdb_test_multiple "maint info line-table" \
+    "maint info line-table w/o a file name" {
     -re "symtab: \[^\n\r\]+${srcfile} \\(\\(struct symtab \\*\\) $hex\\)\r\nlinetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS" {
 	set saw_srcfile 1
 	exp_continue
@@ -518,7 +518,11 @@
 	# Match each symtab to avoid overflowing expect's buffer.
 	exp_continue
     }
-    -re "$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
+    -re "symtab: \[^\n\r\]+ \\(\\(struct symtab \\*\\) $hex\\)\r\nlinetable: \\(\\(struct linetable \\*\\) 0x0\\):\r\nNo line table.\r\n" {
+	# For symtabs with no linetable.
+	exp_continue
+    }
+    -re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
 	# Line table entries can be long too:
 	#
 	#  INDEX    LINE ADDRESS
@@ -531,13 +535,17 @@
 	#  6          45 0x0000000000400740
 	#  (...)
 	#  454       129 0x00007ffff7df1d28
-	#  455         0 0x00007ffff7df1d3f
+	#  455       END 0x00007ffff7df1d3f
 	#
 	# Match each line to avoid overflowing expect's buffer.
 	exp_continue
     }
-    -re "$gdb_prompt $" {
-	gdb_assert $saw_srcfile $test
+    -re "^$decimal\[ \t\]+END\[ \t\]+$hex\r\n" {
+	# Matches an end marker in the above.
+	exp_continue
+    }
+    -re "^$gdb_prompt $" {
+	gdb_assert $saw_srcfile $gdb_test_name
     }
 }
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
index 5405d7d..cff9055 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
@@ -141,3 +141,25 @@
     "Line 21 of .* starts at address .* and ends at .*"
 gdb_test "info line frame3" \
     "Line 31 of .* starts at address .* and ends at .*"
+
+# Ensure that the line table correctly tracks the end of sequence markers.
+set end_seq_count 0
+gdb_test_multiple "maint info line-table" \
+    "count END markers in line table" {
+	-re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
+	    exp_continue
+	}
+	-re "^$decimal\[ \t\]+END\[ \t\]+$hex\r\n" {
+	    incr end_seq_count
+	    exp_continue
+	}
+	-re "^$gdb_prompt $" {
+	    gdb_assert [expr $end_seq_count == 3] $gdb_test_name
+	}
+	-re ".*linetable: \\(\\(struct linetable \\*\\) 0x0\\):\r\nNo line table.\r\n" {
+	    exp_continue
+	}
+	-re ".*linetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS\r\n" {
+	    exp_continue
+	}
+    }

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I002f872248db82a1d4fefdc6b51ff5dbf932d8a8
Gerrit-Change-Number: 525
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-MessageType: newchange

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [review v2] gdb: Include end of sequence markers in the line table
  2019-11-08  0:23 [review] gdb: Include end of sequence markers in the line table Andrew Burgess (Code Review)
@ 2019-11-08 15:03 ` Andrew Burgess (Code Review)
  2019-11-12 12:38 ` [review v3] " Andrew Burgess (Code Review)
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Andrew Burgess (Code Review) @ 2019-11-08 15:03 UTC (permalink / raw)
  To: gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/525
......................................................................

gdb: Include end of sequence markers in the line table

In this commit:

  commit d9b3de22f33e400f7f409cce3acf6c7dab07dd79
  Date:   Wed May 27 14:44:29 2015 -0700

      Add struct to record dwarf line number state machine.

I believe an unintended change was made to how we store the DWARF line
table, the end of sequence markers between sequences of lines were
lost from the line table.

This commit fixes this small oversight and restores the end of
sequence markers.

Given that we've survived this long without noticing is clearly an
indication that this isn't that serious, however, a later patch that I
am developing would benefit from having the markers in place, so I'd
like to restore them.

Having the markers also means that the output of 'maintenance info
line-table' now more closely reflects the DWARF line table.

I've taken this opportunity to improve how 'maintenance info
line-table' displays the end of sequence markers - it now uses the END
keyword, rather than just printing an entry with line number 0.  So we
see this:

  INDEX    LINE ADDRESS
  0          12 0x00000000004003b0
  1          17 0x00000000004003b0
  2          18 0x00000000004003b0
  3         END 0x00000000004003b7
  4           5 0x00000000004004a0
  5           6 0x00000000004004a0
  6         END 0x00000000004004a7

Instead of what we would have seen, which was this:

  INDEX    LINE ADDRESS
  0          12 0x00000000004003b0
  1          17 0x00000000004003b0
  2          18 0x00000000004003b0
  3           0 0x00000000004003b7
  4           5 0x00000000004004a0
  5           6 0x00000000004004a0
  6           0 0x00000000004004a7

I've added a small test that uses 'maintenance info line-table' to
ensure that we don't regress this again.

gdb/ChangeLog:

	* dwarf2read.c (lnp_state_machine::record_line): Include
	end_sequence parameter in debug print out.  Record the line if we
	are at an end_sequence marker even if it's not the start of a
	statement.
	* symmisc.c (maintenance_print_one_line_table): Print end of
	sequence markers with 'END' not '0'.

gdb/testsuite/ChangeLog:

	* gdb.base/maint.exp: Update line table parsing test.
	* gdb.dwarf2/dw2-ranges-base.exp: Add new line table parsing test.

Change-Id: I002f872248db82a1d4fefdc6b51ff5dbf932d8a8
---
M gdb/ChangeLog
M gdb/dwarf2read.c
M gdb/symmisc.c
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.base/maint.exp
M gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
6 files changed, 61 insertions(+), 11 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f5c8a76..f1fa450 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2019-11-08  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* dwarf2read.c (lnp_state_machine::record_line): Include
+	end_sequence parameter in debug print out.  Record the line if we
+	are at an end_sequence marker even if it's not the start of a
+	statement.
+	* symmisc.c (maintenance_print_one_line_table): Print end of
+	sequence markers with 'END' not '0'.
+
 2019-11-08  Tom Tromey  <tromey@adacore.com>
 
 	* top.c (read_command_file): Update.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0a7a033..1070fc3 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -21072,10 +21072,11 @@
     {
       fprintf_unfiltered (gdb_stdlog,
 			  "Processing actual line %u: file %u,"
-			  " address %s, is_stmt %u, discrim %u\n",
+			  " address %s, is_stmt %u, discrim %u%s\n",
 			  m_line, m_file,
 			  paddress (m_gdbarch, m_address),
-			  m_is_stmt, m_discriminator);
+			  m_is_stmt, m_discriminator,
+			  (end_sequence ? "\t(end sequence)" : ""));
     }
 
   file_entry *fe = current_file ();
@@ -21088,7 +21089,8 @@
   else if (m_op_index == 0 || end_sequence)
     {
       fe->included_p = 1;
-      if (m_record_lines_p && (producer_is_codewarrior (m_cu) || m_is_stmt))
+      if (m_record_lines_p
+	  && (producer_is_codewarrior (m_cu) || m_is_stmt || end_sequence))
 	{
 	  if (m_last_subfile != m_cu->get_builder ()->get_current_subfile ()
 	      || end_sequence)
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 24ee255..5ec5cd0 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -1000,8 +1000,12 @@
 	  struct linetable_entry *item;
 
 	  item = &linetable->item [i];
-	  printf_filtered (_("%-6d %6d %s\n"), i, item->line,
-			   core_addr_to_string (item->pc));
+	  printf_filtered ("%-6d ", i);
+	  if (item->line > 0)
+	    printf_filtered ("%6d ", item->line);
+	  else
+	    printf_filtered ("%6s ", _("END"));
+	  printf_filtered ("%s\n", core_addr_to_string (item->pc));
 	}
     }
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 9b7bcf6..c31809c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-11-08  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.base/maint.exp: Update line table parsing test.
+	* gdb.dwarf2/dw2-ranges-base.exp: Add new line table parsing test.
+
 2019-11-02  Tom de Vries  <tdevries@suse.de>
 
 	* gdb.base/advance.exp: Drop superfluous 3rd argument to gdb_test.
diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 15988c7..9022063 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -508,8 +508,8 @@
 # Test that "main info line-table" w/o a file name shows the symtab for
 # $srcfile.
 set saw_srcfile 0
-set test "maint info line-table w/o a file name"
-gdb_test_multiple "maint info line-table" $test {
+gdb_test_multiple "maint info line-table" \
+    "maint info line-table w/o a file name" {
     -re "symtab: \[^\n\r\]+${srcfile} \\(\\(struct symtab \\*\\) $hex\\)\r\nlinetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS" {
 	set saw_srcfile 1
 	exp_continue
@@ -518,7 +518,11 @@
 	# Match each symtab to avoid overflowing expect's buffer.
 	exp_continue
     }
-    -re "$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
+    -re "symtab: \[^\n\r\]+ \\(\\(struct symtab \\*\\) $hex\\)\r\nlinetable: \\(\\(struct linetable \\*\\) 0x0\\):\r\nNo line table.\r\n" {
+	# For symtabs with no linetable.
+	exp_continue
+    }
+    -re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
 	# Line table entries can be long too:
 	#
 	#  INDEX    LINE ADDRESS
@@ -531,13 +535,17 @@
 	#  6          45 0x0000000000400740
 	#  (...)
 	#  454       129 0x00007ffff7df1d28
-	#  455         0 0x00007ffff7df1d3f
+	#  455       END 0x00007ffff7df1d3f
 	#
 	# Match each line to avoid overflowing expect's buffer.
 	exp_continue
     }
-    -re "$gdb_prompt $" {
-	gdb_assert $saw_srcfile $test
+    -re "^$decimal\[ \t\]+END\[ \t\]+$hex\r\n" {
+	# Matches an end marker in the above.
+	exp_continue
+    }
+    -re "^$gdb_prompt $" {
+	gdb_assert $saw_srcfile $gdb_test_name
     }
 }
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
index 5405d7d..cff9055 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
@@ -141,3 +141,25 @@
     "Line 21 of .* starts at address .* and ends at .*"
 gdb_test "info line frame3" \
     "Line 31 of .* starts at address .* and ends at .*"
+
+# Ensure that the line table correctly tracks the end of sequence markers.
+set end_seq_count 0
+gdb_test_multiple "maint info line-table" \
+    "count END markers in line table" {
+	-re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
+	    exp_continue
+	}
+	-re "^$decimal\[ \t\]+END\[ \t\]+$hex\r\n" {
+	    incr end_seq_count
+	    exp_continue
+	}
+	-re "^$gdb_prompt $" {
+	    gdb_assert [expr $end_seq_count == 3] $gdb_test_name
+	}
+	-re ".*linetable: \\(\\(struct linetable \\*\\) 0x0\\):\r\nNo line table.\r\n" {
+	    exp_continue
+	}
+	-re ".*linetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS\r\n" {
+	    exp_continue
+	}
+    }

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I002f872248db82a1d4fefdc6b51ff5dbf932d8a8
Gerrit-Change-Number: 525
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [review v3] gdb: Include end of sequence markers in the line table
  2019-11-08  0:23 [review] gdb: Include end of sequence markers in the line table Andrew Burgess (Code Review)
  2019-11-08 15:03 ` [review v2] " Andrew Burgess (Code Review)
@ 2019-11-12 12:38 ` Andrew Burgess (Code Review)
  2019-11-28  0:47 ` [review v4] " Andrew Burgess (Code Review)
  2019-12-23  1:51 ` Andrew Burgess (Code Review)
  3 siblings, 0 replies; 5+ messages in thread
From: Andrew Burgess (Code Review) @ 2019-11-12 12:38 UTC (permalink / raw)
  To: gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/525
......................................................................

gdb: Include end of sequence markers in the line table

In this commit:

  commit d9b3de22f33e400f7f409cce3acf6c7dab07dd79
  Date:   Wed May 27 14:44:29 2015 -0700

      Add struct to record dwarf line number state machine.

I believe an unintended change was made to how we store the DWARF line
table, the end of sequence markers between sequences of lines were
lost from the line table.

This commit fixes this small oversight and restores the end of
sequence markers.

Given that we've survived this long without noticing is clearly an
indication that this isn't that serious, however, a later patch that I
am developing would benefit from having the markers in place, so I'd
like to restore them.

Having the markers also means that the output of 'maintenance info
line-table' now more closely reflects the DWARF line table.

I've taken this opportunity to improve how 'maintenance info
line-table' displays the end of sequence markers - it now uses the END
keyword, rather than just printing an entry with line number 0.  So we
see this:

  INDEX    LINE ADDRESS
  0          12 0x00000000004003b0
  1          17 0x00000000004003b0
  2          18 0x00000000004003b0
  3         END 0x00000000004003b7
  4           5 0x00000000004004a0
  5           6 0x00000000004004a0
  6         END 0x00000000004004a7

Instead of what we would have seen, which was this:

  INDEX    LINE ADDRESS
  0          12 0x00000000004003b0
  1          17 0x00000000004003b0
  2          18 0x00000000004003b0
  3           0 0x00000000004003b7
  4           5 0x00000000004004a0
  5           6 0x00000000004004a0
  6           0 0x00000000004004a7

I've added a small test that uses 'maintenance info line-table' to
ensure that we don't regress this again.

gdb/ChangeLog:

	* dwarf2read.c (lnp_state_machine::record_line): Include
	end_sequence parameter in debug print out.  Record the line if we
	are at an end_sequence marker even if it's not the start of a
	statement.
	* symmisc.c (maintenance_print_one_line_table): Print end of
	sequence markers with 'END' not '0'.

gdb/testsuite/ChangeLog:

	* gdb.base/maint.exp: Update line table parsing test.
	* gdb.dwarf2/dw2-ranges-base.exp: Add new line table parsing test.

Change-Id: I002f872248db82a1d4fefdc6b51ff5dbf932d8a8
---
M gdb/ChangeLog
M gdb/dwarf2read.c
M gdb/symmisc.c
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.base/maint.exp
M gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
6 files changed, 61 insertions(+), 11 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e8104f3..ad07ca2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2019-11-12  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* dwarf2read.c (lnp_state_machine::record_line): Include
+	end_sequence parameter in debug print out.  Record the line if we
+	are at an end_sequence marker even if it's not the start of a
+	statement.
+	* symmisc.c (maintenance_print_one_line_table): Print end of
+	sequence markers with 'END' not '0'.
+
 2019-11-11  Tom Tromey  <tom@tromey.com>
 
 	* tui/tui.c (tui_initialize_readline): Add new bindable readline
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 0a7a033..1070fc3 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -21072,10 +21072,11 @@
     {
       fprintf_unfiltered (gdb_stdlog,
 			  "Processing actual line %u: file %u,"
-			  " address %s, is_stmt %u, discrim %u\n",
+			  " address %s, is_stmt %u, discrim %u%s\n",
 			  m_line, m_file,
 			  paddress (m_gdbarch, m_address),
-			  m_is_stmt, m_discriminator);
+			  m_is_stmt, m_discriminator,
+			  (end_sequence ? "\t(end sequence)" : ""));
     }
 
   file_entry *fe = current_file ();
@@ -21088,7 +21089,8 @@
   else if (m_op_index == 0 || end_sequence)
     {
       fe->included_p = 1;
-      if (m_record_lines_p && (producer_is_codewarrior (m_cu) || m_is_stmt))
+      if (m_record_lines_p
+	  && (producer_is_codewarrior (m_cu) || m_is_stmt || end_sequence))
 	{
 	  if (m_last_subfile != m_cu->get_builder ()->get_current_subfile ()
 	      || end_sequence)
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 24ee255..5ec5cd0 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -1000,8 +1000,12 @@
 	  struct linetable_entry *item;
 
 	  item = &linetable->item [i];
-	  printf_filtered (_("%-6d %6d %s\n"), i, item->line,
-			   core_addr_to_string (item->pc));
+	  printf_filtered ("%-6d ", i);
+	  if (item->line > 0)
+	    printf_filtered ("%6d ", item->line);
+	  else
+	    printf_filtered ("%6s ", _("END"));
+	  printf_filtered ("%s\n", core_addr_to_string (item->pc));
 	}
     }
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 9c69443..ba9a2b6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-11-12  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.base/maint.exp: Update line table parsing test.
+	* gdb.dwarf2/dw2-ranges-base.exp: Add new line table parsing test.
+
 2019-11-10  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdb.python/py-symbol.exp: Add test for
diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 15988c7..9022063 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -508,8 +508,8 @@
 # Test that "main info line-table" w/o a file name shows the symtab for
 # $srcfile.
 set saw_srcfile 0
-set test "maint info line-table w/o a file name"
-gdb_test_multiple "maint info line-table" $test {
+gdb_test_multiple "maint info line-table" \
+    "maint info line-table w/o a file name" {
     -re "symtab: \[^\n\r\]+${srcfile} \\(\\(struct symtab \\*\\) $hex\\)\r\nlinetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS" {
 	set saw_srcfile 1
 	exp_continue
@@ -518,7 +518,11 @@
 	# Match each symtab to avoid overflowing expect's buffer.
 	exp_continue
     }
-    -re "$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
+    -re "symtab: \[^\n\r\]+ \\(\\(struct symtab \\*\\) $hex\\)\r\nlinetable: \\(\\(struct linetable \\*\\) 0x0\\):\r\nNo line table.\r\n" {
+	# For symtabs with no linetable.
+	exp_continue
+    }
+    -re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
 	# Line table entries can be long too:
 	#
 	#  INDEX    LINE ADDRESS
@@ -531,13 +535,17 @@
 	#  6          45 0x0000000000400740
 	#  (...)
 	#  454       129 0x00007ffff7df1d28
-	#  455         0 0x00007ffff7df1d3f
+	#  455       END 0x00007ffff7df1d3f
 	#
 	# Match each line to avoid overflowing expect's buffer.
 	exp_continue
     }
-    -re "$gdb_prompt $" {
-	gdb_assert $saw_srcfile $test
+    -re "^$decimal\[ \t\]+END\[ \t\]+$hex\r\n" {
+	# Matches an end marker in the above.
+	exp_continue
+    }
+    -re "^$gdb_prompt $" {
+	gdb_assert $saw_srcfile $gdb_test_name
     }
 }
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
index 5405d7d..cff9055 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
@@ -141,3 +141,25 @@
     "Line 21 of .* starts at address .* and ends at .*"
 gdb_test "info line frame3" \
     "Line 31 of .* starts at address .* and ends at .*"
+
+# Ensure that the line table correctly tracks the end of sequence markers.
+set end_seq_count 0
+gdb_test_multiple "maint info line-table" \
+    "count END markers in line table" {
+	-re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
+	    exp_continue
+	}
+	-re "^$decimal\[ \t\]+END\[ \t\]+$hex\r\n" {
+	    incr end_seq_count
+	    exp_continue
+	}
+	-re "^$gdb_prompt $" {
+	    gdb_assert [expr $end_seq_count == 3] $gdb_test_name
+	}
+	-re ".*linetable: \\(\\(struct linetable \\*\\) 0x0\\):\r\nNo line table.\r\n" {
+	    exp_continue
+	}
+	-re ".*linetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS\r\n" {
+	    exp_continue
+	}
+    }

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I002f872248db82a1d4fefdc6b51ff5dbf932d8a8
Gerrit-Change-Number: 525
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [review v4] gdb: Include end of sequence markers in the line table
  2019-11-08  0:23 [review] gdb: Include end of sequence markers in the line table Andrew Burgess (Code Review)
  2019-11-08 15:03 ` [review v2] " Andrew Burgess (Code Review)
  2019-11-12 12:38 ` [review v3] " Andrew Burgess (Code Review)
@ 2019-11-28  0:47 ` Andrew Burgess (Code Review)
  2019-12-23  1:51 ` Andrew Burgess (Code Review)
  3 siblings, 0 replies; 5+ messages in thread
From: Andrew Burgess (Code Review) @ 2019-11-28  0:47 UTC (permalink / raw)
  To: gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/525
......................................................................

gdb: Include end of sequence markers in the line table

In this commit:

  commit d9b3de22f33e400f7f409cce3acf6c7dab07dd79
  Date:   Wed May 27 14:44:29 2015 -0700

      Add struct to record dwarf line number state machine.

I believe an unintended change was made to how we store the DWARF line
table, the end of sequence markers between sequences of lines were
lost from the line table.

This commit fixes this small oversight and restores the end of
sequence markers.

Given that we've survived this long without noticing is clearly an
indication that this isn't that serious, however, a later patch that I
am developing would benefit from having the markers in place, so I'd
like to restore them.

Having the markers also means that the output of 'maintenance info
line-table' now more closely reflects the DWARF line table.

I've taken this opportunity to improve how 'maintenance info
line-table' displays the end of sequence markers - it now uses the END
keyword, rather than just printing an entry with line number 0.  So we
see this:

  INDEX    LINE ADDRESS
  0          12 0x00000000004003b0
  1          17 0x00000000004003b0
  2          18 0x00000000004003b0
  3         END 0x00000000004003b7
  4           5 0x00000000004004a0
  5           6 0x00000000004004a0
  6         END 0x00000000004004a7

Instead of what we would have seen, which was this:

  INDEX    LINE ADDRESS
  0          12 0x00000000004003b0
  1          17 0x00000000004003b0
  2          18 0x00000000004003b0
  3           0 0x00000000004003b7
  4           5 0x00000000004004a0
  5           6 0x00000000004004a0
  6           0 0x00000000004004a7

I've added a small test that uses 'maintenance info line-table' to
ensure that we don't regress this again.

gdb/ChangeLog:

	* dwarf2read.c (lnp_state_machine::record_line): Include
	end_sequence parameter in debug print out.  Record the line if we
	are at an end_sequence marker even if it's not the start of a
	statement.
	* symmisc.c (maintenance_print_one_line_table): Print end of
	sequence markers with 'END' not '0'.

gdb/testsuite/ChangeLog:

	* gdb.base/maint.exp: Update line table parsing test.
	* gdb.dwarf2/dw2-ranges-base.exp: Add new line table parsing test.

Change-Id: I002f872248db82a1d4fefdc6b51ff5dbf932d8a8
---
M gdb/ChangeLog
M gdb/dwarf2read.c
M gdb/symmisc.c
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.base/maint.exp
M gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
6 files changed, 61 insertions(+), 11 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6bd149b..a3d9090 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2019-11-28  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* dwarf2read.c (lnp_state_machine::record_line): Include
+	end_sequence parameter in debug print out.  Record the line if we
+	are at an end_sequence marker even if it's not the start of a
+	statement.
+	* symmisc.c (maintenance_print_one_line_table): Print end of
+	sequence markers with 'END' not '0'.
+
 2019-11-27  Christian Biesinger  <cbiesinger@google.com>
 
 	* NEWS: Mention the new multithreaded symbol loading.
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 40626a1..fa106a8 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -21084,10 +21084,11 @@
     {
       fprintf_unfiltered (gdb_stdlog,
 			  "Processing actual line %u: file %u,"
-			  " address %s, is_stmt %u, discrim %u\n",
+			  " address %s, is_stmt %u, discrim %u%s\n",
 			  m_line, m_file,
 			  paddress (m_gdbarch, m_address),
-			  m_is_stmt, m_discriminator);
+			  m_is_stmt, m_discriminator,
+			  (end_sequence ? "\t(end sequence)" : ""));
     }
 
   file_entry *fe = current_file ();
@@ -21100,7 +21101,8 @@
   else if (m_op_index == 0 || end_sequence)
     {
       fe->included_p = 1;
-      if (m_record_lines_p && (producer_is_codewarrior (m_cu) || m_is_stmt))
+      if (m_record_lines_p
+	  && (producer_is_codewarrior (m_cu) || m_is_stmt || end_sequence))
 	{
 	  if (m_last_subfile != m_cu->get_builder ()->get_current_subfile ()
 	      || end_sequence)
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 69d035d..9e51043 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -1000,8 +1000,12 @@
 	  struct linetable_entry *item;
 
 	  item = &linetable->item [i];
-	  printf_filtered (_("%-6d %6d %s\n"), i, item->line,
-			   core_addr_to_string (item->pc));
+	  printf_filtered ("%-6d ", i);
+	  if (item->line > 0)
+	    printf_filtered ("%6d ", item->line);
+	  else
+	    printf_filtered ("%6s ", _("END"));
+	  printf_filtered ("%s\n", core_addr_to_string (item->pc));
 	}
     }
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6b520e1..caf228d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2019-11-28  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb.base/maint.exp: Update line table parsing test.
+	* gdb.dwarf2/dw2-ranges-base.exp: Add new line table parsing test.
+
+2019-11-28  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* lib/gdb.exp (skip_btrace_tests): Return 1 if the test fails to
 	compile.
 	(skip_btrace_pt_tests): Likewise.
diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 15988c7..9022063 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -508,8 +508,8 @@
 # Test that "main info line-table" w/o a file name shows the symtab for
 # $srcfile.
 set saw_srcfile 0
-set test "maint info line-table w/o a file name"
-gdb_test_multiple "maint info line-table" $test {
+gdb_test_multiple "maint info line-table" \
+    "maint info line-table w/o a file name" {
     -re "symtab: \[^\n\r\]+${srcfile} \\(\\(struct symtab \\*\\) $hex\\)\r\nlinetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS" {
 	set saw_srcfile 1
 	exp_continue
@@ -518,7 +518,11 @@
 	# Match each symtab to avoid overflowing expect's buffer.
 	exp_continue
     }
-    -re "$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
+    -re "symtab: \[^\n\r\]+ \\(\\(struct symtab \\*\\) $hex\\)\r\nlinetable: \\(\\(struct linetable \\*\\) 0x0\\):\r\nNo line table.\r\n" {
+	# For symtabs with no linetable.
+	exp_continue
+    }
+    -re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
 	# Line table entries can be long too:
 	#
 	#  INDEX    LINE ADDRESS
@@ -531,13 +535,17 @@
 	#  6          45 0x0000000000400740
 	#  (...)
 	#  454       129 0x00007ffff7df1d28
-	#  455         0 0x00007ffff7df1d3f
+	#  455       END 0x00007ffff7df1d3f
 	#
 	# Match each line to avoid overflowing expect's buffer.
 	exp_continue
     }
-    -re "$gdb_prompt $" {
-	gdb_assert $saw_srcfile $test
+    -re "^$decimal\[ \t\]+END\[ \t\]+$hex\r\n" {
+	# Matches an end marker in the above.
+	exp_continue
+    }
+    -re "^$gdb_prompt $" {
+	gdb_assert $saw_srcfile $gdb_test_name
     }
 }
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
index 5405d7d..cff9055 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ranges-base.exp
@@ -141,3 +141,25 @@
     "Line 21 of .* starts at address .* and ends at .*"
 gdb_test "info line frame3" \
     "Line 31 of .* starts at address .* and ends at .*"
+
+# Ensure that the line table correctly tracks the end of sequence markers.
+set end_seq_count 0
+gdb_test_multiple "maint info line-table" \
+    "count END markers in line table" {
+	-re "^$decimal\[ \t\]+$decimal\[ \t\]+$hex\r\n" {
+	    exp_continue
+	}
+	-re "^$decimal\[ \t\]+END\[ \t\]+$hex\r\n" {
+	    incr end_seq_count
+	    exp_continue
+	}
+	-re "^$gdb_prompt $" {
+	    gdb_assert [expr $end_seq_count == 3] $gdb_test_name
+	}
+	-re ".*linetable: \\(\\(struct linetable \\*\\) 0x0\\):\r\nNo line table.\r\n" {
+	    exp_continue
+	}
+	-re ".*linetable: \\(\\(struct linetable \\*\\) $hex\\):\r\nINDEX\[ \t\]+LINE\[ \t\]+ADDRESS\r\n" {
+	    exp_continue
+	}
+    }

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I002f872248db82a1d4fefdc6b51ff5dbf932d8a8
Gerrit-Change-Number: 525
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-MessageType: newpatchset

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [review v4] gdb: Include end of sequence markers in the line table
  2019-11-08  0:23 [review] gdb: Include end of sequence markers in the line table Andrew Burgess (Code Review)
                   ` (2 preceding siblings ...)
  2019-11-28  0:47 ` [review v4] " Andrew Burgess (Code Review)
@ 2019-12-23  1:51 ` Andrew Burgess (Code Review)
  3 siblings, 0 replies; 5+ messages in thread
From: Andrew Burgess (Code Review) @ 2019-12-23  1:51 UTC (permalink / raw)
  To: gdb-patches

Andrew Burgess has abandoned this change. ( https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/525 )

Change subject: gdb: Include end of sequence markers in the line table
......................................................................


Abandoned

I've now posted this patch to the mailing list.
-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I002f872248db82a1d4fefdc6b51ff5dbf932d8a8
Gerrit-Change-Number: 525
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-MessageType: abandon

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-12-23  1:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08  0:23 [review] gdb: Include end of sequence markers in the line table Andrew Burgess (Code Review)
2019-11-08 15:03 ` [review v2] " Andrew Burgess (Code Review)
2019-11-12 12:38 ` [review v3] " Andrew Burgess (Code Review)
2019-11-28  0:47 ` [review v4] " Andrew Burgess (Code Review)
2019-12-23  1:51 ` Andrew Burgess (Code Review)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).