Re: [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems
> On 16 Feb 2023, at 7:09 pm, Marco Elver wrote:
>
> On Thu, Feb 16, 2023 at 07:12AM +, Christophe Leroy wrote:
>>
>>
>> Le 16/02/2023 à 06:09, Rohan McLure a écrit :
>>> KCSAN instruments calls to atomic builtins, and will in turn call these
>>> builtins itself. As such, architectures supporting KCSAN must have
>>> compiler support for these atomic primitives.
>>>
>>> Since 32-bit systems are unlikely to have 64-bit compiler builtins,
>>> provide a stub for each missing builtin, and use BUG() to assert
>>> unreachability.
>>>
>>> In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these
>>> locally. Move these definitions to be accessible to all 32-bit
>>> architectures that do not provide the necessary builtins, with opt in
>>> for PowerPC and xtensa.
>>>
>>> Signed-off-by: Rohan McLure
>>> Reviewed-by: Max Filippov
>>
>> This series should also be addressed to KCSAN Maintainers, shouldn't it ?
>>
>> KCSAN
>> M: Marco Elver
>> R: Dmitry Vyukov
>> L: kasan-...@googlegroups.com
>> S: Maintained
>> F: Documentation/dev-tools/kcsan.rst
>> F: include/linux/kcsan*.h
>> F: kernel/kcsan/
>> F: lib/Kconfig.kcsan
>> F: scripts/Makefile.kcsan
>>
>>
>>> ---
>>> Previously issued as a part of a patch series adding KCSAN support to
>>> 64-bit.
>>> Link:
>>> https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4...@ellerman.id.au/T/#t
>>> v1: Remove __has_builtin check, as gcc is not obligated to inline
>>> builtins detected using this check, but instead is permitted to supply
>>> them in libatomic:
>>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734
>>> Instead, opt-in PPC32 and xtensa.
>>> ---
>>> arch/xtensa/lib/Makefile | 1 -
>>> kernel/kcsan/Makefile | 2 ++
>>> arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0
>>> 3 files changed, 2 insertions(+), 1 deletion(-)
>>> rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%)
>>>
>>> diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile
>>> index 7ecef0519a27..d69356dc97df 100644
>>> --- a/arch/xtensa/lib/Makefile
>>> +++ b/arch/xtensa/lib/Makefile
>>> @@ -8,5 +8,4 @@ lib-y += memcopy.o memset.o checksum.o \
>>> divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.o \
>>> usercopy.o strncpy_user.o strnlen_user.o
>>> lib-$(CONFIG_PCI) += pci-auto.o
>>> -lib-$(CONFIG_KCSAN) += kcsan-stubs.o
>>> KCSAN_SANITIZE_kcsan-stubs.o := n
>>> diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
>>> index 8cf70f068d92..86dd713d8855 100644
>>> --- a/kernel/kcsan/Makefile
>>> +++ b/kernel/kcsan/Makefile
>>> @@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \
>>> -fno-stack-protector -DDISABLE_BRANCH_PROFILING
>>>
>>> obj-y := core.o debugfs.o report.o
>>> +obj-$(CONFIG_PPC32) += stubs.o
>>> +obj-$(CONFIG_XTENSA) += stubs.o
>>
>> Not sure it is acceptable to do it that way.
>>
>> There should likely be something like a CONFIG_ARCH_WANTS_KCSAN_STUBS in
>> KCSAN's Kconfig then PPC32 and XTENSA should select it.
>
> The longer I think about it, since these stubs all BUG() anyway, perhaps
> we ought to just avoid them altogether. If you delete all the stubs from
> ppc and xtensa, but do this:
>
> | diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
> | index 54d077e1a2dc..8169d6dadd0e 100644
> | --- a/kernel/kcsan/core.c
> | +++ b/kernel/kcsan/core.c
> | @@ -1261,7 +1261,9 @@ static __always_inline void
> kcsan_atomic_builtin_memorder(int memorder)
> | DEFINE_TSAN_ATOMIC_OPS(8);
> | DEFINE_TSAN_ATOMIC_OPS(16);
> | DEFINE_TSAN_ATOMIC_OPS(32);
> | +#ifdef CONFIG_64BIT
> | DEFINE_TSAN_ATOMIC_OPS(64);
> | +#endif
> |
> | void __tsan_atomic_thread_fence(int memorder);
> | void __tsan_atomic_thread_fence(int memorder)
>
> Does that work?
This makes much more sense. Rather than assume that kcsan is the only
consumer of __atomic_*_8, and stubbing accordingly, we should just
remove its mention from relevant sub-archs.
Re: [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems
On Thu, Feb 16, 2023 at 07:12AM +, Christophe Leroy wrote:
>
>
> Le 16/02/2023 à 06:09, Rohan McLure a écrit :
> > KCSAN instruments calls to atomic builtins, and will in turn call these
> > builtins itself. As such, architectures supporting KCSAN must have
> > compiler support for these atomic primitives.
> >
> > Since 32-bit systems are unlikely to have 64-bit compiler builtins,
> > provide a stub for each missing builtin, and use BUG() to assert
> > unreachability.
> >
> > In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these
> > locally. Move these definitions to be accessible to all 32-bit
> > architectures that do not provide the necessary builtins, with opt in
> > for PowerPC and xtensa.
> >
> > Signed-off-by: Rohan McLure
> > Reviewed-by: Max Filippov
>
> This series should also be addressed to KCSAN Maintainers, shouldn't it ?
>
> KCSAN
> M:Marco Elver
> R:Dmitry Vyukov
> L:kasan-...@googlegroups.com
> S:Maintained
> F:Documentation/dev-tools/kcsan.rst
> F:include/linux/kcsan*.h
> F:kernel/kcsan/
> F:lib/Kconfig.kcsan
> F:scripts/Makefile.kcsan
>
>
> > ---
> > Previously issued as a part of a patch series adding KCSAN support to
> > 64-bit.
> > Link:
> > https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4...@ellerman.id.au/T/#t
> > v1: Remove __has_builtin check, as gcc is not obligated to inline
> > builtins detected using this check, but instead is permitted to supply
> > them in libatomic:
> > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734
> > Instead, opt-in PPC32 and xtensa.
> > ---
> > arch/xtensa/lib/Makefile | 1 -
> > kernel/kcsan/Makefile | 2 ++
> > arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0
> > 3 files changed, 2 insertions(+), 1 deletion(-)
> > rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%)
> >
> > diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile
> > index 7ecef0519a27..d69356dc97df 100644
> > --- a/arch/xtensa/lib/Makefile
> > +++ b/arch/xtensa/lib/Makefile
> > @@ -8,5 +8,4 @@ lib-y += memcopy.o memset.o checksum.o \
> >divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.o \
> >usercopy.o strncpy_user.o strnlen_user.o
> > lib-$(CONFIG_PCI) += pci-auto.o
> > -lib-$(CONFIG_KCSAN) += kcsan-stubs.o
> > KCSAN_SANITIZE_kcsan-stubs.o := n
> > diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
> > index 8cf70f068d92..86dd713d8855 100644
> > --- a/kernel/kcsan/Makefile
> > +++ b/kernel/kcsan/Makefile
> > @@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \
> > -fno-stack-protector -DDISABLE_BRANCH_PROFILING
> >
> > obj-y := core.o debugfs.o report.o
> > +obj-$(CONFIG_PPC32) += stubs.o
> > +obj-$(CONFIG_XTENSA) += stubs.o
>
> Not sure it is acceptable to do it that way.
>
> There should likely be something like a CONFIG_ARCH_WANTS_KCSAN_STUBS in
> KCSAN's Kconfig then PPC32 and XTENSA should select it.
The longer I think about it, since these stubs all BUG() anyway, perhaps
we ought to just avoid them altogether. If you delete all the stubs from
ppc and xtensa, but do this:
| diff --git a/kernel/kcsan/core.c b/kernel/kcsan/core.c
| index 54d077e1a2dc..8169d6dadd0e 100644
| --- a/kernel/kcsan/core.c
| +++ b/kernel/kcsan/core.c
| @@ -1261,7 +1261,9 @@ static __always_inline void
kcsan_atomic_builtin_memorder(int memorder)
| DEFINE_TSAN_ATOMIC_OPS(8);
| DEFINE_TSAN_ATOMIC_OPS(16);
| DEFINE_TSAN_ATOMIC_OPS(32);
| +#ifdef CONFIG_64BIT
| DEFINE_TSAN_ATOMIC_OPS(64);
| +#endif
|
| void __tsan_atomic_thread_fence(int memorder);
| void __tsan_atomic_thread_fence(int memorder)
Does that work?
Re: [PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems
Le 16/02/2023 à 06:09, Rohan McLure a écrit :
> KCSAN instruments calls to atomic builtins, and will in turn call these
> builtins itself. As such, architectures supporting KCSAN must have
> compiler support for these atomic primitives.
>
> Since 32-bit systems are unlikely to have 64-bit compiler builtins,
> provide a stub for each missing builtin, and use BUG() to assert
> unreachability.
>
> In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these
> locally. Move these definitions to be accessible to all 32-bit
> architectures that do not provide the necessary builtins, with opt in
> for PowerPC and xtensa.
>
> Signed-off-by: Rohan McLure
> Reviewed-by: Max Filippov
This series should also be addressed to KCSAN Maintainers, shouldn't it ?
KCSAN
M: Marco Elver
R: Dmitry Vyukov
L: kasan-...@googlegroups.com
S: Maintained
F: Documentation/dev-tools/kcsan.rst
F: include/linux/kcsan*.h
F: kernel/kcsan/
F: lib/Kconfig.kcsan
F: scripts/Makefile.kcsan
> ---
> Previously issued as a part of a patch series adding KCSAN support to
> 64-bit.
> Link:
> https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4...@ellerman.id.au/T/#t
> v1: Remove __has_builtin check, as gcc is not obligated to inline
> builtins detected using this check, but instead is permitted to supply
> them in libatomic:
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734
> Instead, opt-in PPC32 and xtensa.
> ---
> arch/xtensa/lib/Makefile | 1 -
> kernel/kcsan/Makefile | 2 ++
> arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0
> 3 files changed, 2 insertions(+), 1 deletion(-)
> rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%)
>
> diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile
> index 7ecef0519a27..d69356dc97df 100644
> --- a/arch/xtensa/lib/Makefile
> +++ b/arch/xtensa/lib/Makefile
> @@ -8,5 +8,4 @@ lib-y += memcopy.o memset.o checksum.o \
> divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.o \
> usercopy.o strncpy_user.o strnlen_user.o
> lib-$(CONFIG_PCI) += pci-auto.o
> -lib-$(CONFIG_KCSAN) += kcsan-stubs.o
> KCSAN_SANITIZE_kcsan-stubs.o := n
> diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
> index 8cf70f068d92..86dd713d8855 100644
> --- a/kernel/kcsan/Makefile
> +++ b/kernel/kcsan/Makefile
> @@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \
> -fno-stack-protector -DDISABLE_BRANCH_PROFILING
>
> obj-y := core.o debugfs.o report.o
> +obj-$(CONFIG_PPC32) += stubs.o
> +obj-$(CONFIG_XTENSA) += stubs.o
Not sure it is acceptable to do it that way.
There should likely be something like a CONFIG_ARCH_WANTS_KCSAN_STUBS in
KCSAN's Kconfig then PPC32 and XTENSA should select it.
>
> KCSAN_INSTRUMENT_BARRIERS_selftest.o := y
> obj-$(CONFIG_KCSAN_SELFTEST) += selftest.o
> diff --git a/arch/xtensa/lib/kcsan-stubs.c b/kernel/kcsan/stubs.c
> similarity index 100%
> rename from arch/xtensa/lib/kcsan-stubs.c
> rename to kernel/kcsan/stubs.c
[PATCH 1/2] kcsan: xtensa: Add atomic builtin stubs for 32-bit systems
KCSAN instruments calls to atomic builtins, and will in turn call these
builtins itself. As such, architectures supporting KCSAN must have
compiler support for these atomic primitives.
Since 32-bit systems are unlikely to have 64-bit compiler builtins,
provide a stub for each missing builtin, and use BUG() to assert
unreachability.
In commit 725aea873261 ("xtensa: enable KCSAN"), xtensa implements these
locally. Move these definitions to be accessible to all 32-bit
architectures that do not provide the necessary builtins, with opt in
for PowerPC and xtensa.
Signed-off-by: Rohan McLure
Reviewed-by: Max Filippov
---
Previously issued as a part of a patch series adding KCSAN support to
64-bit.
Link:
https://lore.kernel.org/linuxppc-dev/167646486000.1421441.10070059569986228558.b4...@ellerman.id.au/T/#t
v1: Remove __has_builtin check, as gcc is not obligated to inline
builtins detected using this check, but instead is permitted to supply
them in libatomic:
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108734
Instead, opt-in PPC32 and xtensa.
---
arch/xtensa/lib/Makefile | 1 -
kernel/kcsan/Makefile | 2 ++
arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c | 0
3 files changed, 2 insertions(+), 1 deletion(-)
rename arch/xtensa/lib/kcsan-stubs.c => kernel/kcsan/stubs.c (100%)
diff --git a/arch/xtensa/lib/Makefile b/arch/xtensa/lib/Makefile
index 7ecef0519a27..d69356dc97df 100644
--- a/arch/xtensa/lib/Makefile
+++ b/arch/xtensa/lib/Makefile
@@ -8,5 +8,4 @@ lib-y += memcopy.o memset.o checksum.o \
divsi3.o udivsi3.o modsi3.o umodsi3.o mulsi3.o umulsidi3.o \
usercopy.o strncpy_user.o strnlen_user.o
lib-$(CONFIG_PCI) += pci-auto.o
-lib-$(CONFIG_KCSAN) += kcsan-stubs.o
KCSAN_SANITIZE_kcsan-stubs.o := n
diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
index 8cf70f068d92..86dd713d8855 100644
--- a/kernel/kcsan/Makefile
+++ b/kernel/kcsan/Makefile
@@ -12,6 +12,8 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \
-fno-stack-protector -DDISABLE_BRANCH_PROFILING
obj-y := core.o debugfs.o report.o
+obj-$(CONFIG_PPC32) += stubs.o
+obj-$(CONFIG_XTENSA) += stubs.o
KCSAN_INSTRUMENT_BARRIERS_selftest.o := y
obj-$(CONFIG_KCSAN_SELFTEST) += selftest.o
diff --git a/arch/xtensa/lib/kcsan-stubs.c b/kernel/kcsan/stubs.c
similarity index 100%
rename from arch/xtensa/lib/kcsan-stubs.c
rename to kernel/kcsan/stubs.c
--
2.37.2
