Re: [RFC/PATCH 3/3] btf_encoder: Func generation fix

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 12, 2020 at 4:19 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Thu, Nov 12, 2020 at 4:08 PM Andrii Nakryiko
> <andrii.nakryiko@xxxxxxxxx> wrote:
> >
> > So I looked at your vmlinux image. I think we should just keep
> > everything mostly as it it right now (without changes in this patch),
> > but add just two simple checks:
> >
> > 1. Skip if fn->declaration (ignore correctly marked func declarations)
> > 2. Skip if DW_AT_inline: 1 (ignore inlined functions).
> >
> > I'd keep the named arguments check as is, I think it's helpful. 1)
> > will skip stuff that's explicitly marked as declaration. 2) inline
> > check will partially mitigate dropping of fn->external check (and we
> > can't really attach to inlined functions).
>
> I thought DW_AT_inline is an indication that the function was marked "inline"
> in C code. That doesn't mean that the function was actually inlined.
> So I don't think pahole should check that bit.

According to DWARF spec, there are 4 possible values:

DW_INL_not_inlined = 0            Not declared inline nor inlined by
the compiler
DW_INL_inlined = 1                Not declared inline but inlined by
the compiler
DW_INL_declared_not_inlined = 2   Declared inline but not inlined by
the compiler
DW_INL_declared_inlined = 3       Declared inline and inlined by the compiler

So DW_INL_inlined is supposed to be added to functions that are not
marked inline, but were nevertheless inlined. I saw this for one of
vfs_getattr entries in DWARF, which clearly is not marked inline.

But also that DWARF entry had proper args with names, so it would work
fine as well. I don't know, with DWARF it's always some guessing game.
Let's leave DW_AT_inline alone for now.

Important part is skipping declarations (when they are marked as
such), though I'm not claiming it will solve the problem completely...
:)



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux