[PATCH dwarves] btf_encoder: filter var names before btf name check

[PATCH dwarves] btf_encoder: filter var names before btf name check

[Alan Maguire]

...as this means we avoid seeing multiple messages of the form

PAHOLE: Warning: Found invalid variable name when encoding btf, ignored (sym: '__UNIQUE_ID___addressable__RNvXsa_NtCs2vSfyxKMEZc_4core4charNtB5_11EscapeDebugNtNtNtNtB7_4iter6traits10exact_size17ExactSizeIterator3len57').

Such symbols are filtered anyway due to the __UNIQUE_ID prefix, so
warning about them is not necessary; simply reordering the checks
should ensure we will not see these warnings.

Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 btf_encoder.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 5954238..e507e60 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -2241,6 +2241,9 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 		if (!name)
 			continue;
 
+		if (filter_variable_name(name))
+			continue;
+
 		/* Check for invalid BTF names */
 		if (!btf_name_valid(name)) {
 			dump_invalid_symbol("Found invalid variable name when encoding btf",
@@ -2251,9 +2254,6 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
 				return -1;
 		}
 
-		if (filter_variable_name(name))
-			continue;
-
 		if (var->ip.tag.type == 0) {
 			fprintf(stderr, "error: found variable '%s' in CU '%s' that has void type\n",
 				name, cu->name);
-- 
2.43.5

Re: [PATCH dwarves] btf_encoder: filter var names before btf name check

[Miguel Ojeda]

On Fri, Oct 18, 2024 at 11:43 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> ...as this means we avoid seeing multiple messages of the form
>
> PAHOLE: Warning: Found invalid variable name when encoding btf, ignored (sym: '__UNIQUE_ID___addressable__RNvXsa_NtCs2vSfyxKMEZc_4core4charNtB5_11EscapeDebugNtNtNtNtB7_4iter6traits10exact_size17ExactSizeIterator3len57').
>
> Such symbols are filtered anyway due to the __UNIQUE_ID prefix, so
> warning about them is not necessary; simply reordering the checks
> should ensure we will not see these warnings.
>
> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

FWIW, I applied the patch and the `__UNIQUE_ID` warnings indeed go
away (but I don't know if those warnings would be useful to see/fix
for other reasons):

Tested-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

Re: [PATCH dwarves] btf_encoder: filter var names before btf name check

[Arnaldo Carvalho de Melo]

On Tue, Oct 22, 2024 at 08:21:09PM +0200, Miguel Ojeda wrote:
> On Fri, Oct 18, 2024 at 11:43 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > ...as this means we avoid seeing multiple messages of the form
> >
> > PAHOLE: Warning: Found invalid variable name when encoding btf, ignored (sym: '__UNIQUE_ID___addressable__RNvXsa_NtCs2vSfyxKMEZc_4core4charNtB5_11EscapeDebugNtNtNtNtB7_4iter6traits10exact_size17ExactSizeIterator3len57').
> >
> > Such symbols are filtered anyway due to the __UNIQUE_ID prefix, so
> > warning about them is not necessary; simply reordering the checks
> > should ensure we will not see these warnings.
> >
> > Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> 
> FWIW, I applied the patch and the `__UNIQUE_ID` warnings indeed go
> away (but I don't know if those warnings would be useful to see/fix
> for other reasons):

Maybe we should have a debug mode where anything that is skipped gets
reported.
 
> Tested-by: Miguel Ojeda <ojeda@kernel.org>

Thanks, since this hasn't hit 'master' yet, I added it to the cset,

- Arnaldo

Re: [PATCH dwarves] btf_encoder: filter var names before btf name check

[Stephen Brennan]

Alan Maguire <alan.maguire@oracle.com> writes:
> ...as this means we avoid seeing multiple messages of the form
>
> PAHOLE: Warning: Found invalid variable name when encoding btf, ignored (sym: '__UNIQUE_ID___addressable__RNvXsa_NtCs2vSfyxKMEZc_4core4charNtB5_11EscapeDebugNtNtNtNtB7_4iter6traits10exact_size17ExactSizeIterator3len57').

a beautiful variable name :P

> Such symbols are filtered anyway due to the __UNIQUE_ID prefix, so
> warning about them is not necessary; simply reordering the checks
> should ensure we will not see these warnings.
>
> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

Sorry I'm late to review. This is a good catch and is clearly correct
to me.

Reviewed-by: Stephen Brennan <stephen.s.brennan@oracle.com>

> ---
>  btf_encoder.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 5954238..e507e60 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -2241,6 +2241,9 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
>  		if (!name)
>  			continue;
>  
> +		if (filter_variable_name(name))
> +			continue;
> +
>  		/* Check for invalid BTF names */
>  		if (!btf_name_valid(name)) {
>  			dump_invalid_symbol("Found invalid variable name when encoding btf",
> @@ -2251,9 +2254,6 @@ static int btf_encoder__encode_cu_variables(struct btf_encoder *encoder)
>  				return -1;
>  		}
>  
> -		if (filter_variable_name(name))
> -			continue;
> -
>  		if (var->ip.tag.type == 0) {
>  			fprintf(stderr, "error: found variable '%s' in CU '%s' that has void type\n",
>  				name, cu->name);
> -- 
> 2.43.5

Re: [PATCH dwarves] btf_encoder: filter var names before btf name check

[Arnaldo Carvalho de Melo]

On Tue, Oct 22, 2024 at 03:17:09PM -0700, Stephen Brennan wrote:
> Alan Maguire <alan.maguire@oracle.com> writes:
> > ...as this means we avoid seeing multiple messages of the form

> > PAHOLE: Warning: Found invalid variable name when encoding btf, ignored (sym: '__UNIQUE_ID___addressable__RNvXsa_NtCs2vSfyxKMEZc_4core4charNtB5_11EscapeDebugNtNtNtNtB7_4iter6traits10exact_size17ExactSizeIterator3len57').
> 
> a beautiful variable name :P

A unique one :-)
 
> > Such symbols are filtered anyway due to the __UNIQUE_ID prefix, so
> > warning about them is not necessary; simply reordering the checks
> > should ensure we will not see these warnings.

> > Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

> Sorry I'm late to review. This is a good catch and is clearly correct
> to me.

Thanks, since it hasn't moved to master, I'm adding it.
 
> Reviewed-by: Stephen Brennan <stephen.s.brennan@oracle.com>

- Arnaldo