Re: [PATCH 2/2] btf_encoder: Fix function generation

[Jiri Olsa <jolsa@redhat.com>]

On Fri, Nov 13, 2020 at 02:28:32PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 13, 2020 at 04:12:22PM +0100, Jiri Olsa escreveu:
> > Current conditions for picking up function records break
> > BTF data on some gcc versions.
> > 
> > Some function records can appear with no arguments but with
> > declaration tag set, so moving the 'fn->declaration' in front
> > of other checks.
> > 
> > Then checking if argument names are present and finally checking
> > ftrace filter if it's present. If ftrace filter is not available,
> > using the external tag to filter out non external functions.
> 
> Humm has_arg_names() will return true for a:
> 
> void foo(void)
> 
> function, which I think is right, but can't this function appear
> multiple times in different CUs and we end up with the same function
> multiple times in the BTF info?

so declarations should be filtered by the fn->declaration
check, if gcc has it ;-)

and if it goes through, the should_generate_function sets/checks
flag if the function was already generated

jirka

> 
> - Arnaldo
>  
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  btf_encoder.c | 24 ++++++++++--------------
> >  1 file changed, 10 insertions(+), 14 deletions(-)
> > 
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index d531651b1e9e..de471bc754b1 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -612,25 +612,21 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
> >  		const char *name;
> >  
> >  		/*
> > -		 * The functions_cnt != 0 means we parsed all necessary
> > -		 * kernel symbols and we are using ftrace location filter
> > -		 * for functions. If it's not available keep the current
> > -		 * dwarf declaration check.
> > +		 * Skip functions that:
> > +		 *   - are marked as declarations
> > +		 *   - do not have full argument names
> > +		 *   - are not in ftrace list (if it's available)
> > +		 *   - are not external (in case ftrace filter is not available)
> >  		 */
> > +		if (fn->declaration)
> > +			continue;
> > +		if (!has_arg_names(cu, &fn->proto))
> > +			continue;
> >  		if (functions_cnt) {
> > -			/*
> > -			 * We check following conditions:
> > -			 *   - argument names are defined
> > -			 *   - there's symbol and address defined for the function
> > -			 *   - function address belongs to ftrace locations
> > -			 *   - function is generated only once
> > -			 */
> > -			if (!has_arg_names(cu, &fn->proto))
> > -				continue;
> >  			if (!should_generate_function(btfe, function__name(fn, cu)))
> >  				continue;
> >  		} else {
> > -			if (fn->declaration || !fn->external)
> > +			if (!fn->external)
> >  				continue;
> >  		}
> >  
> > -- 
> > 2.26.2
> > 
> 
> -- 
> 
> - Arnaldo
>