[RFC,gdb/symtab] Fix FAILs in gdb.base/langs.exp with gcc-11

Message ID 20210720110226.GA22905@delia.home
State New
Headers show
Series
  • [RFC,gdb/symtab] Fix FAILs in gdb.base/langs.exp with gcc-11
Related show

Commit Message

Tom de Vries July 20, 2021, 11:02 a.m.
Hi,

[ For readability, in the following these path shorthands are used:
- $src:  /home/vries/gdb_versions/devel/src/gdb/testsuite
- $build: /home/vries/gdb_versions/devel/build/gdb/testsuite . ]

With gcc-11 (with -gdwarf-5 default) I run into the following FAIL, showed in
contrast to the -gdwarf-4 output:
...
 (gdb) up^M
-#1  0x0000000000400520 in foo__Fi (x=10000) at langs2.cxx:5^M
+#1  0x0000000000400520 in foo__Fi (x=10000) at $build/langs2.cxx:5^M
 5         return csub (x / 2);^M
-(gdb) PASS: gdb.base/langs.exp: up to foo in langs.exp
+(gdb) FAIL: gdb.base/langs.exp: up to foo in langs.exp
...

Looking at the dwarf info, with dwarf 4 we have:
...
 <0><243>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <249>   DW_AT_name        : $src/gdb.base/langs2.c
    <24d>   DW_AT_comp_dir    : $build
...
and:
...
 The Directory Table is empty.

 The File Name Table (offset 0x22c):
  Entry Dir     Time    Size    Name
  1     0       0       0       langs2.cxx
...
and with dwarf 5:
...
 <0><242>: Abbrev Number: 2 (DW_TAG_compile_unit)
    <248>   DW_AT_name        : $src/gdb.base/langs2.c
    <24c>   DW_AT_comp_dir    : $build
...
and:
...
 The Directory Table (offset 0x1d6, lines 1, columns 1):
  Entry Name
  0     : $build

 The File Name Table (offset 0x1e0, lines 2, columns 2):
  Entry Dir     Name
  0     0       : $src/gdb.base/langs2.c
  1     0       : langs2.cxx
...

It's good to note at this point that langs2.c starts with:
...
 $ cat gdb/testsuite/gdb.base/langs2.c
 /* This is intended to be a vague simulation of cfront output.  */
 #line 1 "langs2.cxx"
...

So the information in both dwarf 4 and 5 cases is correct and equal.  It's
just that in the dwarf 4 case, the "Dir 0" refers to the DW_AT_comp_dir, which
is $build, and in the dwarf 5 case, the "Dir 0" refers to an explicit
Directory Table entry "$build".

Given that there's no difference in information, we'd expect the same
behaviour, so ideally we'd fix this in gdb rather than updating the
test-case.

[ Note that with gcc-10 and -gdwarf-5 the test doesn't fail because while the
.debug_info contribution has version 5, the .debug_line contribution still
has version 3. ]

My first attempt at fixing this was to ignore the "Dir 0" entry:
...

Patch

diff --git a/gdb/dwarf2/line-header.h b/gdb/dwarf2/line-header.h
index 4cacb8ca344..19c7edd3217 100644
--- a/gdb/dwarf2/line-header.h
+++ b/gdb/dwarf2/line-header.h
@@ -93,6 +93,8 @@  struct line_header
       vec_index = index - 1;
     if (vec_index < 0 || vec_index >= m_include_dirs.size ())
       return NULL;
+    if (version >= 5 && index == 0)
+      return NULL
     return m_include_dirs[vec_index];
   }

...
but that causes a regression (with gcc-11 -gdwarf-5) in
gdb.tui/tui-layout-asm-short-prog.exp: tui_get_begin_asm_address fails to
return an address.

Looking at the line table info, it's a bit odd since both directory and file
are duplicated:
...
 The Directory Table (offset 0x22, lines 2, columns 1):
  Entry Name
  0     (indirect line string, offset: 0x0): $src/gdb.tui
  1     (indirect line string, offset: 0x0): $src/gdb.tui

 The File Name Table (offset 0x30, lines 2, columns 2):
  Entry Dir     Name
  0     0       (indirect line string, offset: 0x39): \
                  tui-layout-asm-short-prog.S
  1     1       (indirect line string, offset: 0x39): \
                  tui-layout-asm-short-prog.S
...
I've filed a gas PR about that: PR 28102 - "[gas, --gdwarf-5] Duplicate file
and dir".

Anyway, tui_get_begin_asm_address fails to return an address, because
find_line_pc fails, which fails because in find_line_symtab we fail to match
tui-layout-asm-short-prog.S (without line-table info) with
$src/gdb.tui/tui-layout-asm-short-prog.S (with line-table info).

That is, without the patch we simply have one symtab:
...
$ gdb -q -batch $exec "maint expand-symtabs" -ex "maint info symtab"
{ objfile $exec ((struct objfile *) 0x289bd40)
  { ((struct compunit_symtab *) 0x2f240f0)
    debugformat DWARF 2
    producer GNU AS 2.35.1
    dirname $build
    blockvector ((struct blockvector *) 0x2f24270)
    user ((struct compunit_symtab *) (null))
        { symtab $src/gdb.tui/tui-layout-asm-short-prog.S \
	  ((struct symtab *) 0x2f24180)
          fullname (null)
          linetable ((struct linetable *) 0x2f24290)
        }
  }
}
...
while with the patch we have instead two symtabs:
...
{ objfile $exec ((struct objfile *) 0x290fd40)
  { ((struct compunit_symtab *) 0x2f990f0)
    debugformat DWARF 2
    producer GNU AS 2.35.1
    dirname $build
    blockvector ((struct blockvector *) 0x2f992a0)
    user ((struct compunit_symtab *) (null))
        { symtab tui-layout-asm-short-prog.S ((struct symtab *) 0x2f99180)
          fullname (null)
          linetable ((struct linetable *) 0x0)
        }
        { symtab $src/gdb.tui/tui-layout-asm-short-prog.S \
	    ((struct symtab *) 0x2f991b0)
          fullname (null)
          linetable ((struct linetable *) 0x2f992c0)
        }
  }
}
...

I realized I could fix this by modifying the fix to:
...
diff --git a/gdb/dwarf2/line-header.h b/gdb/dwarf2/line-header.h
index 4cacb8ca344..ad1451c884e 100644
--- a/gdb/dwarf2/line-header.h
+++ b/gdb/dwarf2/line-header.h
@@ -93,6 +93,13 @@  struct line_header
       vec_index = index - 1;
     if (vec_index < 0 || vec_index >= m_include_dirs.size ())
       return NULL;
+    if (version >= 5)
+      {
+       if (index == 0)
+         return NULL;
+       if (strcmp (m_include_dirs[vec_index], m_include_dirs[0]) == 0)
+         return NULL;
+      }
     return m_include_dirs[vec_index];
   }

...
and confirmed that the regressing gdb.tui test passes again, but was not sure
if this is a good idea.

So instead, I tried to fix this at the opposite end: in
symtab_to_filename_for_display:
...
diff --git a/gdb/source.c b/gdb/source.c
index 7d1934bd6c9..c15f6716598 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1272,7 +1272,14 @@  symtab_to_filename_for_display (struct symtab *symtab)
   else if (filename_display_string == filename_display_absolute)
     return symtab_to_fullname (symtab);
   else if (filename_display_string == filename_display_relative)
-    return symtab->filename;
+    {
+      struct compunit_symtab *cust = SYMTAB_COMPUNIT (symtab);
+      if (cust->dirname != nullptr
+         && (strncmp (cust->dirname, symtab->filename, strlen (cust->dirname))
+             == 0))
+       return lbasename (symtab->filename);
+      return symtab->filename;
+    }
   else
     internal_error (__FILE__, __LINE__, _("invalid filename_display_string"));
 }
...
but that caused a regression in gdb.base/realname-expand.exp:
...
 (gdb) rbreak realname-expand-real.c:func^M
-Breakpoint 1 at 0x4004bb: file realname-expand-link.c, line 21.^M
+Breakpoint 1 at 0x4004bb: file \
  $build/outputs/gdb.base/realname-expand/realname-expand-link.c, line 21.^M
 void func(void);^M
-(gdb) PASS: gdb.base/realname-expand.exp: rbreak realname-expand-real.c:func
+(gdb) FAIL: gdb.base/realname-expand.exp: rbreak realname-expand-real.c:func
...

[ This regression did not reproduce initially on a system where I have a
symbolic link /home/vries/gdb_version -> /data/gdb_versions, but after
wrapping the test invocation in ( cd $(pwd -P); ... ) it also reproduced
there. ]

We could fix this by:
- making the patch conditional on set basenames-may-differ (which is on in
  the test-case), or
- updating the test-case.

I don't understand the problem well enough to judge whether the former is a
good idea, and the latter seems not great since I've started out trying to
avoid updating another test-case.

So I reverted back to the original fix in line-header.h, which both fixes the
initial FAIL and does not cause regressions.

Tested on x86_64-linux with gcc-11 (with -gdwarf-5 default).

Any comments?

Thanks,
- Tom

[gdb/symtab] Fix FAILs in gdb.base/langs.exp with gcc-11

gdb/ChangeLog:

2021-07-20  Tom de Vries  <tdevries@suse.de>

	PR symtab/28108
	* gdb/dwarf2/line-header.h (line_header::include_dir_at): For dwarf 5,
	ignore entry at 0, and other entries equal to entry 0.

---
 gdb/dwarf2/line-header.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gdb/dwarf2/line-header.h b/gdb/dwarf2/line-header.h
index 4cacb8ca344..42952b14989 100644
--- a/gdb/dwarf2/line-header.h
+++ b/gdb/dwarf2/line-header.h
@@ -93,6 +93,15 @@  struct line_header
       vec_index = index - 1;
     if (vec_index < 0 || vec_index >= m_include_dirs.size ())
       return NULL;
+    if (version >= 5)
+      {
+	if (index == 0)
+	  return NULL;
+	if (m_include_dirs[vec_index] != nullptr
+	    && m_include_dirs[0] != nullptr
+	    && strcmp (m_include_dirs[vec_index], m_include_dirs[0]) == 0)
+	  return NULL;
+      }
     return m_include_dirs[vec_index];
   }