[PATCH] Getter and setter for Dwfl's offline_next_address

[PATCH] Getter and setter for Dwfl's offline_next_address

[Martin Rodriguez Reboredo]

Added new functions dwfl_get_offline_next_address and
dwfl_set_offline_next_address which will get plus set said field from
the Dwfl struct. This is a requirement for listing functions from their
addresses when using libdwfl offline, otherwise wrong symbols are going
to be returned.

Signed-off-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
---
This is meant to replace a patch that only had the setter [1].

Link: https://sourceware.org/pipermail/elfutils-devel/2024q1/006874.html [1]

 libdwfl/libdwfl.h            |  6 +++++
 libdwfl/offline.c            | 12 ++++++++++
 tests/dwfl-offline-address.c | 43 ++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 tests/dwfl-offline-address.c

diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
index 49ad6664..08aa61c4 100644
--- a/libdwfl/libdwfl.h
+++ b/libdwfl/libdwfl.h
@@ -109,6 +109,12 @@ extern int dwfl_errno (void);
 extern const char *dwfl_errmsg (int err);
 
 
+/* Get the next offline address.  */
+extern GElf_Addr dwfl_get_offline_next_address (Dwfl *dwfl);
+
+/* Set the next offline address.  */
+extern void dwfl_set_offline_next_address (Dwfl *dwfl, GElf_Addr addr);
+
 /* Start reporting the current set of segments and modules to the library.
    All existing segments are wiped.  Existing modules are marked to be
    deleted, and will not be found via dwfl_addrmodule et al if they are not
diff --git a/libdwfl/offline.c b/libdwfl/offline.c
index e9ab0cc1..ca770fac 100644
--- a/libdwfl/offline.c
+++ b/libdwfl/offline.c
@@ -35,6 +35,18 @@
 #include "libdwflP.h"
 #include <fcntl.h>
 
+GElf_Addr
+dwfl_get_offline_next_address (Dwfl *dwfl)
+{
+  return dwfl->offline_next_address;
+}
+
+void
+dwfl_set_offline_next_address (Dwfl *dwfl, GElf_Addr addr)
+{
+  dwfl->offline_next_address = addr;
+}
+
 /* Since dwfl_report_elf lays out the sections already, this will only be
    called when the section headers of the debuginfo file are being
    consulted instead, or for the section placed at 0.  With binutils
diff --git a/tests/dwfl-offline-address.c b/tests/dwfl-offline-address.c
new file mode 100644
index 00000000..9a33b95a
--- /dev/null
+++ b/tests/dwfl-offline-address.c
@@ -0,0 +1,43 @@
+#include <config.h>
+#include <assert.h>
+#include <inttypes.h>
+#include <sys/types.h>
+#include <stdio.h>
+#include <stdio_ext.h>
+#include <stdlib.h>
+#include <string.h>
+#include <locale.h>
+#include <argp.h>
+#include ELFUTILS_HEADER(dwfl)
+#include <dwarf.h>
+#include "system.h"
+
+#define OFFLINE_REDZONE 0x10000
+
+static const Dwfl_Callbacks offline_callbacks =
+  {
+    .find_debuginfo = INTUSE(dwfl_standard_find_debuginfo),
+    .section_address = INTUSE(dwfl_offline_section_address),
+  };
+
+int
+main (int argc, char **argv)
+{
+  Dwfl *dwfl = dwfl_begin (&offline_callbacks);
+  assert (dwfl != NULL);
+
+  if (dwfl_get_offline_next_address (dwfl) != OFFLINE_REDZONE)
+    {
+      dwfl_end (dwfl);
+      return 1;
+    }
+
+  int result = 0;
+  dwfl_set_offline_next_address (dwfl, 0);
+  if (dwfl_get_offline_next_address (dwfl) != 0)
+	  result = 1;
+
+  dwfl_end (dwfl);
+
+  return result;
+}
-- 
2.44.0

Re: [PATCH] Getter and setter for Dwfl's offline_next_address

[Aaron Merey]

Hi Martin,

On Wed, Mar 6, 2024 at 2:23 PM Martin Rodriguez Reboredo
<yakoyoku@gmail.com> wrote:
>
> diff --git a/tests/dwfl-offline-address.c b/tests/dwfl-offline-address.c
> new file mode 100644
> index 00000000..9a33b95a
> --- /dev/null
> +++ b/tests/dwfl-offline-address.c
> @@ -0,0 +1,43 @@
> +#include <config.h>
> +#include <assert.h>
> +#include <inttypes.h>
> +#include <sys/types.h>
> +#include <stdio.h>
> +#include <stdio_ext.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <locale.h>
> +#include <argp.h>
> +#include ELFUTILS_HEADER(dwfl)
> +#include <dwarf.h>
> +#include "system.h"
> +
> +#define OFFLINE_REDZONE 0x10000
> +
> +static const Dwfl_Callbacks offline_callbacks =
> +  {
> +    .find_debuginfo = INTUSE(dwfl_standard_find_debuginfo),
> +    .section_address = INTUSE(dwfl_offline_section_address),
> +  };
> +
> +int
> +main (int argc, char **argv)
> +{
> +  Dwfl *dwfl = dwfl_begin (&offline_callbacks);
> +  assert (dwfl != NULL);
> +
> +  if (dwfl_get_offline_next_address (dwfl) != OFFLINE_REDZONE)
> +    {
> +      dwfl_end (dwfl);
> +      return 1;
> +    }
> +
> +  int result = 0;
> +  dwfl_set_offline_next_address (dwfl, 0);
> +  if (dwfl_get_offline_next_address (dwfl) != 0)
> +         result = 1;
> +
> +  dwfl_end (dwfl);
> +
> +  return result;
> +}

This test needs to be run under a testrun command in a tests/run-*.sh script
in order to have the result recorded by 'make check'.

Also there are a few places in libdwfl/ and src/ where we manually set
dwfl->offline_next_address.  We could use the setter here instead.

Aaron

Re: [PATCH] Getter and setter for Dwfl's offline_next_address

[Mark Wielaard]

Hi Martin,

On Wed, Mar 06, 2024 at 04:22:49PM -0300, Martin Rodriguez Reboredo wrote:
> Added new functions dwfl_get_offline_next_address and
> dwfl_set_offline_next_address which will get plus set said field from
> the Dwfl struct. This is a requirement for listing functions from their
> addresses when using libdwfl offline, otherwise wrong symbols are going
> to be returned.

Could you show an example of when/where you need it and what address
you set it to?

The offline_next_address is only relevant for ET_REL files (like
object files or kernel modules). In general you will need to check the
bias, which various dwfl functions return to know the difference
between the addresses in the ELF, Dwarf or the module load address.

That said, readelf, nm and dwfl_argp do "cheat" by setting the
offline_next_address to zero if they know they are just inspecting a
single object file. So maybe this is a functionality we need to
expose. But I don't fully understand why you need both a getter and a
setter for any arbitrary address.

Thanks,

Mark

Re: [PATCH] Getter and setter for Dwfl's offline_next_address

[Martin Rodriguez Reboredo]

On 3/20/24 19:52, Mark Wielaard wrote:
> Hi Martin,
> 
> On Wed, Mar 06, 2024 at 04:22:49PM -0300, Martin Rodriguez Reboredo wrote:
>> Added new functions dwfl_get_offline_next_address and
>> dwfl_set_offline_next_address which will get plus set said field from
>> the Dwfl struct. This is a requirement for listing functions from their
>> addresses when using libdwfl offline, otherwise wrong symbols are going
>> to be returned.
> 
> Could you show an example of when/where you need it and what address
> you set it to?

For example, this test program reports the name and location pointed by
the passed address.

$ ./report /home/yakoyoku/.debug/.build-id/.../elf 0x0003281f
0x000000000003281f
??
??:0

But due to the mentioned bias both of them are unknown or out of range.

> The offline_next_address is only relevant for ET_REL files (like
> object files or kernel modules). In general you will need to check the
> bias, which various dwfl functions return to know the difference
> between the addresses in the ELF, Dwarf or the module load address.
> 
> That said, readelf, nm and dwfl_argp do "cheat" by setting the
> offline_next_address to zero if they know they are just inspecting a
> single object file. So maybe this is a functionality we need to
> expose. But I don't fully understand why you need both a getter and a
> setter for any arbitrary address.

I've erroneously thought that setting `offline_next_address` was a
requirement to obtain what I've needed. But if I use
`dwfl_module_getelf` or `dwfl_module_getdwarf` I can get the correct
name and location.

0x000000000003281f
_ZNSt11__copy_moveILb0ELb0ESt26random_access_iterator_tagE8__copy_mIPcPhEET0_T_S6_S5_
/usr/include/c++/13.2.1/bits/stl_algobase.h:388,18

So this patch is kinda pointless, but at least I've managed to learn
what I was missing. Anyways, thanks for the heads-up. :)

Re: [PATCH] Getter and setter for Dwfl's offline_next_address

[Mark Wielaard]

Hi Martin,

On Sun, Mar 24, 2024 at 11:11:21AM -0300, Martin Rodriguez Reboredo wrote:
> On 3/20/24 19:52, Mark Wielaard wrote:
> >Could you show an example of when/where you need it and what address
> >you set it to?
> 
> For example, this test program reports the name and location pointed by
> the passed address.
> 
> $ ./report /home/yakoyoku/.debug/.build-id/.../elf 0x0003281f
> 0x000000000003281f
> ??
> ??:0
> 
> But due to the mentioned bias both of them are unknown or out of range.
> 
> >The offline_next_address is only relevant for ET_REL files (like
> >object files or kernel modules). In general you will need to check the
> >bias, which various dwfl functions return to know the difference
> >between the addresses in the ELF, Dwarf or the module load address.
> >
> >That said, readelf, nm and dwfl_argp do "cheat" by setting the
> >offline_next_address to zero if they know they are just inspecting a
> >single object file. So maybe this is a functionality we need to
> >expose. But I don't fully understand why you need both a getter and a
> >setter for any arbitrary address.
> 
> I've erroneously thought that setting `offline_next_address` was a
> requirement to obtain what I've needed. But if I use
> `dwfl_module_getelf` or `dwfl_module_getdwarf` I can get the correct
> name and location.
> 
> 0x000000000003281f
> _ZNSt11__copy_moveILb0ELb0ESt26random_access_iterator_tagE8__copy_mIPcPhEET0_T_S6_S5_
> /usr/include/c++/13.2.1/bits/stl_algobase.h:388,18
> 
> So this patch is kinda pointless, but at least I've managed to learn
> what I was missing. Anyways, thanks for the heads-up. :)

I don't think it was pointless. Clearly our documentation is not very
good (and given eu-readelf and eu-nm do cheat, maybe our interface/api
isn't really good either).

Maybe you could post your code for that ./report program and what you
had to do to get it to print the correct address/symbols. Then we at
least have some documentation for others which hit the same issue.

Thanks,

Mark

Re: [PATCH] Getter and setter for Dwfl's offline_next_address

[Martin Rodriguez Reboredo]

On 3/24/24 14:08, Mark Wielaard wrote:
> Hi Martin,
> 
> On Sun, Mar 24, 2024 at 11:11:21AM -0300, Martin Rodriguez Reboredo wrote:
>> [...]
>> So this patch is kinda pointless, but at least I've managed to learn
>> what I was missing. Anyways, thanks for the heads-up. :)
> 
> I don't think it was pointless. Clearly our documentation is not very
> good (and given eu-readelf and eu-nm do cheat, maybe our interface/api
> isn't really good either).

At least to get the gist of it the header comments were good enough, it
was this particular case that I misunderstood. I could do what I wanted
with the API, so no problem with that.

> Maybe you could post your code for that ./report program and what you
> had to do to get it to print the correct address/symbols. Then we at
> least have some documentation for others which hit the same issue.

     #ifdef USE_DEMANGLE
     #include <cxxabi.h>
     #endif
     #include <elfutils/libdwfl.h>
     #include <dwarf.h>
     #include <inttypes.h>
     #include <stdio.h>
     #include <stdlib.h>

     static const char *symname(const char *name)
     {
     #ifdef USE_DEMANGLE
       // Require GNU v3 ABI by the "_Z" prefix.
       if (name[0] == '_' && name[1] == 'Z') {
         int status = -1;
         char *dsymname = __cxa_demangle(name, demangle_buffer,
                 &demangle_buffer_len, &status);
         if (status == 0)
           name = demangle_buffer = dsymname;
       }
     #endif
       return name;
     }

     static int get_addr_width(Dwfl_Module *mod)
     {
       // Try to find the address width if possible.
       static int width = 0;
       if (width == 0 && mod != NULL) {
         Dwarf_Addr bias;
         Elf *elf = dwfl_module_getelf(mod, &bias);
         if (elf != NULL) {
           GElf_Ehdr ehdr_mem;
           GElf_Ehdr *ehdr = gelf_getehdr(elf, &ehdr_mem);
           if (ehdr != NULL)
             width = ehdr->e_ident[EI_CLASS] == ELFCLASS32 ?
                 8 :
                 16;
         }
       }
       if (width == 0)
         width = 16;

       return width;
     }

     int main(int argc, const char **argv)
     {
       if (argc != 3)
         return 1;

       Dwfl *dwfl = dwfl_begin(&offline_callbacks);
       if (!dwfl)
         return 1;
       //dwfl->offline_next_address = 0; // What I thought it was needed

       if (!dwfl_report_offline(dwfl, "", argv[1], -1)) {
         dwfl_end(dwfl);
         return 1;
       }
       if (dwfl_report_end(dwfl, NULL, NULL)) {
         dwfl_end(dwfl);
         return 1;
       }

       char *endp = NULL;
       GElf_Addr addr = strtoumax(argv[2], &endp, 16), bias;

       Dwfl_Module *mod = dwfl_addrmodule(dwfl, addr);

       int width = get_addr_width(mod);
       printf("0x%.*" PRIx64 "%s", width, addr, "\n");

       dwfl_module_getdwarf(mod, &bias); // This line did the trick!

       const char *name = dwfl_module_addrname(mod, addr + bias);
       name = name != NULL ? symname(name) : "??";
       printf("%s%c", name, '\n');

       GElf_Sym s;
       GElf_Off off = 0;
       name = dwfl_module_addrinfo(mod, addr + bias, &off, &s, NULL, NULL, NULL);

       Dwfl_Line *line = dwfl_module_getsrc(mod, addr + bias);
       if (!line)
         line = dwfl_getsrc(dwfl, addr + bias);
       if (line) {
         int nline, column;
         const char *filename =
           dwfl_lineinfo(line, &addr, &nline, &column, NULL, NULL);
         printf("%s:%i,%i\n", filename, nline, column);
       } else {
         printf("??:0\n");
       }

       dwfl_end(dwfl);

       return 0;
     }

> Thanks,
> 
> Mark