Bug 28220 - dwarf_location_attr returns high-bit junk from .debug_addr when fetching 32-bit addresses
Summary: dwarf_location_attr returns high-bit junk from .debug_addr when fetching 32-b...
Status: RESOLVED FIXED
Alias: None
Product: elfutils
Classification: Unclassified
Component: libdw (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks: 28191
  Show dependency treegraph
 
Reported: 2021-08-11 12:00 UTC by gprocida
Modified: 2021-09-12 19:45 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2021-08-26 00:00:00


Attachments
test case (5.08 KB, application/x-compressed-tar)
2021-08-11 12:01 UTC, gprocida
Details

Note You need to log in before you can comment on or make changes to this bug.
Description gprocida 2021-08-11 12:00:35 UTC
In libdw, there are snippets of code of the form:

dwarf_formaddr.c:
  71   if (address_size == 4)
  72     *addr = read_4ubyte_unaligned (dbg, datap);
  73   else
  74     *addr = read_8ubyte_unaligned (dbg, datap);

However, when I use the public interfaces in order to read locations, it goes wrong when for Clang-built 32-bit objects with DWARF 5 debug information, with extra junk in the upper 32 bits.

The attachment contains a full example. It's possible that I'm using the API incorrectly. This is the debug output for the problematic case:

clang 5 32
bias=0
bias=0
dwarf=0x55f103cf8a10 elf=0x55f103cf5610
 version=5 unit_type=1 cu_die.addr=0x7fc3032e0078 cu_die.cu=0x55f103cf8e98 cu_die.abbrev=0 sub_die.addr=0 sub_die.cu=0 sub_die.abbrev=0
 unit_id=0 address_size=4 offset_size=4
  dwarf_string_attr(die, 1b)
 comp_dir=/usr/local/google/home/gprocida/dev/libabigail/b195152239
   child.addr=0x7fc3032e008a child.cu=0x55f103cf8e98 child.abbrev=0
    dwarf_string_attr(die, 3)
    dwarf_bool_attr(die, 3f)
    dwarf_location_attr(die, 2)
    attribute code=0x2 form=0x18 (expected=18)
     expr=0x55f103cf8ff8 exprlen=1
     op.atom=a1 op.number=0 op.number2=0 op.offset=0
     result code=0x11 form=0x1 (expected=1)
     address=0x1000400010000
   variable name=x name2=x external=1 location=1000400010000
   child.addr=0x7fc3032e0095 child.cu=0x55f103cf8e98 child.abbrev=0
   child.addr=0x7fc3032e0099 child.cu=0x55f103cf8e98 child.abbrev=0
    dwarf_string_attr(die, 3)
    dwarf_bool_attr(die, 3f)
    dwarf_location_attr(die, 2)
    attribute code=0x2 form=0x18 (expected=18)
     expr=0x55f103cf9048 exprlen=1
     op.atom=a1 op.number=1 op.number2=0 op.offset=0
     result code=0x11 form=0x1 (expected=1)
     address=0x6265440000010004
   variable name=y name2=y external=1 location=6265440000010004

Note that variable x's address includes variable y's address as the upper 32 bits and that variable y has junk in the upper 32 bits.
Comment 1 gprocida 2021-08-11 12:01:23 UTC
Created attachment 13614 [details]
test case
Comment 2 Mark Wielaard 2021-08-26 17:12:02 UTC
That was an interesting bug. The issue is the use of DW_OP_addrx which creates a fake attribute of DW_FORM_addr pointing to the actual address. This fake attribute also has a fake CU. We didn't set the correct address size on this fake CU.

The following patch fixes it:

commit d18228697f750e651bf0bdf19abdaab0a4217008
Author: Mark Wielaard <mark@klomp.org>
Date:   Thu Aug 26 19:05:45 2021 +0200

    PR28220

diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c
index 9e944b86..a368feb8 100644
--- a/libdw/dwarf_begin_elf.c
+++ b/libdw/dwarf_begin_elf.c
@@ -224,6 +224,23 @@ valid_p (Dwarf *result)
       result = NULL;
     }
 
+  /* We are setting up some "fake" CUs, which need an address size.
+     Check the ELF class to come up with something reasonable.  */
+  int elf_addr_size = 8;
+  if (result != NULL)
+    {
+      GElf_Ehdr ehdr;
+      if (gelf_getehdr (result->elf, &ehdr) == NULL)
+	{
+	  Dwarf_Sig8_Hash_free (&result->sig8_hash);
+	  __libdw_seterrno (DWARF_E_INVALID_ELF);
+	  free (result);
+	  result = NULL;
+	}
+      else if (ehdr.e_ident[EI_CLASS] == ELFCLASS32)
+	elf_addr_size = 4;
+    }
+
   /* For dwarf_location_attr () we need a "fake" CU to indicate
      where the "fake" attribute data comes from.  This is a block
      inside the .debug_loc or .debug_loclists section.  */
@@ -247,8 +264,9 @@ valid_p (Dwarf *result)
 	    = (result->sectiondata[IDX_debug_loc]->d_buf
 	       + result->sectiondata[IDX_debug_loc]->d_size);
 	  result->fake_loc_cu->locs = NULL;
-	  result->fake_loc_cu->address_size = 0;
-	  result->fake_loc_cu->version = 0;
+	  result->fake_loc_cu->address_size = elf_addr_size;
+	  result->fake_loc_cu->offset_size = 4;
+	  result->fake_loc_cu->version = 4;
 	  result->fake_loc_cu->split = NULL;
 	}
     }
@@ -274,8 +292,9 @@ valid_p (Dwarf *result)
 	    = (result->sectiondata[IDX_debug_loclists]->d_buf
 	       + result->sectiondata[IDX_debug_loclists]->d_size);
 	  result->fake_loclists_cu->locs = NULL;
-	  result->fake_loclists_cu->address_size = 0;
-	  result->fake_loclists_cu->version = 0;
+	  result->fake_loclists_cu->address_size = elf_addr_size;
+	  result->fake_loclists_cu->offset_size = 4;
+	  result->fake_loclists_cu->version = 5;
 	  result->fake_loclists_cu->split = NULL;
 	}
     }
@@ -306,8 +325,9 @@ valid_p (Dwarf *result)
 	    = (result->sectiondata[IDX_debug_addr]->d_buf
 	       + result->sectiondata[IDX_debug_addr]->d_size);
 	  result->fake_addr_cu->locs = NULL;
-	  result->fake_addr_cu->address_size = 0;
-	  result->fake_addr_cu->version = 0;
+	  result->fake_addr_cu->address_size = elf_addr_size;
+	  result->fake_addr_cu->offset_size = 4;
+	  result->fake_addr_cu->version = 5;
 	  result->fake_addr_cu->split = NULL;
 	}
     }

Also on https://code.wildebeest.org/git/user/mjw/elfutils/commit/?h=fake_cu_elf_addr_size

Thanks for the testcases I'll try to incorporate them into the testsuite.
Comment 3 gprocida 2021-08-26 22:47:02 UTC
Thanks for the fast fix.

I've confirmed by basic parser working with the test case and with the original library we had the problem with.

This plus a libabigail change to use dwarf_getlocation_attr fixes the issue with abidw. I'll post that patch to the list tomorrow.
Comment 4 Mark Wielaard 2021-09-08 16:01:09 UTC
Turns out we could have caught this with the varlocs testcase.
Before the fix:

$ for i in testfile-vars-*.o; do echo $i; tests/varlocs --debug --exprlocs -e $i | grep exprloc; done
testfile-vars-clang-dwarf4-32.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}
testfile-vars-clang-dwarf4-64.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}
testfile-vars-clang-dwarf5-32.o
    location (exprloc) {addr: 0x400000000}
    location (exprloc) {addr: 0x616c630000000004}
testfile-vars-clang-dwarf5-64.o
    location (exprloc) {addr: 0x0}
    location (exprloc) {addr: 0x4}
testfile-vars-gcc-dwarf4-32.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}
testfile-vars-gcc-dwarf4-64.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}
testfile-vars-gcc-dwarf5-32.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}
testfile-vars-gcc-dwarf5-64.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}

With the patch:

$ for i in testfile-vars-*.o; do echo $i; LD_LIBRARY_PATH=libelf:libdw tests/varlocs --debug --exprlocs -e $i | grep exprloc; done
testfile-vars-clang-dwarf4-32.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}
testfile-vars-clang-dwarf4-64.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}
testfile-vars-clang-dwarf5-32.o
    location (exprloc) {addr: 0x0}
    location (exprloc) {addr: 0x4}
testfile-vars-clang-dwarf5-64.o
    location (exprloc) {addr: 0x0}
    location (exprloc) {addr: 0x4}
testfile-vars-gcc-dwarf4-32.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}
testfile-vars-gcc-dwarf4-64.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}
testfile-vars-gcc-dwarf5-32.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}
testfile-vars-gcc-dwarf5-64.o
    location (exprloc) {addr(0x0)}
    location (exprloc) {addr(0x4)}
Comment 5 Mark Wielaard 2021-09-08 20:15:03 UTC
Proposed patch and testcase:
https://sourceware.org/pipermail/elfutils-devel/2021q3/004149.html
Comment 6 Mark Wielaard 2021-09-12 19:45:28 UTC
commit 52b0d9caf5575a62322c9fbe920b69444dd09162
Author: Mark Wielaard <mark@klomp.org>
Date:   Thu Aug 26 19:05:45 2021 +0200

    libdw: set address size, offset size and version on fake CUs
    
    There are three "fake CUs" that are associated with .debug_loc,
    .debug_loclist and .debug_addr.  These fake CUs are used for "fake
    attributes" to provide values that are stored in these sections
    instead of in the .debug_info section. These fake CUs didn't have the
    address size, offset size and DWARF version set. This meant that
    values that depended on those properties might not be interpreted
    correctly. One example was the value associated with a DW_OP_addrx
    (which comes from the .debug_addr section).
    
    Add a testcase using varlocs to test that addresses can correctly be
    retrieved for gcc/clang, DWARF4/5 and 32/64 bits objects.
    
    https://sourceware.org/bugzilla/show_bug.cgi?id=28220
    
    Signed-off-by: Mark Wielaard <mark@klomp.org>