lkml.org 
[lkml]   [2008]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
Date
From
SubjectRe: [PATCH] debug: fix BUILD_BUG_ON() for non-constant expressions
Boaz Harrosh wrote:
> Ingo Molnar wrote:
>> * Rusty Russell <rusty@rustcorp.com.au> wrote:
>>
>>> On Monday 18 August 2008 03:33:19 Ingo Molnar wrote:
>>>> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
>>>>> Gag me now.
>>>>>
>>>>> Why not just do
>>>>>
>>>>> #define __BBO(c) sizeof(const char[1 - 2*!!(c)])
>>>>> #define __BBONC(c) __BBO(!__builtin_constant_p(c))
>>>>> #define BUILD_BUG_ON_ZERO(c) (__BBO(c) - __BBONC(c))
>>>>> #define BUILD_BUG_ON(c) (void)BUILD_BUG_ON_ZERO(c)
>>>>>
>>>>> and be done with it?
>>>> yeah, i first tried a few variants of that (compile-time warnings are
>>>> much better than link time warnings), but none worked when i tested
>>>> various failure modes.
>>> Hey, I thought I was the "undisputed ruler of Ugly-land".
>>>
>>> How about this instead:
>>>
>>> #define BUILD_BUG_ON(condition) \
>>> do { \
>>> static struct { char arr[1 - 2*!!(condition)]; } x __maybe_unused; \
>>> } while(0)
>> hm, have you tried it and do we get a severe enough link error about
>> that? If the macro gets ignored by the compiler that's really a hard
>> error - such things are essential safeguards of kernel sanity:
>>
>> /*
>> * Build-time sanity checks on the kernel image and module
>> * area mappings. (these are purely build-time and produce no code)
>> */
>> BUILD_BUG_ON(MODULES_VADDR < KERNEL_IMAGE_START);
>> BUILD_BUG_ON(MODULES_VADDR-KERNEL_IMAGE_START < KERNEL_IMAGE_SIZE);
>> BUILD_BUG_ON(MODULES_LEN + KERNEL_IMAGE_SIZE > 2*PUD_SIZE);
>> BUILD_BUG_ON((KERNEL_IMAGE_START & ~PMD_MASK) != 0);
>> BUILD_BUG_ON((MODULES_VADDR & ~PMD_MASK) != 0);
>> BUILD_BUG_ON(!(MODULES_VADDR > __START_KERNEL));
>> BUILD_BUG_ON(!(((MODULES_END - 1) & PGDIR_MASK) ==
>> (__START_KERNEL & PGDIR_MASK)));
>> BUILD_BUG_ON(__fix_to_virt(__end_of_fixed_addresses) <= MODULES_END);
>>
>> ( and propagating them into runtime failures not only increases bloat,
>> it also makes failures harder to debug. These checks 'run' _early_. )
>>
>> Link time warnings are easy enough to miss.
>>
>> So unless there's a better way of doing it all at compile time (i'd
>> really prefer that!) i'd prefer the link time error about botched
>> BUILD_BUG_ON() conditions - as my commits introduce.
>>
>> Ingo
>> --
>
> #define BUILD_BUG_ON(condition) \
> do {
> enum { bad = !!(condition)}; \
> static struct { char arr[1 - 2*bad]; } x __maybe_unused; \
> } while(0)
>
> the enum definition will not let in anything not compile-time constant.
> But then I fail on: (include/linux/virtio_config.h:99)
>
> if (__builtin_constant_p(fbit))
> BUILD_BUG_ON(fbit >= 32);
>
> is that code broken?
>
> Boaz
>

with this patch:
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index aaa998f..91d98f2 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -468,7 +468,12 @@ struct sysinfo {
};

/* Force a compilation error if condition is true */
-#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+// #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#define BUILD_BUG_ON(condition) \
+do { \
+ enum { bad = !!(condition)}; \
+ static struct { char arr[1 - 2*bad]; } x __maybe_unused;\
+} while(0)

/* Force a compilation error if condition is true, but also produce a
result (of value 0 and type size_t), so the expression can be used

I found another BUILD_BUG_ON() none const bug, here:

diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index e4765b7..1469a38 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -5133,9 +5133,9 @@ static void niu_init_tx_bmac(struct niu *np, u64 min, u64 max)

static void niu_init_tx_mac(struct niu *np)
{
- u64 min, max;
+ u64 max;

- min = 64;
+ enum { min = 64 } ;
if (np->dev->mtu > ETH_DATA_LEN)
max = 9216;
else


\
 
 \ /
  Last update: 2008-08-18 14:35    [W:0.097 / U:0.540 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site