* [PATCH] perf tools: Fix arm64 build error with gcc-11
@ 2021-02-09 11:33 ` Jianlin Lv
0 siblings, 0 replies; 10+ messages in thread
From: Jianlin Lv @ 2021-02-09 11:33 UTC (permalink / raw)
To: john.garry, will, mathieu.poirier, leo.yan, peterz, mingo, acme,
mark.rutland, alexander.shishkin, jolsa, namhyung
Cc: Jianlin.Lv, linux-arm-kernel, linux-kernel
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] perf tools: Fix arm64 build error with gcc-11
@ 2021-02-09 11:33 ` Jianlin Lv
0 siblings, 0 replies; 10+ messages in thread
From: Jianlin Lv @ 2021-02-09 11:33 UTC (permalink / raw)
To: john.garry, will, mathieu.poirier, leo.yan, peterz, mingo, acme,
mark.rutland, alexander.shishkin, jolsa, namhyung
Cc: Jianlin.Lv, linux-kernel, linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] perf tools: Fix arm64 build error with gcc-11
2021-02-09 11:33 ` Jianlin Lv
@ 2021-02-09 12:17 ` Leo Yan
-1 siblings, 0 replies; 10+ messages in thread
From: Leo Yan @ 2021-02-09 12:17 UTC (permalink / raw)
To: Jianlin Lv
Cc: john.garry, will, mathieu.poirier, peterz, mingo, acme,
mark.rutland, alexander.shishkin, jolsa, namhyung,
linux-arm-kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf tools: Fix arm64 build error with gcc-11
@ 2021-02-09 12:17 ` Leo Yan
0 siblings, 0 replies; 10+ messages in thread
From: Leo Yan @ 2021-02-09 12:17 UTC (permalink / raw)
To: Jianlin Lv
Cc: mark.rutland, mathieu.poirier, peterz, jolsa, john.garry,
linux-kernel, acme, alexander.shishkin, mingo, namhyung, will,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf tools: Fix arm64 build error with gcc-11
2021-02-09 12:17 ` Leo Yan
@ 2021-02-09 14:18 ` John Garry
-1 siblings, 0 replies; 10+ messages in thread
From: John Garry @ 2021-02-09 14:18 UTC (permalink / raw)
To: Leo Yan, Jianlin Lv
Cc: will, mathieu.poirier, peterz, mingo, acme, mark.rutland,
alexander.shishkin, jolsa, namhyung, linux-arm-kernel,
linux-kernel
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
> .
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf tools: Fix arm64 build error with gcc-11
@ 2021-02-09 14:18 ` John Garry
0 siblings, 0 replies; 10+ messages in thread
From: John Garry @ 2021-02-09 14:18 UTC (permalink / raw)
To: Leo Yan, Jianlin Lv
Cc: mark.rutland, mathieu.poirier, peterz, jolsa, linux-kernel, acme,
alexander.shishkin, mingo, namhyung, will, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf tools: Fix arm64 build error with gcc-11
2021-02-09 14:18 ` John Garry
@ 2021-02-09 14:44 ` Leo Yan
-1 siblings, 0 replies; 10+ messages in thread
From: Leo Yan @ 2021-02-09 14:44 UTC (permalink / raw)
To: John Garry
Cc: Jianlin Lv, will, mathieu.poirier, peterz, mingo, acme,
mark.rutland, alexander.shishkin, jolsa, namhyung,
linux-arm-kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] perf tools: Fix arm64 build error with gcc-11
@ 2021-02-09 14:44 ` Leo Yan
0 siblings, 0 replies; 10+ messages in thread
From: Leo Yan @ 2021-02-09 14:44 UTC (permalink / raw)
To: John Garry
Cc: mark.rutland, mathieu.poirier, peterz, Jianlin Lv, jolsa,
linux-kernel, acme, alexander.shishkin, mingo, namhyung, will,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] perf tools: Fix arm64 build error with gcc-11
2021-02-09 12:17 ` Leo Yan
@ 2021-02-10 2:31 ` Jianlin Lv
-1 siblings, 0 replies; 10+ messages in thread
From: Jianlin Lv @ 2021-02-10 2:31 UTC (permalink / raw)
To: leo.yan
Cc: john.garry, will, mathieu.poirier, peterz, mingo, acme,
Mark Rutland, alexander.shishkin, jolsa, namhyung,
linux-arm-kernel, linux-kernel
> -----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
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] perf tools: Fix arm64 build error with gcc-11
@ 2021-02-10 2:31 ` Jianlin Lv
0 siblings, 0 replies; 10+ messages in thread
From: Jianlin Lv @ 2021-02-10 2:31 UTC (permalink / raw)
To: leo.yan
Cc: Mark Rutland, mathieu.poirier, peterz, jolsa, john.garry,
linux-kernel, acme, alexander.shishkin, mingo, namhyung, will,
linux-arm-kernel
> -----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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-02-10 2:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-09 11:33 [PATCH] perf tools: Fix arm64 build error with gcc-11 Jianlin Lv
2021-02-09 11:33 ` Jianlin Lv
2021-02-09 12:17 ` Leo Yan
2021-02-09 12:17 ` Leo Yan
2021-02-09 14:18 ` John Garry
2021-02-09 14:18 ` John Garry
2021-02-09 14:44 ` Leo Yan
2021-02-09 14:44 ` Leo Yan
2021-02-10 2:31 ` Jianlin Lv
2021-02-10 2:31 ` Jianlin Lv
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.