[05/14] libdw: Fix dwarf_macro_getsrcfiles for DWARF 5

Message ID bdd28a8a6fa01c0472119b8fdea2dce055f8544a.1695837512.git.osandov@fb.com
State Committed
Headers
Series elfutils: DWARF package (.dwp) file support |

Commit Message

Omar Sandoval Sept. 27, 2023, 6:20 p.m. UTC
  From: Omar Sandoval <osandov@fb.com>

Dwarf_Macro_Op_Table::is_64bit conflates the address size and the offset
size: for .debug_macinfo, it is initialized based on the compilation
unit's address size, but for .debug_macro, it is initialized based on
the macro unit's offset size.  is_64bit is used to determine the address
size to pass to __libdw_getsrclines.  For a 64-bit architecture using
DWARF 5 with 32-bit offsets (the common case), this fails because
read_srclines checks that the given address size matches the address
size from the line number program header.

Fix it by splitting is_64bit into separate address_size and offset_size
members.

Fixes: fb90bf3f84b5 ("Support .debug_macro")
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 libdw/ChangeLog                 | 11 +++++++++++
 libdw/dwarf_getmacros.c         | 17 ++++++++++++++---
 libdw/dwarf_macro_getsrcfiles.c |  3 +--
 libdw/libdwP.h                  |  3 ++-
 4 files changed, 28 insertions(+), 6 deletions(-)
  

Comments

Mark Wielaard Oct. 3, 2023, 9:29 p.m. UTC | #1
Hi Omar,

On Wed, Sep 27, 2023 at 11:20:54AM -0700, Omar Sandoval wrote:
> Dwarf_Macro_Op_Table::is_64bit conflates the address size and the offset
> size: for .debug_macinfo, it is initialized based on the compilation
> unit's address size, but for .debug_macro, it is initialized based on
> the macro unit's offset size.  is_64bit is used to determine the address
> size to pass to __libdw_getsrclines.  For a 64-bit architecture using
> DWARF 5 with 32-bit offsets (the common case), this fails because
> read_srclines checks that the given address size matches the address
> size from the line number program header.
> 
> Fix it by splitting is_64bit into separate address_size and offset_size
> members.

Very nice catch. You are right that for DWARF5 the line table header
already gives the address_size. It is slightly embarassing our sanity
check here failed. Thanks for fixing.

I had to think a bit on why we didn't need to set the address_size for
the fake_cu. But that is only needed to read address class
forms. Which aren't valid forms in macro data.

Applied,

Mark
  

Patch

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 7528c093..d3f36cc8 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -7,6 +7,17 @@ 
 	* dwarf_macro_param2.c (dwarf_macro_param2): Change form condition to
 	switch statement and add DW_FORM_line_strp, DW_FORM_strp_sup,
 	DW_FORM_strx, and DW_FORM_strx[1-4].
+	* dwarf_getmacros.c (get_macinfo_table): Replace assignment of
+	table->is_64bit with assignments of table->address_size and
+	table->offset_size.
+	(get_table_for_offset): Ditto.
+	(read_macros): Get fake CU offset_size from table->offset_size instead
+	of table->is_64bit.
+	* dwarf_macro_getsrcfiles.c (dwarf_macro_getsrcfiles): Get
+	address_size for __libdw_getsrclines from table->address_size instead
+	of table->is_64bit.
+	* libdwP.h (Dwarf_Macro_Op_Table): Replace is_64bit with address_size
+	and offset_size.
 
 2023-02-22  Mark Wielaard  <mark@klomp.org>
 
diff --git a/libdw/dwarf_getmacros.c b/libdw/dwarf_getmacros.c
index fd929669..b7d98af4 100644
--- a/libdw/dwarf_getmacros.c
+++ b/libdw/dwarf_getmacros.c
@@ -134,7 +134,8 @@  get_macinfo_table (Dwarf *dbg, Dwarf_Word macoff, Dwarf_Die *cudie)
   table->offset = macoff;
   table->sec_index = IDX_debug_macinfo;
   table->line_offset = line_offset;
-  table->is_64bit = cudie->cu->address_size == 8;
+  table->address_size = cudie->cu->address_size;
+  table->offset_size = cudie->cu->offset_size;
   table->comp_dir = __libdw_getcompdir (cudie);
 
   return table;
@@ -182,6 +183,15 @@  get_table_for_offset (Dwarf *dbg, Dwarf_Word macoff,
 	  return NULL;
     }
 
+  uint8_t address_size;
+  if (cudie != NULL)
+    address_size = cudie->cu->address_size;
+  else
+    {
+      char *ident = elf_getident (dbg->elf, NULL);
+      address_size = ident[EI_CLASS] == ELFCLASS32 ? 4 : 8;
+    }
+
   /* """The macinfo entry types defined in this standard may, but
      might not, be described in the table""".
 
@@ -258,7 +268,8 @@  get_table_for_offset (Dwarf *dbg, Dwarf_Word macoff,
     .line_offset = line_offset,
     .header_len = readp - startp,
     .version = version,
-    .is_64bit = is_64bit,
+    .address_size = address_size,
+    .offset_size = is_64bit ? 8 : 4,
 
     /* NULL if CUDIE is NULL or DW_AT_comp_dir is absent.  */
     .comp_dir = __libdw_getcompdir (cudie),
@@ -368,7 +379,7 @@  read_macros (Dwarf *dbg, int sec_index,
 	.dbg = dbg,
 	.sec_idx = sec_index,
 	.version = table->version,
-	.offset_size = table->is_64bit ? 8 : 4,
+	.offset_size = table->offset_size,
 	.str_off_base = str_offsets_base_off (dbg, (cudie != NULL
 						    ? cudie->cu: NULL)),
 	.startp = (void *) startp + offset,
diff --git a/libdw/dwarf_macro_getsrcfiles.c b/libdw/dwarf_macro_getsrcfiles.c
index 3b1794b1..4e8deeeb 100644
--- a/libdw/dwarf_macro_getsrcfiles.c
+++ b/libdw/dwarf_macro_getsrcfiles.c
@@ -72,8 +72,7 @@  dwarf_macro_getsrcfiles (Dwarf *dbg, Dwarf_Macro *macro,
 	 will be broken.  */
 
       if (__libdw_getsrclines (dbg, line_offset, table->comp_dir,
-			       table->is_64bit ? 8 : 4,
-			       NULL, &table->files) < 0)
+			       table->address_size, NULL, &table->files) < 0)
 	table->files = (void *) -1;
     }
 
diff --git a/libdw/libdwP.h b/libdw/libdwP.h
index 5cbdc279..c3fe9f93 100644
--- a/libdw/libdwP.h
+++ b/libdw/libdwP.h
@@ -526,7 +526,8 @@  typedef struct
   Dwarf_Half header_len;
 
   uint16_t version;
-  bool is_64bit;
+  uint8_t address_size;
+  uint8_t offset_size;
   uint8_t sec_index;	/* IDX_debug_macro or IDX_debug_macinfo.  */
 
   /* Shows where in TABLE each opcode is defined.  Since opcode 0 is