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

[Tom de Vries <tdevries@suse.de>]

On 7/20/21 1:02 PM, Tom de Vries wrote:
> 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:
> ...
> 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?
> 

Ping.

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];
>    }
>  
>