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

[Yonghong Song <yhs@fb.com>]

On 11/12/20 4:30 PM, Andrii Nakryiko wrote:
> On Thu, Nov 12, 2020 at 4:19 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Thu, Nov 12, 2020 at 4:08 PM Andrii Nakryiko
>> <andrii.nakryiko@gmail.com> 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.

I looked at llvm source code, llvm only tries to assign DW_INL_inlined
and also only at certain conditions. Not sure about gcc. Probably 
similar. So this field is not reliable, esp. without it does not mean it 
is not inlined.

> 
> 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...
> :)
>