On Tue, Jul 26, 2022 at 12:21 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com>
wrote:
Hi Lianbo,
thanks for the review.
On 2022/07/25 15:30, lijiang wrote:
> Hi, Kazu
> Thank you for the fix.
>
> On Fri, Jul 22, 2022 at 12:45 PM HAGIO KAZUHITO(萩尾 一仁) <
k-hagio-ab(a)nec.com>
> wrote:
>
>> Without the patch, the following gcc-11 compliation warning is emitted
>> for symbols.c:
>>
>> symbols.c: In function 'cmd_p':
>> symbols.c:7412:38: warning: writing 1 byte into a region of size 0
>> [-Wstringop-overflow=]
>> 7412 | *(cpuspec-1) = ':';
>> | ~~~~~~~~~~~~~^~~~~
>>
>
>
> I have one question about it:
> This issue only appears in gcc-11 and earlier, I tested it with gcc-12,
but
> it disappeared. It's interesting.
I think that the warning is a false detection. This patch has no behavior
change, just a cosmetic change to avoid the warning. I'm not sure, but it
might be a gcc bug and fixed in gcc-12.
Probably. Let's leave it there, if it still appears in the future, we can
pick it up. Any thoughts?
The other three patches look good to me: Ack.
[PATCH 1/4] Fix gcc-11 compilation warnings on filesys.c
[PATCH 3/4] Fix gcc-11 compilation warning on makedumpfile.c
[PATCH 4/4] Fix gcc-11 compilation warning on kvmdump.c
Thanks.
Lianbo
Thanks,
Kazu
>
> In addition, a new warning was found(gcc-12), but it is not related to
this
> patch.
> ...
> gcc -c -g -DX86_64 -DLZO -DGDB_10_2 -g -O2 lkcd_v1.c -DMCLX -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
-Wformat-security
> lkcd_v1.c: In function ‘dump_lkcd_environment_v1’:
> lkcd_v1.c:252:20: warning: the comparison will always evaluate as ‘true’
> for the address of ‘dh_panic_string’ will never be NULL [-Waddress]
> 252 | dh && dh->dh_panic_string &&
> | ^~
> In file included from lkcd_v1.c:21:
> lkcd_vmdump_v1.h:108:30: note: ‘dh_panic_string’ declared here
> 108 | char dh_panic_string[DUMP_PANIC_LEN];
> | ^~~~~~~~~~~~~~~
> gcc -c -g -DX86_64 -DLZO -DGDB_10_2 -g -O2 lkcd_v2_v3.c -DMCLX -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
-Wformat-security
> lkcd_v2_v3.c: In function ‘dump_lkcd_environment_v2_v3’:
> lkcd_v2_v3.c:310:20: warning: the comparison will always evaluate as
‘true’
> for the address of ‘dh_panic_string’ will never be NULL [-Waddress]
> 310 | dh && dh->dh_panic_string &&
> | ^~
> In file included from lkcd_v2_v3.c:21:
> lkcd_vmdump_v2_v3.h:205:30: note: ‘dh_panic_string’ declared here
> 205 | char dh_panic_string[DUMP_PANIC_LEN];
> | ^~~~~~~~~~~~~~~
> gcc -c -g -DX86_64 -DLZO -DGDB_10_2 -g -O2 lkcd_v5.c -DMCLX -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
-Wformat-security
> lkcd_v5.c: In function ‘dump_lkcd_environment_v5’:
> lkcd_v5.c:273:20: warning: the comparison will always evaluate as ‘true’
> for the address of ‘dh_panic_string’ will never be NULL [-Waddress]
> 273 | dh && dh->dh_panic_string &&
> | ^~
> In file included from lkcd_v5.c:20:
> lkcd_dump_v5.h:163:30: note: ‘dh_panic_string’ declared here
> 163 | char dh_panic_string[DUMP_PANIC_LEN];
> | ^~~~~~~~~~~~~~~
> gcc -c -g -DX86_64 -DLZO -DGDB_10_2 -g -O2 lkcd_v7.c -DMCLX -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
-Wformat-security
> lkcd_v7.c: In function ‘dump_lkcd_environment_v7’:
> lkcd_v7.c:350:20: warning: the comparison will always evaluate as ‘true’
> for the address of ‘dh_panic_string’ will never be NULL [-Waddress]
> 350 | dh && dh->dh_panic_string &&
> | ^~
> In file included from lkcd_v7.c:21:
> lkcd_dump_v5.h:163:30: note: ‘dh_panic_string’ declared here
> 163 | char dh_panic_string[DUMP_PANIC_LEN];
> | ^~~~~~~~~~~~~~~
> gcc -c -g -DX86_64 -DLZO -DGDB_10_2 -g -O2 lkcd_v8.c -DMCLX -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
-Wformat-security
> lkcd_v8.c: In function ‘dump_lkcd_environment_v8’:
> lkcd_v8.c:546:20: warning: the comparison will always evaluate as ‘true’
> for the address of ‘dh_panic_string’ will never be NULL [-Waddress]
> 546 | dh && dh->dh_panic_string &&
> | ^~
> In file included from lkcd_v8.c:23:
> lkcd_dump_v8.h:165:30: note: ‘dh_panic_string’ declared here
> 165 | char dh_panic_string[DUMP_PANIC_LEN];
> ...
>
> Thanks.
> Lianbo
>
>
>> Signed-off-by: Kazuhito Hagio <k-hagio-ab(a)nec.com>
>> ---
>> symbols.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/symbols.c b/symbols.c
>> index bee1faf..42c4eb4 100644
>> --- a/symbols.c
>> +++ b/symbols.c
>> @@ -7351,7 +7351,7 @@ cmd_p(void)
>> unsigned radix;
>> int do_load_module_filter;
>> char buf1[BUFSIZE];
>> - char *cpuspec;
>> + char *cpuspec, *p;
>>
>> do_load_module_filter = radix = 0;
>>
>> @@ -7386,7 +7386,7 @@ cmd_p(void)
>> if (argerrs || !args[optind])
>> cmd_usage(pc->curcmd, SYNOPSIS);
>>
>> - cpuspec = strrchr(args[optind], ':');
>> + p = cpuspec = strrchr(args[optind], ':');
>> if (cpuspec)
>> *cpuspec++ = NULLCHAR;
>>
>> @@ -7409,7 +7409,7 @@ cmd_p(void)
>> sp->name);
>> else
>> /* maybe a valid C expression (e.g. ':') */
>> - *(cpuspec-1) = ':';
>> + *p = ':';
>> }
>>
>> process_gdb_output(concat_args(buf1, 0, TRUE), radix,
>> --
>> 2.31.1
>>