RE: [PATCH] perf/probe: Search both .eh_frame and .debug_frame sections for probe location

[åæéå / HIRAMATUïMASAMI]

From: Mark Wielaard [mailto:mjw@xxxxxxxxxx]
>
>Hi Hemant,
>
>On Thu, 2015-09-24 at 07:46 +0530, Hemant Kumar wrote:
>> perf probe through debuginfo__find_probes() in util/probe-finder.c
>> checks for the functions' frame descriptions in either .eh_frame section
>> of an ELF or the .debug_frame. The check is based on whether either one
>> of these sections is present. But sometimes, it may happen that,
>> .eh_frame, even if present, may not be complete and may miss some
>> descriptions.
>
>Right. Depending on distro, toolchain defaults, arch, build flags, etc.
>CFI might be found in either .eh_frame and/or .debug_frame. To be sure
>you find the CFI covering an address you will always have to investigate
>both if available.

OK, I didn't care about that.

>
>I am not too familiar with the code so there might be a reason for
>setting and reusing the pf->cfi to do the search twice. But might it not
>be more clear to just store both pf->cfi_eh and pf->cfi_debug and then
>check both in call_probe_finder () with the dwarf_cfi_addrframe () call?
>Which is the only place I see actually using the cfi.

Right, but since call_probe_finder can be called repeatedly on same binary,
we should keep pf->cfi for caching CFI too.

>BTW. Not really related to this patch since the following was already in
>the code, and is most likely always correct anyway:
>
>> +	if (elf_section_by_name(elf, &ehdr, &shdr, ".eh_frame", NULL) &&
>> +	    shdr.sh_type == SHT_PROGBITS) {
>> +		pf->cfi = dwarf_getcfi_elf(elf);
>
>But that SHT_PROGBITS check is only necessary because of a bug in
>elfutils < 0.156. For 0.156+ dwarf_getcfi_elf () will properly return
>NULL in case you happen to be looking at a separate debug file that
>has .eh_frame as NOBITS. In theory this prevents getting the CFI if the
>file has stripped away the shdrs. Which is reasonable, there are
>probably also other things that rely on the shdrs.

Ah, I had just wanted to avoid introducing new ifdefs.

> But dwarf_getcfi_elf
>is able to also get you the CFI with just the phdrs.

Hmm, how can I make such binary? I can fix it, but we need a
testcase for that.

Thanks!

>
>Cheers,
>
>Mark
N‹§²æ¸›yú²X¬¶ÇvØ–)Þ{.nlj·¥Š{±‘êX§¶›¡Ü}©ž²ÆzÚj:+v‰¨¾«‘êZ+€Êzf£¢·hšˆ§~†­†Ûÿû®w¥¢¸?™¨è&¢)ßf”ùy§m…á«a¶Úÿ0¶ìå