linux-kernel - Re: [PATCH] utilize _Static_assert() for BUILD_BUG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <50C9217B.1010801@att.net>
Date:	Wed, 12 Dec 2012 18:29:47 -0600
From:	Daniel Santos <danielfsantos@....net>
To:	Rusty Russell <rusty@...tcorp.com.au>
CC:	Jan Beulich <JBeulich@...e.com>, david.daney@...ium.com,
	kosaki.motohiro@...fujitsu.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the
 compiler supports it

Wow, it's really easy to miss parallel development on the same issue.  
Sorry for my late response to this thread.  I started another thread 
addressing these issues (as well as a few others) back in September 
(https://lkml.org/lkml/2012/9/28/1136).  I've finally gotten ACKs from 
maintainers with v6 of the patches (here 
https://lkml.org/lkml/2012/11/20/621) and I'm just waiting for 3.8-rc1 
to re-submit them.  I actually submitted these patches back in June as 
part of a larger patch set, but broke it apart in September (I had way 
to many changes for one patch set)


On 11/07/2012 05:24 PM, Rusty Russell wrote:
> Jan Beulich<JBeulich@...e.com>  writes:
>>>>> On 07.11.12 at 02:03, Rusty Russell<rusty@...tcorp.com.au>  wrote:
>>>>> __COUNTER__:
>>>>>          Used to make a unique id.  Let's define __UNIQUE_ID(prefix) for
>>>>>          this (using __COUNTER__ or __LINE__). 4.3 and above.
>>>> The fallback to __LINE__ is not necessarily correct in all cases,
>>>> namely when deep macro expansions result in two instances used
>>>> on the same source line, or a use in a header matches line number
>>>> wise a second use in another header or the source file.
>>>>
>>>> I considered to option of a fallback (and hence an abstraction in
>>>> compiler*.h) when doing this, but I didn't want to do something
>>>> that would break in perhaps vary obscure ways.
>>> I was thinking of my own code in moduleparam.h, but elfnote.h does the
>>> same trick.  In both cases, it'll simply fail compile if we fallback and
>>> __LINE__ isn't unique.
>>>
>>> So again, I don't think we're going to see many uses, and no existing
>>> use will bite us.
>> That's exactly the point - we can't know (because there's no
>> guarantee there aren't - or won't be by the time it might get
>> merged - any two conflicting uses of BUILD_BUG_ON...().
> Conflicting BUILD_BUG_ON() is completely harmless: two identical
> undefines are OK.  The other cases cause a compile error, and one which
> is trivial to fix, and I'm happy to fix it if it does.
>
> I've never seen a construction in the kernel which would silently break
> if __UNIQUE_ID() isn't actually unique.  That makes sense, because you
> if the symbol wasn't exposed you wouldn't need a __UNIQUE_ID.  And if
> it's exposed, the compiler will give an error on multiple definition.
>
> (Obviously you can't have non-static __UNIQUE_ID() because it's only
> ever unique across a single gcc compile).
I was considering __COUNTER__, but in the end, others felt that the 
inconsistency could cause problems later.  However, with the 
__compiletime_error function attribute, the result will only be that the 
actual error message emitted will be incorrect, since the first error 
attribute assigned to the extern function will be used.  Thus, compiling 
this code:

{extern void func(void) __attribute__((error("hello")));}
{extern void func(void) __attribute__((error("goodbye"))); func();}

would result in an error with the message "hello".

>>>> #define __build_bug_on_failed(n) __build_bug_on_##n##_failed
>>>> #define _BUILD_BUG_ON(n, condition)				\
>>>> 	do {							\
>>>> 		extern void __compiletime_error(#condition)	\
>>>> 		__build_bug_on_failed(n)(void);			\
>>>> 		if (condition) __build_bug_on_failed(n)();	\
>>>> 	} while(0)
>> Actually, I just noticed that __linktime_error() really is a compile
>> time error when gcc supports __attribute__(__error__()), so
>> probably using that one instead of __compiletime_error() here
>> would be the cleaner approach.
>>
>> The one thing puzzling me here is that __linktime_error() gets
>> defined even if __CHECKER__ isn't defined, while
>> __compiletime_error() doesn't - an apparent inconsistency
>> between 746a2a838deec3ef86ef6b7c3edd4207b9a351aa
>> (Kosaki?) and 1399ff86f2a2bbacbbe68fa00c5f8c752b344723
>> (David?), with apparently the latter being too lax, as it only
>> introduced a use inside a !__CHECKER__ section.
> Yes, __linktime_error() makes no sense, since it's identical to
> __compiletime_error().  It's also a terrible name.  Let's kill it.
Definitely.  My patch set kills that sucker too.
>
> I hadn't noticed BUILD_BUG.  We should fix that as discussed here, and
> simply make BUILD_BUG_ON() call it.
>
>>> Subject: __UNIQUE_ID()
>>>
>>> Jan Beulich points out __COUNTER__ (gcc 4.3 and above), so let's use
>>> that to create unique ids.  This is better than __LINE__ which we use
>>> today, so provide a wrapper.
>>>
>>> Signed-off-by: Rusty Russell<rusty@...tcorp.com.au>
>> Acked-by: Jan Beulich<jbeulich@...e.com>
> Thanks,
> Rusty.
https://lkml.org/lkml/2012/9/28/1136
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/