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 <[email protected]>
---
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
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 <[email protected]>
> > > ---
> > > 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
> -----Original Message-----
> From: Leo Yan <[email protected]>
> Sent: Tuesday, February 9, 2021 8:17 PM
> To: Jianlin Lv <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Mark Rutland
> <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-arm-
> [email protected]; [email protected]
> 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 <[email protected]>
> > ---
> > 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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
>> ---
>> 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
> .
>