[PATCH] linux/bug.h: make BUILD_BUG_ON generate compile-time error

[PATCH] linux/bug.h: make BUILD_BUG_ON generate compile-time error

[Daniel Santos]

[-- Attachment #1: Type: text/plain, Size: 637 bytes --]

This is pretty straight-forward. __compiletime_error is defined in
compiler-gcc4.h with appropriate version checks, so shouldn't break on
older compilers. This makes the difference between getting an error at
link-time and getting one at compile time. Example:

/home/daniel/proj/kernel/grbtest/grbtest2.c:83:2: error: call to
‘__build_bug_on_failed’ declared with attribute error: BUILD_BUG_ON
failed: !__builtin_constant_p(count)

The above points me directly to where I called BUILD_BUG_ON. Note that
I'm moving __build_bug_on_failed out of the global scope, just so it can
be re-declared with different attributes each time.

Daniel

[-- Attachment #2: 0006-linux-bug.h-make-BUILD_BUG_ON-generate-compile-time-.patch --]
[-- Type: text/x-patch, Size: 1076 bytes --]

>From 3af9a38225e6d074ef6b22e6c99c2503bc581953 Mon Sep 17 00:00:00 2001
From: Daniel Santos <daniel.santos@pobox.com>
Date: Mon, 4 Jun 2012 06:13:41 -0500
Subject: linux/bug.h: make BUILD_BUG_ON generate compile-time error

---
 include/linux/bug.h |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/bug.h b/include/linux/bug.h
index 72961c3..d35d6e9 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -48,11 +48,12 @@ struct pt_regs;
 #ifndef __OPTIMIZE__
 #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
 #else
-extern int __build_bug_on_failed;
-#define BUILD_BUG_ON(condition)					\
-	do {							\
-		((void)sizeof(char[1 - 2*!!(condition)]));	\
-		if (condition) __build_bug_on_failed = 1;	\
+#define BUILD_BUG_ON(condition)						\
+	do {								\
+		extern void __build_bug_on_failed(void)			\
+		__compiletime_error("BUILD_BUG_ON failed: " #condition);\
+		((void)sizeof(char[1 - 2*!!(condition)]));		\
+		if (condition) __build_bug_on_failed();			\
 	} while(0)
 #endif
 
-- 
1.7.3.4

Re: [PATCH] linux/bug.h: make BUILD_BUG_ON generate compile-time error

[Paul Gortmaker]

On 12-06-04 07:35 AM, Daniel Santos wrote:
> This is pretty straight-forward. __compiletime_error is defined in
> compiler-gcc4.h with appropriate version checks, so shouldn't break on
> older compilers. This makes the difference between getting an error at
> link-time and getting one at compile time. Example:

It is such a rarity for it to be triggered, that I wonder if it
really matters whether it happens at compile time, or you wait
the extra minute for link time.  Regardless, you've got your
commit log written here, and not in the patch (which was attached
and not inline; preventing comment) and you've not got any
Signed-off line. 

> 
> /home/daniel/proj/kernel/grbtest/grbtest2.c:83:2: error: call to
> ‘__build_bug_on_failed’ declared with attribute error: BUILD_BUG_ON
> failed: !__builtin_constant_p(count)

Also the string "BUILD_BUG_ON failed" is in itself misleading, as if
the macro itself has issues, vs. say using the word "triggered" or 
"tripped" vs. "failed".  You also don't want to put the whole
meaningless paths in commit logs normally (the /home/me/mydir part).

> 
> The above points me directly to where I called BUILD_BUG_ON. Note that
> I'm moving __build_bug_on_failed out of the global scope, just so it can
> be re-declared with different attributes each time.

It wasn't clear to me why you'd want to re-declare things with
different attributes ever.

Paul.

> 
> Daniel

Re: [PATCH] linux/bug.h: make BUILD_BUG_ON generate compile-time error

[Daniel Santos]

On 06/04/2012 09:32 AM, Paul Gortmaker wrote:
> On 12-06-04 07:35 AM, Daniel Santos wrote:
>> This is pretty straight-forward. __compiletime_error is defined in
>> compiler-gcc4.h with appropriate version checks, so shouldn't break on
>> older compilers. This makes the difference between getting an error at
>> link-time and getting one at compile time. Example:
> It is such a rarity for it to be triggered, that I wonder if it
> really matters whether it happens at compile time, or you wait
> the extra minute for link time.  Regardless, you've got your
> commit log written here, and not in the patch (which was attached
> and not inline; preventing comment) and you've not got any
> Signed-off line. 
I apologize for the quality of patch issues.  I've just read about "git
--signoff" and I'll do that next time.  As far as the patch being
attached instead of inline, I haven't gotten my mailer client to not
screw them up yet (I'm using Thunderbird, and apparently I need to
install an external editor add-on or switch to another mail client).

As for the rarity, it's fairly common for me right now since I'm working
on stuff that depends upon compile-time constants and
__builtin_constant_p tests, since I have so many of them, it can be
tricky to track down which one in particular fails.  Since gcc added
this feature specifically for these types of checks, I thought it
appropriate that it should be used here.
>> /home/daniel/proj/kernel/grbtest/grbtest2.c:83:2: error: call to
>> ‘__build_bug_on_failed’ declared with attribute error: BUILD_BUG_ON
>> failed: !__builtin_constant_p(count)
> Also the string "BUILD_BUG_ON failed" is in itself misleading, as if
> the macro itself has issues, vs. say using the word "triggered" or 
> "tripped" vs. "failed".  You also don't want to put the whole
> meaningless paths in commit logs normally (the /home/me/mydir part).
ok, good point, I guess I'm thinking along the lines of a assertions
"failing".  So maybe "BUILD_BUG_ON triggered by: " #condition?
>
>> The above points me directly to where I called BUILD_BUG_ON. Note that
>> I'm moving __build_bug_on_failed out of the global scope, just so it can
>> be re-declared with different attributes each time.
> It wasn't clear to me why you'd want to re-declare things with
> different attributes ever.
Sorry for not clarifying this. If I have two lines like this:

BUILD_BUG_ON(!__builtin_constant_p(count));
BUILD_BUG_ON(size < 1024);

Each will declare extern void __build_bug_on_failed(void), but with
different attributes.  The first will be delcared with
__attribute__((error("BUILD_BUG_ON failed:
!__builtin_constant_p(count)"))) and the second with
__attribute__((error("BUILD_BUG_ON failed: size < 1024"))).  The
advantage is that the error output will tell you exactly what triggered
the failure, which can be especially important if you have a macro that
expands to more than one BUILD_BUG_ON check on the same line.

But I guess the really sad part that both of us have missed is that
BUILD_BUG() already uses this attribute, except via a different macro --
which means that now, in compiler-gcc4.h, we have two macros that both
expand to __attribute__(((error(msg))), except that __linktime_error
uses "__error__", where __compiletime_error uses "error".  Also, this
feature was introduced in 4.3.0 and I was not able to find any bug
reports indicating that it had problems, so that would mean the compiler
version check for __linktime_error (4.3.x) is correct and
__compiletime_error (4.4.x) is 1 minor revision late.

Then again, perhaps these macros exist to be turned off by __CHECKER__.

Daniel