2009-10-13 07:08:55

by Uwe Kleine-König

[permalink] [raw]
Subject: [PATCH] serial/atmel_serial: Fix another fallout of the change to BUILD_BUG_ON

Commit 8c87df457cb5 fixed BUILD_BUG_ON with the result that some
expressions (e.g. "not really constant" ones) result in a build failure.

Some of these were fixed in 8c87df457cb5, but not serial/atmel_serial.

Signed-off-by: Uwe Kleine-König <[email protected]>
Cc: Jan Beulich <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Haavard Skinnemoen <[email protected]>
Cc: Alan Cox <[email protected]>
---
Hello,

currently 27 arm defconfigs fail because of that error, see

http://armlinux.simtec.co.uk/kautobuild/2.6.32-rc4-git1/errors.html

Best regards
Uwe

drivers/serial/atmel_serial.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
index 3551c5c..b37d75f 100644
--- a/drivers/serial/atmel_serial.c
+++ b/drivers/serial/atmel_serial.c
@@ -1531,7 +1531,7 @@ static int __devinit atmel_serial_probe(struct platform_device *pdev)
void *data;
int ret;

- BUILD_BUG_ON(!is_power_of_2(ATMEL_SERIAL_RINGSIZE));
+ MAYBE_BUILD_BUG_ON(!is_power_of_2(ATMEL_SERIAL_RINGSIZE));

port = &atmel_ports[pdev->id];
port->backup_imr = 0;
--
1.6.4.3


2009-10-13 07:33:15

by Haavard Skinnemoen

[permalink] [raw]
Subject: Re: [PATCH] serial/atmel_serial: Fix another fallout of the change to BUILD_BUG_ON

Uwe Kleine-König <[email protected]> wrote:
> Commit 8c87df457cb5 fixed BUILD_BUG_ON with the result that some
> expressions (e.g. "not really constant" ones) result in a build failure.
>
> Some of these were fixed in 8c87df457cb5, but not serial/atmel_serial.

This patch fixes the same issue:

http://lkml.org/lkml/2009/10/6/305

> - BUILD_BUG_ON(!is_power_of_2(ATMEL_SERIAL_RINGSIZE));
> + MAYBE_BUILD_BUG_ON(!is_power_of_2(ATMEL_SERIAL_RINGSIZE));

What's the difference between BUILD_BUG_ON() and MAYBE_BUILD_BUG_ON()?

Haavard

2009-10-13 07:40:43

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH] serial/atmel_serial: Fix another fallout of the change to BUILD_BUG_ON

>>> Haavard Skinnemoen <[email protected]> 13.10.09 09:26 >>>
>Uwe Kleine-König <[email protected]> wrote:
>> Commit 8c87df457cb5 fixed BUILD_BUG_ON with the result that some
>> expressions (e.g. "not really constant" ones) result in a build failure.
>>
>> Some of these were fixed in 8c87df457cb5, but not serial/atmel_serial.
>
>This patch fixes the same issue:
>
>http://lkml.org/lkml/2009/10/6/305
>
>> - BUILD_BUG_ON(!is_power_of_2(ATMEL_SERIAL_RINGSIZE));
>> + MAYBE_BUILD_BUG_ON(!is_power_of_2(ATMEL_SERIAL_RINGSIZE));
>
>What's the difference between BUILD_BUG_ON() and MAYBE_BUILD_BUG_ON()?

The latter (at present) generally only serves as an annotation. It should
probably be renamed and converted to the (linking time) detecting
mechanism Rusty suggested (though I'm not sure that model would
allow all non-constant [at parsing time] uses to be detected - clearly
there would remain potential for build issues if the compiler isn't able
to reduce the expression to a constant during optimization).

Jan

2009-10-13 08:25:30

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH] serial/atmel_serial: Fix another fallout of the change to BUILD_BUG_ON

Hello Andrew,

On Tue, Oct 13, 2009 at 09:26:44AM +0200, Haavard Skinnemoen wrote:
> Uwe Kleine-K?nig <[email protected]> wrote:
> > Commit 8c87df457cb5 fixed BUILD_BUG_ON with the result that some
> > expressions (e.g. "not really constant" ones) result in a build failure.
> >
> > Some of these were fixed in 8c87df457cb5, but not serial/atmel_serial.
>
> This patch fixes the same issue:
>
> http://lkml.org/lkml/2009/10/6/305
Can you please take this and add my Ack?

If you cannot find it anymore in your inbox, you can get it from
http://article.gmane.org/gmane.linux.kernel/898575/raw

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |

2009-10-19 06:08:48

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] serial/atmel_serial: Fix another fallout of the change to BUILD_BUG_ON

On Tue, 13 Oct 2009 06:10:01 pm Jan Beulich wrote:
> >>> Haavard Skinnemoen <[email protected]> 13.10.09 09:26 >>>
> >Uwe Kleine-König <[email protected]> wrote:
> >> Commit 8c87df457cb5 fixed BUILD_BUG_ON with the result that some
> >> expressions (e.g. "not really constant" ones) result in a build failure.
> >>
> >> Some of these were fixed in 8c87df457cb5, but not serial/atmel_serial.
> >
> >This patch fixes the same issue:
> >
> >http://lkml.org/lkml/2009/10/6/305
> >
> >> - BUILD_BUG_ON(!is_power_of_2(ATMEL_SERIAL_RINGSIZE));
> >> + MAYBE_BUILD_BUG_ON(!is_power_of_2(ATMEL_SERIAL_RINGSIZE));
> >
> >What's the difference between BUILD_BUG_ON() and MAYBE_BUILD_BUG_ON()?
>
> The latter (at present) generally only serves as an annotation. It should
> probably be renamed and converted to the (linking time) detecting
> mechanism Rusty suggested (though I'm not sure that model would
> allow all non-constant [at parsing time] uses to be detected - clearly
> there would remain potential for build issues if the compiler isn't able
> to reduce the expression to a constant during optimization).

How's this? It's not quite valid C, but it "works":

Subject: [PATCH] Rename, strengthen MAYBE_BUILD_BUG_ON()

There are some cases where we really want the parser to break if it can,
but BUG_ON() if it can't. We can do that using sizeof() and a BUG_ON():
the compiler will only emit the test if it is non-constant.

Signed-off-by: Rusty Russell <[email protected]>

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -220,7 +220,7 @@ static inline enum zone_type gfp_zone(gf
((1 << ZONES_SHIFT) - 1);

if (__builtin_constant_p(bit))
- MAYBE_BUILD_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
+ BUILD_OR_RUNTIME_BUG_ON((GFP_ZONE_BAD >> bit) & 1);
else {
#ifdef CONFIG_DEBUG_VM
BUG_ON((GFP_ZONE_BAD >> bit) & 1);
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -686,8 +686,16 @@ struct sysinfo {
/* Force a compilation error if condition is true */
#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))

-/* Force a compilation error if condition is constant and true */
-#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
+/**
+ * BUILD_OR_RUNTIME_BUG_ON - break compile or BUG_ON() when run.
+ * cond: condition which should be false.
+ *
+ * There are cases where compile should simply break if the compiler
+ * can deduce the condition is true at parse time; if it can't, it
+ * should be tested at runtime.
+ */
+#define BUILD_OR_RUNTIME_BUG_ON(cond) \
+ BUG_ON(sizeof(char[1 - 2 * !!(cond)]) != 1)

/* 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
diff --git a/include/linux/kmemcheck.h b/include/linux/kmemcheck.h
--- a/include/linux/kmemcheck.h
+++ b/include/linux/kmemcheck.h
@@ -152,7 +152,7 @@ static inline bool kmemcheck_is_obj_init
\
_n = (long) &((ptr)->name##_end) \
- (long) &((ptr)->name##_begin); \
- MAYBE_BUILD_BUG_ON(_n < 0); \
+ BUILD_OR_RUNTIME_BUG_ON(_n < 0); \
\
kmemcheck_mark_initialized(&((ptr)->name##_begin), _n); \
} while (0)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -109,7 +109,7 @@ static inline bool virtio_has_feature(co
unsigned int fbit)
{
/* Did you forget to fix assumptions on max features? */
- MAYBE_BUILD_BUG_ON(fbit >= 32);
+ BUILD_OR_RUNTIME_BUG_ON(fbit >= 32);

if (fbit < VIRTIO_TRANSPORT_F_START)
virtio_check_driver_offered_feature(vdev, fbit);

2009-10-19 08:18:49

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH] serial/atmel_serial: Fix another fallout of the change to BUILD_BUG_ON

>>> Rusty Russell <[email protected]> 19.10.09 08:08 >>>
>How's this? It's not quite valid C, but it "works":

But that's not what you proposed initially, i.e. generating a link time
error if a compile time error can't be generated (and only if even a link
time error isn't possible, a run time one should be forced).

And btw., why do you think this isn't valid C?

Jan

2009-10-19 14:32:47

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] serial/atmel_serial: Fix another fallout of the change to BUILD_BUG_ON

On Mon, 19 Oct 2009 06:49:40 pm Jan Beulich wrote:
> >>> Rusty Russell <[email protected]> 19.10.09 08:08 >>>
> >How's this? It's not quite valid C, but it "works":
>
> But that's not what you proposed initially, i.e. generating a link time
> error if a compile time error can't be generated (and only if even a link
> time error isn't possible, a run time one should be forced).

Yeah, this was cleverer. A compile time is nicer than link time. And this
is *actually* what I want: a compile fail if the compiler knows enough, runtime
otherwise.

> And btw., why do you think this isn't valid C?

>From my glance at ISO C, non-positive sized variable length arrays are
invalid. gcc here seems to give the expected results (eg. sizeof gives
a negative result).

Cheers,
Rusty.

2009-10-21 12:28:08

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] serial/atmel_serial: Fix another fallout of the change to BUILD_BUG_ON

On Tue, Oct 13, 2009 at 10:24:47AM +0200, Uwe Kleine-K?nig wrote:
> Hello Andrew,
>
> On Tue, Oct 13, 2009 at 09:26:44AM +0200, Haavard Skinnemoen wrote:
> > Uwe Kleine-K?nig <[email protected]> wrote:
> > > Commit 8c87df457cb5 fixed BUILD_BUG_ON with the result that some
> > > expressions (e.g. "not really constant" ones) result in a build failure.
> > >
> > > Some of these were fixed in 8c87df457cb5, but not serial/atmel_serial.
> >
> > This patch fixes the same issue:
> >
> > http://lkml.org/lkml/2009/10/6/305
> Can you please take this and add my Ack?
>
> If you cannot find it anymore in your inbox, you can get it from
> http://article.gmane.org/gmane.linux.kernel/898575/raw

So what's happening with this?

2009-10-21 13:57:08

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH] serial/atmel_serial: Fix another fallout of the change to BUILD_BUG_ON

Hello,

On Wed, Oct 21, 2009 at 01:27:09PM +0100, Russell King - ARM Linux wrote:
> On Tue, Oct 13, 2009 at 10:24:47AM +0200, Uwe Kleine-K?nig wrote:
> > Hello Andrew,
> >
> > On Tue, Oct 13, 2009 at 09:26:44AM +0200, Haavard Skinnemoen wrote:
> > > Uwe Kleine-K?nig <[email protected]> wrote:
> > > > Commit 8c87df457cb5 fixed BUILD_BUG_ON with the result that some
> > > > expressions (e.g. "not really constant" ones) result in a build failure.
> > > >
> > > > Some of these were fixed in 8c87df457cb5, but not serial/atmel_serial.
> > >
> > > This patch fixes the same issue:
> > >
> > > http://lkml.org/lkml/2009/10/6/305
> > Can you please take this and add my Ack?
> >
> > If you cannot find it anymore in your inbox, you can get it from
> > http://article.gmane.org/gmane.linux.kernel/898575/raw
>
> So what's happening with this?
The patch by Haavard Skinnemoen is in -mm as

atmel_serial-fix-bad-build_bug_on-usage.patch

.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |