Re: [PATCH RFC 0/3] list inline expansions in .BTF.inline

Alexei Starovoitov
2025-04-28T20:51:58Z

On Fri, Apr 25, 2025 at 11:41 AM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> > We propose to re-encode DWARF location expressions into a custom BTF
> > location expression format. It operates on a stack of values, similar to
> > DWARF's location expressions, but is stripped of unused operators,
> > while allowing future expansions.
>
> A stack machine seems overkill. I'm certainly not an expert on DWARF
> location expressions, but I think we need to get away from arbitrarily
> complex expressions, even if they are simpler than DWARF ones. I don't
> think we want consumers implementing any kind of interpreter or VM.

+1
This was already brought up at lsfmm.
stack machine in BTF is not an option.
I see the appeal of simple inline_encoder__encode_location() logic
that takes dwarf VM and re-encodes things pretty much as-is.
This is a wrong trade-off.
pahole side needs to do full expression analysis and emit
easy to parse location expression.

> > ID | Operator Name | Operands[...]
> > ----+----------------------+-------------------------------------------
> > 0 | LOC_END_OF_EXPR | _none_
> > 1 | LOC_SIGNED_CONST_1 | s8: constant's value
> > 2 | LOC_SIGNED_CONST_2 | s16: constant's value
> > 3 | LOC_SIGNED_CONST_4 | s32: constant's value
> > 4 | LOC_SIGNED_CONST_8 | s64: constant's value
> > 5 | LOC_UNSIGNED_CONST_1 | u8: constant's value
> > 6 | LOC_UNSIGNED_CONST_2 | u16: constant's value
> > 7 | LOC_UNSIGNED_CONST_4 | u32: constant's value
> > 8 | LOC_UNSIGNED_CONST_8 | u64: constant's value
> > 9 | LOC_REGISTER | u8: DWARF register number from the ABI
> > 10 | LOC_REGISTER_OFFSET | u8: DWARF register number from the ABI
> > | s64: offset added to the register's value
> > 11 | LOC_DEREF | u8: size of the deref'd type

LOC_END_OF_EXPR shouldn't be there. This is just an artifact of stack VM.
I don't see patch 3 using LOC_DEREF.
register vs register_offset is another artifact of dwarf.
It looks to me we only need:
- register
- load_from_reg_plus_offset
- constant

>
> I think supporting fully inlined functions as part of this work would
> add a lot of value to users. It doesn't necessarily have to happen in
> the first patchset, but we probably want to plan on doing it.
>
> Regarding BTF, another option is to just leave `callee_id` unset, right?
> Consumers should be able to recognize BTF for the inlined function isn't
> available and then act accordingly. For bpftrace, that probably means
> not allowing function argument access.

For fully inlined we still need callee_id otherwise bpftrace won't know
argument types. It shouldn't be hard to emit btf even for fully
inlined functions, but we need to be smart not to bloat BTF to dwarf sizes.