Re: [RFC] elfutils: Checks for debuginfo file without .debug extension as well

[Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>]

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

Hi Mark,

Thanks for the patch. Indeed this is optimized one compared to what I've
proposed.

On Friday 19 February 2016 08:41 PM, Mark Wielaard wrote:
> Hi Ravi,
>
> On Fri, 2016-02-19 at 15:07 +0530, Ravi Bangoria wrote:
>> Thanks for the sample program. I can see it tries to simulate what
>> systemtap do.
>> Here is the output on ubunut powerpc.
>>
>> Without patch:
>>
>>     # ./dwfl_find_kernel
>>     Finding kernel release: 3.13.0-76-generic
>>     Debuginfo path: +:.debug:/usr/lib/debug
>>     setup report modname: kernel, filename: /boot/vmlinux-3.13.0-76-generic
>>     setup report modname: xfrm_user, filename:
>> /lib/modules/3.13.0-76-generic/kernel/net/xfrm/xfrm_user.ko
>>     Found kernel file: /boot/vmlinux-3.13.0-76-generic
>>     getdwarf name: kernel
>>     kernel without DWARF
>>     Couldn't get kernel DWARF
>>
>> With patch:
>>
>>     # ./dwfl_find_kernel
>>     Finding kernel release: 3.13.0-76-generic
>>     Debuginfo path: +:.debug:/usr/lib/debug
>>     setup report modname: kernel, filename: /boot/vmlinux-3.13.0-76-generic
>>     setup report modname: xfrm_user, filename:
>> /lib/modules/3.13.0-76-generic/kernel/net/xfrm/xfrm_user.ko
>>     Found kernel file: /boot/vmlinux-3.13.0-76-generic
>>     getdwarf name: kernel
>>     mainfile: /boot/vmlinux-3.13.0-76-generic, debugfile:
>> /usr/lib/debug/boot/vmlinux-3.13.0-76-generic
>>     Found kernel DWARF file: /usr/lib/debug/boot/vmlinux-3.13.0-76-generic
> Thanks, that looks like what I expected.
> I also got access to a somewhat newer version of ubuntu ppc64le and that
> has an additional issue. It has the vmlinux file installed unreadable
> (except for root). That causes almost the same issue, but slightly
> differently, now no kernel at all is found... Although in that case it
> can be worked around be using /usr/lib/debug as base. But the main issue
> is exactly as you describe.

Can you please provide me system configurations like ubuntu versrion,
kernel version, stap version, elfutils version etc. I'll check if I'll 
be able
to get similar system.

>>> It still doesn't show the full search path (for that we should hack
>>> find_debuginfo_in_path and try_open in libdwfl/find-debuginfo.c a bit),
>>> but it should be a start to better understand why the current search
>>> isn't finding the separate kernel debug file.
>> Sry, I'm little bit confused in your last two paragraphs.
> That is probably because I was still a little confused myself :)
>
>> 1. Does this means change I've proposed is at wrong place i.e.
>> find_debuginfo
> Not wrong. It correctly finds the kernel. But it could be done a little
> more efficient I believe. With your change we do the search through all
> possible directories (and subdirs) twice. We could do it slightly more
> efficient by moving the logic into find_debuginfo_in_path itself.
>
> The issue is really that if find_debuginfo_in_path is passed in a "null"
> debuglink_name then it invents one (basename + .debug). But if it is
> "null" then that is really just one guess. We should as you indicate
> also try the basename itself. So that is what the attached patch does,
> it checks if debuglink_name is NULL and if we are checking in a debug
> directory (or subdirectory), but not the main file location, then we try
> both basename.debug and basename.
>
> That should do the same thing as your suggested patch, but makes it
> slightly more efficient. We try less directories/files. And if the
> kernel image exists we should find it earlier.
>
> Could you try out if this variant of the patch works for you?

I've tested your patch and it's working fine with Ubuntu 14.04 with 
kernel 3.13,
stap 2.3 and elfutils 0.165.

But I've a little doubt here. Here is the code snip of try_kernel_name
from libdwfl/linux-kernel-modules.c

     static int
     try_kernel_name (Dwfl *dwfl, char **fname, bool try_debug)
     {
LINE A:
       int fd = ((((dwfl->callbacks->debuginfo_path
                    ? *dwfl->callbacks->debuginfo_path : NULL)
                   ?: DEFAULT_DEBUGINFO_PATH)[0] == ':') ? -1
                 : TEMP_FAILURE_RETRY (open64 (*fname, O_RDONLY)));

       if (fd < 0)
         {
LINE B:
           /* look for "vmlinux" files.  */
           fd = INTUSE(dwfl_standard_find_debuginfo) (&fakemod, NULL, 
NULL, 0,
                                                      *fname, basename 
(*fname), 0,
&fakemod.debug.name);
           if (fd < 0 && try_debug)
LINE C:
             /* look for "vmlinux.debug" files.  */
             fd = INTUSE(dwfl_standard_find_debuginfo) (&fakemod, NULL, 
NULL, 0,
                                                        *fname, NULL, 0,
&fakemod.debug.name);

try_kernel_name is doing almost same thing what I have proposed. Now let's
say we want to go ahead with your patch, than call to 
dwfl_standard_find_debuginfo
in LINE C will look for both vmlinux and vmlinux.debug right? But it has 
already
checked for vmlinux in LINE B. So, in this case we have to modify 
try_kernel_name
as well.

Please correct me if I'm wrong.

Regards,
Ravi