Hi all,
After merging the akpm tree, today's linux-next build (powerpc
allyesconfig) produced warnings like this:
kernel/kcov.c:296:14: warning: conflicting types for built-in function '__sanitizer_cov_trace_switch'; expected 'void(long unsigned int, void *)' [-Wbuiltin-declaration-mismatch]
296 | void notrace __sanitizer_cov_trace_switch(u64 val, u64 *cases)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
ld: warning: orphan section `.data..Lubsan_data177' from `arch/powerpc/oprofile/op_model_pa6t.o' being placed in section `.data..Lubsan_data177'
(lots of these latter ones)
I don't know what produced these, but it is in the akpm-current or
akpm trees.
--
Cheers,
Stephen Rothwell
On Fri, 4 Dec 2020 21:00:00 +1100 Stephen Rothwell <[email protected]> wrote:
> Hi all,
>
> After merging the akpm tree, today's linux-next build (powerpc
> allyesconfig) produced warnings like this:
>
> kernel/kcov.c:296:14: warning: conflicting types for built-in function '__sanitizer_cov_trace_switch'; expected 'void(long unsigned int, void *)' [-Wbuiltin-declaration-mismatch]
> 296 | void notrace __sanitizer_cov_trace_switch(u64 val, u64 *cases)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
Odd. clang wants that signature, according to
https://clang.llvm.org/docs/SanitizerCoverage.html. But gcc seems to
want a different signature. Beats me - best I can do is to cc various
likely culprits ;)
Which gcc version? Did you recently update gcc?
> ld: warning: orphan section `.data..Lubsan_data177' from `arch/powerpc/oprofile/op_model_pa6t.o' being placed in section `.data..Lubsan_data177'
>
> (lots of these latter ones)
>
> I don't know what produced these, but it is in the akpm-current or
> akpm trees.
Hi Andrew,
On Fri, 4 Dec 2020 21:19:23 -0800 Andrew Morton <[email protected]> wrote:
>
> Odd. clang wants that signature, according to
> https://clang.llvm.org/docs/SanitizerCoverage.html. But gcc seems to
> want a different signature. Beats me - best I can do is to cc various
> likely culprits ;)
>
> Which gcc version? Did you recently update gcc?
gcc (Debian 10.2.0-15) 10.2.0
Not recently upgraded.
--
Cheers,
Stephen Rothwell
On Sat, Dec 5, 2020 at 6:19 AM Andrew Morton <[email protected]> wrote:
>
> On Fri, 4 Dec 2020 21:00:00 +1100 Stephen Rothwell <[email protected]> wrote:
>
> > Hi all,
> >
> > After merging the akpm tree, today's linux-next build (powerpc
> > allyesconfig) produced warnings like this:
> >
> > kernel/kcov.c:296:14: warning: conflicting types for built-in function '__sanitizer_cov_trace_switch'; expected 'void(long unsigned int, void *)' [-Wbuiltin-declaration-mismatch]
> > 296 | void notrace __sanitizer_cov_trace_switch(u64 val, u64 *cases)
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Odd. clang wants that signature, according to
> https://clang.llvm.org/docs/SanitizerCoverage.html. But gcc seems to
> want a different signature. Beats me - best I can do is to cc various
> likely culprits ;)
>
> Which gcc version? Did you recently update gcc?
>
> > ld: warning: orphan section `.data..Lubsan_data177' from `arch/powerpc/oprofile/op_model_pa6t.o' being placed in section `.data..Lubsan_data177'
> >
> > (lots of these latter ones)
> >
> > I don't know what produced these, but it is in the akpm-current or
> > akpm trees.
I can reproduce this in x86_64 build as well but only if I enable
UBSAN as well. There were some recent UBSAN changes by Kees, so maybe
that's what affected the warning.
Though, the warning itself looks legit and unrelated to UBSAN. In
fact, if the compiler expects long and we accept u64, it may be broken
on 32-bit arches...
I have gcc version 10.2.0 (Debian 10.2.0-15)
On next-20201207
config is defconfig +
CONFIG_KCOV=y
CONFIG_KCOV_ENABLE_COMPARISONS=y
CONFIG_UBSAN=y
$ make -j8 kernel/kcov.o
CC kernel/kcov.o
kernel/kcov.c:296:14: warning: conflicting types for built-in function
‘__sanitizer_cov_trace_switch’; expected ‘void(long unsigned int,
void *)’ [-Wbuiltin-declaration-mismatch]
296 | void notrace __sanitizer_cov_trace_switch(u64 val, u64 *cases)
On Mon, Dec 7, 2020 at 1:08 PM Dmitry Vyukov <[email protected]> wrote:
> > > Hi all,
> > >
> > > After merging the akpm tree, today's linux-next build (powerpc
> > > allyesconfig) produced warnings like this:
> > >
> > > kernel/kcov.c:296:14: warning: conflicting types for built-in function '__sanitizer_cov_trace_switch'; expected 'void(long unsigned int, void *)' [-Wbuiltin-declaration-mismatch]
> > > 296 | void notrace __sanitizer_cov_trace_switch(u64 val, u64 *cases)
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Odd. clang wants that signature, according to
> > https://clang.llvm.org/docs/SanitizerCoverage.html. But gcc seems to
> > want a different signature. Beats me - best I can do is to cc various
> > likely culprits ;)
> >
> > Which gcc version? Did you recently update gcc?
> >
> > > ld: warning: orphan section `.data..Lubsan_data177' from `arch/powerpc/oprofile/op_model_pa6t.o' being placed in section `.data..Lubsan_data177'
> > >
> > > (lots of these latter ones)
> > >
> > > I don't know what produced these, but it is in the akpm-current or
> > > akpm trees.
>
> I can reproduce this in x86_64 build as well but only if I enable
> UBSAN as well. There were some recent UBSAN changes by Kees, so maybe
> that's what affected the warning.
> Though, the warning itself looks legit and unrelated to UBSAN. In
> fact, if the compiler expects long and we accept u64, it may be broken
> on 32-bit arches...
No, I think it works, the argument should be uint64.
I think both gcc and clang signatures are correct and both want
uint64_t. The question is just how uint64_t is defined :) The old
printf joke that one can't write portable format specifier for
uint64_t.
What I know so far:
clang 11 does not produce this warning even with obviously wrong
signatures (e.g. short).
I wasn't able to trigger it with gcc on 32-bits at all. KCOV is not
supported on i386 and on arm I got no warnings even with obviously
wrong signatures (e.g. short).
Using "(unsigned long val, void *cases)" fixes the warning on x86_64.
I am still puzzled why gcc considers this as a builtin because we
don't enable -fsanitizer-coverage on this file. I am also puzzled how
UBSAN affects things.
We could change the signature to long, but it feels wrong/dangerous
because the variable should really be 64-bits (long is broken on
32-bits).
Or we could introduce a typedef that is long on 64-bits and 'long
long' on 32-bits.
On Mon, 7 Dec 2020 at 13:38, 'Dmitry Vyukov' via kasan-dev
<[email protected]> wrote:
> On Mon, Dec 7, 2020 at 1:08 PM Dmitry Vyukov <[email protected]> wrote:
> > > > Hi all,
> > > >
> > > > After merging the akpm tree, today's linux-next build (powerpc
> > > > allyesconfig) produced warnings like this:
> > > >
> > > > kernel/kcov.c:296:14: warning: conflicting types for built-in function '__sanitizer_cov_trace_switch'; expected 'void(long unsigned int, void *)' [-Wbuiltin-declaration-mismatch]
> > > > 296 | void notrace __sanitizer_cov_trace_switch(u64 val, u64 *cases)
> > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > Odd. clang wants that signature, according to
> > > https://clang.llvm.org/docs/SanitizerCoverage.html. But gcc seems to
> > > want a different signature. Beats me - best I can do is to cc various
> > > likely culprits ;)
> > >
> > > Which gcc version? Did you recently update gcc?
> > >
> > > > ld: warning: orphan section `.data..Lubsan_data177' from `arch/powerpc/oprofile/op_model_pa6t.o' being placed in section `.data..Lubsan_data177'
> > > >
> > > > (lots of these latter ones)
> > > >
> > > > I don't know what produced these, but it is in the akpm-current or
> > > > akpm trees.
> >
> > I can reproduce this in x86_64 build as well but only if I enable
> > UBSAN as well. There were some recent UBSAN changes by Kees, so maybe
> > that's what affected the warning.
> > Though, the warning itself looks legit and unrelated to UBSAN. In
> > fact, if the compiler expects long and we accept u64, it may be broken
> > on 32-bit arches...
>
> No, I think it works, the argument should be uint64.
>
> I think both gcc and clang signatures are correct and both want
> uint64_t. The question is just how uint64_t is defined :) The old
> printf joke that one can't write portable format specifier for
> uint64_t.
>
> What I know so far:
> clang 11 does not produce this warning even with obviously wrong
> signatures (e.g. short).
> I wasn't able to trigger it with gcc on 32-bits at all. KCOV is not
> supported on i386 and on arm I got no warnings even with obviously
> wrong signatures (e.g. short).
> Using "(unsigned long val, void *cases)" fixes the warning on x86_64.
>
> I am still puzzled why gcc considers this as a builtin because we
> don't enable -fsanitizer-coverage on this file. I am also puzzled how
> UBSAN affects things.
It might be some check-for-builtins check gone wrong if it enables any
one of the sanitizers. That would be confirmed if it works with
UBSAN_SANITIZE_kcov.o := n
> We could change the signature to long, but it feels wrong/dangerous
> because the variable should really be 64-bits (long is broken on
> 32-bits).
> Or we could introduce a typedef that is long on 64-bits and 'long
> long' on 32-bits.
Hi Stephen,
On Fri, 4 Dec 2020 21:00:00 +1100 Stephen Rothwell <[email protected]> wrote:
>
> Hi all,
>
> After merging the akpm tree, today's linux-next build (powerpc
> allyesconfig) produced warnings like this:
>
> ld: warning: orphan section `.data..Lubsan_data177' from `arch/powerpc/oprofile/op_model_pa6t.o' being placed in section `.data..Lubsan_data177'
>
> (lots of these latter ones)
781584 of them today!
> I don't know what produced these, but it is in the akpm-current or
> akpm trees.
Presumably the result of commit
186c3e18dba3 ("ubsan: enable for all*config builds")
from the akpm-current tree.
arch/powerpc/kernel/vmlinux.lds.S has:
#ifdef CONFIG_PPC32
.data : AT(ADDR(.data) - LOAD_OFFSET) {
DATA_DATA
#ifdef CONFIG_UBSAN
*(.data..Lubsan_data*)
*(.data..Lubsan_type*)
#endif
*(.data.rel*)
*(SDATA_MAIN)
added by commit
beba24ac5913 ("powerpc/32: Add .data..Lubsan_data*/.data..Lubsan_type* sections explicitly")
in 2018, but no equivalent for 64 bit.
I will try the following patch tomorrow:
From: Stephen Rothwell <[email protected]>
Date: Tue, 8 Dec 2020 22:58:24 +1100
Subject: [PATCH] powerpc: Add .data..Lubsan_data*/.data..Lubsan_type* sections explicitly
Similarly to commit
beba24ac5913 ("powerpc/32: Add .data..Lubsan_data*/.data..Lubsan_type* sections explicitly")
since CONFIG_UBSAN bits can now be enabled for all*config.
Signed-off-by: Stephen Rothwell <[email protected]>
---
arch/powerpc/kernel/vmlinux.lds.S | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index 3b4c26e94328..0318ba436f34 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -296,6 +296,10 @@ SECTIONS
#else
.data : AT(ADDR(.data) - LOAD_OFFSET) {
DATA_DATA
+#ifdef CONFIG_UBSAN
+ *(.data..Lubsan_data*)
+ *(.data..Lubsan_type*)
+#endif
*(.data.rel*)
*(.toc1)
*(.branch_lt)
--
2.29.2
--
Cheers,
Stephen Rothwell
Hi Michael,
On Wed, 09 Dec 2020 15:44:35 +1100 Michael Ellerman <[email protected]> wrote:
>
> They should really be in DATA_DATA or similar shouldn't they?
No other architecture appears t need them ...
--
Cheers,
Stephen Rothwell
Stephen Rothwell <[email protected]> writes:
> Hi Stephen,
>
> On Fri, 4 Dec 2020 21:00:00 +1100 Stephen Rothwell <[email protected]> wrote:
>>
>> Hi all,
>>
>> After merging the akpm tree, today's linux-next build (powerpc
>> allyesconfig) produced warnings like this:
>>
>> ld: warning: orphan section `.data..Lubsan_data177' from `arch/powerpc/oprofile/op_model_pa6t.o' being placed in section `.data..Lubsan_data177'
>>
>> (lots of these latter ones)
>
> 781584 of them today!
>
>> I don't know what produced these, but it is in the akpm-current or
>> akpm trees.
>
> Presumably the result of commit
>
> 186c3e18dba3 ("ubsan: enable for all*config builds")
>
> from the akpm-current tree.
>
> arch/powerpc/kernel/vmlinux.lds.S has:
>
> #ifdef CONFIG_PPC32
> .data : AT(ADDR(.data) - LOAD_OFFSET) {
> DATA_DATA
> #ifdef CONFIG_UBSAN
> *(.data..Lubsan_data*)
> *(.data..Lubsan_type*)
> #endif
> *(.data.rel*)
> *(SDATA_MAIN)
>
> added by commit
>
> beba24ac5913 ("powerpc/32: Add .data..Lubsan_data*/.data..Lubsan_type* sections explicitly")
>
> in 2018, but no equivalent for 64 bit.
They should really be in DATA_DATA or similar shouldn't they?
cheers
Hi all,
On Tue, 8 Dec 2020 23:01:57 +1100 Stephen Rothwell <[email protected]> wrote:
>
> I will try the following patch tomorrow:
>
> From: Stephen Rothwell <[email protected]>
> Date: Tue, 8 Dec 2020 22:58:24 +1100
> Subject: [PATCH] powerpc: Add .data..Lubsan_data*/.data..Lubsan_type* sections explicitly
>
> Similarly to commit
>
> beba24ac5913 ("powerpc/32: Add .data..Lubsan_data*/.data..Lubsan_type* sections explicitly")
>
> since CONFIG_UBSAN bits can now be enabled for all*config.
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> arch/powerpc/kernel/vmlinux.lds.S | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
> index 3b4c26e94328..0318ba436f34 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -296,6 +296,10 @@ SECTIONS
> #else
> .data : AT(ADDR(.data) - LOAD_OFFSET) {
> DATA_DATA
> +#ifdef CONFIG_UBSAN
> + *(.data..Lubsan_data*)
> + *(.data..Lubsan_type*)
> +#endif
> *(.data.rel*)
> *(.toc1)
> *(.branch_lt)
> --
> 2.29.2
This got rid of all the warnings.
--
Cheers,
Stephen Rothwell
On Tue, Dec 08, 2020 at 11:01:57PM +1100, Stephen Rothwell wrote:
> Hi Stephen,
>
> On Fri, 4 Dec 2020 21:00:00 +1100 Stephen Rothwell <[email protected]> wrote:
> >
> > Hi all,
> >
> > After merging the akpm tree, today's linux-next build (powerpc
> > allyesconfig) produced warnings like this:
> >
> > ld: warning: orphan section `.data..Lubsan_data177' from `arch/powerpc/oprofile/op_model_pa6t.o' being placed in section `.data..Lubsan_data177'
> >
> > (lots of these latter ones)
>
> 781584 of them today!
>
> > I don't know what produced these, but it is in the akpm-current or
> > akpm trees.
>
> Presumably the result of commit
>
> 186c3e18dba3 ("ubsan: enable for all*config builds")
>
> from the akpm-current tree.
>
> arch/powerpc/kernel/vmlinux.lds.S has:
>
> #ifdef CONFIG_PPC32
> .data : AT(ADDR(.data) - LOAD_OFFSET) {
> DATA_DATA
> #ifdef CONFIG_UBSAN
> *(.data..Lubsan_data*)
> *(.data..Lubsan_type*)
> #endif
> *(.data.rel*)
> *(SDATA_MAIN)
>
> added by commit
>
> beba24ac5913 ("powerpc/32: Add .data..Lubsan_data*/.data..Lubsan_type* sections explicitly")
>
> in 2018, but no equivalent for 64 bit.
>
> I will try the following patch tomorrow:
>
> From: Stephen Rothwell <[email protected]>
> Date: Tue, 8 Dec 2020 22:58:24 +1100
> Subject: [PATCH] powerpc: Add .data..Lubsan_data*/.data..Lubsan_type* sections explicitly
>
> Similarly to commit
>
> beba24ac5913 ("powerpc/32: Add .data..Lubsan_data*/.data..Lubsan_type* sections explicitly")
>
> since CONFIG_UBSAN bits can now be enabled for all*config.
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> arch/powerpc/kernel/vmlinux.lds.S | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
> index 3b4c26e94328..0318ba436f34 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -296,6 +296,10 @@ SECTIONS
> #else
> .data : AT(ADDR(.data) - LOAD_OFFSET) {
> DATA_DATA
> +#ifdef CONFIG_UBSAN
> + *(.data..Lubsan_data*)
> + *(.data..Lubsan_type*)
> +#endif
> *(.data.rel*)
> *(.toc1)
> *(.branch_lt)
> --
> 2.29.2
>
> --
> Cheers,
> Stephen Rothwell
Reviewed-by: Kees Cook <[email protected]>
Thanks for figuring this one out. :) Andrew, can you add this to your
ubsan patch stack, or do you want me to resend it to you directly?
--
Kees Cook
On Mon, Dec 7, 2020 at 1:52 PM Marco Elver <[email protected]> wrote:
>
> On Mon, 7 Dec 2020 at 13:38, 'Dmitry Vyukov' via kasan-dev
> <[email protected]> wrote:
> > On Mon, Dec 7, 2020 at 1:08 PM Dmitry Vyukov <[email protected]> wrote:
> > > > > Hi all,
> > > > >
> > > > > After merging the akpm tree, today's linux-next build (powerpc
> > > > > allyesconfig) produced warnings like this:
> > > > >
> > > > > kernel/kcov.c:296:14: warning: conflicting types for built-in function '__sanitizer_cov_trace_switch'; expected 'void(long unsigned int, void *)' [-Wbuiltin-declaration-mismatch]
> > > > > 296 | void notrace __sanitizer_cov_trace_switch(u64 val, u64 *cases)
> > > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >
> > > > Odd. clang wants that signature, according to
> > > > https://clang.llvm.org/docs/SanitizerCoverage.html. But gcc seems to
> > > > want a different signature. Beats me - best I can do is to cc various
> > > > likely culprits ;)
> > > >
> > > > Which gcc version? Did you recently update gcc?
> > > >
> > > > > ld: warning: orphan section `.data..Lubsan_data177' from `arch/powerpc/oprofile/op_model_pa6t.o' being placed in section `.data..Lubsan_data177'
> > > > >
> > > > > (lots of these latter ones)
> > > > >
> > > > > I don't know what produced these, but it is in the akpm-current or
> > > > > akpm trees.
> > >
> > > I can reproduce this in x86_64 build as well but only if I enable
> > > UBSAN as well. There were some recent UBSAN changes by Kees, so maybe
> > > that's what affected the warning.
> > > Though, the warning itself looks legit and unrelated to UBSAN. In
> > > fact, if the compiler expects long and we accept u64, it may be broken
> > > on 32-bit arches...
> >
> > No, I think it works, the argument should be uint64.
> >
> > I think both gcc and clang signatures are correct and both want
> > uint64_t. The question is just how uint64_t is defined :) The old
> > printf joke that one can't write portable format specifier for
> > uint64_t.
> >
> > What I know so far:
> > clang 11 does not produce this warning even with obviously wrong
> > signatures (e.g. short).
> > I wasn't able to trigger it with gcc on 32-bits at all. KCOV is not
> > supported on i386 and on arm I got no warnings even with obviously
> > wrong signatures (e.g. short).
> > Using "(unsigned long val, void *cases)" fixes the warning on x86_64.
> >
> > I am still puzzled why gcc considers this as a builtin because we
> > don't enable -fsanitizer-coverage on this file. I am also puzzled how
> > UBSAN affects things.
>
> It might be some check-for-builtins check gone wrong if it enables any
> one of the sanitizers. That would be confirmed if it works with
>
> UBSAN_SANITIZE_kcov.o := n
Yes, it "fixes" the warning.
Initially I thought it's not a good solution because we want to detect
UBSAN bugs in KCOV. But on second thought, if UBSAN detects a bug in
KCOV, it may lead to infinite recursion. We already disable all other
sanitizers on KCOV for this reason, so it's reasonable to disable
UBSAN as well. And as a side effect it "resolves" the warning as well.
I mailed:
https://lore.kernel.org/lkml/[email protected]/T/#u
Thanks
> > We could change the signature to long, but it feels wrong/dangerous
> > because the variable should really be 64-bits (long is broken on
> > 32-bits).
> > Or we could introduce a typedef that is long on 64-bits and 'long
> > long' on 32-bits.
Stephen Rothwell <[email protected]> writes:
> Hi Michael,
>
> On Wed, 09 Dec 2020 15:44:35 +1100 Michael Ellerman <[email protected]> wrote:
>>
>> They should really be in DATA_DATA or similar shouldn't they?
>
> No other architecture appears t need them ...
Any arch with orphan-handling=warn should see them I thought?
cheers
Hi Michael,
On Thu, 10 Dec 2020 11:19:45 +1100 Michael Ellerman <[email protected]> wrote:
>
> Stephen Rothwell <[email protected]> writes:
> >
> > On Wed, 09 Dec 2020 15:44:35 +1100 Michael Ellerman <[email protected]> wrote:
> >>
> >> They should really be in DATA_DATA or similar shouldn't they?
> >
> > No other architecture appears t need them ...
>
> Any arch with orphan-handling=warn should see them I thought?
I did an x86_64 allyesconfig build (same compiler (more or less) and
same source) and it produces none of these sections. The only
difference in UBSAN CONFIG_ options was that CONFIG_UBSAN_UNREACHABLE
is not set in the x86_64 build.
--
Cheers,
Stephen Rothwell