Re: [PATCH] DWARFv5: Fix for the filename complaint when printing macro.

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-01-20 11:21 p.m., Nitika.Achra--- via Gdb-patches wrote:
> From: nitachra <Nitika.Achra@amd.com>
> 
> A gentle reminder!
> 
> Regards,
> Nitika
> 
> ---
> GDB complaints "During symbol reading: symtab found for file,
> but that file is not covered in the compilation unit's macro information.
> No symbol "MAX_SIZE" in the current context" when printing the macro with
> the testcase file macro.c given below. This patch fixes this and prints
> the correct value.
> 
> When gdb reads file number from DW_MACRO_start_file data in .debug_macro
> section, it will do name lookup in file_names and include_directories
> index table of .debug_line section.
> 
> A snippet of .debug_macro is as follows-
> 
> DW_MACRO_start_file - lineno: 0 filenum: 1
>   DW_MACRO_define_strx - lineno: 3 macro: MAX_SIZE 10
> DW_MACRO_end_file
> 
> Index of the file for the lookup in file_names table in .debug_line is (filenum - 1)
> 
> A snippet of .debug_line is as follows-
> 
> file_names[  0]:
>            name: "macro.c"
>       dir_index: 0
>    md5_checksum: 0ee310711170f2dd9844b522e844207d
> 
> dir_index will be used to lookup the path in include_directories table-
> 
> include_directories[  0] = "/home/nitika/workspace"
> include_directories[  1] = "/usr/include"
> include_directories[  2] = "/usr/include/x86_64-linux-gnu/bits"
> include_directories[  3] = "/usr/include/x86_64-linux-gnu/sys"
> include_directories[  4] = "/usr/include/x86_64-linux-gnu/gnu"
> include_directories[  5] = "up_llvm/llvm-project/build/lib/clang/11.0.0/include"
> include_directories[  6] = "/usr/include/x86_64-linux-gnu/bits/types"
> 
> Prior to DWARFv5 include_directories[0] has no info, so a lookup at that
> index will return NULL and we have file name "macro.c". But in dwarfv5,
> current directory is explicitly represented at index 0 (as shown in the
> snippet above), hence after lookup you get "/home/nitika/workspace/macro.c".
> (compilation directory is /home/nitika/workspace)
> 
> Test case used (macro.c):
> 
>  #define MAX_SIZE 10
> int main(void)
> {
>    int size = 0;
>    size = size + MAX_SIZE;
> 
>    printf("\n The value of size is [%d]\n",size);
> 
>    return 0;
> }
> 
> clang -gdwarf-5 -fdebug-macro  macro.c -o macro.out
> 
> Before the patch:
> 
> gdb -q  macro.out -ex "start" -ex "set complaints 1"  -ex "p MAX_SIZE"
> Reading symbols from macro.out...
> Temporary breakpoint 1 at 0x4004df: file macro.c, line 7.
> Starting program: /home/nitika/workspace/macro.out
> 
> Temporary breakpoint 1, main () at macro.c:7
> 7          int size = 0;
> During symbol reading: symtab found for 'macro.c',
> but that file is not covered in the compilation unit's macro information.
> No symbol "MAX_SIZE" in current context.
> 
> After the patch:
> 
> gdb -q  macro.out -ex "start" -ex "p MAX_SIZE"
> Reading symbols from macro.out...
> Temporary breakpoint 1 at 0x4004df: file macro.c, line 7.
> Starting program: /home/nitika/workspace/macro.out
> 
> Temporary breakpoint 1, main () at macro.c:7
> 7          int size = 0;
> $1 = 10
> 
> Tested by running the testsuite before and after the patch with
> -gdwarf-5 and there is no increase in the number of test cases
> that fails. Used clang 11.0.0.

But are there some tests that now pass with your patch?  If not, it means
this is not covered by the testsuite.  This is a bit surprising, because
it's a really basic usage of a macro, I would expect that there exists a
test for this.  Can you please check the macro-related tests (look for
gdb.*/*mac*.exp), see if there exists a test for this kind of thing?  If
there isn't, then we would need to add one.

> 
> gdb/ChangeLog:
> 
> 	* macrotab.c (macro_lookup_inclusion): In DWARF version 5, macro
> 	filename includes the full directory path. Changing the if condition
> 	to include the comparison for full filename.
> 	* macrotab.h (macro_lookup_inclusion): Changing the definition,
> 	symtab as parameter instead of only the filename.
> 	* macroscope.c (sal_macro_scope): Passing the symtab instead of
> 	filename while calling macro_lookup_inclusion.
> 	* dwarf2/line-header.c (file_file_name): As per the comment in
> 	its definition in line-header.h, it should return file name
> 	relative to the compilation directory of file number I in this
> 	object's file name table.
> ---
>  gdb/dwarf2/line-header.c | 8 --------
>  gdb/macroscope.c         | 2 +-
>  gdb/macrotab.c           | 7 ++++---
>  gdb/macrotab.h           | 2 +-
>  4 files changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/gdb/dwarf2/line-header.c b/gdb/dwarf2/line-header.c
> index 7575297f96..8975a03285 100644
> --- a/gdb/dwarf2/line-header.c
> +++ b/gdb/dwarf2/line-header.c
> @@ -69,14 +69,6 @@ line_header::file_file_name (int file) const
>      {
>        const file_entry *fe = file_name_at (file);
>  
> -      if (!IS_ABSOLUTE_PATH (fe->name))
> -	{
> -	  const char *dir = fe->include_dir (this);
> -	  if (dir != NULL)
> -	    return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
> -							  fe->name,
> -							  (char *) NULL));
> -	}
>        return make_unique_xstrdup (fe->name);
>      }
>    else
> diff --git a/gdb/macroscope.c b/gdb/macroscope.c
> index e2abdd1bdd..a994068003 100644
> --- a/gdb/macroscope.c
> +++ b/gdb/macroscope.c
> @@ -50,7 +50,7 @@ sal_macro_scope (struct symtab_and_line sal)
>    gdb::unique_xmalloc_ptr<struct macro_scope> ms (XNEW (struct macro_scope));
>  
>    main_file = macro_main (COMPUNIT_MACRO_TABLE (cust));
> -  inclusion = macro_lookup_inclusion (main_file, sal.symtab->filename);
> +  inclusion = macro_lookup_inclusion (main_file, sal.symtab);
>  
>    if (inclusion)
>      {
> diff --git a/gdb/macrotab.c b/gdb/macrotab.c
> index 96b29e345a..fae14b3c3c 100644
> --- a/gdb/macrotab.c
> +++ b/gdb/macrotab.c
> @@ -504,10 +504,11 @@ macro_include (struct macro_source_file *source,
>  
>  
>  struct macro_source_file *
> -macro_lookup_inclusion (struct macro_source_file *source, const char *name)
> +macro_lookup_inclusion (struct macro_source_file *source, struct symtab *symtab)
>  {
>    /* Is SOURCE itself named NAME?  */
> -  if (filename_cmp (name, source->filename) == 0)
> +  if (filename_cmp (symtab->filename, source->filename) == 0 ||
> +      filename_cmp (symtab->fullname, macro_source_fullname(source).c_str()) == 0)

Space before parenthesis, binary operator on the next line, line too long.  Format
like this:

  if (filename_cmp (symtab->filename, source->filename) == 0
      || filename_cmp (symtab->fullname,
		       macro_source_fullname (source).c_str ()) == 0)

>      return source;
>  
>    /* It's not us.  Try all our children, and return the lowest.  */
> @@ -519,7 +520,7 @@ macro_lookup_inclusion (struct macro_source_file *source, const char *name)
>      for (child = source->includes; child; child = child->next_included)
>        {
>  	struct macro_source_file *result
> -	  = macro_lookup_inclusion (child, name);
> +	  = macro_lookup_inclusion (child, symtab);
>  
>  	if (result)
>  	  {
> diff --git a/gdb/macrotab.h b/gdb/macrotab.h
> index ea539e872e..6cbd3bc474 100644
> --- a/gdb/macrotab.h
> +++ b/gdb/macrotab.h
> @@ -235,7 +235,7 @@ void macro_define_special (struct macro_table *table);
>     least-nested inclusion --- the one closest to the main source file.  */
>  struct macro_source_file *macro_lookup_inclusion
>  			  (struct macro_source_file *source,
> -			   const char *name);
> +			   struct symtab *symtab);

The documentation just above this will need to be updated.

Simon