Re: [PATCH] compiler.h: don't use temporary variable in __compiletime_assert()

[James Hogan]

Hi Johannes,

On 13/05/14 08:31, Johannes Berg wrote:
> On Mon, 2014-05-12 at 14:16 -0700, Andrew Morton wrote:
> 
>>> I don't understand why your patch should break things, I suspect it's
>>> related to the sparse behaviour you're trying to work around, but can we
>>> please drop this patch until a more portable workaround can be found?
>>
>> Older gcc's often have this problem.
>>
>> I suppose that build bustage is more serious than sparse false
>> positives so yes, let's please try to find an alternative.
> 
> Since most people probably don't use sparse without compiling (in fact
> it's pretty difficult to do so) I'll just send a patch to disable
> __compiletime_assert for sparse - any objections?

I came up with the hack below yesterday evening. Is that the sort of
thing you mean? (feel free to use this or do something else, just my
2p).

Cheers
James


Subject: [PATCH] compiler.h: avoid sparse errors in
 __compiletime_error_fallback()

Usually, BUG_ON and friends aren't even evaluated in sparse, but
recently compiletime_assert_atomic_type() was added, and that now
results in a sparse warning every time it is used.

The reason turns out to be the temporary variable, after it sparse no
longer considers the value to be a constant, and results in a warning
and an error. The error is the more annoying part of this as it
suppresses any further warnings in the same file, hiding other problems.

Unfortunately the condition cannot be simply expanded out to avoid the
temporary variable since it breaks compiletime_assert on old versions of
GCC such as GCC 4.2.4 which the latest metag compiler is based on.

Therefore #ifndef __CHECKER__ out the __compiletime_error_fallback which
uses the potentially negative size array to trigger a conditional
compiler error, so that sparse doesn't see it.

Signed-off-by: James Hogan <james.hogan@xxxxxxxxxx>
Cc: Johannes Berg <johannes.berg@xxxxxxxxx>
Cc: Daniel Santos <daniel.santos@xxxxxxxxx>
Cc: Luciano Coelho <luciano.coelho@xxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
It's not particularly pretty, if you can think of a better solution that
doesn't break old GCC I'm all ears.
---
 include/linux/compiler.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index ee7239ea1583..64fdfe1cfcf0 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -323,9 +323,18 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
 #endif
 #ifndef __compiletime_error
 # define __compiletime_error(message)
-# define __compiletime_error_fallback(condition) \
+/*
+ * Sparse complains of variable sized arrays due to the temporary variable in
+ * __compiletime_assert. Unfortunately we can't just expand it out to make
+ * sparse see a constant array size without breaking compiletime_assert on old
+ * versions of GCC (e.g. 4.2.4), so hide the array from sparse altogether.
+ */
+# ifndef __CHECKER__
+#  define __compiletime_error_fallback(condition) \
 	do { ((void)sizeof(char[1 - 2 * condition])); } while (0)
-#else
+# endif
+#endif
+#ifndef __compiletime_error_fallback
 # define __compiletime_error_fallback(condition) do { } while (0)
 #endif
 
-- 
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/