| Message ID | 20210209113357.1535104-1-Jianlin.Lv@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | perf tools: Fix arm64 build error with gcc-11 | expand |
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
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 > . >
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
> -----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
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 */
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(-)