Re: [PATCH dwarves 2/4] btf_encoder: Refactor elf_functions__new() with struct btf_encoder as argument

On 1/13/26 10:32 AM, Ihor Solodrai wrote:
> On 1/13/26 5:13 AM, Alan Maguire wrote:
>> From: Yonghong Song <[email protected]>
>>
>> For elf_functions__new(), replace original argument 'Elf *elf' with
>> 'struct btf_encoder *encoder' for future use.
>>
>> Signed-off-by: Yonghong Song <[email protected]>
>> ---
>>   btf_encoder.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/btf_encoder.c b/btf_encoder.c
>> index 2c3cef9..5bc61cb 100644
>> --- a/btf_encoder.c
>> +++ b/btf_encoder.c
>> @@ -187,11 +187,13 @@ static inline void elf_functions__delete(struct elf_functions *funcs)
>>   
>>   static int elf_functions__collect(struct elf_functions *functions);
>>   
>> -struct elf_functions *elf_functions__new(Elf *elf)
>> +struct elf_functions *elf_functions__new(struct btf_encoder *encoder)
> Hi Alan, Yonghong,
>
> I assume "future use" refers to this patch:
> https://lore.kernel.org/dwarves/[email protected]/
>
> Do I understand correctly that you're passing btf_encoder here in
> order to detect that the `encoder->dotted_true_signature` feature flag
> is set? If so, I think this is a bit of an overkill.
>
> How about just store the flag in struct elf_functions, pass it to the
> elf_functions__new() directly and set it there:
>
> 	funcs->elf = elf;
> 	funcs->dotted_true_signature = dotted_true_signature; // <--
> 	err = elf_functions__collect(funcs);
> 	if (err < 0)
> 		goto out_delete;
>
> And even then, it doesn't feel right to me that the contents of the
> *ELF* functions table changes based on a feature flag. But we are
> discarding the suffixes currently, so I understand why this was done.
>
> Taking a step back, I remember Yonghong mentioned some pushback both
> from LLVM and DWARF side regarding the introduction of true signatures
> to DWARF data. Is there a feasible path forward landing all that?

Yes. My previous dwarf format (https://github.com/llvm/llvm-project/pull/165310)
gets resistance from llvm esp. dwarf community.

There are two possible solutions going forward:
   1. use existing dwarf data (e.g. locations, etc) to extract true signatures.
   2. generate vmlinux BTF directly from compiler and make sure true signatures
      are encoded in that BTF. Currently gcc is able to generate BTF but
      do not have changed signatures.

The second approach is more complicated so I prefer to try option 1 first.
With option 1, I think pahole already has lots of checking for various
inconsistency or ambiguity. But llvm generated dwarf may have some
difference from gcc generated dwarf. For example, for function
__blkcg_rstat_flush() in patch 1, gcc has abstract origin for that function,
but clang does not have it in dwarf. I will need to sort out
these things.

>
> I haven't followed this work in detail, so apologies if I missed
> anything. Just want to have a high-level understanding of the
> situation.
>
> Thank you!
>
>
>>   {
>>   	struct elf_functions *funcs;
>> +	Elf *elf;
>>   	int err;
>>   
>> +	elf = encoder->cu->elf;
>>   	funcs = calloc(1, sizeof(*funcs));
>>   	if (!funcs) {
>>   		err = -ENOMEM;
>> @@ -1552,7 +1554,7 @@ static struct elf_functions *btf_encoder__elf_functions(struct btf_encoder *enco
>>   
>>   	funcs = elf_functions__find(encoder->cu->elf, &encoder->elf_functions_list);
>>   	if (!funcs) {
>> -		funcs = elf_functions__new(encoder->cu->elf);
>> +		funcs = elf_functions__new(encoder);
>>   		if (funcs)
>>   			list_add(&funcs->node, &encoder->elf_functions_list);
>>   	}