linux-kernel - Re: [PATCH v4 6/9] compiler.h, bug.h: Prevent double error messages with 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: <20121031110609.GD16410@liondog.tnic>
Date:	Wed, 31 Oct 2012 12:06:10 +0100
From:	Borislav Petkov <bp@...en8.de>
To:	Daniel Santos <daniel.santos@...ox.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Andi Kleen <ak@...ux.intel.com>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christopher Li <sparse@...isli.org>,
	David Daney <david.daney@...ium.com>,
	David Howells <dhowells@...hat.com>,
	Joe Perches <joe@...ches.com>,
	Josh Triplett <josh@...htriplett.org>,
	Konstantin Khlebnikov <khlebnikov@...nvz.org>,
	linux-sparse@...r.kernel.org,
	Michel Lespinasse <walken@...gle.com>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Pavel Pisa <pisa@....felk.cvut.cz>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Steven Rostedt <rostedt@...dmis.org>,
	David Rientjes <rientjes@...gle.com>
Subject: Re: [PATCH v4 6/9] compiler.h, bug.h: Prevent double error messages
 with BUILD_BUG{,_ON}

On Wed, Oct 31, 2012 at 12:34:45AM -0500, Daniel Santos wrote:
> Yes, the __build_bug_on_failed message is much more informative.  This
> will only increase with these patches.  For example, the line
> 
> BUILD_BUG_ON(sizeof(*c) != 4);
> 
> emits this error:
> 
> arch/x86/kernel/cpu/amd.c: In function ‘early_init_amd’:
> arch/x86/kernel/cpu/amd.c:486:2: error: call to
> ‘__build_bug_on_failed_486’ declared with attribute error: BUILD_BUG_ON
> failed: sizeof(*c) != 4
> make[1]: *** [arch/x86/kernel/cpu/amd.o] Error 1
> make: *** [arch/x86/kernel/cpu/amd.o] Error 2
> 
> It's true that there is some redundancy in there as well as the
> gibberish line number embedded in the function name, but the end of the
> line spits out the exact statement that failed.

I guess that's as good as it gets. But it's fine IMO, it tells you
exactly what you need to know.

> But as far as rather the fallback is first or the __compiletime_error
> function is a matter of asthetics, since it's really an either/or
> situation.  Either the __build_bug_on_failedxxx function will be
> declared with __attribute__((error(message))) and the fallback will
> expand to a no-op, or the fallback will produce code that (presumably
> always?) breaks the build.  For insurance, a link-time error will occur
> if the fallback code fails to break the build.

Right, but my suggestion was to have the more informative message always
trigger first, if possible and if gcc supports it (practically, more
and more systems will be upgrading gcc which has the error attribute
with time) and have the less informative one be the more seldom one. The
"fallback" naming is just a minor issue.

This way, the error message would be precise on most modern toolchains.
Older toolchains will issue something about negative array size, which
is not really helpful so one would have to fire up an editor and
actually look at the code :).

> Realistically, a single macro could be defined in compiler*.h that
> encapsulates the entirety of this mechanism and only exposes a "black
> box" macro, that will simply expand to something that breaks the build
> in the most appropriate fashion based upon the version of gcc.  In
> essence, the new BUILD_BUG_ON_MSG macro attempts to fill that roll.

Yes.

> I guess I'll fix it up (and address the emails on the other patches)
> and do a v5 then for the whole set? (is that the right way to resubmit
> with these corrections?)

Well, you could wait a couple of days first to gather feedback from
other people and then resend. This way you give chance to people to take
a look without them seeing too many versions of the patchset and getting
confused.

What I always do is send out the patchset, collect and discuss changes,
add in the required changes and test it while the discussions go on.
After they settle down (and they do in a couple of days, in most cases)
I then send out the newly tested version out.

That whole exercise takes more or less a week if you're doing other
stuff in between :)

-- 
Regards/Gruss,
    Boris.
--
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/