diff mbox series

[RFC,bpf-next,37/52] rcupdate: fix access helpers for incomplete struct pointers on GCC < 10

Message ID 20220628194812.1453059-38-alexandr.lobakin@intel.com (mailing list archive)
State RFC
Delegated to: BPF
Headers show
Series bpf, xdp: introduce and use Generic Hints/metadata | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-1 fail Logs for Kernel LATEST on ubuntu-latest with gcc
bpf/vmtest-bpf-next-VM_Test-2 fail Logs for Kernel LATEST on ubuntu-latest with llvm-15
bpf/vmtest-bpf-next-VM_Test-3 fail Logs for Kernel LATEST on z15 with gcc
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/apply fail Patch does not apply to bpf-next

Commit Message

Alexander Lobakin June 28, 2022, 7:47 p.m. UTC
It's been found that currently it is impossible to use RCU for
incomplete struct pointers.
RCU access helpers have the following construct:

	typeof(*p) *local = ...

GCC versions older than 10 don't look at the whole sentence and
believe that there's a dereference happening inside the typeof(),
although it does not.
As RCU doesn't imply any dereference, but only the way to store and
access pointers, this is not a valid case. Moreover, Clang and GCC
10 onwards evaluate it with no issues.
Fix this by introducing a new macro, __rcutype(), which will take
care of pointer annotations inside the RCU access helpers, in two
different ways depending on the compiler used. For sane compilers,
leave it as it is for now, as it ensures that the passed argument
is a pointer, and for the affected ones use...
`typeof(0 ? (p) : (p))`. As:

void fc(void) { }

...
	pr_info("%d", __builtin_types_compatible(typeof(*fn) *, typeof(fn)));
	pr_info("%d", __builtin_types_compatible(typeof(*fn) *, typeof(&fn)));
	pr_info("%d", __builtin_types_compatible(typeof(*fn) *,
						 typeof(0 ? (fn) : (fn)));

emits:

011

and we can't use the second for non-functions.

Fixes: ca5ecddfa8fc ("rcu: define __rcu address space modifier for sparse")
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
---
 include/linux/rcupdate.h | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)
diff mbox series

Patch

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 1a32036c918c..f5971fccf852 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -358,18 +358,33 @@  static inline void rcu_preempt_sleep_check(void) { }
  * (e.g., __srcu), should this make sense in the future.
  */
 
+/*
+ * Unfortunately, GCC versions older than 10 don't look at the whole sentence
+ * and treat `typeof(*(p)) *` as dereferencing although it is not. This makes
+ * it impossible to use those helpers with pointers to incomplete structures.
+ * Plain `typeof(p)` is not the same, as `typeof(func)` returns the type of a
+ * function, not a pointer to it, as `typeof(*(func)) *` does.
+ * `typeof(<anything> ? (func) : (func))` is silly; however, it works just as
+ * the original definition.
+ */
+#if defined(CONFIG_CC_IS_GCC) && CONFIG_GCC_VERSION < 100000
+#define __rcutype(p, ...)	typeof(0 ? (p) : (p)) __VA_ARGS__
+#else
+#define __rcutype(p, ...)	typeof(*(p)) __VA_ARGS__ *
+#endif
+
 #ifdef __CHECKER__
 #define rcu_check_sparse(p, space) \
-	((void)(((typeof(*p) space *)p) == p))
+	((void)((__rcutype(p, space))(p) == (p)))
 #else /* #ifdef __CHECKER__ */
 #define rcu_check_sparse(p, space)
 #endif /* #else #ifdef __CHECKER__ */
 
 #define __unrcu_pointer(p, local)					\
 ({									\
-	typeof(*p) *local = (typeof(*p) *__force)(p);			\
+	__rcutype(p) local = (__rcutype(p, __force))(p);		\
 	rcu_check_sparse(p, __rcu);					\
-	((typeof(*p) __force __kernel *)(local)); 			\
+	((__rcutype(p, __force __kernel))(local)); 			\
 })
 /**
  * unrcu_pointer - mark a pointer as not being RCU protected
@@ -382,29 +397,29 @@  static inline void rcu_preempt_sleep_check(void) { }
 
 #define __rcu_access_pointer(p, local, space) \
 ({ \
-	typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
+	__rcutype(p) local = (__rcutype(p, __force))READ_ONCE(p); \
 	rcu_check_sparse(p, space); \
-	((typeof(*p) __force __kernel *)(local)); \
+	((__rcutype(p, __force __kernel))(local)); \
 })
 #define __rcu_dereference_check(p, local, c, space) \
 ({ \
 	/* Dependency order vs. p above. */ \
-	typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
+	__rcutype(p) local = (__rcutype(p, __force))READ_ONCE(p); \
 	RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_check() usage"); \
 	rcu_check_sparse(p, space); \
-	((typeof(*p) __force __kernel *)(local)); \
+	((__rcutype(p, __force __kernel))(local)); \
 })
 #define __rcu_dereference_protected(p, local, c, space) \
 ({ \
 	RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_protected() usage"); \
 	rcu_check_sparse(p, space); \
-	((typeof(*p) __force __kernel *)(p)); \
+	((__rcutype(p, __force __kernel))(p)); \
 })
 #define __rcu_dereference_raw(p, local) \
 ({ \
 	/* Dependency order vs. p above. */ \
-	typeof(p) local = READ_ONCE(p); \
-	((typeof(*p) __force __kernel *)(local)); \
+	__rcutype(p) local = READ_ONCE(p); \
+	((__rcutype(p, __force __kernel))(local)); \
 })
 #define rcu_dereference_raw(p) __rcu_dereference_raw(p, __UNIQUE_ID(rcu))
 
@@ -412,7 +427,7 @@  static inline void rcu_preempt_sleep_check(void) { }
  * RCU_INITIALIZER() - statically initialize an RCU-protected global variable
  * @v: The value to statically initialize with.
  */
-#define RCU_INITIALIZER(v) (typeof(*(v)) __force __rcu *)(v)
+#define RCU_INITIALIZER(v) (__rcutype(v, __force __rcu))(v)
 
 /**
  * rcu_assign_pointer() - assign to RCU-protected pointer