DWARFv5: Handle DW_MACRO_define_strx and DW_MACRO_undef_strx macro entries.

Message ID 20200628165752.5379-1-Nitika.Achra@amd.com
State Superseded
Headers show
Series
  • DWARFv5: Handle DW_MACRO_define_strx and DW_MACRO_undef_strx macro entries.
Related show

Commit Message

nitachra June 28, 2020, 4:57 p.m.
GDB is not able to print the macro values with -gdwarf-5 for clang
generated binaries. Clang is emitting DW_MACRO_define_strx and
DW_MACRO_undef_strx entries in .debug_macro section which are not
supported in GDB. This patch handles them.

DW_MACRO_define_strx and DW_MACRO_undef_strx are added in DWARFv5.
They have two operands. The first operand encodes the line number of
the #define or #undef macro directive. The second operand identifies
a string; it is represented using an unsigned LEB128 encoded value,
which is interpreted as a zero-based index into an array of offsets
in the .debug_str_offsets section. This is as per the section 6.3.2.1
of Dwarf Debugging Information Format Version 5.

Test case used:
 #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 "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;
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.

gdb/dwarf2/ChangeLog:

*macro.c (dwarf_decode_macro_bytes): Handle DW_MACRO_define_strx
 and DW_MACRO_undef_strx.
 (dwarf_decode_macros): Likewise
*read.c (dwarf_decode_macros): Pass str_offsets_base in the parameters
 which is the value of DW_AT_str_offsets_base.
*macro.h (dwarf_decode_macros): Modify the definition to include
 str_offsets_base.

gdb/ChangeLog:

*macrotab.c (macro_lookup_inclusion): In DWARF version 5 filename
includes the full directory path.
---
 gdb/dwarf2/macro.c | 63 +++++++++++++++++++++++++++++++++++++++++++---
 gdb/dwarf2/macro.h |  1 +
 gdb/dwarf2/read.c  |  4 ++-
 gdb/macrotab.c     |  3 ++-
 4 files changed, 66 insertions(+), 5 deletions(-)

-- 
2.17.1

Comments

Simon Marchi July 11, 2020, 6:46 p.m. | #1
git tells me this when applying:

  Applying: DWARFv5: Handle DW_MACRO_define_strx and DW_MACRO_undef_strx macro entries.
  .git/rebase-apply/patch:50: trailing whitespace.

  warning: 1 line adds whitespace errors.

please find and fix it.


On 2020-06-28 12:57 p.m., nitachra wrote:
> GDB is not able to print the macro values with -gdwarf-5 for clang

> generated binaries. Clang is emitting DW_MACRO_define_strx and

> DW_MACRO_undef_strx entries in .debug_macro section which are not

> supported in GDB. This patch handles them.

> 

> DW_MACRO_define_strx and DW_MACRO_undef_strx are added in DWARFv5.

> They have two operands. The first operand encodes the line number of

> the #define or #undef macro directive. The second operand identifies

> a string; it is represented using an unsigned LEB128 encoded value,

> which is interpreted as a zero-based index into an array of offsets

> in the .debug_str_offsets section. This is as per the section 6.3.2.1

> of Dwarf Debugging Information Format Version 5.

> 

> Test case used:

>  #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 "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;

> 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.


Hmm this is a bit misleading, as clang 11.0.0 is not released yet.  You must
have tested with a preview build.  Please indicate the exact version.

But thanks for specifying, because I only have clang 10 right now, and I didn't
see DW_MACRO_define_strx getting generated.

> 

> gdb/dwarf2/ChangeLog:


There is no gdb/dwarf2/ChangeLog file.  This will go in gdb/ChangeLog.

> *macro.c (dwarf_decode_macro_bytes): Handle DW_MACRO_define_strx

>  and DW_MACRO_undef_strx.

>  (dwarf_decode_macros): Likewise


Maybe it's just your mail client, but just to be sure, the format is:

<tab>*<space><filename>...

Wrapped lines are ligned with the *.

> *read.c (dwarf_decode_macros): Pass str_offsets_base in the parameters

>  which is the value of DW_AT_str_offsets_base.

> *macro.h (dwarf_decode_macros): Modify the definition to include

>  str_offsets_base.

> 

> gdb/ChangeLog:

> 

> *macrotab.c (macro_lookup_inclusion): In DWARF version 5 filename

> includes the full directory path.

> ---

>  gdb/dwarf2/macro.c | 63 +++++++++++++++++++++++++++++++++++++++++++---

>  gdb/dwarf2/macro.h |  1 +

>  gdb/dwarf2/read.c  |  4 ++-

>  gdb/macrotab.c     |  3 ++-

>  4 files changed, 66 insertions(+), 5 deletions(-)

> 

> diff --git a/gdb/dwarf2/macro.c b/gdb/dwarf2/macro.c

> index d047d806f8..2f5f3aa827 100644

> --- a/gdb/dwarf2/macro.c

> +++ b/gdb/dwarf2/macro.c

> @@ -427,6 +427,7 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,

>  			  const struct dwarf2_section_info *section,

>  			  int section_is_gnu, int section_is_dwz,

>  			  unsigned int offset_size,

> +			  ULONGEST str_offsets_base,

>  			  htab_t include_hash)

>  {

>    struct objfile *objfile = per_objfile->objfile;

> @@ -561,6 +562,50 @@ dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,

>            }

>            break;

>  

> +	case DW_MACRO_define_strx:

> +	case DW_MACRO_undef_strx:

> +	  {

> +	    unsigned int bytes_read;

> +	    int line;

> +	    int offset_index;

> +	    const char* body;

> +	    const gdb_byte* info_ptr;


Space before asterisk.

> +	    ULONGEST str_offset;


Declare all of these on first use (where applicable, doesn't apply
to `bytes_read` of course).

> +

> +	    line = read_unsigned_leb128 (abfd, mac_ptr, &bytes_read);

> +	    mac_ptr += bytes_read;

> +	    offset_index = read_unsigned_leb128 (abfd, mac_ptr, &bytes_read);

> +	    mac_ptr += bytes_read;

> +	    struct dwarf2_section_info str_offsets_section = per_objfile->per_bfd->str_offsets;


This line is too long.

When using a .dwo, the .debug_macro section is in the dwo (.debug_macro.dwo)?
If so, should we use the string offset and string sections from the dwo?

We don't have a testsuite board for DWARF5 + split-dwarf currently, I think
it would be useful.

> +	    str_offsets_section.read (objfile);

> +	    info_ptr = (str_offsets_section.buffer

> +			+ str_offsets_base

> +			+ offset_index * offset_size);

> +	    if (offset_size == 4)

> +	      str_offset = bfd_get_32 (abfd, info_ptr);

> +	    else

> +	      str_offset = bfd_get_64 (abfd, info_ptr);


Can we get some bound checking here, to avoid reading past the section buffer?

> +	    body = per_objfile->per_bfd->str.read_string (objfile,

> +							  str_offset,

> +							  "DW_FORM_strx");


The "DW_FORM_strx" is used for error reporting inside `read_string`.  I'm not
sure it's exact DW_FORM_strx here, because forms are used for attribute values,
which is not the case here.  Perhaps you could use "DW_MACRO_define_strx" and
"DW_MACRO_undef_strx", it would produce error messages like:

  DW_MACRO_define_strx used without %s section [in module %s]
  DW_MACRO_define_strx pointing outside of %s section [in module %s]

which I think make sense.

> +	    

> +	    if (! current_file)


current_file == nullptr

> +             {

> +               /* DWARF violation as no main source is present.  */

> +               complaint (_("debug info with no main source gives macro %s "

> +                            "on line %d: %s"),

> +                            macinfo_type == DW_MACRO_define_strx ? _("definition")

> +                            : _("undefinition"), line, body);

> +               break;


There are spaces that should be tabs here.

Simon
Tom Tromey July 11, 2020, 10:54 p.m. | #2
>> +	    struct dwarf2_section_info str_offsets_section = per_objfile->per_bfd->str_offsets;


Simon> This line is too long.

It's also slightly bad to copy a section like this.  In weird cases I
think it can lead to it being read more than once.

We should probably delete operator= here, but that's a pain right now.
I think I have some patches toward this somewhere, but I haven't
finished them yet.  (Also, I think the DWP code does do this at the
moment.)

Better here to use a reference or a pointer.

Tom
Simon Marchi July 11, 2020, 11:05 p.m. | #3
On 2020-07-11 6:54 p.m., Tom Tromey wrote:
>>> +	    struct dwarf2_section_info str_offsets_section = per_objfile->per_bfd->str_offsets;

> 

> Simon> This line is too long.

> 

> It's also slightly bad to copy a section like this.  In weird cases I

> think it can lead to it being read more than once.

> 

> We should probably delete operator= here, but that's a pain right now.

> I think I have some patches toward this somewhere, but I haven't

> finished them yet.  (Also, I think the DWP code does do this at the

> moment.)

> 

> Better here to use a reference or a pointer.

> 

> Tom

> 


Ah, I didn't notice that it wasn't a pointer, thanks.

Simon

Patch

diff --git a/gdb/dwarf2/macro.c b/gdb/dwarf2/macro.c
index d047d806f8..2f5f3aa827 100644
--- a/gdb/dwarf2/macro.c
+++ b/gdb/dwarf2/macro.c
@@ -427,6 +427,7 @@  dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
 			  const struct dwarf2_section_info *section,
 			  int section_is_gnu, int section_is_dwz,
 			  unsigned int offset_size,
+			  ULONGEST str_offsets_base,
 			  htab_t include_hash)
 {
   struct objfile *objfile = per_objfile->objfile;
@@ -561,6 +562,50 @@  dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
           }
           break;
 
+	case DW_MACRO_define_strx:
+	case DW_MACRO_undef_strx:
+	  {
+	    unsigned int bytes_read;
+	    int line;
+	    int offset_index;
+	    const char* body;
+	    const gdb_byte* info_ptr;
+	    ULONGEST str_offset;
+
+	    line = read_unsigned_leb128 (abfd, mac_ptr, &bytes_read);
+	    mac_ptr += bytes_read;
+	    offset_index = read_unsigned_leb128 (abfd, mac_ptr, &bytes_read);
+	    mac_ptr += bytes_read;
+	    struct dwarf2_section_info str_offsets_section = per_objfile->per_bfd->str_offsets;
+	    str_offsets_section.read (objfile);
+	    info_ptr = (str_offsets_section.buffer
+			+ str_offsets_base
+			+ offset_index * offset_size);
+	    if (offset_size == 4)
+	      str_offset = bfd_get_32 (abfd, info_ptr);
+	    else
+	      str_offset = bfd_get_64 (abfd, info_ptr);
+	    body = per_objfile->per_bfd->str.read_string (objfile,
+							  str_offset,
+							  "DW_FORM_strx");
+	    
+	    if (! current_file)
+             {
+               /* DWARF violation as no main source is present.  */
+               complaint (_("debug info with no main source gives macro %s "
+                            "on line %d: %s"),
+                            macinfo_type == DW_MACRO_define_strx ? _("definition")
+                            : _("undefinition"), line, body);
+               break;
+             }
+
+	    if (macinfo_type == DW_MACRO_define_strx)
+	      parse_macro_definition (current_file, line, body);
+	    else
+	      macro_undef (current_file, line, body);
+	   }
+	   break;
+
         case DW_MACRO_start_file:
           {
             unsigned int bytes_read;
@@ -671,7 +716,7 @@  dwarf_decode_macro_bytes (dwarf2_per_objfile *per_objfile,
 					  new_mac_ptr, include_mac_end,
 					  current_file, lh, section,
 					  section_is_gnu, is_dwz, offset_size,
-					  include_hash);
+					  str_offsets_base, include_hash);
 
 		htab_remove_elt (include_hash, (void *) new_mac_ptr);
 	      }
@@ -712,7 +757,8 @@  dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
 		     buildsym_compunit *builder,
 		     const dwarf2_section_info *section,
 		     const struct line_header *lh, unsigned int offset_size,
-		     unsigned int offset, int section_is_gnu)
+		     unsigned int offset, ULONGEST str_offsets_base,
+		     int section_is_gnu)
 {
   bfd *abfd;
   const gdb_byte *mac_ptr, *mac_end;
@@ -814,6 +860,17 @@  dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
 	    mac_ptr += offset_size;
 	  }
 	  break;
+	case DW_MACRO_define_strx:
+	case DW_MACRO_undef_strx:
+	  {
+	    unsigned int bytes_read;
+
+	    read_unsigned_leb128 (abfd, mac_ptr, &bytes_read);
+	    mac_ptr += bytes_read;
+	    read_unsigned_leb128 (abfd, mac_ptr, &bytes_read);
+	    mac_ptr += bytes_read;
+	  }
+	  break;
 
 	case DW_MACRO_import:
 	case DW_MACRO_import_sup:
@@ -861,5 +918,5 @@  dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
   *slot = (void *) mac_ptr;
   dwarf_decode_macro_bytes (per_objfile, builder, abfd, mac_ptr, mac_end,
 			    current_file, lh, section, section_is_gnu, 0,
-			    offset_size, include_hash.get ());
+			    offset_size, str_offsets_base, include_hash.get ());
 }
diff --git a/gdb/dwarf2/macro.h b/gdb/dwarf2/macro.h
index d874068947..dc396e1b70 100644
--- a/gdb/dwarf2/macro.h
+++ b/gdb/dwarf2/macro.h
@@ -28,6 +28,7 @@  extern void dwarf_decode_macros (dwarf2_per_objfile *per_objfile,
 				 const struct line_header *lh,
 				 unsigned int offset_size,
 				 unsigned int offset,
+				 ULONGEST str_offsets_base,
 				 int section_is_gnu);
 
 #endif /* GDB_DWARF2_MACRO_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index e3073fe43c..a0e197c748 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -23353,8 +23353,10 @@  dwarf_decode_macros (struct dwarf2_cu *cu, unsigned int offset,
 
   buildsym_compunit *builder = cu->get_builder ();
 
+  ULONGEST str_offsets_base = *cu->str_offsets_base;
+
   dwarf_decode_macros (per_objfile, builder, section, lh,
-		       offset_size, offset, section_is_gnu);
+		       offset_size, offset, str_offsets_base, section_is_gnu);
 }
 
 /* Return the .debug_loc section to use for CU.
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index 63cd30148a..606f084bc9 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -507,7 +507,8 @@  struct macro_source_file *
 macro_lookup_inclusion (struct macro_source_file *source, const char *name)
 {
   /* Is SOURCE itself named NAME?  */
-  if (filename_cmp (name, source->filename) == 0)
+  if (filename_cmp (name, source->filename) == 0 ||
+      filename_cmp (basename (name), basename (source->filename)) == 0)
     return source;
 
   /* It's not us.  Try all our children, and return the lowest.  */