This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Thanks Mark,

Please find my comments.

On Monday 22 February 2016 07:15 PM, Mark Wielaard wrote:
Hi Ravi,

On Sat, 2016-02-20 at 19:10 +0530, Ravi Bangoria wrote:
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:
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.
Ubuntu 15.10 wily. 4.2.0-27-generic #32-Ubuntu SMP ppc64le
elfutils-0.163-4ubuntu1 stap from systemtap git.

Yes you are right. I checked it. stap is not working on system with
above configurations.


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.
Great, thanks for testing.

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.
Interesting find. And I think you are right.

In our case on ubuntu ppc64le, LINE A would find the vmlinux image
already and we would never reach LINE B or C. The separate debuginfo
would then be found, through dwfl_standard_find_debuginfo, when we want
to get the DWARF for the kernel.

But on other arches where the main image isn't called vmlinux we would
indeed hit them. When try_debug == true then we only need to do C now,
otherwise we only need to do B.

Updated patch attached. This time with updated commit message and
ChangeLog entry. Does this look correct to you?

This patch looks fine to me. I've tested it on Ubuntu. I think we can go ahead
with this.

Regards,
Ravi


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]