* [PATCH] kallsyms: work around bogus -Wrestrict warning
@ 2020-01-07 21:40 Arnd Bergmann
2020-01-07 22:25 ` Andrew Morton
2020-01-08 1:26 ` Masami Hiramatsu
0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2020-01-07 21:40 UTC (permalink / raw)
To: Andrew Morton
Cc: Oleksandr Natalenko, linux-mm, Arnd Bergmann,
Arnaldo Carvalho de Melo, Will Deacon, Masami Hiramatsu,
Song Liu, Alexey Dobriyan, Marc Zyngier, Thomas Gleixner,
linux-kernel
gcc -O3 produces some really odd warnings for this file:
kernel/kallsyms.c: In function 'sprint_symbol':
kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
strcpy(buffer, name);
^~~~~~~~~~~~~~~~~~~~
kernel/kallsyms.c: In function 'sprint_symbol_no_offset':
kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
strcpy(buffer, name);
^~~~~~~~~~~~~~~~~~~~
kernel/kallsyms.c: In function 'sprint_backtrace':
kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
strcpy(buffer, name);
^~~~~~~~~~~~~~~~~~~~
This obviously cannot be since it is preceded by an 'if (name != buffer)'
check.
Using sprintf() instead of strcpy() is a bit wasteful but is
the best workaround I could come up with.
Fixes: mmtom ("init/Kconfig: enable -O3 for all arches")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
kernel/kallsyms.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index d812b90f4c86..726b8eeb223e 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -366,7 +366,7 @@ static int __sprint_symbol(char *buffer, unsigned long address,
return sprintf(buffer, "0x%lx", address - symbol_offset);
if (name != buffer)
- strcpy(buffer, name);
+ sprintf(buffer, "%s", name);
len = strlen(buffer);
offset -= symbol_offset;
--
2.20.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] kallsyms: work around bogus -Wrestrict warning
2020-01-07 21:40 [PATCH] kallsyms: work around bogus -Wrestrict warning Arnd Bergmann
@ 2020-01-07 22:25 ` Andrew Morton
2020-01-08 9:23 ` Oleksandr Natalenko
2020-01-08 1:26 ` Masami Hiramatsu
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2020-01-07 22:25 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Oleksandr Natalenko, linux-mm, Arnaldo Carvalho de Melo,
Will Deacon, Masami Hiramatsu, Song Liu, Alexey Dobriyan,
Marc Zyngier, Thomas Gleixner, linux-kernel
On Tue, 7 Jan 2020 22:40:26 +0100 Arnd Bergmann <arnd@arndb.de> wrote:
> gcc -O3 produces some really odd warnings for this file:
>
> kernel/kallsyms.c: In function 'sprint_symbol':
> kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> strcpy(buffer, name);
> ^~~~~~~~~~~~~~~~~~~~
> kernel/kallsyms.c: In function 'sprint_symbol_no_offset':
> kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> strcpy(buffer, name);
> ^~~~~~~~~~~~~~~~~~~~
> kernel/kallsyms.c: In function 'sprint_backtrace':
> kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> strcpy(buffer, name);
> ^~~~~~~~~~~~~~~~~~~~
>
> This obviously cannot be since it is preceded by an 'if (name != buffer)'
> check.
>
> Using sprintf() instead of strcpy() is a bit wasteful but is
> the best workaround I could come up with.
>
> ...
>
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -366,7 +366,7 @@ static int __sprint_symbol(char *buffer, unsigned long address,
> return sprintf(buffer, "0x%lx", address - symbol_offset);
>
> if (name != buffer)
> - strcpy(buffer, name);
> + sprintf(buffer, "%s", name);
> len = strlen(buffer);
> offset -= symbol_offset;
gee, is that even worth "fixing"? Oleksandr, I've seen a couple of
these false positives. Do we know if anyone is taking them to the gcc
developers?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kallsyms: work around bogus -Wrestrict warning
2020-01-07 21:40 [PATCH] kallsyms: work around bogus -Wrestrict warning Arnd Bergmann
2020-01-07 22:25 ` Andrew Morton
@ 2020-01-08 1:26 ` Masami Hiramatsu
2020-01-09 22:02 ` Andrew Morton
1 sibling, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2020-01-08 1:26 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Morton, Oleksandr Natalenko, linux-mm,
Arnaldo Carvalho de Melo, Will Deacon, Masami Hiramatsu,
Song Liu, Alexey Dobriyan, Marc Zyngier, Thomas Gleixner,
linux-kernel
Hi Arnd,
On Tue, 7 Jan 2020 22:40:26 +0100
Arnd Bergmann <arnd@arndb.de> wrote:
> gcc -O3 produces some really odd warnings for this file:
>
> kernel/kallsyms.c: In function 'sprint_symbol':
> kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> strcpy(buffer, name);
> ^~~~~~~~~~~~~~~~~~~~
> kernel/kallsyms.c: In function 'sprint_symbol_no_offset':
> kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> strcpy(buffer, name);
> ^~~~~~~~~~~~~~~~~~~~
> kernel/kallsyms.c: In function 'sprint_backtrace':
> kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> strcpy(buffer, name);
> ^~~~~~~~~~~~~~~~~~~~
>
> This obviously cannot be since it is preceded by an 'if (name != buffer)'
> check.
Hmm, this looks like a bug in gcc.
>
> Using sprintf() instead of strcpy() is a bit wasteful but is
> the best workaround I could come up with.
>
> Fixes: mmtom ("init/Kconfig: enable -O3 for all arches")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> kernel/kallsyms.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index d812b90f4c86..726b8eeb223e 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -366,7 +366,7 @@ static int __sprint_symbol(char *buffer, unsigned long address,
> return sprintf(buffer, "0x%lx", address - symbol_offset);
>
> if (name != buffer)
> - strcpy(buffer, name);
> + sprintf(buffer, "%s", name);
BTW, this seems not happen. kallsyms_lookup() (and it's subfunctions)
always stores the result into buffer unless name == NULL.
Maybe we can remove these 2 lines?
(and add a comment line for kallsyms_lookup() so that it guarantees the
symbol name always stored in namebuf argument)
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kallsyms: work around bogus -Wrestrict warning
2020-01-07 22:25 ` Andrew Morton
@ 2020-01-08 9:23 ` Oleksandr Natalenko
0 siblings, 0 replies; 6+ messages in thread
From: Oleksandr Natalenko @ 2020-01-08 9:23 UTC (permalink / raw)
To: Andrew Morton
Cc: Arnd Bergmann, linux-mm, Arnaldo Carvalho de Melo, Will Deacon,
Masami Hiramatsu, Song Liu, Alexey Dobriyan, Marc Zyngier,
Thomas Gleixner, linux-kernel
Hi.
On Tue, Jan 07, 2020 at 02:25:12PM -0800, Andrew Morton wrote:
> gee, is that even worth "fixing"? Oleksandr, I've seen a couple of
> these false positives. Do we know if anyone is taking them to the gcc
> developers?
I'm not aware of such an effort. I tend to blame compiler as an option
of last resort, but if Arnd gathers enough examples (since he's working
on fixing/working around those), it would be reasonable to suggest him to
hand over those findings to gcc bugtracker.
--
Best regards,
Oleksandr Natalenko (post-factum)
Senior Software Maintenance Engineer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kallsyms: work around bogus -Wrestrict warning
2020-01-08 1:26 ` Masami Hiramatsu
@ 2020-01-09 22:02 ` Andrew Morton
2020-01-09 22:18 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2020-01-09 22:02 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Arnd Bergmann, Oleksandr Natalenko, linux-mm,
Arnaldo Carvalho de Melo, Will Deacon, Song Liu, Alexey Dobriyan,
Marc Zyngier, Thomas Gleixner, linux-kernel
On Wed, 8 Jan 2020 10:26:02 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Hi Arnd,
>
> On Tue, 7 Jan 2020 22:40:26 +0100
> Arnd Bergmann <arnd@arndb.de> wrote:
>
> > gcc -O3 produces some really odd warnings for this file:
> >
> > kernel/kallsyms.c: In function 'sprint_symbol':
> > kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> > strcpy(buffer, name);
> > ^~~~~~~~~~~~~~~~~~~~
> > kernel/kallsyms.c: In function 'sprint_symbol_no_offset':
> > kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> > strcpy(buffer, name);
> > ^~~~~~~~~~~~~~~~~~~~
> > kernel/kallsyms.c: In function 'sprint_backtrace':
> > kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> > strcpy(buffer, name);
> > ^~~~~~~~~~~~~~~~~~~~
> >
> > This obviously cannot be since it is preceded by an 'if (name != buffer)'
> > check.
>
> Hmm, this looks like a bug in gcc.
Yes, we're getting a lot of such reports. I don't think current gcc is
ready for this patch so I'll drop it, sorry.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kallsyms: work around bogus -Wrestrict warning
2020-01-09 22:02 ` Andrew Morton
@ 2020-01-09 22:18 ` Arnd Bergmann
0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2020-01-09 22:18 UTC (permalink / raw)
To: Andrew Morton
Cc: Masami Hiramatsu, Oleksandr Natalenko, Linux-MM,
Arnaldo Carvalho de Melo, Will Deacon, Song Liu, Alexey Dobriyan,
Marc Zyngier, Thomas Gleixner, linux-kernel
On Thu, Jan 9, 2020 at 11:02 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Wed, 8 Jan 2020 10:26:02 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> > Hi Arnd,
> >
> > On Tue, 7 Jan 2020 22:40:26 +0100
> > Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > > gcc -O3 produces some really odd warnings for this file:
> > >
> > > kernel/kallsyms.c: In function 'sprint_symbol':
> > > kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> > > strcpy(buffer, name);
> > > ^~~~~~~~~~~~~~~~~~~~
> > > kernel/kallsyms.c: In function 'sprint_symbol_no_offset':
> > > kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> > > strcpy(buffer, name);
> > > ^~~~~~~~~~~~~~~~~~~~
> > > kernel/kallsyms.c: In function 'sprint_backtrace':
> > > kernel/kallsyms.c:369:3: error: 'strcpy' source argument is the same as destination [-Werror=restrict]
> > > strcpy(buffer, name);
> > > ^~~~~~~~~~~~~~~~~~~~
> > >
> > > This obviously cannot be since it is preceded by an 'if (name != buffer)'
> > > check.
> >
> > Hmm, this looks like a bug in gcc.
>
> Yes, we're getting a lot of such reports. I don't think current gcc is
> ready for this patch so I'll drop it, sorry.
I've been building with gcc-8 and got around 20 false positive
warnings, three real bugs
and a few files that introduce increased stack usage. I have sent
patches for every one
of these and have a clean randconfig builds again on arm, arm64 and
x86 (a few thousand
so far).
Most of the false-positive warnings are for understandable reasons and easy to
work around, the one above is probably the most blatant screwup by gcc.
My feeling is that we can deal with the warnings here and I wouldn't
mind getting
it enabled in mainline from that perspective, but there are two caveats:
- v5.6 is probably too early since we're close to the merge window and a lot of
my fixups have not been merged yet
- I have no good estimate of how many runtime failures there will be.
Oleksandr hasn't found any issues after running with -O3 kernels for
a longer time, but any significant change to the toolchain likely causes
problems for somebody.
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-09 22:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-07 21:40 [PATCH] kallsyms: work around bogus -Wrestrict warning Arnd Bergmann
2020-01-07 22:25 ` Andrew Morton
2020-01-08 9:23 ` Oleksandr Natalenko
2020-01-08 1:26 ` Masami Hiramatsu
2020-01-09 22:02 ` Andrew Morton
2020-01-09 22:18 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).