Re: [PATCH v2 1/3] btf_encoder: skip functions consuming packed structs passed by value on stack

["Alexis Lothoré" <alexis.lothore@bootlin.com>]

Hello Ihor,

thanks for the prompt feedback and testing !

On Thu Jul 3, 2025 at 8:17 PM CEST, Ihor Solodrai wrote:
> On 7/3/25 2:02 AM, Alexis Lothoré (eBPF Foundation) wrote:

[...]

>>   		/* do not exclude functions with optimized-out parameters; they
>>   		 * may still be _called_ with the right parameter values, they
>>   		 * just do not _use_ them.  Only exclude functions with
>> -		 * unexpected register use or multiple inconsistent prototypes.
>> +		 * unexpected register use, multiple inconsistent prototypes or
>> +		 * uncertain parameters location
>>   		 */
>> -		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto;
>> +		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto && !state->uncertain_parm_loc;
>
>
> Is it possible for a function to have uncertain_parm_loc in one CU,
> but not in another?
>
> If yes, we still don't want the function in BTF, right?

TBH, my understanding about those discrepancies between CUs about the same
functions and how pahole handle them is still a bit fragile. Have you got
any example about how it could be the case ?

If it _can_ happen, I guess you are suggesting to make sure that copies are
compared in saved_functions_combine and their uncertain_loc_parm flag are
aligned. Something like this:

uncertain_parm_loc = a->uncertain_parm_loc | b->uncertain_parm_loc;
[...]
a->uncertain_parm_loc = b->uncertain_parm_loc = uncertain_parm_loc;

>> @@ -2693,6 +2736,9 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>>   		if (!func)
>>   			continue;
>>   
>> +		if (ftype__has_uncertain_arg_loc(cu, &fn->proto))
>> +			fn->proto.uncertain_parm_loc = 1;
>> +
>>   		err = btf_encoder__save_func(encoder, fn, func);
>
> I think checking and setting uncertain_parm_loc flag should be done
> inside btf_encoder__save_func(), because that's where we inspect DWARF
> function prototype and add a new btf_encoder_func_state.

ACK, it can be moved there

Thanks,

Alexis

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com