Re: [patch] Fix report_r_debug for prelinked libraries

[Mark Wielaard <mark@klomp.org>]

[-- Attachment #1: Type: text/plain, Size: 2250 bytes --]

On Thu, Jul 24, 2014 at 08:50:28PM +0200, Jan Kratochvil wrote:
> eu-stack: "Callback returned failure" for seemingly OK shared libraries
> https://bugzilla.redhat.com/show_bug.cgi?id=1112610#c2
> 
> attaching the fix.  I find it obviously right.
> It also has no testsuite regressions.
> 
> The problem was that user dumping the core file had DSOs prelinked but
> downloaded DSOs from RPMs are not prelinked.  l_addr is then zero but it
> cannot be determined from the DSO file.

Right. I admit it wasn't immediately obvious to me. It is obvious once
you realize that l_addr is the difference between runtime addresses and
ELF file at the time of core dump. But the link_map code might use a
differently pre-linked version of that ELF file. And you can recalculate
that difference by using l_ld from the core file and elf_dynamic_vaddr
from the current ELF file phdrs, which both point to the dynamic section.

> Unfortunately I do not provide a testcase.  It fixes the Bug above as tested
> on its core file but I cannot disclose the core file (or its parts).
> And despite I did try I haven't reproduced it by a hand-made testcase.
>
> The problem is the modules placing code got overcomplicated and IMO it could
> be simplified one day.  There is the former placement which works somehow and
> there is the placement by patches of mine which force it here and there to the
> right positions.  Trying to exploit failure of the add-on code of mine always
> ended up on the former code placing it right on its own.

Thanks for trying. I am convinced of the solution even without a testcase.

Just one nitpick with the following comment:
>  
> -      GElf_Addr l_addr = addrs[0];
> +      // unused:
> +      // GElf_Addr l_addr = addrs[0];
>        GElf_Addr l_name = addrs[1];
>        GElf_Addr l_ld = addrs[2];

Could you expand that comment a little to say why it is unused and why we
need to recalculate it? Maybe "Unused: l_addr is the difference between
the address in memory and the ELF file when the core was created. We need
to recalculate the difference below because the ELF file we use might be
differently pre-linked." Or something similar.

OK with an expanded comment.

Thanks,

Mark