Re: [RFC PATCH 2/9] btf_encoder: store,use section-relative addresses in ELF function representation

[Alan Maguire <alan.maguire@oracle.com>]

On 29/11/2024 09:07, Eduard Zingerman wrote:
> On Thu, 2024-11-28 at 01:23 +0000, Ihor Solodrai wrote:
>> From: Alan Maguire <alan.maguire@oracle.com>
>>
>> This will help us do more accurate DWARF/ELF matching.
> 
> It would be good to have a more detailed explanation here.
> E.g. number of generated functions differs with this patch:
> 
> # without this patch
> $ bpftool btf dump file /home/eddy/work/tmp/old.btf | grep "\] FUNC '" | wc -l
> 48056
> # with this patch
> $ bpftool btf dump file /home/eddy/work/tmp/new.btf | grep "\] FUNC '" | wc -l
> 48189
> 
> It would be helpful to peek one of newly added functions and explain
> why it was previously excluded.
>

I've also found the opposite situation; that some functions are not
present after this patch that were before. In your case - where more
were matched - I suspect the extra address hint allowed us to do a
better job of matching ELF and DWARF such that we matched .isra.0
functions that represent actual function boundaries rather than .cold
subfunctions that had different signatures. I'm working on a respin of
the initial patches that ensures we get the same (or more) number of
functions, but it shouldn't change substantially anything that follows
that Ihor has done.

>>
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>> Signed-off-by: Ihor Solodrai <ihor.solodrai@pm.me>
>> ---
>>  btf_encoder.c | 37 +++++++++++++++++++++++++++++++------
>>  1 file changed, 31 insertions(+), 6 deletions(-)
>>
>> diff --git a/btf_encoder.c b/btf_encoder.c
>> index 98e4d7d..01d7094 100644
>> --- a/btf_encoder.c
>> +++ b/btf_encoder.c
>> @@ -88,6 +88,7 @@ struct btf_encoder_func_state {
>>  struct elf_function {
>>  	const char	*name;
>>  	char		*alias;
>> +	uint32_t	addr;
>>  	size_t		prefixlen;
>>  	struct btf_encoder_func_state state;
>>  };
>> @@ -131,6 +132,7 @@ struct btf_encoder {
>>  		int		    allocated;
>>  		int		    cnt;
>>  		int		    suffix_cnt; /* number of .isra, .part etc */
>> +		uint64_t	    base_addr;
> 
> This field is set, but never read.
> 
>>  	} functions;
>>  };
>>  
> 
> [...]
>