2021-12-04 12:59:15

by Marco Elver

[permalink] [raw]
Subject: [PATCH -rcu] kcsan: Turn barrier instrumentation into macros

Some architectures use barriers in 'extern inline' functions, from which
we should not refer to static inline functions.

For example, building Alpha with gcc and W=1 shows:

./include/asm-generic/barrier.h:70:30: warning: 'kcsan_rmb' is static but used in inline function 'pmd_offset' which is not static
70 | #define smp_rmb() do { kcsan_rmb(); __smp_rmb(); } while (0)
| ^~~~~~~~~
./arch/alpha/include/asm/pgtable.h:293:9: note: in expansion of macro 'smp_rmb'
293 | smp_rmb(); /* see above */
| ^~~~~~~

Which seems to warn about 6.7.4#3 of the C standard:
"An inline definition of a function with external linkage shall not
contain a definition of a modifiable object with static or thread
storage duration, and shall not contain a reference to an identifier
with internal linkage."

Fix it by turning barrier instrumentation into macros, which matches
definitions in <asm/barrier.h>.

Perhaps we can revert this change in future, when there are no more
'extern inline' users left.

Link: https://lkml.kernel.org/r/[email protected]
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
---
include/linux/kcsan-checks.h | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h
index 9d2c869167f2..92f3843d9ebb 100644
--- a/include/linux/kcsan-checks.h
+++ b/include/linux/kcsan-checks.h
@@ -241,28 +241,30 @@ static inline void __kcsan_disable_current(void) { }
* disabled with the __no_kcsan function attribute.
*
* Also see definition of __tsan_atomic_signal_fence() in kernel/kcsan/core.c.
+ *
+ * These are all macros, like <asm/barrier.h>, since some architectures use them
+ * in non-static inline functions.
*/
#define __KCSAN_BARRIER_TO_SIGNAL_FENCE(name) \
- static __always_inline void kcsan_##name(void) \
- { \
+ do { \
barrier(); \
__atomic_signal_fence(__KCSAN_BARRIER_TO_SIGNAL_FENCE_##name); \
barrier(); \
- }
-__KCSAN_BARRIER_TO_SIGNAL_FENCE(mb)
-__KCSAN_BARRIER_TO_SIGNAL_FENCE(wmb)
-__KCSAN_BARRIER_TO_SIGNAL_FENCE(rmb)
-__KCSAN_BARRIER_TO_SIGNAL_FENCE(release)
+ } while (0)
+#define kcsan_mb() __KCSAN_BARRIER_TO_SIGNAL_FENCE(mb)
+#define kcsan_wmb() __KCSAN_BARRIER_TO_SIGNAL_FENCE(wmb)
+#define kcsan_rmb() __KCSAN_BARRIER_TO_SIGNAL_FENCE(rmb)
+#define kcsan_release() __KCSAN_BARRIER_TO_SIGNAL_FENCE(release)
#elif defined(CONFIG_KCSAN_WEAK_MEMORY) && defined(__KCSAN_INSTRUMENT_BARRIERS__)
#define kcsan_mb __kcsan_mb
#define kcsan_wmb __kcsan_wmb
#define kcsan_rmb __kcsan_rmb
#define kcsan_release __kcsan_release
#else /* CONFIG_KCSAN_WEAK_MEMORY && ... */
-static inline void kcsan_mb(void) { }
-static inline void kcsan_wmb(void) { }
-static inline void kcsan_rmb(void) { }
-static inline void kcsan_release(void) { }
+#define kcsan_mb() do { } while (0)
+#define kcsan_wmb() do { } while (0)
+#define kcsan_rmb() do { } while (0)
+#define kcsan_release() do { } while (0)
#endif /* CONFIG_KCSAN_WEAK_MEMORY && ... */

/**
--
2.34.1.400.ga245620fadb-goog



2021-12-04 15:12:40

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH -rcu] kcsan: Turn barrier instrumentation into macros

On Sat, Dec 04, 2021 at 01:57:03PM +0100, Marco Elver wrote:
> Some architectures use barriers in 'extern inline' functions, from which
> we should not refer to static inline functions.
>
> For example, building Alpha with gcc and W=1 shows:
>
> ./include/asm-generic/barrier.h:70:30: warning: 'kcsan_rmb' is static but used in inline function 'pmd_offset' which is not static
> 70 | #define smp_rmb() do { kcsan_rmb(); __smp_rmb(); } while (0)
> | ^~~~~~~~~
> ./arch/alpha/include/asm/pgtable.h:293:9: note: in expansion of macro 'smp_rmb'
> 293 | smp_rmb(); /* see above */
> | ^~~~~~~
>
> Which seems to warn about 6.7.4#3 of the C standard:
> "An inline definition of a function with external linkage shall not
> contain a definition of a modifiable object with static or thread
> storage duration, and shall not contain a reference to an identifier
> with internal linkage."
>
> Fix it by turning barrier instrumentation into macros, which matches
> definitions in <asm/barrier.h>.
>
> Perhaps we can revert this change in future, when there are no more
> 'extern inline' users left.
>
> Link: https://lkml.kernel.org/r/[email protected]
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Marco Elver <[email protected]>

Queued and pushed, thank you!

Thanx, Paul

> ---
> include/linux/kcsan-checks.h | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/kcsan-checks.h b/include/linux/kcsan-checks.h
> index 9d2c869167f2..92f3843d9ebb 100644
> --- a/include/linux/kcsan-checks.h
> +++ b/include/linux/kcsan-checks.h
> @@ -241,28 +241,30 @@ static inline void __kcsan_disable_current(void) { }
> * disabled with the __no_kcsan function attribute.
> *
> * Also see definition of __tsan_atomic_signal_fence() in kernel/kcsan/core.c.
> + *
> + * These are all macros, like <asm/barrier.h>, since some architectures use them
> + * in non-static inline functions.
> */
> #define __KCSAN_BARRIER_TO_SIGNAL_FENCE(name) \
> - static __always_inline void kcsan_##name(void) \
> - { \
> + do { \
> barrier(); \
> __atomic_signal_fence(__KCSAN_BARRIER_TO_SIGNAL_FENCE_##name); \
> barrier(); \
> - }
> -__KCSAN_BARRIER_TO_SIGNAL_FENCE(mb)
> -__KCSAN_BARRIER_TO_SIGNAL_FENCE(wmb)
> -__KCSAN_BARRIER_TO_SIGNAL_FENCE(rmb)
> -__KCSAN_BARRIER_TO_SIGNAL_FENCE(release)
> + } while (0)
> +#define kcsan_mb() __KCSAN_BARRIER_TO_SIGNAL_FENCE(mb)
> +#define kcsan_wmb() __KCSAN_BARRIER_TO_SIGNAL_FENCE(wmb)
> +#define kcsan_rmb() __KCSAN_BARRIER_TO_SIGNAL_FENCE(rmb)
> +#define kcsan_release() __KCSAN_BARRIER_TO_SIGNAL_FENCE(release)
> #elif defined(CONFIG_KCSAN_WEAK_MEMORY) && defined(__KCSAN_INSTRUMENT_BARRIERS__)
> #define kcsan_mb __kcsan_mb
> #define kcsan_wmb __kcsan_wmb
> #define kcsan_rmb __kcsan_rmb
> #define kcsan_release __kcsan_release
> #else /* CONFIG_KCSAN_WEAK_MEMORY && ... */
> -static inline void kcsan_mb(void) { }
> -static inline void kcsan_wmb(void) { }
> -static inline void kcsan_rmb(void) { }
> -static inline void kcsan_release(void) { }
> +#define kcsan_mb() do { } while (0)
> +#define kcsan_wmb() do { } while (0)
> +#define kcsan_rmb() do { } while (0)
> +#define kcsan_release() do { } while (0)
> #endif /* CONFIG_KCSAN_WEAK_MEMORY && ... */
>
> /**
> --
> 2.34.1.400.ga245620fadb-goog
>