[PATCH] perf tools: Fix arm64 build error with gcc-11

[PATCH] perf tools: Fix arm64 build error with gcc-11

[Jianlin Lv]

gcc version: 11.0.0 20210208 (experimental) (GCC)

Following build error on arm64:

.......
In function ‘printf’,
    inlined from ‘regs_dump__printf’ at util/session.c:1141:3,
    inlined from ‘regs__printf’ at util/session.c:1169:2:
/usr/include/aarch64-linux-gnu/bits/stdio2.h:107:10: \
  error: ‘%-5s’ directive argument is null [-Werror=format-overflow=]

107 |   return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, \
                __va_arg_pack ());

......
In function ‘fprintf’,
  inlined from ‘perf_sample__fprintf_regs.isra’ at \
    builtin-script.c:622:14:
/usr/include/aarch64-linux-gnu/bits/stdio2.h:100:10: \
	error: ‘%5s’ directive argument is null [-Werror=format-overflow=]
  100 |   return __fprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1, __fmt,
  101 |                         __va_arg_pack ());

cc1: all warnings being treated as errors
.......

This patch fixes Wformat-overflow warnings by replacing the return
value NULL of perf_reg_name with "unknown".

Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
---
 tools/perf/arch/arm64/include/perf_regs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/arch/arm64/include/perf_regs.h b/tools/perf/arch/arm64/include/perf_regs.h
index baaa5e64a3fb..901419f907c0 100644
--- a/tools/perf/arch/arm64/include/perf_regs.h
+++ b/tools/perf/arch/arm64/include/perf_regs.h
@@ -85,10 +85,10 @@ static inline const char *perf_reg_name(int id)
 	case PERF_REG_ARM64_PC:
 		return "pc";
 	default:
-		return NULL;
+		return "unknown";
 	}
 
-	return NULL;
+	return "unknown";
 }
 
 #endif /* ARCH_PERF_REGS_H */
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] perf tools: Fix arm64 build error with gcc-11

[Leo Yan]

Hi Jianlin,

On Tue, Feb 09, 2021 at 07:33:57PM +0800, Jianlin Lv wrote:
> gcc version: 11.0.0 20210208 (experimental) (GCC)
> 
> Following build error on arm64:
> 
> .......
> In function ‘printf’,
>     inlined from ‘regs_dump__printf’ at util/session.c:1141:3,
>     inlined from ‘regs__printf’ at util/session.c:1169:2:
> /usr/include/aarch64-linux-gnu/bits/stdio2.h:107:10: \
>   error: ‘%-5s’ directive argument is null [-Werror=format-overflow=]
> 
> 107 |   return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, \
>                 __va_arg_pack ());
> 
> ......
> In function ‘fprintf’,
>   inlined from ‘perf_sample__fprintf_regs.isra’ at \
>     builtin-script.c:622:14:
> /usr/include/aarch64-linux-gnu/bits/stdio2.h:100:10: \
> 	error: ‘%5s’ directive argument is null [-Werror=format-overflow=]
>   100 |   return __fprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1, __fmt,
>   101 |                         __va_arg_pack ());
> 
> cc1: all warnings being treated as errors
> .......
> 
> This patch fixes Wformat-overflow warnings by replacing the return
> value NULL of perf_reg_name with "unknown".
> 
> Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> ---
>  tools/perf/arch/arm64/include/perf_regs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/arch/arm64/include/perf_regs.h b/tools/perf/arch/arm64/include/perf_regs.h
> index baaa5e64a3fb..901419f907c0 100644
> --- a/tools/perf/arch/arm64/include/perf_regs.h
> +++ b/tools/perf/arch/arm64/include/perf_regs.h
> @@ -85,10 +85,10 @@ static inline const char *perf_reg_name(int id)
>  	case PERF_REG_ARM64_PC:
>  		return "pc";
>  	default:
> -		return NULL;
> +		return "unknown";
>  	}
>  
> -	return NULL;
> +	return "unknown";

This issue is a common issue crossing all archs.  So it's better to
change the code in the places where calls perf_reg_name(), e.g. in
util/session.c:

--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1135,12 +1135,14 @@ static void branch_stack__printf(struct perf_sample *sample, bool callstack)
 static void regs_dump__printf(u64 mask, u64 *regs)
 {
        unsigned rid, i = 0;
+       char *reg_name;
 
        for_each_set_bit(rid, (unsigned long *) &mask, sizeof(mask) * 8) {
                u64 val = regs[i++];
 
+               reg_name = perf_reg_name(rid);
                printf(".... %-5s 0x%016" PRIx64 "\n",
-                      perf_reg_name(rid), val);
+                      reg_name ?: "Unknown", val);
        }
 }

And another potential issue is the format specifier "%-5s", it prints
out maximum to 5 chars, but actually string "Unknown" has 7 chars.
Actually the format specifier breaks other archs register names, e.g.
[1][2], seems to me, it's better to change as "%-8s", you might need
to use a dedicated patch for format specifier changes.

Thanks,
Leo


[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/arch/powerpc/include/perf_regs.h#n57
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/arch/csky/include/perf_regs.h#n83

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] perf tools: Fix arm64 build error with gcc-11

[John Garry]

On 09/02/2021 12:17, Leo Yan wrote:
> Hi Jianlin,
> 
> On Tue, Feb 09, 2021 at 07:33:57PM +0800, Jianlin Lv wrote:
>> gcc version: 11.0.0 20210208 (experimental) (GCC)
>>
>> Following build error on arm64:
>>
>> .......
>> In function ‘printf’,
>>      inlined from ‘regs_dump__printf’ at util/session.c:1141:3,
>>      inlined from ‘regs__printf’ at util/session.c:1169:2:
>> /usr/include/aarch64-linux-gnu/bits/stdio2.h:107:10: \
>>    error: ‘%-5s’ directive argument is null [-Werror=format-overflow=]
>>
>> 107 |   return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, \
>>                  __va_arg_pack ());
>>
>> ......
>> In function ‘fprintf’,
>>    inlined from ‘perf_sample__fprintf_regs.isra’ at \
>>      builtin-script.c:622:14:
>> /usr/include/aarch64-linux-gnu/bits/stdio2.h:100:10: \
>> 	error: ‘%5s’ directive argument is null [-Werror=format-overflow=]
>>    100 |   return __fprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1, __fmt,
>>    101 |                         __va_arg_pack ());
>>
>> cc1: all warnings being treated as errors
>> .......
>>
>> This patch fixes Wformat-overflow warnings by replacing the return
>> value NULL of perf_reg_name with "unknown".
>>
>> Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
>> ---
>>   tools/perf/arch/arm64/include/perf_regs.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/arch/arm64/include/perf_regs.h b/tools/perf/arch/arm64/include/perf_regs.h
>> index baaa5e64a3fb..901419f907c0 100644
>> --- a/tools/perf/arch/arm64/include/perf_regs.h
>> +++ b/tools/perf/arch/arm64/include/perf_regs.h
>> @@ -85,10 +85,10 @@ static inline const char *perf_reg_name(int id)
>>   	case PERF_REG_ARM64_PC:
>>   		return "pc";
>>   	default:
>> -		return NULL;
>> +		return "unknown";
>>   	}
>>   
>> -	return NULL;
>> +	return "unknown";
> 
> This issue is a common issue crossing all archs.  So it's better to
> change the code in the places where calls perf_reg_name(), e.g. in
> util/session.c:
> 
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1135,12 +1135,14 @@ static void branch_stack__printf(struct perf_sample *sample, bool callstack)
>   static void regs_dump__printf(u64 mask, u64 *regs)
>   {
>          unsigned rid, i = 0;
> +       char *reg_name;
>   
>          for_each_set_bit(rid, (unsigned long *) &mask, sizeof(mask) * 8) {
>                  u64 val = regs[i++];
>   
> +               reg_name = perf_reg_name(rid);
>                  printf(".... %-5s 0x%016" PRIx64 "\n",
> -                      perf_reg_name(rid), val);
> +                      reg_name ?: "Unknown", val);
>          }
>   }
> 
> And another potential issue is the format specifier "%-5s", it prints
> out maximum to 5 chars, 

Doesn't the width field specify the min, not max, number of characters?

Cheers,
John

> but actually string "Unknown" has 7 chars.
> Actually the format specifier breaks other archs register names, e.g.
> [1][2], seems to me, it's better to change as "%-8s", you might need
> to use a dedicated patch for format specifier changes.
> 
> Thanks,
> Leo
> 
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/arch/powerpc/include/perf_regs.h#n57
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/arch/csky/include/perf_regs.h#n83
> .
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] perf tools: Fix arm64 build error with gcc-11

[Leo Yan]

On Tue, Feb 09, 2021 at 02:18:26PM +0000, John Garry wrote:
> On 09/02/2021 12:17, Leo Yan wrote:
> > Hi Jianlin,
> > 
> > On Tue, Feb 09, 2021 at 07:33:57PM +0800, Jianlin Lv wrote:
> > > gcc version: 11.0.0 20210208 (experimental) (GCC)
> > > 
> > > Following build error on arm64:
> > > 
> > > .......
> > > In function ‘printf’,
> > >      inlined from ‘regs_dump__printf’ at util/session.c:1141:3,
> > >      inlined from ‘regs__printf’ at util/session.c:1169:2:
> > > /usr/include/aarch64-linux-gnu/bits/stdio2.h:107:10: \
> > >    error: ‘%-5s’ directive argument is null [-Werror=format-overflow=]
> > > 
> > > 107 |   return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, \
> > >                  __va_arg_pack ());
> > > 
> > > ......
> > > In function ‘fprintf’,
> > >    inlined from ‘perf_sample__fprintf_regs.isra’ at \
> > >      builtin-script.c:622:14:
> > > /usr/include/aarch64-linux-gnu/bits/stdio2.h:100:10: \
> > > 	error: ‘%5s’ directive argument is null [-Werror=format-overflow=]
> > >    100 |   return __fprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1, __fmt,
> > >    101 |                         __va_arg_pack ());
> > > 
> > > cc1: all warnings being treated as errors
> > > .......
> > > 
> > > This patch fixes Wformat-overflow warnings by replacing the return
> > > value NULL of perf_reg_name with "unknown".
> > > 
> > > Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> > > ---
> > >   tools/perf/arch/arm64/include/perf_regs.h | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/perf/arch/arm64/include/perf_regs.h b/tools/perf/arch/arm64/include/perf_regs.h
> > > index baaa5e64a3fb..901419f907c0 100644
> > > --- a/tools/perf/arch/arm64/include/perf_regs.h
> > > +++ b/tools/perf/arch/arm64/include/perf_regs.h
> > > @@ -85,10 +85,10 @@ static inline const char *perf_reg_name(int id)
> > >   	case PERF_REG_ARM64_PC:
> > >   		return "pc";
> > >   	default:
> > > -		return NULL;
> > > +		return "unknown";
> > >   	}
> > > -	return NULL;
> > > +	return "unknown";
> > 
> > This issue is a common issue crossing all archs.  So it's better to
> > change the code in the places where calls perf_reg_name(), e.g. in
> > util/session.c:
> > 
> > --- a/tools/perf/util/session.c
> > +++ b/tools/perf/util/session.c
> > @@ -1135,12 +1135,14 @@ static void branch_stack__printf(struct perf_sample *sample, bool callstack)
> >   static void regs_dump__printf(u64 mask, u64 *regs)
> >   {
> >          unsigned rid, i = 0;
> > +       char *reg_name;
> >          for_each_set_bit(rid, (unsigned long *) &mask, sizeof(mask) * 8) {
> >                  u64 val = regs[i++];
> > +               reg_name = perf_reg_name(rid);
> >                  printf(".... %-5s 0x%016" PRIx64 "\n",
> > -                      perf_reg_name(rid), val);
> > +                      reg_name ?: "Unknown", val);
> >          }
> >   }
> > 
> > And another potential issue is the format specifier "%-5s", it prints
> > out maximum to 5 chars,
> 
> Doesn't the width field specify the min, not max, number of characters?

Thanks for correction, John.

I wrongly understood it and sorry for confusion.  Wiki says [1]:

"The Width field specifies a minimum number of characters to output,
and is typically used to pad fixed-width fields in tabulated output,
where the fields would otherwise be smaller, although it does not
cause truncation of oversized fields."

Thanks,
Leo

[1] https://en.wikipedia.org/wiki/Printf_format_string#Width_field

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH] perf tools: Fix arm64 build error with gcc-11

[Jianlin Lv]

> -----Original Message-----
> From: Leo Yan <leo.yan@linaro.org>
> Sent: Tuesday, February 9, 2021 8:17 PM
> To: Jianlin Lv <Jianlin.Lv@arm.com>
> Cc: john.garry@huawei.com; will@kernel.org; mathieu.poirier@linaro.org;
> peterz@infradead.org; mingo@redhat.com; acme@kernel.org; Mark Rutland
> <Mark.Rutland@arm.com>; alexander.shishkin@linux.intel.com;
> jolsa@redhat.com; namhyung@kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] perf tools: Fix arm64 build error with gcc-11
> 
> Hi Jianlin,
> 
> On Tue, Feb 09, 2021 at 07:33:57PM +0800, Jianlin Lv wrote:
> > gcc version: 11.0.0 20210208 (experimental) (GCC)
> >
> > Following build error on arm64:
> >
> > .......
> > In function ‘printf’,
> >     inlined from ‘regs_dump__printf’ at util/session.c:1141:3,
> >     inlined from ‘regs__printf’ at util/session.c:1169:2:
> > /usr/include/aarch64-linux-gnu/bits/stdio2.h:107:10: \
> >   error: ‘%-5s’ directive argument is null [-Werror=format-overflow=]
> >
> > 107 |   return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, \
> >                 __va_arg_pack ());
> >
> > ......
> > In function ‘fprintf’,
> >   inlined from ‘perf_sample__fprintf_regs.isra’ at \
> >     builtin-script.c:622:14:
> > /usr/include/aarch64-linux-gnu/bits/stdio2.h:100:10: \
> > 	error: ‘%5s’ directive argument is null [-Werror=format-overflow=]
> >   100 |   return __fprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1, __fmt,
> >   101 |                         __va_arg_pack ());
> >
> > cc1: all warnings being treated as errors .......
> >
> > This patch fixes Wformat-overflow warnings by replacing the return
> > value NULL of perf_reg_name with "unknown".
> >
> > Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> > ---
> >  tools/perf/arch/arm64/include/perf_regs.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/arch/arm64/include/perf_regs.h
> > b/tools/perf/arch/arm64/include/perf_regs.h
> > index baaa5e64a3fb..901419f907c0 100644
> > --- a/tools/perf/arch/arm64/include/perf_regs.h
> > +++ b/tools/perf/arch/arm64/include/perf_regs.h
> > @@ -85,10 +85,10 @@ static inline const char *perf_reg_name(int id)
> >  	case PERF_REG_ARM64_PC:
> >  		return "pc";
> >  	default:
> > -		return NULL;
> > +		return "unknown";
> >  	}
> >
> > -	return NULL;
> > +	return "unknown";
> 
> This issue is a common issue crossing all archs.  So it's better to change the
> code in the places where calls perf_reg_name(), e.g. in
> util/session.c:
> 
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1135,12 +1135,14 @@ static void branch_stack__printf(struct
> perf_sample *sample, bool callstack)  static void regs_dump__printf(u64
> mask, u64 *regs)  {
>         unsigned rid, i = 0;
> +       char *reg_name;
> 
>         for_each_set_bit(rid, (unsigned long *) &mask, sizeof(mask) * 8) {
>                 u64 val = regs[i++];
> 
> +               reg_name = perf_reg_name(rid);
>                 printf(".... %-5s 0x%016" PRIx64 "\n",
> -                      perf_reg_name(rid), val);
> +                      reg_name ?: "Unknown", val);
>         }
>  }
> 

Thanks for your comments, I will send a v2 of the patch today.

Jianlin


> And another potential issue is the format specifier "%-5s", it prints out
> maximum to 5 chars, but actually string "Unknown" has 7 chars.
> Actually the format specifier breaks other archs register names, e.g.
> [1][2], seems to me, it's better to change as "%-8s", you might need to use a
> dedicated patch for format specifier changes.
> 
> Thanks,
> Leo
> 
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/
> perf/arch/powerpc/include/perf_regs.h#n57
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/
> perf/arch/csky/include/perf_regs.h#n83
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel