[PATCH] Setter for Dwfl's offline_next_address

[PATCH] Setter for Dwfl's offline_next_address

[Martin Rodriguez Reboredo]

Added a new function dwfl_set_offline_next_addres which will 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>
---
 libdwfl/libdwfl.h | 3 +++
 libdwfl/offline.c | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/libdwfl/libdwfl.h b/libdwfl/libdwfl.h
index 49ad6664..0ee12b58 100644
--- a/libdwfl/libdwfl.h
+++ b/libdwfl/libdwfl.h
@@ -109,6 +109,9 @@ extern int dwfl_errno (void);
 extern const char *dwfl_errmsg (int err);
 
 
+/* 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..f65486d3 100644
--- a/libdwfl/offline.c
+++ b/libdwfl/offline.c
@@ -35,6 +35,12 @@
 #include "libdwflP.h"
 #include <fcntl.h>
 
+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
-- 
2.44.0

Re: [PATCH] Setter for Dwfl's offline_next_address

[Mark Wielaard]

Hi Martin,

On Fri, Mar 01, 2024 at 05:04:05PM -0300, Martin Rodriguez Reboredo wrote:
> Added a new function dwfl_set_offline_next_addres which will 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 give an example or testcase for this?

Thanks,

Mark

Re: [PATCH] Setter for Dwfl's offline_next_address

[Martin Rodriguez Reboredo]

Oh hi Mark!

On 3/2/24 17:47, Mark Wielaard wrote:
> Hi Martin,
> 
> On Fri, Mar 01, 2024 at 05:04:05PM -0300, Martin Rodriguez Reboredo wrote:
>> Added a new function dwfl_set_offline_next_addres which will 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 give an example or testcase for this?

This is intended for the Linux kernel perf tool so you might see it in
action when I publish the changes. In regards to testing I thought that
it was not needed due to the patch being a simple setter, but as
requested I can think something in the lines of.

     int
     main (int argc, char **argv)
     {
       Dwfl *dwfl = dwfl_begin (&offline_callbacks);
       assert (dwfl != NULL);

       if (dwfl->offline_next_address != OFFLINE_REDZONE)
         {
           dwfl_end (dwfl);
           return 1;
         }

       int result = 0;
       dwfl_set_offline_next_address (dwfl, 0);
       if (dwfl->offline_next_address != 0)
               result = 1;

       dwfl_end (dwfl);

       return result;
     }

But this will require libdwflP.h to be included, maybe if I add a getter
too it'd remedy it. Thoughts?

> 
> Thanks,
> 
> Mark

Re: [PATCH] Setter for Dwfl's offline_next_address

[Mark Wielaard]

Hi Martin,

On Sat, Mar 02, 2024 at 07:43:38PM -0300, Martin Rodriguez Reboredo wrote:
> On 3/2/24 17:47, Mark Wielaard wrote:
> >On Fri, Mar 01, 2024 at 05:04:05PM -0300, Martin Rodriguez Reboredo wrote:
> >>Added a new function dwfl_set_offline_next_addres which will 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 give an example or testcase for this?
> 
> This is intended for the Linux kernel perf tool so you might see it in
> action when I publish the changes. In regards to testing I thought that
> it was not needed due to the patch being a simple setter, but as
> requested I can think something in the lines of.

It would be interesting to see the perf tool patch. I don't understand
the use case. So I assume perf currently does something which is wrong
and with your patch calling this new dwfl_set_offline_next_addres it
will do the right thing. That is what I was thinking of when asking
for an example or testcase. I agree that on itself such a simple
setter doesn't need a dedicated testcase. But maybe we can come up
with a testcase given the right context.

Cheers,

Mark

Re: [PATCH] Setter for Dwfl's offline_next_address

[Martin Rodriguez Reboredo]

On 3/2/24 21:05, Mark Wielaard wrote:
> Hi Martin,
> 
> [...]
> 
> It would be interesting to see the perf tool patch. I don't understand
> the use case. So I assume perf currently does something which is wrong
> and with your patch calling this new dwfl_set_offline_next_addres it
> will do the right thing. That is what I was thinking of when asking
> for an example or testcase. I agree that on itself such a simple
> setter doesn't need a dedicated testcase. But maybe we can come up
> with a testcase given the right context.

Erm, I think I have a more clear example. When I use libdw to do
something like addr2line with an ELF file I fail to get the address
source line. Then I saw that eu-addr2line sets offline_next_address to
zero. The following is the program I've used to see the source info of
an address.

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

       Dwfl *dwfl = dwfl_begin(&offline_callbacks);
       if (!dwfl)
         return 1;
       dwfl_set_offline_next_address(dwfl, 0);

       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);

       Dwfl_Module *mod = dwfl_addrmodule(dwfl, addr);

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

       GElf_Sym s;
       GElf_Off off = 0;
       const char *name =
         dwfl_module_addrinfo(mod, addr, &off, &s, NULL, NULL, NULL);

       Dwfl_Line *line = dwfl_module_getsrc(mod, addr);
       if (!line)
         line = dwfl_getsrc(dwfl, addr);
       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;
     }

It could print the symbol name but that would've made the program
much longer but I think that this should be clear now.

> 
> Cheers,
> 
> Mark