Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, 2014-02-27 at 12:55 -0800, Christopher Li wrote:
> On Thu, Feb 27, 2014 at 12:39 PM, Joe Perches wrote:
> > Maybe the evaluate.c "size = bits_in_char;" assignment
> >
> > if (size == 1 && is_bool_type(type)) {
> > - warning(expr->pos, "expression using sizeof bool");
> > + if (Wsizeof_bool)
> > + warning(expr->pos, "expression using sizeof bool");
> > size = bits_in_char;
> > }
> >
> > should be
> >
> > size = sizeof(_Bool) * 8;
>
> The reason to use bits_in_ is to allow sparse application to over write
> the size of int etc. If you don't like the bits_in_char here. You can
> introduce
> bits_in_bool and set that to sizeof(Bool)*8 by default. That way you don't
> hard code it.
There already is a bits_in_bool and it's default 1.
$ git grep -E "\bbits_in_\w+\s*=[^=]"
lib.c: bits_in_long = 64;
lib.c: bits_in_pointer = 64;
target.c:int bits_in_bool = 1;
target.c:int bits_in_char = 8;
target.c:int bits_in_short = 16;
target.c:int bits_in_int = 32;
target.c:int bits_in_long = 32;
target.c:int bits_in_longlong = 64;
target.c:int bits_in_longlonglong = 128;
target.c:int bits_in_float = 32;
target.c:int bits_in_double = 64;
target.c:int bits_in_longdouble = 80;
target.c:int bits_in_pointer = 32;
target.c:int bits_in_enum = 32;
> > And also, in sparse.1, Josh Triplett is shown as
> > the maintainer. Maybe that should be changed to
> > Christopher Li
>
> Maybe a separate patch.
That's fine with me too. If you're the maintainer,
I think you should do that patch. I don't see a need
for me to send any more right now though.
cheers, Joe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 1:00 PM, Joe Perches wrote: > Of course > The change has applied and pushed. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 1:00 PM, Joe Perches wrote: > On Thu, 2014-02-27 at 12:44 -0800, Christopher Li wrote: >> On Thu, Feb 27, 2014 at 12:26 PM, H. Peter Anvin wrote: >> > I would. >> Joe, I assume you are OK with this patch, the default is now off. > > Of course Let me know if you are going to have a V3 or not. Thanks Chris -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, 2014-02-27 at 12:44 -0800, Christopher Li wrote: > On Thu, Feb 27, 2014 at 12:26 PM, H. Peter Anvin wrote: > > I would. > Joe, I assume you are OK with this patch, the default is now off. Of course > diff --git a/lib.c b/lib.c [] > @@ -226,6 +226,7 @@ int Wparen_string = 0; > +int Wsizeof_bool = 0; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 12:39 PM, Joe Perches wrote:
> Please use V3 as I stuffed up the alphabetic order
> of sizeof and shadow.
Please send it your V3 then :-)
>
> I'm not sure it matters much, but the linux-kernel
> Makefile wouldn't need to be changed if Wsizeof_bool
> is default 0.
It seems default to off is the right thing to do.
>
> Here's a couple of other nits:
>
> Maybe the evaluate.c "size = bits_in_char;" assignment
>
> if (size == 1 && is_bool_type(type)) {
> - warning(expr->pos, "expression using sizeof bool");
> + if (Wsizeof_bool)
> + warning(expr->pos, "expression using sizeof bool");
> size = bits_in_char;
> }
>
> should be
>
> size = sizeof(_Bool) * 8;
The reason to use bits_in_ is to allow sparse application to over write
the size of int etc. If you don't like the bits_in_char here. You can introduce
bits_in_bool and set that to sizeof(Bool)*8 by default. That way you don't
hard code it.
> And also, in sparse.1, Josh Triplett is shown as
> the maintainer. Maybe that should be changed to
> Christopher Li
Maybe a separate patch.
Waiting for your V3.
Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 12:26 PM, H. Peter Anvin wrote:
> I would.
>
Joe, I assume you are OK with this patch, the default is now off.
Chris
Allow an override to emit or not the sizeof(bool) warning.
Add a "-Wsizeof-bool" description to the manpage.
Signed-off-by: Joe Perches
Reviewed-by: Josh Triplett
Signed-off-by: Christopher Li
---
evaluate.c | 3 ++-
lib.c | 2 ++
lib.h | 1 +
sparse.1 | 8
4 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/evaluate.c b/evaluate.c
index 6655615..a45f59b 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2057,7 +2057,8 @@ static struct symbol *evaluate_sizeof(struct
expression *expr)
}
if (size == 1 && is_bool_type(type)) {
-warning(expr->pos, "expression using sizeof bool");
+if (Wsizeof_bool)
+warning(expr->pos, "expression using sizeof bool");
size = bits_in_char;
}
diff --git a/lib.c b/lib.c
index 51b97fd..844797d 100644
--- a/lib.c
+++ b/lib.c
@@ -226,6 +226,7 @@ int Wparen_string = 0;
int Wptr_subtraction_blows = 0;
int Wreturn_void = 0;
int Wshadow = 0;
+int Wsizeof_bool = 0;
int Wtransparent_union = 0;
int Wtypesign = 0;
int Wundef = 0;
@@ -438,6 +439,7 @@ static const struct warning {
{ "ptr-subtraction-blows", _subtraction_blows },
{ "return-void", _void },
{ "shadow", },
+{ "sizeof-bool", _bool },
{ "transparent-union", _union },
{ "typesign", },
{ "undef", },
diff --git a/lib.h b/lib.h
index f09b338..f6cd9b4 100644
--- a/lib.h
+++ b/lib.h
@@ -120,6 +120,7 @@ extern int Wparen_string;
extern int Wptr_subtraction_blows;
extern int Wreturn_void;
extern int Wshadow;
+extern int Wsizeof_bool;
extern int Wtransparent_union;
extern int Wtypesign;
extern int Wundef;
diff --git a/sparse.1 b/sparse.1
index cd6be26..54da09b 100644
--- a/sparse.1
+++ b/sparse.1
@@ -297,6 +297,14 @@ Such declarations can lead to error-prone code.
Sparse does not issue these warnings by default.
.
.TP
+.B \-Wsizeof-bool
+Warn when checking the sizeof a _Bool.
+
+C99 does not specify the sizeof a _Bool. gcc uses 1.
+
+Sparse does not issue these warnings by default.
+.
+.TP
.B \-Wtransparent\-union
Warn about any declaration using the GCC extension
\fB__attribute__((transparent_union))\fR.
--
1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, 2014-02-27 at 12:26 -0800, H. Peter Anvin wrote:
> On February 27, 2014 12:22:45 PM PST, Christopher Li
> wrote:
> >OK. I get it nobody wants a sizeof(_Bool) warning.
> >I am going to apply this patch.
Please use V3 as I stuffed up the alphabetic order
of sizeof and shadow.
> >Should we change the default to off then?
> I would.
I'm not sure it matters much, but the linux-kernel
Makefile wouldn't need to be changed if Wsizeof_bool
is default 0.
Here's a couple of other nits:
Maybe the evaluate.c "size = bits_in_char;" assignment
if (size == 1 && is_bool_type(type)) {
- warning(expr->pos, "expression using sizeof bool");
+ if (Wsizeof_bool)
+ warning(expr->pos, "expression using sizeof bool");
size = bits_in_char;
}
should be
size = sizeof(_Bool) * 8;
And also, in sparse.1, Josh Triplett is shown as
the maintainer. Maybe that should be changed to
Christopher Li
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
I would. On February 27, 2014 12:22:45 PM PST, Christopher Li wrote: >On Wed, Feb 26, 2014 at 8:32 PM, H. Peter Anvin wrote: >> >> Quite frankly, this is silly in my opinion, *and* it is not >guaranteed >> by C either (read about "trap representations"). >>> >> Anything that moves data around in a generic fashion. It can be as >> simple as: >> >> memcpy(foo, bar, sizeof *foo); > >OK. I get it nobody wants a sizeof(_Bool) warning. > >I am going to apply this patch. > >Should we change the default to off then? > >Thanks. > >Chris -- Sent from my mobile phone. Please pardon brevity and lack of formatting. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Wed, Feb 26, 2014 at 8:32 PM, H. Peter Anvin wrote: > > Quite frankly, this is silly in my opinion, *and* it is not guaranteed > by C either (read about "trap representations"). >> > Anything that moves data around in a generic fashion. It can be as > simple as: > > memcpy(foo, bar, sizeof *foo); OK. I get it nobody wants a sizeof(_Bool) warning. I am going to apply this patch. Should we change the default to off then? Thanks. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 27/02/14 16:52, H. Peter Anvin wrote: > On 02/27/2014 08:10 AM, Dan Carpenter wrote: >> >> That last assumption has to change for the Meta architecture. >> >> https://lwn.net/Articles/522188/ >> >> On meta, the structs and unions are padded to 4 bytes unless they are >> explicitly marked as __packed. >> > > One have to wonder how likely they are to have that not continually break... Well it's only when it deals with stored/portable data formats that it's an issue, and in those cases some amount of care over packing has usually already been taken. It most often causes problems when one small struct/union containing only chars or shorts is nested inside another struct which is marked as packed (on the unfortunately mistaken assumption that the inner struct cannot have padding at the end so doesn't need packing itself). But yeh, it is a PITA. Cheers James -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 02/27/2014 08:10 AM, Dan Carpenter wrote: > > That last assumption has to change for the Meta architecture. > > https://lwn.net/Articles/522188/ > > On meta, the structs and unions are padded to 4 bytes unless they are > explicitly marked as __packed. > One have to wonder how likely they are to have that not continually break... -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 07:48:35AM -0800, H. Peter Anvin wrote: > > Do we have a fairly comprehensive list of what these extrastandard > > requirements / assumptions are? It might be a good idea to have one > > that we can point to, so that (a) people who are trying to define a > > new architecture knows what they need to handle, (b) and so we can > > give a list of things that static code analyzers like smatch and > > coverity and sparse should be able to suppress (perhaps in a Linux > > kernel-only mode). > > > > No, but I think we can certainly make a list... a lot of it right now > sits in various people's heads. > > Here are a couple: > > - Bytes are 8 bits > - Signed integers will be 2's complement > - sizeof char, short, int, long, and long long will be 1, 2, 4, 4, 8 or > 1, 2, 4, 8, 8 on 32- and 64-bit processors, respectively. > - sizeof(long) == sizeof(void *) > - NULL is represented by all zero > - Structures will not add padding as long as all the members are > naturally aligned. > That last assumption has to change for the Meta architecture. https://lwn.net/Articles/522188/ On meta, the structs and unions are padded to 4 bytes unless they are explicitly marked as __packed. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 07:48:35AM -0800, H. Peter Anvin wrote: > No, but I think we can certainly make a list... a lot of it right now > sits in various people's heads. > > Here are a couple: > > - Bytes are 8 bits > - Signed integers will be 2's complement > - sizeof char, short, int, long, and long long will be 1, 2, 4, 4, 8 or ^ ^ bool, ,1 , > 1, 2, 4, 8, 8 on 32- and 64-bit processors, respectively. > - sizeof(long) == sizeof(void *) > - NULL is represented by all zero > - Structures will not add padding as long as all the members are > naturally aligned. > > Someone want to set up a collaborative document of some kind and collect > more? https://wiki.kernel.org/ but I hear spammers love it. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 02/27/2014 07:24 AM, Theodore Ts'o wrote: > On Thu, Feb 27, 2014 at 07:10:25AM -0800, H. Peter Anvin wrote: >> Keep in mind, too, that for the kernel we don't care about the full >> C standard but a subset. We rely on extrastandard behavior all over >> the place. For all ABIs supported by the kernel, sizeof(_Book) == 1 >> and so everything is sane. > > Do we have a fairly comprehensive list of what these extrastandard > requirements / assumptions are? It might be a good idea to have one > that we can point to, so that (a) people who are trying to define a > new architecture knows what they need to handle, (b) and so we can > give a list of things that static code analyzers like smatch and > coverity and sparse should be able to suppress (perhaps in a Linux > kernel-only mode). > No, but I think we can certainly make a list... a lot of it right now sits in various people's heads. Here are a couple: - Bytes are 8 bits - Signed integers will be 2's complement - sizeof char, short, int, long, and long long will be 1, 2, 4, 4, 8 or 1, 2, 4, 8, 8 on 32- and 64-bit processors, respectively. - sizeof(long) == sizeof(void *) - NULL is represented by all zero - Structures will not add padding as long as all the members are naturally aligned. Someone want to set up a collaborative document of some kind and collect more? -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 07:10:25AM -0800, H. Peter Anvin wrote: > Keep in mind, too, that for the kernel we don't care about the full > C standard but a subset. We rely on extrastandard behavior all over > the place. For all ABIs supported by the kernel, sizeof(_Book) == 1 > and so everything is sane. Do we have a fairly comprehensive list of what these extrastandard requirements / assumptions are? It might be a good idea to have one that we can point to, so that (a) people who are trying to define a new architecture knows what they need to handle, (b) and so we can give a list of things that static code analyzers like smatch and coverity and sparse should be able to suppress (perhaps in a Linux kernel-only mode). - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
Keep in mind, too, that for the kernel we don't care about the full C standard but a subset. We rely on extrastandard behavior all over the place. For all ABIs supported by the kernel, sizeof(_Book) == 1 and so everything is sane. On February 27, 2014 12:25:29 AM PST, Borislav Petkov wrote: >On Wed, Feb 26, 2014 at 07:42:37PM -0800, H. Peter Anvin wrote: >> sizeof(_Bool), like for many other types, is ABI-dependent, but that >> doesn't mean it is illegitimate. >> >> I don't think C99 says that it is invalid (which means C99 doesn't >> permit is to be a packed bitmap.) > >Ok, but what can be said about the __pcpu_size_call() use case where we >do sizeof(bool)? We have there accessors for sizes 1,2,4 and 8. Can we >simply assume that the ABI will give us a size of bool which is one of >those? > >What if sizeof(bool) is 3? > >Or, are we saying that sizeof(bool) will always be of some natural, >native size like byte, short, int or long so we're good there? > >Thanks. -- Sent from my mobile phone. Please pardon brevity and lack of formatting. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Wed, Feb 26, 2014 at 07:42:37PM -0800, H. Peter Anvin wrote: > sizeof(_Bool), like for many other types, is ABI-dependent, but that > doesn't mean it is illegitimate. > > I don't think C99 says that it is invalid (which means C99 doesn't > permit is to be a packed bitmap.) Ok, but what can be said about the __pcpu_size_call() use case where we do sizeof(bool)? We have there accessors for sizes 1,2,4 and 8. Can we simply assume that the ABI will give us a size of bool which is one of those? What if sizeof(bool) is 3? Or, are we saying that sizeof(bool) will always be of some natural, native size like byte, short, int or long so we're good there? Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Wed, Feb 26, 2014 at 07:42:37PM -0800, H. Peter Anvin wrote: sizeof(_Bool), like for many other types, is ABI-dependent, but that doesn't mean it is illegitimate. I don't think C99 says that it is invalid (which means C99 doesn't permit is to be a packed bitmap.) Ok, but what can be said about the __pcpu_size_call() use case where we do sizeof(bool)? We have there accessors for sizes 1,2,4 and 8. Can we simply assume that the ABI will give us a size of bool which is one of those? What if sizeof(bool) is 3? Or, are we saying that sizeof(bool) will always be of some natural, native size like byte, short, int or long so we're good there? Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
Keep in mind, too, that for the kernel we don't care about the full C standard but a subset. We rely on extrastandard behavior all over the place. For all ABIs supported by the kernel, sizeof(_Book) == 1 and so everything is sane. On February 27, 2014 12:25:29 AM PST, Borislav Petkov b...@alien8.de wrote: On Wed, Feb 26, 2014 at 07:42:37PM -0800, H. Peter Anvin wrote: sizeof(_Bool), like for many other types, is ABI-dependent, but that doesn't mean it is illegitimate. I don't think C99 says that it is invalid (which means C99 doesn't permit is to be a packed bitmap.) Ok, but what can be said about the __pcpu_size_call() use case where we do sizeof(bool)? We have there accessors for sizes 1,2,4 and 8. Can we simply assume that the ABI will give us a size of bool which is one of those? What if sizeof(bool) is 3? Or, are we saying that sizeof(bool) will always be of some natural, native size like byte, short, int or long so we're good there? Thanks. -- Sent from my mobile phone. Please pardon brevity and lack of formatting. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 07:10:25AM -0800, H. Peter Anvin wrote: Keep in mind, too, that for the kernel we don't care about the full C standard but a subset. We rely on extrastandard behavior all over the place. For all ABIs supported by the kernel, sizeof(_Book) == 1 and so everything is sane. Do we have a fairly comprehensive list of what these extrastandard requirements / assumptions are? It might be a good idea to have one that we can point to, so that (a) people who are trying to define a new architecture knows what they need to handle, (b) and so we can give a list of things that static code analyzers like smatch and coverity and sparse should be able to suppress (perhaps in a Linux kernel-only mode). - Ted -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 02/27/2014 07:24 AM, Theodore Ts'o wrote: On Thu, Feb 27, 2014 at 07:10:25AM -0800, H. Peter Anvin wrote: Keep in mind, too, that for the kernel we don't care about the full C standard but a subset. We rely on extrastandard behavior all over the place. For all ABIs supported by the kernel, sizeof(_Book) == 1 and so everything is sane. Do we have a fairly comprehensive list of what these extrastandard requirements / assumptions are? It might be a good idea to have one that we can point to, so that (a) people who are trying to define a new architecture knows what they need to handle, (b) and so we can give a list of things that static code analyzers like smatch and coverity and sparse should be able to suppress (perhaps in a Linux kernel-only mode). No, but I think we can certainly make a list... a lot of it right now sits in various people's heads. Here are a couple: - Bytes are 8 bits - Signed integers will be 2's complement - sizeof char, short, int, long, and long long will be 1, 2, 4, 4, 8 or 1, 2, 4, 8, 8 on 32- and 64-bit processors, respectively. - sizeof(long) == sizeof(void *) - NULL is represented by all zero - Structures will not add padding as long as all the members are naturally aligned. Someone want to set up a collaborative document of some kind and collect more? -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 07:48:35AM -0800, H. Peter Anvin wrote: No, but I think we can certainly make a list... a lot of it right now sits in various people's heads. Here are a couple: - Bytes are 8 bits - Signed integers will be 2's complement - sizeof char, short, int, long, and long long will be 1, 2, 4, 4, 8 or ^ ^ bool, ,1 , 1, 2, 4, 8, 8 on 32- and 64-bit processors, respectively. - sizeof(long) == sizeof(void *) - NULL is represented by all zero - Structures will not add padding as long as all the members are naturally aligned. Someone want to set up a collaborative document of some kind and collect more? https://wiki.kernel.org/ but I hear spammers love it. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 07:48:35AM -0800, H. Peter Anvin wrote: Do we have a fairly comprehensive list of what these extrastandard requirements / assumptions are? It might be a good idea to have one that we can point to, so that (a) people who are trying to define a new architecture knows what they need to handle, (b) and so we can give a list of things that static code analyzers like smatch and coverity and sparse should be able to suppress (perhaps in a Linux kernel-only mode). No, but I think we can certainly make a list... a lot of it right now sits in various people's heads. Here are a couple: - Bytes are 8 bits - Signed integers will be 2's complement - sizeof char, short, int, long, and long long will be 1, 2, 4, 4, 8 or 1, 2, 4, 8, 8 on 32- and 64-bit processors, respectively. - sizeof(long) == sizeof(void *) - NULL is represented by all zero - Structures will not add padding as long as all the members are naturally aligned. That last assumption has to change for the Meta architecture. https://lwn.net/Articles/522188/ On meta, the structs and unions are padded to 4 bytes unless they are explicitly marked as __packed. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 02/27/2014 08:10 AM, Dan Carpenter wrote: That last assumption has to change for the Meta architecture. https://lwn.net/Articles/522188/ On meta, the structs and unions are padded to 4 bytes unless they are explicitly marked as __packed. One have to wonder how likely they are to have that not continually break... -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 27/02/14 16:52, H. Peter Anvin wrote: On 02/27/2014 08:10 AM, Dan Carpenter wrote: That last assumption has to change for the Meta architecture. https://lwn.net/Articles/522188/ On meta, the structs and unions are padded to 4 bytes unless they are explicitly marked as __packed. One have to wonder how likely they are to have that not continually break... Well it's only when it deals with stored/portable data formats that it's an issue, and in those cases some amount of care over packing has usually already been taken. It most often causes problems when one small struct/union containing only chars or shorts is nested inside another struct which is marked as packed (on the unfortunately mistaken assumption that the inner struct cannot have padding at the end so doesn't need packing itself). But yeh, it is a PITA. Cheers James -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Wed, Feb 26, 2014 at 8:32 PM, H. Peter Anvin h...@zytor.com wrote: Quite frankly, this is silly in my opinion, *and* it is not guaranteed by C either (read about trap representations). Anything that moves data around in a generic fashion. It can be as simple as: memcpy(foo, bar, sizeof *foo); OK. I get it nobody wants a sizeof(_Bool) warning. I am going to apply this patch. Should we change the default to off then? Thanks. Chris -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
I would. On February 27, 2014 12:22:45 PM PST, Christopher Li spa...@chrisli.org wrote: On Wed, Feb 26, 2014 at 8:32 PM, H. Peter Anvin h...@zytor.com wrote: Quite frankly, this is silly in my opinion, *and* it is not guaranteed by C either (read about trap representations). Anything that moves data around in a generic fashion. It can be as simple as: memcpy(foo, bar, sizeof *foo); OK. I get it nobody wants a sizeof(_Bool) warning. I am going to apply this patch. Should we change the default to off then? Thanks. Chris -- Sent from my mobile phone. Please pardon brevity and lack of formatting. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, 2014-02-27 at 12:26 -0800, H. Peter Anvin wrote:
On February 27, 2014 12:22:45 PM PST, Christopher Li spa...@chrisli.org
wrote:
OK. I get it nobody wants a sizeof(_Bool) warning.
I am going to apply this patch.
Please use V3 as I stuffed up the alphabetic order
of sizeof and shadow.
Should we change the default to off then?
I would.
I'm not sure it matters much, but the linux-kernel
Makefile wouldn't need to be changed if Wsizeof_bool
is default 0.
Here's a couple of other nits:
Maybe the evaluate.c size = bits_in_char; assignment
if (size == 1 is_bool_type(type)) {
- warning(expr-pos, expression using sizeof bool);
+ if (Wsizeof_bool)
+ warning(expr-pos, expression using sizeof bool);
size = bits_in_char;
}
should be
size = sizeof(_Bool) * 8;
And also, in sparse.1, Josh Triplett is shown as
the maintainer. Maybe that should be changed to
Christopher Li
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 12:26 PM, H. Peter Anvin h...@zytor.com wrote:
I would.
Joe, I assume you are OK with this patch, the default is now off.
Chris
Allow an override to emit or not the sizeof(bool) warning.
Add a -Wsizeof-bool description to the manpage.
Signed-off-by: Joe Perches j...@perches.com
Reviewed-by: Josh Triplett j...@joshtriplett.org
Signed-off-by: Christopher Li spa...@chrisli.org
---
evaluate.c | 3 ++-
lib.c | 2 ++
lib.h | 1 +
sparse.1 | 8
4 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/evaluate.c b/evaluate.c
index 6655615..a45f59b 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2057,7 +2057,8 @@ static struct symbol *evaluate_sizeof(struct
expression *expr)
}
if (size == 1 is_bool_type(type)) {
-warning(expr-pos, expression using sizeof bool);
+if (Wsizeof_bool)
+warning(expr-pos, expression using sizeof bool);
size = bits_in_char;
}
diff --git a/lib.c b/lib.c
index 51b97fd..844797d 100644
--- a/lib.c
+++ b/lib.c
@@ -226,6 +226,7 @@ int Wparen_string = 0;
int Wptr_subtraction_blows = 0;
int Wreturn_void = 0;
int Wshadow = 0;
+int Wsizeof_bool = 0;
int Wtransparent_union = 0;
int Wtypesign = 0;
int Wundef = 0;
@@ -438,6 +439,7 @@ static const struct warning {
{ ptr-subtraction-blows, Wptr_subtraction_blows },
{ return-void, Wreturn_void },
{ shadow, Wshadow },
+{ sizeof-bool, Wsizeof_bool },
{ transparent-union, Wtransparent_union },
{ typesign, Wtypesign },
{ undef, Wundef },
diff --git a/lib.h b/lib.h
index f09b338..f6cd9b4 100644
--- a/lib.h
+++ b/lib.h
@@ -120,6 +120,7 @@ extern int Wparen_string;
extern int Wptr_subtraction_blows;
extern int Wreturn_void;
extern int Wshadow;
+extern int Wsizeof_bool;
extern int Wtransparent_union;
extern int Wtypesign;
extern int Wundef;
diff --git a/sparse.1 b/sparse.1
index cd6be26..54da09b 100644
--- a/sparse.1
+++ b/sparse.1
@@ -297,6 +297,14 @@ Such declarations can lead to error-prone code.
Sparse does not issue these warnings by default.
.
.TP
+.B \-Wsizeof-bool
+Warn when checking the sizeof a _Bool.
+
+C99 does not specify the sizeof a _Bool. gcc uses 1.
+
+Sparse does not issue these warnings by default.
+.
+.TP
.B \-Wtransparent\-union
Warn about any declaration using the GCC extension
\fB__attribute__((transparent_union))\fR.
--
1.8.5.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 12:39 PM, Joe Perches j...@perches.com wrote:
Please use V3 as I stuffed up the alphabetic order
of sizeof and shadow.
Please send it your V3 then :-)
I'm not sure it matters much, but the linux-kernel
Makefile wouldn't need to be changed if Wsizeof_bool
is default 0.
It seems default to off is the right thing to do.
Here's a couple of other nits:
Maybe the evaluate.c size = bits_in_char; assignment
if (size == 1 is_bool_type(type)) {
- warning(expr-pos, expression using sizeof bool);
+ if (Wsizeof_bool)
+ warning(expr-pos, expression using sizeof bool);
size = bits_in_char;
}
should be
size = sizeof(_Bool) * 8;
The reason to use bits_in_ is to allow sparse application to over write
the size of int etc. If you don't like the bits_in_char here. You can introduce
bits_in_bool and set that to sizeof(Bool)*8 by default. That way you don't
hard code it.
And also, in sparse.1, Josh Triplett is shown as
the maintainer. Maybe that should be changed to
Christopher Li
Maybe a separate patch.
Waiting for your V3.
Chris
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, 2014-02-27 at 12:44 -0800, Christopher Li wrote: On Thu, Feb 27, 2014 at 12:26 PM, H. Peter Anvin h...@zytor.com wrote: I would. Joe, I assume you are OK with this patch, the default is now off. Of course diff --git a/lib.c b/lib.c [] @@ -226,6 +226,7 @@ int Wparen_string = 0; +int Wsizeof_bool = 0; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 1:00 PM, Joe Perches j...@perches.com wrote: On Thu, 2014-02-27 at 12:44 -0800, Christopher Li wrote: On Thu, Feb 27, 2014 at 12:26 PM, H. Peter Anvin h...@zytor.com wrote: I would. Joe, I assume you are OK with this patch, the default is now off. Of course Let me know if you are going to have a V3 or not. Thanks Chris -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, Feb 27, 2014 at 1:00 PM, Joe Perches j...@perches.com wrote: Of course The change has applied and pushed. Chris -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Thu, 2014-02-27 at 12:55 -0800, Christopher Li wrote:
On Thu, Feb 27, 2014 at 12:39 PM, Joe Perches j...@perches.com wrote:
Maybe the evaluate.c size = bits_in_char; assignment
if (size == 1 is_bool_type(type)) {
- warning(expr-pos, expression using sizeof bool);
+ if (Wsizeof_bool)
+ warning(expr-pos, expression using sizeof bool);
size = bits_in_char;
}
should be
size = sizeof(_Bool) * 8;
The reason to use bits_in_ is to allow sparse application to over write
the size of int etc. If you don't like the bits_in_char here. You can
introduce
bits_in_bool and set that to sizeof(Bool)*8 by default. That way you don't
hard code it.
There already is a bits_in_bool and it's default 1.
$ git grep -E \bbits_in_\w+\s*=[^=]
lib.c: bits_in_long = 64;
lib.c: bits_in_pointer = 64;
target.c:int bits_in_bool = 1;
target.c:int bits_in_char = 8;
target.c:int bits_in_short = 16;
target.c:int bits_in_int = 32;
target.c:int bits_in_long = 32;
target.c:int bits_in_longlong = 64;
target.c:int bits_in_longlonglong = 128;
target.c:int bits_in_float = 32;
target.c:int bits_in_double = 64;
target.c:int bits_in_longdouble = 80;
target.c:int bits_in_pointer = 32;
target.c:int bits_in_enum = 32;
And also, in sparse.1, Josh Triplett is shown as
the maintainer. Maybe that should be changed to
Christopher Li
Maybe a separate patch.
That's fine with me too. If you're the maintainer,
I think you should do that patch. I don't see a need
for me to send any more right now though.
cheers, Joe
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 02/26/2014 08:26 PM, Ben Pfaff wrote: > >> Because sizeof(_Bool) is a little bit special compare to sizeof(long). >> In the case of long, all sizeof(long) * 8 bits are use in the actual value. >> But for the _Bool, only the 1 bit is used in the 8 bits size. In other words, >> the _Bool has a special case of the actual bit size is not a multiple of 8. Quite frankly, this is silly in my opinion, *and* it is not guaranteed by C either (read about "trap representations"). >> Sparse has two hats, it is a C compiler front end, and more often it is >> used in the Linux kernel source sanitize checking. Depending on the sizeof >> _Bool sounds a little bit suspicious in the kernel. I would love to the heard >> your actual usage case of the sizeof(_Bool). Why do you care about this >> warning? Anything that moves data around in a generic fashion. It can be as simple as: memcpy(foo, bar, sizeof *foo); -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Wed, Feb 26, 2014 at 08:19:57PM -0800, H. Peter Anvin wrote: > On 02/26/2014 08:00 PM, Ben Pfaff wrote: > > > > The commit *relaxed* sparse behavior: because previously sizeof(bool) > > was an error. I'm not in favor of any diagnostic at all for > > sizeof(bool), but my recollection is that a sparse maintainer wanted it > > to yield one. > > Still not clear as to why. The discussion is here: http://comments.gmane.org/gmane.comp.parsers.sparse/2462 Quoting from that discussion, the core of Christopher Li's argument was this: > On Mon, May 9, 2011 at 1:02 PM, Ben Pfaff nicira.com> wrote: > > Thank you for applying my patch. It does work for me, in the sense > > that I get a warning instead of an error now, but I'm not so happy to > > get any diagnostic at all. Is there some reason why sizeof(_Bool) > > warrants a warning when, say, sizeof(long) does not? After all, both > > sizes are implementation defined. > Because sizeof(_Bool) is a little bit special compare to sizeof(long). > In the case of long, all sizeof(long) * 8 bits are use in the actual value. > But for the _Bool, only the 1 bit is used in the 8 bits size. In other words, > the _Bool has a special case of the actual bit size is not a multiple of 8. > Sparse has two hats, it is a C compiler front end, and more often it is > used in the Linux kernel source sanitize checking. Depending on the sizeof > _Bool sounds a little bit suspicious in the kernel. I would love to the heard > your actual usage case of the sizeof(_Bool). Why do you care about this > warning? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 02/26/2014 08:00 PM, Ben Pfaff wrote: > > The commit *relaxed* sparse behavior: because previously sizeof(bool) > was an error. I'm not in favor of any diagnostic at all for > sizeof(bool), but my recollection is that a sparse maintainer wanted it > to yield one. > Still not clear as to why. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Wed, Feb 26, 2014 at 07:38:46PM -0800, Joe Perches wrote: > (adding Ben Pfaff and Christopher Li) > > On Wed, 2014-02-26 at 19:29 -0800, H. Peter Anvin wrote: > > On 02/26/2014 06:58 PM, Josh Triplett wrote: > > > On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: > > >> Allow an override to emit or not the sizeof(bool) warning > > >> Add a description to the manpage. > > >> > > >> Signed-off-by: Joe Perches > > > > > > Reviewed-by: Josh Triplett > > > > > > > I have to admit that this particular warning is a bit odd to me. I'm > > wondering what kind of bugs it was intended to catch. > > > > In particular, things that incorrectly assumes the size of bool to be > > anything in particular would seem unlikely to actually use sizeof(). > > Dunno, the commit log for the commit that added it doesn't quite > match the code and is seemingly unaware that the c99 spec doesn't > specify sizeof(bool). The commit *relaxed* sparse behavior: because previously sizeof(bool) was an error. I'm not in favor of any diagnostic at all for sizeof(bool), but my recollection is that a sparse maintainer wanted it to yield one. I don't care about the particular result for sizeof(bool) as long as it matches the ABI. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
sizeof(_Bool), like for many other types, is ABI-dependent, but that doesn't mean it is illegitimate. I don't think C99 says that it is invalid (which means C99 doesn't permit is to be a packed bitmap.) On February 26, 2014 7:38:46 PM PST, Joe Perches wrote: >(adding Ben Pfaff and Christopher Li) > >On Wed, 2014-02-26 at 19:29 -0800, H. Peter Anvin wrote: >> On 02/26/2014 06:58 PM, Josh Triplett wrote: >> > On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: >> >> Allow an override to emit or not the sizeof(bool) warning >> >> Add a description to the manpage. >> >> >> >> Signed-off-by: Joe Perches >> > >> > Reviewed-by: Josh Triplett >> > >> >> I have to admit that this particular warning is a bit odd to me. I'm >> wondering what kind of bugs it was intended to catch. >> >> In particular, things that incorrectly assumes the size of bool to be >> anything in particular would seem unlikely to actually use sizeof(). > >Dunno, the commit log for the commit that added it doesn't quite >match the code and is seemingly unaware that the c99 spec doesn't >specify sizeof(bool). > >Ben Pfaff >Date: Wed May 4 16:39:54 2011 -0700 > >evaluate: Allow sizeof(_Bool) to succeed. > > Without this commit, sizeof(_Bool) provokes an error with "cannot size >expression" because _Bool is a 1-bit type and thus not a multiple of a >full >byte in size. But sizeof(_Bool) is valid C that should evaluate to 1, >so >this commit fixes the problem and adds a regression test. > >Signed-off-by: Ben Pfaff >Signed-off-by: Christopher Li -- Sent from my mobile phone. Please pardon brevity and lack of formatting. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
(adding Ben Pfaff and Christopher Li) On Wed, 2014-02-26 at 19:29 -0800, H. Peter Anvin wrote: > On 02/26/2014 06:58 PM, Josh Triplett wrote: > > On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: > >> Allow an override to emit or not the sizeof(bool) warning > >> Add a description to the manpage. > >> > >> Signed-off-by: Joe Perches > > > > Reviewed-by: Josh Triplett > > > > I have to admit that this particular warning is a bit odd to me. I'm > wondering what kind of bugs it was intended to catch. > > In particular, things that incorrectly assumes the size of bool to be > anything in particular would seem unlikely to actually use sizeof(). Dunno, the commit log for the commit that added it doesn't quite match the code and is seemingly unaware that the c99 spec doesn't specify sizeof(bool). Ben Pfaff Date: Wed May 4 16:39:54 2011 -0700 evaluate: Allow sizeof(_Bool) to succeed. Without this commit, sizeof(_Bool) provokes an error with "cannot size expression" because _Bool is a 1-bit type and thus not a multiple of a full byte in size. But sizeof(_Bool) is valid C that should evaluate to 1, so this commit fixes the problem and adds a regression test. Signed-off-by: Ben Pfaff Signed-off-by: Christopher Li -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 02/26/2014 06:58 PM, Josh Triplett wrote: > On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: >> Allow an override to emit or not the sizeof(bool) warning >> Add a description to the manpage. >> >> Signed-off-by: Joe Perches > > Reviewed-by: Josh Triplett > I have to admit that this particular warning is a bit odd to me. I'm wondering what kind of bugs it was intended to catch. In particular, things that incorrectly assumes the size of bool to be anything in particular would seem unlikely to actually use sizeof(). -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote:
> Allow an override to emit or not the sizeof(bool) warning
> Add a description to the manpage.
>
> Signed-off-by: Joe Perches
Reviewed-by: Josh Triplett
> evaluate.c | 3 ++-
> lib.c | 2 ++
> lib.h | 1 +
> sparse.1 | 9 +
> 4 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/evaluate.c b/evaluate.c
> index 6655615..a45f59b 100644
> --- a/evaluate.c
> +++ b/evaluate.c
> @@ -2057,7 +2057,8 @@ static struct symbol *evaluate_sizeof(struct expression
> *expr)
> }
>
> if (size == 1 && is_bool_type(type)) {
> - warning(expr->pos, "expression using sizeof bool");
> + if (Wsizeof_bool)
> + warning(expr->pos, "expression using sizeof bool");
> size = bits_in_char;
> }
>
> diff --git a/lib.c b/lib.c
> index bf3e91c..676b72e 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -225,6 +225,7 @@ int Wone_bit_signed_bitfield = 1;
> int Wparen_string = 0;
> int Wptr_subtraction_blows = 0;
> int Wreturn_void = 0;
> +int Wsizeof_bool = 1;
> int Wshadow = 0;
> int Wtransparent_union = 0;
> int Wtypesign = 0;
> @@ -437,6 +438,7 @@ static const struct warning {
> { "paren-string", _string },
> { "ptr-subtraction-blows", _subtraction_blows },
> { "return-void", _void },
> + { "sizeof-bool", _bool },
> { "shadow", },
> { "transparent-union", _union },
> { "typesign", },
> diff --git a/lib.h b/lib.h
> index f09b338..7e3432f 100644
> --- a/lib.h
> +++ b/lib.h
> @@ -119,6 +119,7 @@ extern int Wone_bit_signed_bitfield;
> extern int Wparen_string;
> extern int Wptr_subtraction_blows;
> extern int Wreturn_void;
> +extern int Wsizeof_bool;
> extern int Wshadow;
> extern int Wtransparent_union;
> extern int Wtypesign;
> diff --git a/sparse.1 b/sparse.1
> index cd6be26..b4546be 100644
> --- a/sparse.1
> +++ b/sparse.1
> @@ -288,6 +288,15 @@ programs consider this poor style, and those programs
> can use
> Sparse does not issue these warnings by default.
> .
> .TP
> +.B \-Wsizeof-bool
> +Warn when checking the sizeof a _Bool.
> +
> +C99 does not specify the sizeof a _Bool. gcc uses 1.
> +
> +Sparse issues these warnings by default. To turn them off, use
> +\fB\-Wno\-sizeof\-bool\fR.
> +.
> +.TP
> .B \-Wshadow
> Warn when declaring a symbol which shadows a declaration with the same name
> in
> an outer scope.
> --
> 1.8.1.2.459.gbcd45b4.dirty
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2] sparse: Allow override of sizeof(bool) warning
Allow an override to emit or not the sizeof(bool) warning
Add a description to the manpage.
Signed-off-by: Joe Perches
---
evaluate.c | 3 ++-
lib.c | 2 ++
lib.h | 1 +
sparse.1 | 9 +
4 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/evaluate.c b/evaluate.c
index 6655615..a45f59b 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2057,7 +2057,8 @@ static struct symbol *evaluate_sizeof(struct expression
*expr)
}
if (size == 1 && is_bool_type(type)) {
- warning(expr->pos, "expression using sizeof bool");
+ if (Wsizeof_bool)
+ warning(expr->pos, "expression using sizeof bool");
size = bits_in_char;
}
diff --git a/lib.c b/lib.c
index bf3e91c..676b72e 100644
--- a/lib.c
+++ b/lib.c
@@ -225,6 +225,7 @@ int Wone_bit_signed_bitfield = 1;
int Wparen_string = 0;
int Wptr_subtraction_blows = 0;
int Wreturn_void = 0;
+int Wsizeof_bool = 1;
int Wshadow = 0;
int Wtransparent_union = 0;
int Wtypesign = 0;
@@ -437,6 +438,7 @@ static const struct warning {
{ "paren-string", _string },
{ "ptr-subtraction-blows", _subtraction_blows },
{ "return-void", _void },
+ { "sizeof-bool", _bool },
{ "shadow", },
{ "transparent-union", _union },
{ "typesign", },
diff --git a/lib.h b/lib.h
index f09b338..7e3432f 100644
--- a/lib.h
+++ b/lib.h
@@ -119,6 +119,7 @@ extern int Wone_bit_signed_bitfield;
extern int Wparen_string;
extern int Wptr_subtraction_blows;
extern int Wreturn_void;
+extern int Wsizeof_bool;
extern int Wshadow;
extern int Wtransparent_union;
extern int Wtypesign;
diff --git a/sparse.1 b/sparse.1
index cd6be26..b4546be 100644
--- a/sparse.1
+++ b/sparse.1
@@ -288,6 +288,15 @@ programs consider this poor style, and those programs can
use
Sparse does not issue these warnings by default.
.
.TP
+.B \-Wsizeof-bool
+Warn when checking the sizeof a _Bool.
+
+C99 does not specify the sizeof a _Bool. gcc uses 1.
+
+Sparse issues these warnings by default. To turn them off, use
+\fB\-Wno\-sizeof\-bool\fR.
+.
+.TP
.B \-Wshadow
Warn when declaring a symbol which shadows a declaration with the same name in
an outer scope.
--
1.8.1.2.459.gbcd45b4.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2] sparse: Allow override of sizeof(bool) warning
Allow an override to emit or not the sizeof(bool) warning
Add a description to the manpage.
Signed-off-by: Joe Perches j...@perches.com
---
evaluate.c | 3 ++-
lib.c | 2 ++
lib.h | 1 +
sparse.1 | 9 +
4 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/evaluate.c b/evaluate.c
index 6655615..a45f59b 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2057,7 +2057,8 @@ static struct symbol *evaluate_sizeof(struct expression
*expr)
}
if (size == 1 is_bool_type(type)) {
- warning(expr-pos, expression using sizeof bool);
+ if (Wsizeof_bool)
+ warning(expr-pos, expression using sizeof bool);
size = bits_in_char;
}
diff --git a/lib.c b/lib.c
index bf3e91c..676b72e 100644
--- a/lib.c
+++ b/lib.c
@@ -225,6 +225,7 @@ int Wone_bit_signed_bitfield = 1;
int Wparen_string = 0;
int Wptr_subtraction_blows = 0;
int Wreturn_void = 0;
+int Wsizeof_bool = 1;
int Wshadow = 0;
int Wtransparent_union = 0;
int Wtypesign = 0;
@@ -437,6 +438,7 @@ static const struct warning {
{ paren-string, Wparen_string },
{ ptr-subtraction-blows, Wptr_subtraction_blows },
{ return-void, Wreturn_void },
+ { sizeof-bool, Wsizeof_bool },
{ shadow, Wshadow },
{ transparent-union, Wtransparent_union },
{ typesign, Wtypesign },
diff --git a/lib.h b/lib.h
index f09b338..7e3432f 100644
--- a/lib.h
+++ b/lib.h
@@ -119,6 +119,7 @@ extern int Wone_bit_signed_bitfield;
extern int Wparen_string;
extern int Wptr_subtraction_blows;
extern int Wreturn_void;
+extern int Wsizeof_bool;
extern int Wshadow;
extern int Wtransparent_union;
extern int Wtypesign;
diff --git a/sparse.1 b/sparse.1
index cd6be26..b4546be 100644
--- a/sparse.1
+++ b/sparse.1
@@ -288,6 +288,15 @@ programs consider this poor style, and those programs can
use
Sparse does not issue these warnings by default.
.
.TP
+.B \-Wsizeof-bool
+Warn when checking the sizeof a _Bool.
+
+C99 does not specify the sizeof a _Bool. gcc uses 1.
+
+Sparse issues these warnings by default. To turn them off, use
+\fB\-Wno\-sizeof\-bool\fR.
+.
+.TP
.B \-Wshadow
Warn when declaring a symbol which shadows a declaration with the same name in
an outer scope.
--
1.8.1.2.459.gbcd45b4.dirty
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote:
Allow an override to emit or not the sizeof(bool) warning
Add a description to the manpage.
Signed-off-by: Joe Perches j...@perches.com
Reviewed-by: Josh Triplett j...@joshtriplett.org
evaluate.c | 3 ++-
lib.c | 2 ++
lib.h | 1 +
sparse.1 | 9 +
4 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/evaluate.c b/evaluate.c
index 6655615..a45f59b 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2057,7 +2057,8 @@ static struct symbol *evaluate_sizeof(struct expression
*expr)
}
if (size == 1 is_bool_type(type)) {
- warning(expr-pos, expression using sizeof bool);
+ if (Wsizeof_bool)
+ warning(expr-pos, expression using sizeof bool);
size = bits_in_char;
}
diff --git a/lib.c b/lib.c
index bf3e91c..676b72e 100644
--- a/lib.c
+++ b/lib.c
@@ -225,6 +225,7 @@ int Wone_bit_signed_bitfield = 1;
int Wparen_string = 0;
int Wptr_subtraction_blows = 0;
int Wreturn_void = 0;
+int Wsizeof_bool = 1;
int Wshadow = 0;
int Wtransparent_union = 0;
int Wtypesign = 0;
@@ -437,6 +438,7 @@ static const struct warning {
{ paren-string, Wparen_string },
{ ptr-subtraction-blows, Wptr_subtraction_blows },
{ return-void, Wreturn_void },
+ { sizeof-bool, Wsizeof_bool },
{ shadow, Wshadow },
{ transparent-union, Wtransparent_union },
{ typesign, Wtypesign },
diff --git a/lib.h b/lib.h
index f09b338..7e3432f 100644
--- a/lib.h
+++ b/lib.h
@@ -119,6 +119,7 @@ extern int Wone_bit_signed_bitfield;
extern int Wparen_string;
extern int Wptr_subtraction_blows;
extern int Wreturn_void;
+extern int Wsizeof_bool;
extern int Wshadow;
extern int Wtransparent_union;
extern int Wtypesign;
diff --git a/sparse.1 b/sparse.1
index cd6be26..b4546be 100644
--- a/sparse.1
+++ b/sparse.1
@@ -288,6 +288,15 @@ programs consider this poor style, and those programs
can use
Sparse does not issue these warnings by default.
.
.TP
+.B \-Wsizeof-bool
+Warn when checking the sizeof a _Bool.
+
+C99 does not specify the sizeof a _Bool. gcc uses 1.
+
+Sparse issues these warnings by default. To turn them off, use
+\fB\-Wno\-sizeof\-bool\fR.
+.
+.TP
.B \-Wshadow
Warn when declaring a symbol which shadows a declaration with the same name
in
an outer scope.
--
1.8.1.2.459.gbcd45b4.dirty
--
To unsubscribe from this list: send the line unsubscribe linux-sparse in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 02/26/2014 06:58 PM, Josh Triplett wrote: On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: Allow an override to emit or not the sizeof(bool) warning Add a description to the manpage. Signed-off-by: Joe Perches j...@perches.com Reviewed-by: Josh Triplett j...@joshtriplett.org I have to admit that this particular warning is a bit odd to me. I'm wondering what kind of bugs it was intended to catch. In particular, things that incorrectly assumes the size of bool to be anything in particular would seem unlikely to actually use sizeof(). -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
(adding Ben Pfaff and Christopher Li) On Wed, 2014-02-26 at 19:29 -0800, H. Peter Anvin wrote: On 02/26/2014 06:58 PM, Josh Triplett wrote: On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: Allow an override to emit or not the sizeof(bool) warning Add a description to the manpage. Signed-off-by: Joe Perches j...@perches.com Reviewed-by: Josh Triplett j...@joshtriplett.org I have to admit that this particular warning is a bit odd to me. I'm wondering what kind of bugs it was intended to catch. In particular, things that incorrectly assumes the size of bool to be anything in particular would seem unlikely to actually use sizeof(). Dunno, the commit log for the commit that added it doesn't quite match the code and is seemingly unaware that the c99 spec doesn't specify sizeof(bool). Ben Pfaff b...@nicira.com Date: Wed May 4 16:39:54 2011 -0700 evaluate: Allow sizeof(_Bool) to succeed. Without this commit, sizeof(_Bool) provokes an error with cannot size expression because _Bool is a 1-bit type and thus not a multiple of a full byte in size. But sizeof(_Bool) is valid C that should evaluate to 1, so this commit fixes the problem and adds a regression test. Signed-off-by: Ben Pfaff b...@nicira.com Signed-off-by: Christopher Li spa...@chrisli.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
sizeof(_Bool), like for many other types, is ABI-dependent, but that doesn't mean it is illegitimate. I don't think C99 says that it is invalid (which means C99 doesn't permit is to be a packed bitmap.) On February 26, 2014 7:38:46 PM PST, Joe Perches j...@perches.com wrote: (adding Ben Pfaff and Christopher Li) On Wed, 2014-02-26 at 19:29 -0800, H. Peter Anvin wrote: On 02/26/2014 06:58 PM, Josh Triplett wrote: On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: Allow an override to emit or not the sizeof(bool) warning Add a description to the manpage. Signed-off-by: Joe Perches j...@perches.com Reviewed-by: Josh Triplett j...@joshtriplett.org I have to admit that this particular warning is a bit odd to me. I'm wondering what kind of bugs it was intended to catch. In particular, things that incorrectly assumes the size of bool to be anything in particular would seem unlikely to actually use sizeof(). Dunno, the commit log for the commit that added it doesn't quite match the code and is seemingly unaware that the c99 spec doesn't specify sizeof(bool). Ben Pfaff b...@nicira.com Date: Wed May 4 16:39:54 2011 -0700 evaluate: Allow sizeof(_Bool) to succeed. Without this commit, sizeof(_Bool) provokes an error with cannot size expression because _Bool is a 1-bit type and thus not a multiple of a full byte in size. But sizeof(_Bool) is valid C that should evaluate to 1, so this commit fixes the problem and adds a regression test. Signed-off-by: Ben Pfaff b...@nicira.com Signed-off-by: Christopher Li spa...@chrisli.org -- Sent from my mobile phone. Please pardon brevity and lack of formatting. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Wed, Feb 26, 2014 at 07:38:46PM -0800, Joe Perches wrote: (adding Ben Pfaff and Christopher Li) On Wed, 2014-02-26 at 19:29 -0800, H. Peter Anvin wrote: On 02/26/2014 06:58 PM, Josh Triplett wrote: On Wed, Feb 26, 2014 at 06:53:14PM -0800, Joe Perches wrote: Allow an override to emit or not the sizeof(bool) warning Add a description to the manpage. Signed-off-by: Joe Perches j...@perches.com Reviewed-by: Josh Triplett j...@joshtriplett.org I have to admit that this particular warning is a bit odd to me. I'm wondering what kind of bugs it was intended to catch. In particular, things that incorrectly assumes the size of bool to be anything in particular would seem unlikely to actually use sizeof(). Dunno, the commit log for the commit that added it doesn't quite match the code and is seemingly unaware that the c99 spec doesn't specify sizeof(bool). The commit *relaxed* sparse behavior: because previously sizeof(bool) was an error. I'm not in favor of any diagnostic at all for sizeof(bool), but my recollection is that a sparse maintainer wanted it to yield one. I don't care about the particular result for sizeof(bool) as long as it matches the ABI. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 02/26/2014 08:00 PM, Ben Pfaff wrote: The commit *relaxed* sparse behavior: because previously sizeof(bool) was an error. I'm not in favor of any diagnostic at all for sizeof(bool), but my recollection is that a sparse maintainer wanted it to yield one. Still not clear as to why. -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On Wed, Feb 26, 2014 at 08:19:57PM -0800, H. Peter Anvin wrote: On 02/26/2014 08:00 PM, Ben Pfaff wrote: The commit *relaxed* sparse behavior: because previously sizeof(bool) was an error. I'm not in favor of any diagnostic at all for sizeof(bool), but my recollection is that a sparse maintainer wanted it to yield one. Still not clear as to why. The discussion is here: http://comments.gmane.org/gmane.comp.parsers.sparse/2462 Quoting from that discussion, the core of Christopher Li's argument was this: On Mon, May 9, 2011 at 1:02 PM, Ben Pfaff blp at nicira.com wrote: Thank you for applying my patch. It does work for me, in the sense that I get a warning instead of an error now, but I'm not so happy to get any diagnostic at all. Is there some reason why sizeof(_Bool) warrants a warning when, say, sizeof(long) does not? After all, both sizes are implementation defined. Because sizeof(_Bool) is a little bit special compare to sizeof(long). In the case of long, all sizeof(long) * 8 bits are use in the actual value. But for the _Bool, only the 1 bit is used in the 8 bits size. In other words, the _Bool has a special case of the actual bit size is not a multiple of 8. Sparse has two hats, it is a C compiler front end, and more often it is used in the Linux kernel source sanitize checking. Depending on the sizeof _Bool sounds a little bit suspicious in the kernel. I would love to the heard your actual usage case of the sizeof(_Bool). Why do you care about this warning? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] sparse: Allow override of sizeof(bool) warning
On 02/26/2014 08:26 PM, Ben Pfaff wrote: Because sizeof(_Bool) is a little bit special compare to sizeof(long). In the case of long, all sizeof(long) * 8 bits are use in the actual value. But for the _Bool, only the 1 bit is used in the 8 bits size. In other words, the _Bool has a special case of the actual bit size is not a multiple of 8. Quite frankly, this is silly in my opinion, *and* it is not guaranteed by C either (read about trap representations). Sparse has two hats, it is a C compiler front end, and more often it is used in the Linux kernel source sanitize checking. Depending on the sizeof _Bool sounds a little bit suspicious in the kernel. I would love to the heard your actual usage case of the sizeof(_Bool). Why do you care about this warning? Anything that moves data around in a generic fashion. It can be as simple as: memcpy(foo, bar, sizeof *foo); -hpa -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
