[RFC] linux/err.h: Refactor IS_ERR_VALUE(x) to improve clang reasoningclang-analyzer/initial_review

Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>

1 files changed, 53 insertions, 0 deletions

diff --git a/include/linux/err.h b/include/linux/err.h
index a139c64aef2a..6ebb9aea23b0 100644
--- a/include/linux/err.h
+++ b/include/linux/err.h
@@ -19,8 +19,61 @@
 
 #ifndef __ASSEMBLY__
 
+#ifdef __clang_analyzer__
+//#define WANT_SIGNED_LONG_IS_ERR_VALUE
+#define WANT_UNSIGNED_LONG_IS_ERR_VALUE
+#endif
+
+#if defined WANT_SIGNED_LONG_IS_ERR_VALUE
+
+/*
+ * This should be an identical check to the "real" IS_ERR_VALUE().
+ *
+ * It's only purpose is to ensure clang-analyzer learns that x != 0
+ * when IS_ERR_VALUE(x) is true. It cannot make this inference from
+ * the normal implementation of IS_ERR_VALUE() above.
+ */
+#define IS_ERR_VALUE(x)                                                        \
+	unlikely(({                                                            \
+		long _l = (long)(void *)(x);                                   \
+		_l < 0 && -MAX_ERRNO <= _l;                                    \
+	}))
+
+#elif defined WANT_UNSIGNED_LONG_IS_ERR_VALUE
+
+/*
+ * This is partway between the two IS_ERR_VALUE() implementations above and
+ * below.
+ *
+ * It is structurally like the one above and explicitly proves that x is
+ * non-zero. However this is not enough to prevent clang-analyzer from later
+ * assuming that x is non-zero.
+ *
+ * This implies that the failure to reason about the range of x is linked to
+ * casting it as an unsigned long.
+ */
+#define IS_ERR_VALUE(x)                                                        \
+	unlikely(({                                                            \
+		unsigned long _l = (unsigned long)(void *)(x);                 \
+		_l != 0 && -MAX_ERRNO <= _l;                                   \
+	}))
+
+#else
+
+/*
+ * This is the original implementation of IS_ERR_VALUE().
+ *
+ * By treating the value as unsigned then the range check can be a single
+ * comparison. Currently clang-analyzer does not use this predicate
+ * to reason about the possible values of x.
+ *
+ * In particular clang-analyzer will explore code paths where
+ * IS_ERR_VALUE(x) is true but then later assume that x is zero.
+ */
 #define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned long)-MAX_ERRNO)
 
+#endif
+
 static inline void * __must_check ERR_PTR(long error)
 {
 	return (void *) error;