[PATCH 1/2] [gdb/symtab] Add producer_is_gas

[PATCH 1/2] [gdb/symtab] Add producer_is_gas

[Tom de Vries]

Add producer_is_gas, a generic way to get the gas version from the
producer string.

Tested on x86_64-linux.
---
 gdb/dwarf2/read.c |  4 ++--
 gdb/producer.c    | 56 +++++++++++++++++++++++++++++++++++++++++++++++
 gdb/producer.h    |  5 +++++
 3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index ea0b2328a3e..24495bfbb89 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -11426,8 +11426,8 @@ check_producer (struct dwarf2_cu *cu)
     cu->producer_is_codewarrior = true;
   else if (producer_is_clang (cu->producer, &major, &minor))
     cu->producer_is_clang = true;
-  else if (startswith (cu->producer, "GNU AS 2.39.0"))
-    cu->producer_is_gas_2_39 = true;
+  else if (producer_is_gas (cu->producer, &major, &minor))
+    cu->producer_is_gas_2_39 = major == 2 && minor == 39;
   else
     {
       /* For other non-GCC compilers, expect their behavior is DWARF version
diff --git a/gdb/producer.c b/gdb/producer.c
index 9fcf749e3d4..903edfbabac 100644
--- a/gdb/producer.c
+++ b/gdb/producer.c
@@ -81,6 +81,45 @@ producer_is_gcc (const char *producer, int *major, int *minor)
 
 /* See producer.h.  */
 
+int
+producer_is_gas (const char *producer, int *major, int *minor)
+{
+  if (producer == nullptr)
+    {
+      /* No producer, don't know.  */
+      return 0;
+    }
+
+  /* Detect prefix.  */
+  const char prefix[] = "GNU AS ";
+  if (!startswith (producer, prefix))
+    {
+      /* Producer is not gas.  */
+      return 0;
+    }
+
+  /* Skip prefix.  */
+  const char *cs = &producer[strlen (prefix)];
+
+  /* Ensure that major/minor are not nullptrs.  */
+  int maj, min;
+  if (major == nullptr)
+    major = &maj;
+  if (minor == nullptr)
+    minor = &min;
+
+  int scanned = sscanf (cs, "%d.%d", major, minor);
+  if (scanned != 2)
+    {
+      /* Unable to scan major/minor version.  */
+      return 0;
+    }
+
+  return 1;
+}
+
+  /* See producer.h.  */
+
 bool
 producer_is_icc_ge_19 (const char *producer)
 {
@@ -251,6 +290,23 @@ Version 18.0 Beta";
     SELF_CHECK (!producer_is_gcc (flang_llvm_exp, &major, &minor));
     SELF_CHECK (producer_is_llvm (flang_llvm_exp));
   }
+
+  {
+    static const char gas_exp[] = "GNU AS 2.39.0";
+    int major = 0, minor = 0;
+    SELF_CHECK (!producer_is_gcc (gas_exp, &major, &minor));
+    SELF_CHECK (producer_is_gas (gas_exp, &major, &minor));
+    SELF_CHECK (major == 2 && minor == 39);
+
+    static const char gas_incomplete_exp[] = "GNU AS ";
+    SELF_CHECK (!producer_is_gas (gas_incomplete_exp, &major, &minor));
+    SELF_CHECK (!producer_is_gcc (gas_incomplete_exp, &major, &minor));
+
+    static const char gas_incomplete_exp_2[] = "GNU AS 2";
+    SELF_CHECK (!producer_is_gas (gas_incomplete_exp_2, &major, &minor));
+    SELF_CHECK (!producer_is_gcc (gas_incomplete_exp_2, &major, &minor));
+  }
+
 }
 }
 }
diff --git a/gdb/producer.h b/gdb/producer.h
index c915979b122..aac5088395a 100644
--- a/gdb/producer.h
+++ b/gdb/producer.h
@@ -30,6 +30,11 @@ extern int producer_is_gcc_ge_4 (const char *producer);
    is NULL or it isn't GCC.  */
 extern int producer_is_gcc (const char *producer, int *major, int *minor);
 
+/* Returns nonzero if the given PRODUCER string is GAS and sets the MAJOR
+   and MINOR versions when not NULL.  Returns zero if the given PRODUCER
+   is NULL or it isn't GAS.  */
+int producer_is_gas (const char *producer, int *major, int *minor);
+
 /* Check for Intel compilers >= 19.0.  */
 extern bool producer_is_icc_ge_19 (const char *producer);
 

base-commit: ca362799ee0ab2bd4d4ea4f521726fefbfbad738
-- 
2.35.3

[PATCH 2/2] [gdb/symtab] Work around gas PR28629

[Tom de Vries]

When running test-case gdb.tui/tui-layout-asm-short-prog.exp on AlmaLinux 9.2
ppc64le, I run into:
...
FAIL: gdb.tui/tui-layout-asm-short-prog.exp: check asm box contents
...

The problem is that we get:
...
    7              [ No Assembly Available ]
...
because tui_get_begin_asm_address doesn't succeed.

In more detail, tui_get_begin_asm_address calls:
...
		    find_line_pc (sal.symtab, sal.line, &addr);
...
with:
...
(gdb) p *sal.symtab
$5 = {next = 0x130393c0, m_compunit = 0x130392f0, m_linetable = 0x0,
  filename = "tui-layout-asm-short-prog.S",
  filename_for_id = "$gdb/build/gdb/testsuite/tui-layout-asm-short-prog.S",
  m_language = language_asm, fullname = 0x0}
(gdb) p sal.line
$6 = 1
...

The problem is the filename_for_id which is the source file prefixed with the
compilation dir rather than the source dir.

This is due to faulty debug info generated by gas, PR28629:
...
    <1a>   DW_AT_name        : tui-layout-asm-short-prog.S
    <1e>   DW_AT_comp_dir    : $gdb/build/gdb/testsuite
    <22>   DW_AT_producer    : GNU AS 2.35.2
...

The DW_AT_name is relative, and it's relative to the DW_AT_comp_dir entry,
making the effective name $gdb/build/gdb/testsuite/tui-layout-asm-short-prog.S.

The bug is fixed starting version 2.38, where we get instead:
...
    <1a>   DW_AT_name        :
             $gdb/src/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.S
    <1e>   DW_AT_comp_dir    : $gdb/build/gdb/testsuite
    <22>   DW_AT_producer    : GNU AS 2.38
...

Work around the faulty debug info by constructing the filename_for_id using
the second directory from the directory table in the .debug_line header:
...
 The Directory Table (offset 0x22, lines 2, columns 1):
  Entry	Name
  0	$gdb/build/gdb/testsuite
  1	$gdb/src/gdb/testsuite/gdb.tui
...

Note that the used gas contains a backport of commit 3417bfca676 ("GAS:
DWARF-5: Ensure that the 0'th entry in the directory table contains the
current working directory."), because directory 0 is correct.  With the
unpatched 2.35.2 release the directory 0 entry is incorrect: it's a copy of
entry 1.

Add a dwarf assembly test-case that reflects the debug info as generated by
unpatched gas 2.35.2.

Tested on x86_64-linux.
---
 gdb/dwarf2/cu.c                               |  1 +
 gdb/dwarf2/cu.h                               |  1 +
 gdb/dwarf2/read.c                             | 37 +++++++-
 .../gdb.dwarf2/dw2-gas-workaround.exp         | 92 +++++++++++++++++++
 4 files changed, 130 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-gas-workaround.exp

diff --git a/gdb/dwarf2/cu.c b/gdb/dwarf2/cu.c
index a908ec908cd..f9b05dfc709 100644
--- a/gdb/dwarf2/cu.c
+++ b/gdb/dwarf2/cu.c
@@ -40,6 +40,7 @@ dwarf2_cu::dwarf2_cu (dwarf2_per_cu_data *per_cu,
     producer_is_icc_lt_14 (false),
     producer_is_codewarrior (false),
     producer_is_clang (false),
+    producer_is_gas_lt_2_38 (false),
     producer_is_gas_2_39 (false),
     processing_has_namespace_info (false),
     load_all_dies (false)
diff --git a/gdb/dwarf2/cu.h b/gdb/dwarf2/cu.h
index 6c611710503..ef8db480e3f 100644
--- a/gdb/dwarf2/cu.h
+++ b/gdb/dwarf2/cu.h
@@ -265,6 +265,7 @@ struct dwarf2_cu
   bool producer_is_icc_lt_14 : 1;
   bool producer_is_codewarrior : 1;
   bool producer_is_clang : 1;
+  bool producer_is_gas_lt_2_38 : 1;
   bool producer_is_gas_2_39 : 1;
 
   /* When true, the file that we're processing is known to have
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 24495bfbb89..1357bca49e4 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -147,6 +147,8 @@ static int dwarf2_locexpr_block_index;
 static int dwarf2_loclist_block_index;
 static int ada_block_index;
 
+static bool producer_is_gas_lt_2_38 (struct dwarf2_cu *cu);
+
 /* Size of .debug_loclists section header for 32-bit DWARF format.  */
 #define LOCLIST_HEADER_SIZE32 12
 
@@ -7665,6 +7667,27 @@ read_file_scope (struct die_info *die, struct dwarf2_cu *cu)
 
   file_and_directory &fnd = find_file_and_directory (die, cu);
 
+  /* GAS supports generating dwarf-5 info starting version 2.35.  Versions
+     2.35-2.37 generate an incorrect CU name attribute: it's relative,
+     implicitly prefixing it with the compilation dir.  Work around this by
+     prefixing it with the source dir instead.  */
+  if (cu->header.version == 5 && !IS_ABSOLUTE_PATH (fnd.get_name ())
+      && producer_is_gas_lt_2_38 (cu))
+    {
+      attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
+      if (attr != nullptr && attr->form_is_unsigned ())
+	{
+	  sect_offset line_offset = (sect_offset) attr->as_unsigned ();
+	  line_header_up lh = dwarf_decode_line_header (line_offset, cu,
+							fnd.get_comp_dir ());
+	  if (lh->version == 5 && lh->is_valid_file_index (1))
+	    {
+	      std::string dir = lh->include_dir_at (1);
+	      fnd.set_comp_dir (std::move (dir));
+	    }
+	}
+    }
+
   cu->start_compunit_symtab (fnd.get_name (), fnd.intern_comp_dir (objfile),
 			     lowpc);
 
@@ -11427,7 +11450,10 @@ check_producer (struct dwarf2_cu *cu)
   else if (producer_is_clang (cu->producer, &major, &minor))
     cu->producer_is_clang = true;
   else if (producer_is_gas (cu->producer, &major, &minor))
-    cu->producer_is_gas_2_39 = major == 2 && minor == 39;
+    {
+      cu->producer_is_gas_lt_2_38 = major < 2 || (major == 2 && minor < 38);
+      cu->producer_is_gas_2_39 = major == 2 && minor == 39;
+    }
   else
     {
       /* For other non-GCC compilers, expect their behavior is DWARF version
@@ -11463,6 +11489,15 @@ producer_is_codewarrior (struct dwarf2_cu *cu)
   return cu->producer_is_codewarrior;
 }
 
+static bool
+producer_is_gas_lt_2_38 (struct dwarf2_cu *cu)
+{
+  if (!cu->checked_producer)
+    check_producer (cu);
+
+  return cu->producer_is_gas_lt_2_38;
+}
+
 static bool
 producer_is_gas_2_39 (struct dwarf2_cu *cu)
 {
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-gas-workaround.exp b/gdb/testsuite/gdb.dwarf2/dw2-gas-workaround.exp
new file mode 100644
index 00000000000..ca2b10f23b3
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-gas-workaround.exp
@@ -0,0 +1,92 @@
+# Copyright 2023 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 line number information in various configurations.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+require dwarf2_support
+
+standard_testfile dw2-lines.c -dw2.S
+
+with_shared_gdb {
+    set func_info_vars [get_func_info bar]
+}
+
+# Helper function.
+proc line_for { l } {
+    global srcfile
+    set line [gdb_get_line_number "$l:" $srcfile]
+    return [expr $line + 1]
+}
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    declare_labels Llines
+    global srcdir subdir srcfile objdir
+    global func_info_vars
+    foreach var $func_info_vars {
+	global $var
+    }
+
+    cu { version 5 } {
+	compile_unit {
+	    {language @DW_LANG_Mips_Assembler}
+	    {name $srcfile}
+	    {comp_dir $objdir}
+	    {stmt_list $Llines DW_FORM_sec_offset}
+	    {producer "GNU AS 2.35.2"}
+	} {
+	    subprogram {
+		{external 1 flag}
+		{name bar}
+		{low_pc $bar_start addr}
+		{high_pc "$bar_start + $bar_len" addr}
+	    }
+	}
+    }
+
+    lines [list version 5] Llines {
+	set diridx1 [include_dir "${srcdir}/${subdir}"]
+	set diridx2 [include_dir "${srcdir}/${subdir}"]
+	file_name "$srcfile" $diridx1
+	file_name "$srcfile" $diridx2
+
+	program {
+	    DW_LNE_set_address bar_label
+	    line [line_for bar_label]
+	    DW_LNS_copy
+
+	    DW_LNE_set_address bar_label_2
+	    DW_LNE_end_sequence
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+gdb_test_no_output "set debug symtab-create 1"
+gdb_test_multiple "ptype bar" "" {
+    -re -wrap "$objdir.*" {
+	fail $gdb_test_name
+    }
+    -re -wrap "" {
+	pass $gdb_test_name
+    }
+}
-- 
2.35.3

Re: [PATCH 1/2] [gdb/symtab] Add producer_is_gas

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Add producer_is_gas, a generic way to get the gas version from the
Tom> producer string.

Tom> Tested on x86_64-linux.
 
Tom> +int
Tom> +producer_is_gas (const char *producer, int *major, int *minor)

I'd suggest a bool return type for new code.

Tom

Re: [PATCH 2/2] [gdb/symtab] Work around gas PR28629

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Work around the faulty debug info by constructing the filename_for_id using
Tom> the second directory from the directory table in the .debug_line header:

Always sad to have to do this, but on the plus side, you've documented
the situation nicely.  Thank you.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH 1/2] [gdb/symtab] Add producer_is_gas

[Tom de Vries]

On 10/31/23 20:30, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Add producer_is_gas, a generic way to get the gas version from the
> Tom> producer string.
> 
> Tom> Tested on x86_64-linux.
>   
> Tom> +int
> Tom> +producer_is_gas (const char *producer, int *major, int *minor)
> 
> I'd suggest a bool return type for new code.

Ack, pushed with that change.

Thanks,
- Tom

Re: [PATCH 2/2] [gdb/symtab] Work around gas PR28629

[Tom de Vries]

On 10/31/23 20:31, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Work around the faulty debug info by constructing the filename_for_id using
> Tom> the second directory from the directory table in the .debug_line header:
> 
> Always sad to have to do this, but on the plus side, you've documented
> the situation nicely.  Thank you.
> 

Agreed, it's a bit sad, but luckily the fix was not too complex.

Thanks for the review, pushed.

- Tom

> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Tom

Re: [PATCH 2/2] [gdb/symtab] Work around gas PR28629

[Carl Love]

Tom:

On Wed, 2023-11-01 at 07:43 +0100, Tom de Vries wrote:
> On 10/31/23 20:31, Tom Tromey wrote:
> > > > > > > "Tom" == Tom de Vries <tdevries@suse.de> writes:
> > 
> > Tom> Work around the faulty debug info by constructing the
> > filename_for_id using
> > Tom> the second directory from the directory table in the
> > .debug_line header:
> > 
> > Always sad to have to do this, but on the plus side, you've
> > documented
> > the situation nicely.  Thank you.
> > 
> 
> Agreed, it's a bit sad, but luckily the fix was not too complex.
> 
> Thanks for the review, pushed.
> 
> - Tom
> 
> > Approved-By: Tom Tromey <tom@tromey.com>
> > 
> > Tom

I see this new test is failing on PowerPC.  I get the message:

(gdb) PASS: gdb.dwarf2/dw2-gas-workaround.exp: set debug symtab-create 1
ptype bar
[symtab-create] start_subfile: name = dw2-lines.c, name_for_id =.../gdb/testsuite/gdb.dwarf2/dw2-lines.c
[symtab-create] start_subfile: name = ,gdb/testsuite/gdb.dwarf2/dw2-lines.c, name_for_id = .../gdb-current-patches/gdb/testsuite/gdb.dwarf2/dw2-l\
ines.c
[symtab-create] start_subfile: found existing symtab with name_for_id ...gdb/testsuite/gdb.dwarf2/dw2-lines.c
[symtab-create] start_subfile: name =.../gdb.dwarf2/dw2-lines.c, name_for_id = .../gdb/testsuite/gdb.dwarf2/dw2-lines.c
[symtab-create] start_subfile: found existing symtab with name_for_id .../gdb/testsuite/gdb.dwarf2/dw2-lines.c
[symtab-create] start_subfile: name = .../gdb/testsuite/gdb.dwarf2/dw2-lines.c, name_for_id = ..../gdb-current-patches/gdb/testsuite/gdb.dwarf2/dw2-l\
ines.c
[symtab-create] start_subfile: found existing symtab with name_for_id ...gdb/testsuite/gdb.dwarf2/dw2-lines.c
type = void (void)

(gdb) FAIL: gdb.dwarf2/dw2-gas-workaround.exp: ptype bar

testcase .../gdb.dwarf2/dw2-gas-workaround.exp completed in 1 seconds


I saw the error on a Power 10 system with Fedora release 36 (Thirty
Six) and gcc (GCC) 12.2.1 20221121 (Red Hat 12.2.1-4) with the gdb
mainline source tree.

Wondering if you had any thoughts as to why the type information for
bar is not seen on PowerPC?  Thanks.

                      Carl 

Re: [PATCH 2/2] [gdb/symtab] Work around gas PR28629

[Tom Tromey]

Carl> I see this new test is failing on PowerPC.  I get the message:
...
Carl> (gdb) FAIL: gdb.dwarf2/dw2-gas-workaround.exp: ptype bar

FWIW this also fails for me on x86-64 Fedora 36.

Tom

Re: [PATCH 2/2] [gdb/symtab] Work around gas PR28629

[Tom de Vries]

On 11/3/23 19:25, Tom Tromey wrote:
> Carl> I see this new test is failing on PowerPC.  I get the message:
> ...
> Carl> (gdb) FAIL: gdb.dwarf2/dw2-gas-workaround.exp: ptype bar
> 
> FWIW this also fails for me on x86-64 Fedora 36.
> 

I managed to reproduce the FAIL by doing this:
...
  pwd=$(pwd -P)
-src=$pwd/src 

+src=../../src
...
in my build script.

Thanks,
- Tom

Re: [PATCH 2/2] [gdb/symtab] Work around gas PR28629

[Tom de Vries]

On 11/5/23 21:14, Tom de Vries wrote:
> On 11/3/23 19:25, Tom Tromey wrote:
>> Carl> I see this new test is failing on PowerPC.  I get the message:
>> ...
>> Carl> (gdb) FAIL: gdb.dwarf2/dw2-gas-workaround.exp: ptype bar
>>
>> FWIW this also fails for me on x86-64 Fedora 36.
>>
> 
> I managed to reproduce the FAIL by doing this:
> ...
>   pwd=$(pwd -P)
> -src=$pwd/src
> +src=../../src
> ...
> in my build script.

Thanks for reporting this.

Fixed by 
https://sourceware.org/pipermail/gdb-patches/2023-November/203802.html .

Thanks,
- Tom