[PATCH] Change how .debug_aranges padding is skipped

[PATCH] Change how .debug_aranges padding is skipped

[Tom Tromey]

When GCC emits .debug_aranges, it adds padding to align the contents
to two times the address size.  GCC has done this for many years --
but there is nothing in the DWARF standard that says this should be
done, and LLVM does not seem to add this padding.

It's simple to detect if the padding exists, though: if the contents
of one .debug_aranges CU (excluding the header) are not a multiple of
the alignment that GCC uses, then anything extra must be padding.

This patch changes gdb to correctly read both styles.  It removes the
requirement that the padding bytes be zero, as this seemed
unnecessarily pedantic to me.

gdb/ChangeLog
2021-06-11  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (create_addrmap_from_aranges): Change padding
	logic.

gdb/testsuite/ChangeLog
2021-06-11  Tom Tromey  <tom@tromey.com>

	* lib/gdb.exp (add_gdb_index, ensure_gdb_index): Add "style"
	parameter.
	* gdb.rust/dwindex.exp: New file.
	* gdb.rust/dwindex.rs: New file.
---
 gdb/ChangeLog                      |  5 ++++
 gdb/dwarf2/read.c                  | 18 +++++--------
 gdb/testsuite/ChangeLog            |  7 +++++
 gdb/testsuite/gdb.rust/dwindex.exp | 43 ++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.rust/dwindex.rs  | 22 +++++++++++++++
 gdb/testsuite/lib/gdb.exp          | 14 +++++++---
 6 files changed, 93 insertions(+), 16 deletions(-)
 create mode 100644 gdb/testsuite/gdb.rust/dwindex.exp
 create mode 100644 gdb/testsuite/gdb.rust/dwindex.rs

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 96009f1418f..f7a48d87ff4 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2639,18 +2639,12 @@ create_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
 
       /* Must pad to an alignment boundary that is twice the address
 	 size.  It is undocumented by the DWARF standard but GCC does
-	 use it.  */
-      for (size_t padding = ((-(addr - section->buffer))
-			     & (2 * address_size - 1));
-	   padding > 0; padding--)
-	if (*addr++ != 0)
-	  {
-	    warning (_("Section .debug_aranges in %s entry at offset %s "
-		       "padding is not zero, ignoring .debug_aranges."),
-		     objfile_name (objfile),
-		     plongest (entry_addr - section->buffer));
-	    return;
-	  }
+	 use it.  However, not every compiler does this.  We can see
+	 whether it has happened by looking at the total length of the
+	 contents of the aranges for this CU -- it if isn't a multiple
+	 of twice the address size, then we skip any leftover
+	 bytes.  */
+      addr += (entry_end - addr) % (2 * address_size);
 
       for (;;)
 	{
diff --git a/gdb/testsuite/gdb.rust/dwindex.exp b/gdb/testsuite/gdb.rust/dwindex.exp
new file mode 100644
index 00000000000..1bc61993240
--- /dev/null
+++ b/gdb/testsuite/gdb.rust/dwindex.exp
@@ -0,0 +1,43 @@
+# Copyright (C) 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that a rustc-produced .debug_aranges can be read.
+
+load_lib rust-support.exp
+if {[skip_rust_tests]} {
+    continue
+}
+
+standard_testfile .rs
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug rust}]} {
+    return -1
+}
+
+if {[ensure_gdb_index $binfile "-dwarf-5"] == -1} {
+    return -1
+}
+
+gdb_exit
+gdb_start
+set testname "file with aranges"
+gdb_test_multiple "file $binfile" "" {
+    -re "warning: Section \\.debug_aranges" {
+	fail $testname
+    }
+    -re -wrap ".*" {
+	pass $testname
+    }
+}
diff --git a/gdb/testsuite/gdb.rust/dwindex.rs b/gdb/testsuite/gdb.rust/dwindex.rs
new file mode 100644
index 00000000000..439876d0630
--- /dev/null
+++ b/gdb/testsuite/gdb.rust/dwindex.rs
@@ -0,0 +1,22 @@
+// Copyright (C) 2021 Free Software Foundation, Inc.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+#![allow(dead_code)]
+#![allow(unused_variables)]
+#![allow(unused_assignments)]
+
+
+fn main () {
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index d8c684c7238..a6453010e07 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -7686,12 +7686,15 @@ proc verify_psymtab_expanded { filename readin } {
 # Add a .gdb_index section to PROGRAM.
 # PROGRAM is assumed to be the output of standard_output_file.
 # Returns the 0 if there is a failure, otherwise 1.
+#
+# STYLE controls which style of index to add, if needed.  The empty
+# string (the default) means .gdb_index; "-dwarf-5" means .debug_names.
 
-proc add_gdb_index { program } {
+proc add_gdb_index { program {style ""} } {
     global srcdir GDB env BUILD_DATA_DIRECTORY
     set contrib_dir "$srcdir/../contrib"
     set env(GDB) "$GDB --data-directory=$BUILD_DATA_DIRECTORY"
-    set result [catch "exec $contrib_dir/gdb-add-index.sh $program" output]
+    set result [catch "exec $contrib_dir/gdb-add-index.sh $style $program" output]
     if { $result != 0 } {
 	verbose -log "result is $result"
 	verbose -log "output is $output"
@@ -7705,8 +7708,11 @@ proc add_gdb_index { program } {
 # (.gdb_index/.debug_names).  Gdb doesn't support building an index from a
 # program already using one.  Return 1 if a .gdb_index was added, return 0
 # if it already contained an index, and -1 if an error occurred.
+#
+# STYLE controls which style of index to add, if needed.  The empty
+# string (the default) means .gdb_index; "-dwarf-5" means .debug_names.
 
-proc ensure_gdb_index { binfile } {
+proc ensure_gdb_index { binfile {style ""} } {
     set testfile [file tail $binfile]
     set test "check if index present"
     gdb_test_multiple "mt print objfiles ${testfile}" $test {
@@ -7717,7 +7723,7 @@ proc ensure_gdb_index { binfile } {
 	    return 0
 	}
 	-re -wrap "Psymtabs.*" {
-	    if { [add_gdb_index $binfile] != "1" } {
+	    if { [add_gdb_index $binfile $style] != "1" } {
 		return -1
 	    }
 	    return 1
-- 
2.26.3

Re: [PATCH] Change how .debug_aranges padding is skipped

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> gdb/ChangeLog
Tom> 2021-06-11  Tom Tromey  <tom@tromey.com>

Tom> 	* dwarf2/read.c (create_addrmap_from_aranges): Change padding
Tom> 	logic.

I'm checking this in.

Tom

Re: [PATCH] Change how .debug_aranges padding is skipped

[Tom de Vries]

On 6/11/21 6:33 PM, Tom Tromey wrote:
> When GCC emits .debug_aranges, it adds padding to align the contents
> to two times the address size.  GCC has done this for many years --
> but there is nothing in the DWARF standard that says this should be
> done, and LLVM does not seem to add this padding.

I recently came across this padding issue when doing the .debug_aranges
support in the dwarf assembler.

I did find evidence for this in the dwarf standard ... eventually.
Looking in the v4 pdf, we find the following.

Here, no mention of padding:
...
6.1.2 Lookup by Address

This header is followed by a variable number of address range
descriptors. Each descriptor is a triple consisting of a segment
selector, the beginning address within that segment of a range of
text or data covered by some entry owned by the corresponding
compilation unit, followed by the non-zero length of that range. A
particular set is terminated by an entry consisting of three
zeroes.
...

Here we find the padding described:
...
7.20 Address Range Table

This header is followed by a series of tuples. Each tuple consists of a
segment, an address and a length. The segment size is given by the
segment_size field of the header; the address and length size are each
given by the address_size field of the header. The first tuple following
the header in each set begins at an offset that is a multiple of the
size of a single tuple (that is, the size of a segment selector plus
twice the size of an address). The header is padded, if necessary,
to that boundary.
...

So, I initially grepped for .debug_aranges and found 6.1.2, and
understood that gcc's padding was unnecessary.  And then I found 7.20.

Having found the padding definition, I think the definition is confusing.

I'm not sure what the offset that is padded is relative too.  In the
dwarf assembler implementation I took it to be the start of the
.debug_aranges contribution, which is what gcc does AFAIU.

And looking at what gcc/gas emits, it gives the .debug_aranges section
an alignment of 8 (with tuple size 16):
...
$ gcc /home/vries/hello.c -g -save-temps
$ readelf -W -S hello.o
  [Nr] Name              Type            Address          Off    Size
ES Flg Lk Inf Al
  [ 9] .debug_aranges    PROGBITS        0000000000000000 0002d8 00002f
00   C  0   0  8
...

In the eventual exec, the section does end up with an alignment of 16:
...
$ readelf -W -S a.out
  [Nr] Name              Type            Address          Off    Size
ES Flg Lk Inf Al
  [27] .debug_aranges    PROGBITS        0000000000000000 001050 000130
00      0   0 16
...
I guess because other objects do have that alignment:
...
$ readelf -S -W /usr/lib64/gcc/x86_64-suse-linux/7/../../../../lib64/crt1.o
  [Nr] Name              Type            Address          Off    Size
ES Flg Lk Inf Al
  [11] .debug_aranges    PROGBITS        0000000000000000 0000e0 000050
00      0   0 16
...

So, AFAIU the offset relative to the start of the .debug_aranges section
after linking may end up as not a multiple of the size of a single
tuple.  Then if that's the offset that was meant by the standard,
gcc/gas needs to be fixed to force an alignment of 16.

Thanks,
- Tom