Re: Overriding -Werror
On Wed, Aug 20, 2014 at 01:43:59AM +0200, Andi Kleen wrote: > On Tue, Aug 19, 2014 at 10:11:30PM +0200, Sam Ravnborg wrote: > > On Tue, Aug 19, 2014 at 06:15:07AM -0700, Andi Kleen wrote: > > > Brian Norris writes: > > > > > > > > 4. better ideas? > > > > > > Just send patches to remove -Werror in all architectures > > > as a tree sweep (and anywhere else where someone misguided add it) > > In arch/sparc/ we have -Werror and this has never troubled us. > > In fact more than once people had tried to provide sloppy code that > > failed due to -Werror so it got fixed up front. > > It's a major mistake to not support future gccs for released source. Until now this has not been an issue - for the sparc bits. If newer gcc's turn out to give legit warnings then if someone provides a little help (configure command-line to build gcc) the warnings will be fixed. And that should hold true for any decent maintained piece of SW. If the intent is to use bleeding edge gcc then -Werrror can be annoying and the right fix would be to make it possible to diable it on the command-line. Sam -- 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: Overriding -Werror
On Tue, Aug 19, 2014 at 10:11:30PM +0200, Sam Ravnborg wrote: > On Tue, Aug 19, 2014 at 06:15:07AM -0700, Andi Kleen wrote: > > Brian Norris writes: > > > > > > 4. better ideas? > > > > Just send patches to remove -Werror in all architectures > > as a tree sweep (and anywhere else where someone misguided add it) > In arch/sparc/ we have -Werror and this has never troubled us. > In fact more than once people had tried to provide sloppy code that > failed due to -Werror so it got fixed up front. It's a major mistake to not support future gccs for released source. -Andi -- a...@linux.intel.com -- Speaking for myself only. -- 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: Overriding -Werror
On Tue, Aug 19, 2014 at 06:15:07AM -0700, Andi Kleen wrote: > Brian Norris writes: > > > > 4. better ideas? > > Just send patches to remove -Werror in all architectures > as a tree sweep (and anywhere else where someone misguided add it) In arch/sparc/ we have -Werror and this has never troubled us. In fact more than once people had tried to provide sloppy code that failed due to -Werror so it got fixed up front. Sam -- 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: Overriding -Werror
On Tue, Aug 19, 2014 at 06:15:07AM -0700, Andi Kleen wrote: > Brian Norris writes: > > > > 4. better ideas? > > Just send patches to remove -Werror in all architectures > as a tree sweep (and anywhere else where someone misguided add it) I cited at least one example in which this was attempted but rejected. (Or at least, it was semi-deliberately stalled.) > Having -Werror anywhere in a shipping release is just plainly a bug, > as it makes it often impossible to build on newer gcc versions. I also feel it is misguided, for other reasons. But there are at least a few who seem to disagree. They seem to feel it is important/necessary for enforcing good practices during development. I'm not sure how to bridge the disagreement, other than to provide workarounds for disabling -Werror. I could try sending -Werror removal patches, but I'm not confident that will go over well. Brian -- 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: Overriding -Werror
Brian Norris writes: > > 4. better ideas? Just send patches to remove -Werror in all architectures as a tree sweep (and anywhere else where someone misguided add it) Having -Werror anywhere in a shipping release is just plainly a bug, as it makes it often impossible to build on newer gcc versions. -Andi -- a...@linux.intel.com -- Speaking for myself only -- 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: Overriding -Werror
On Tue, Aug 19, 2014 at 10:11:30PM +0200, Sam Ravnborg wrote: On Tue, Aug 19, 2014 at 06:15:07AM -0700, Andi Kleen wrote: Brian Norris computersforpe...@gmail.com writes: 4. better ideas? Just send patches to remove -Werror in all architectures as a tree sweep (and anywhere else where someone misguided add it) In arch/sparc/ we have -Werror and this has never troubled us. In fact more than once people had tried to provide sloppy code that failed due to -Werror so it got fixed up front. It's a major mistake to not support future gccs for released source. -Andi -- a...@linux.intel.com -- Speaking for myself only. -- 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: Overriding -Werror
On Wed, Aug 20, 2014 at 01:43:59AM +0200, Andi Kleen wrote: On Tue, Aug 19, 2014 at 10:11:30PM +0200, Sam Ravnborg wrote: On Tue, Aug 19, 2014 at 06:15:07AM -0700, Andi Kleen wrote: Brian Norris computersforpe...@gmail.com writes: 4. better ideas? Just send patches to remove -Werror in all architectures as a tree sweep (and anywhere else where someone misguided add it) In arch/sparc/ we have -Werror and this has never troubled us. In fact more than once people had tried to provide sloppy code that failed due to -Werror so it got fixed up front. It's a major mistake to not support future gccs for released source. Until now this has not been an issue - for the sparc bits. If newer gcc's turn out to give legit warnings then if someone provides a little help (configure command-line to build gcc) the warnings will be fixed. And that should hold true for any decent maintained piece of SW. If the intent is to use bleeding edge gcc then -Werrror can be annoying and the right fix would be to make it possible to diable it on the command-line. Sam -- 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: Overriding -Werror
Brian Norris computersforpe...@gmail.com writes: 4. better ideas? Just send patches to remove -Werror in all architectures as a tree sweep (and anywhere else where someone misguided add it) Having -Werror anywhere in a shipping release is just plainly a bug, as it makes it often impossible to build on newer gcc versions. -Andi -- a...@linux.intel.com -- Speaking for myself only -- 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: Overriding -Werror
On Tue, Aug 19, 2014 at 06:15:07AM -0700, Andi Kleen wrote: Brian Norris computersforpe...@gmail.com writes: 4. better ideas? Just send patches to remove -Werror in all architectures as a tree sweep (and anywhere else where someone misguided add it) I cited at least one example in which this was attempted but rejected. (Or at least, it was semi-deliberately stalled.) Having -Werror anywhere in a shipping release is just plainly a bug, as it makes it often impossible to build on newer gcc versions. I also feel it is misguided, for other reasons. But there are at least a few who seem to disagree. They seem to feel it is important/necessary for enforcing good practices during development. I'm not sure how to bridge the disagreement, other than to provide workarounds for disabling -Werror. I could try sending -Werror removal patches, but I'm not confident that will go over well. Brian -- 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: Overriding -Werror
On Tue, Aug 19, 2014 at 06:15:07AM -0700, Andi Kleen wrote: Brian Norris computersforpe...@gmail.com writes: 4. better ideas? Just send patches to remove -Werror in all architectures as a tree sweep (and anywhere else where someone misguided add it) In arch/sparc/ we have -Werror and this has never troubled us. In fact more than once people had tried to provide sloppy code that failed due to -Werror so it got fixed up front. Sam -- 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: Overriding -Werror
On Aug 15, 2014, at 9:34 PM, Brian Norris wrote:
> Hi Mark,
>
> (BTW, your mailer is creating some pretty long, unwrapped lines. I've
> rewrapped them when quoting below.)
Sorry about that. I'll try to remember to deal with it on my end.
> On Fri, Aug 15, 2014 at 08:36:07PM -0700, Mark D Rustad wrote:
>> On Aug 15, 2014, at 12:33 PM, Brian Norris
>> wrote:
>>> On Fri, Aug 15, 2014 at 02:30:49AM -0700, Jeff Kirsher wrote:
Funny that you bring this up because I have ~60 patches in my queue to
resolve several thousand of these warnings. Half of the patches
actually resolve warnings that can be resolved and the other half
implement compiler diagnostic control macros to help silence warnings.
All these were the work of a co-worker Mark Rustad, below is the patch
he put together to implement the compiler diagnostic control macros.
>>>
>>> While fixing warnings is usually a good thing (at least when done right;
>>> there are plenty of ways to fight with the compiler over silly things,
>>> but that's another discussion),
>>
>> I have said at some presentations on the subject that resolving
>> warnings is not something you want an intern to do.
>
> Nice.
>
>>> I think that my issue is still
>>> orthogonal to the one you're addressing. In my estimation, it is
>>> impossible to guarantee that the entire kernel (including drivers) will
>>> build without any warnings, across all levels of warning verbosity.
>>> Thus, even with a valiant effort to fix or annotate all warnings, we
>>> still won't get to the point where I can build 'make ARCH=mips W=1', if
>>> -Werror is active.
>>
>> Actually, some years ago, I got a MIPS Linux kernel to compile clean
>> with even more warnings than W=12 provides.
>
> Congrats!
>
>> It can be done, but it certainly is not a state that is required and
>> cannot be maintained across all configurations, architectures and
>> compiler versions. This is the real world.
>
> Right, and this is the crux of why I would like to have the option of
> disabling -Werror systematically.
>
>>> Besides, when testing *new* code, it's even more likely to have new
>>> warnings, and I'd like to see as many as possible, without -Werror
>>> getting in the way.
>>
>> I have to say that I rather like -Werror.
>
> It has its uses. I think it can be a pretty good option whenever the
> compiler's warning level is kept to a reasonable level.
>
> -Wextra, for instance, enables a lot of warnings that can be problematic
> for little practical benefit (like -Wsign-compare, which notably is
> explicitly overridden for x86).
Hmm. I wasn't aware that x86 had that disabled. Sign-compare actually does
find some kinds of bugs. Those warnings are worth resolving for that reason.
Being explicit about whether you want the signed or unsigned comparison is
a good thing.
>> One thing that not a lot of people are aware of is that you can
>> selectively allow some warnings. -Wno-error=shadow would allow shadow
>> warnings to be reported without being treated as errors.
>
> That's interesting. I glazed over this option because I misinterpreted
> the second sentence of this note in the gcc manpage:
>
> "Note that specifying -Werror=foo automatically implies -Wfoo.
> However, -Wno-error=foo does not imply anything."
>
> [ Really, -Wno-error=foo doesn't imply anything? So why does it exist? ;) ]
Yeah, it means that it doesn't imply anything about whether the warning
is enabled or not. It will neither enable nor disable the warning as a
side-effect, unlike -Werror= which will enable it. So you can add -Wno
error=... without regard to whether the given warning was enabled or
not. It kind of makes sense, but is not explained very well.
> But now that you mention it, I think it could work as a hack for my
> builds right now, since it isn't overridden by a blanket -Werror found
> later on the command linie. So if I do
>
> make ARCH=mips W=1 KCFLAGS="-Wno-error=...(long list of warnings that break
> the build)..."
And still be able to see them. Not bad.
> then I can systematically perform the build-tests I'd like.
Yes.
>>> So I still think -Werror is fundamentally wrong in some cases, and I
>>> would like to pursue some approach like in my original post.
>>>
>>> BTW, for a little more context: I realize the output of 'make W=[123]'
>>> may not be very useful on its own, sometimes, but it's actually pretty
>>> useful to quickly catch potential issues in new code, by diff'ing the
>>> warnings in the before/after build logs. In this case, it's not helpful
>>> at all if the first build "fails" due to dubious warnings. I'm doing
>>> this in the context of Aiaiai [1]. Right now, I have to keep around a
>>> few local patches to remove -Werror from arch/{mips,sh}.
>>
>> The problem is that when a kernel build throws over 125,000 warnings,
>> it just becomes completely useless. That was what kind of set me off.
>
> Yeah that's bad. But it *still* can provide a helpful diff for tools
>
Re: Overriding -Werror
On Aug 15, 2014, at 9:34 PM, Brian Norris computersforpe...@gmail.com wrote:
Hi Mark,
(BTW, your mailer is creating some pretty long, unwrapped lines. I've
rewrapped them when quoting below.)
Sorry about that. I'll try to remember to deal with it on my end.
On Fri, Aug 15, 2014 at 08:36:07PM -0700, Mark D Rustad wrote:
On Aug 15, 2014, at 12:33 PM, Brian Norris computersforpe...@gmail.com
wrote:
On Fri, Aug 15, 2014 at 02:30:49AM -0700, Jeff Kirsher wrote:
Funny that you bring this up because I have ~60 patches in my queue to
resolve several thousand of these warnings. Half of the patches
actually resolve warnings that can be resolved and the other half
implement compiler diagnostic control macros to help silence warnings.
All these were the work of a co-worker Mark Rustad, below is the patch
he put together to implement the compiler diagnostic control macros.
While fixing warnings is usually a good thing (at least when done right;
there are plenty of ways to fight with the compiler over silly things,
but that's another discussion),
I have said at some presentations on the subject that resolving
warnings is not something you want an intern to do.
Nice.
I think that my issue is still
orthogonal to the one you're addressing. In my estimation, it is
impossible to guarantee that the entire kernel (including drivers) will
build without any warnings, across all levels of warning verbosity.
Thus, even with a valiant effort to fix or annotate all warnings, we
still won't get to the point where I can build 'make ARCH=mips W=1', if
-Werror is active.
Actually, some years ago, I got a MIPS Linux kernel to compile clean
with even more warnings than W=12 provides.
Congrats!
It can be done, but it certainly is not a state that is required and
cannot be maintained across all configurations, architectures and
compiler versions. This is the real world.
Right, and this is the crux of why I would like to have the option of
disabling -Werror systematically.
Besides, when testing *new* code, it's even more likely to have new
warnings, and I'd like to see as many as possible, without -Werror
getting in the way.
I have to say that I rather like -Werror.
It has its uses. I think it can be a pretty good option whenever the
compiler's warning level is kept to a reasonable level.
-Wextra, for instance, enables a lot of warnings that can be problematic
for little practical benefit (like -Wsign-compare, which notably is
explicitly overridden for x86).
Hmm. I wasn't aware that x86 had that disabled. Sign-compare actually does
find some kinds of bugs. Those warnings are worth resolving for that reason.
Being explicit about whether you want the signed or unsigned comparison is
a good thing.
One thing that not a lot of people are aware of is that you can
selectively allow some warnings. -Wno-error=shadow would allow shadow
warnings to be reported without being treated as errors.
That's interesting. I glazed over this option because I misinterpreted
the second sentence of this note in the gcc manpage:
Note that specifying -Werror=foo automatically implies -Wfoo.
However, -Wno-error=foo does not imply anything.
[ Really, -Wno-error=foo doesn't imply anything? So why does it exist? ;) ]
Yeah, it means that it doesn't imply anything about whether the warning
is enabled or not. It will neither enable nor disable the warning as a
side-effect, unlike -Werror= which will enable it. So you can add -Wno
error=... without regard to whether the given warning was enabled or
not. It kind of makes sense, but is not explained very well.
But now that you mention it, I think it could work as a hack for my
builds right now, since it isn't overridden by a blanket -Werror found
later on the command linie. So if I do
make ARCH=mips W=1 KCFLAGS=-Wno-error=...(long list of warnings that break
the build)...
And still be able to see them. Not bad.
then I can systematically perform the build-tests I'd like.
Yes.
So I still think -Werror is fundamentally wrong in some cases, and I
would like to pursue some approach like in my original post.
BTW, for a little more context: I realize the output of 'make W=[123]'
may not be very useful on its own, sometimes, but it's actually pretty
useful to quickly catch potential issues in new code, by diff'ing the
warnings in the before/after build logs. In this case, it's not helpful
at all if the first build fails due to dubious warnings. I'm doing
this in the context of Aiaiai [1]. Right now, I have to keep around a
few local patches to remove -Werror from arch/{mips,sh}.
The problem is that when a kernel build throws over 125,000 warnings,
it just becomes completely useless. That was what kind of set me off.
Yeah that's bad. But it *still* can provide a helpful diff for tools
like Aiaiai. The difference between 125,000 and 125,001 warnings still
can be determined automatically for new code, although
Re: Overriding -Werror
Hi Mark,
(BTW, your mailer is creating some pretty long, unwrapped lines. I've
rewrapped them when quoting below.)
On Fri, Aug 15, 2014 at 08:36:07PM -0700, Mark D Rustad wrote:
> On Aug 15, 2014, at 12:33 PM, Brian Norris
> wrote:
> > On Fri, Aug 15, 2014 at 02:30:49AM -0700, Jeff Kirsher wrote:
> >> Funny that you bring this up because I have ~60 patches in my queue to
> >> resolve several thousand of these warnings. Half of the patches
> >> actually resolve warnings that can be resolved and the other half
> >> implement compiler diagnostic control macros to help silence warnings.
> >> All these were the work of a co-worker Mark Rustad, below is the patch
> >> he put together to implement the compiler diagnostic control macros.
> >
> > While fixing warnings is usually a good thing (at least when done right;
> > there are plenty of ways to fight with the compiler over silly things,
> > but that's another discussion),
>
> I have said at some presentations on the subject that resolving
> warnings is not something you want an intern to do.
Nice.
> > I think that my issue is still
> > orthogonal to the one you're addressing. In my estimation, it is
> > impossible to guarantee that the entire kernel (including drivers) will
> > build without any warnings, across all levels of warning verbosity.
> > Thus, even with a valiant effort to fix or annotate all warnings, we
> > still won't get to the point where I can build 'make ARCH=mips W=1', if
> > -Werror is active.
>
> Actually, some years ago, I got a MIPS Linux kernel to compile clean
> with even more warnings than W=12 provides.
Congrats!
> It can be done, but it certainly is not a state that is required and
> cannot be maintained across all configurations, architectures and
> compiler versions. This is the real world.
Right, and this is the crux of why I would like to have the option of
disabling -Werror systematically.
> > Besides, when testing *new* code, it's even more likely to have new
> > warnings, and I'd like to see as many as possible, without -Werror
> > getting in the way.
>
> I have to say that I rather like -Werror.
It has its uses. I think it can be a pretty good option whenever the
compiler's warning level is kept to a reasonable level.
-Wextra, for instance, enables a lot of warnings that can be problematic
for little practical benefit (like -Wsign-compare, which notably is
explicitly overridden for x86).
> One thing that not a lot of people are aware of is that you can
> selectively allow some warnings. -Wno-error=shadow would allow shadow
> warnings to be reported without being treated as errors.
That's interesting. I glazed over this option because I misinterpreted
the second sentence of this note in the gcc manpage:
"Note that specifying -Werror=foo automatically implies -Wfoo.
However, -Wno-error=foo does not imply anything."
[ Really, -Wno-error=foo doesn't imply anything? So why does it exist? ;) ]
But now that you mention it, I think it could work as a hack for my
builds right now, since it isn't overridden by a blanket -Werror found
later on the command linie. So if I do
make ARCH=mips W=1 KCFLAGS="-Wno-error=...(long list of warnings that break
the build)..."
then I can systematically perform the build-tests I'd like.
> > So I still think -Werror is fundamentally wrong in some cases, and I
> > would like to pursue some approach like in my original post.
> >
> > BTW, for a little more context: I realize the output of 'make W=[123]'
> > may not be very useful on its own, sometimes, but it's actually pretty
> > useful to quickly catch potential issues in new code, by diff'ing the
> > warnings in the before/after build logs. In this case, it's not helpful
> > at all if the first build "fails" due to dubious warnings. I'm doing
> > this in the context of Aiaiai [1]. Right now, I have to keep around a
> > few local patches to remove -Werror from arch/{mips,sh}.
>
> The problem is that when a kernel build throws over 125,000 warnings,
> it just becomes completely useless. That was what kind of set me off.
Yeah that's bad. But it *still* can provide a helpful diff for tools
like Aiaiai. The difference between 125,000 and 125,001 warnings still
can be determined automatically for new code, although it's not helpful
if every "new" warning is actually just because you used a new core
header that causes a lot of warnings.
> I did wind up pushing this rock further up the hill than I really
> meant to. Still, I got the build under 1,400 warnings, and I now know
> how to address most of them in a systematic way.
>
> >> commit 7b9aace02b2405f0714bc08c424b72e6962f1c2e
> >> Author: Mark Rustad
> >> Date: Fri Aug 15 01:43:44 2014 -0700
> >>
> >>compiler: Add diagnostic control macros
> >>
> >>Add macros to control diagnostic messages where needed. These
> >>are used to silence warning messages that are expected, normal
> >>and do not indicate any sort of problem. Reducing the stream
> >>of
Re: Overriding -Werror
Brian,
On Aug 15, 2014, at 12:33 PM, Brian Norris wrote:
> Hi,
>
> On Fri, Aug 15, 2014 at 02:30:49AM -0700, Jeff Kirsher wrote:
>> Funny that you bring this up because I have ~60 patches in my queue to
>> resolve several thousand of these warnings. Half of the patches
>> actually resolve warnings that can be resolved and the other half
>> implement compiler diagnostic control macros to help silence warnings.
>> All these were the work of a co-worker Mark Rustad, below is the patch
>> he put together to implement the compiler diagnostic control macros.
>
> While fixing warnings is usually a good thing (at least when done right;
> there are plenty of ways to fight with the compiler over silly things,
> but that's another discussion),
I have said at some presentations on the subject that resolving warnings is not
something you want an intern to do.
> I think that my issue is still
> orthogonal to the one you're addressing. In my estimation, it is
> impossible to guarantee that the entire kernel (including drivers) will
> build without any warnings, across all levels of warning verbosity.
> Thus, even with a valiant effort to fix or annotate all warnings, we
> still won't get to the point where I can build 'make ARCH=mips W=1', if
> -Werror is active.
Actually, some years ago, I got a MIPS Linux kernel to compile clean with even
more warnings than W=12 provides. It can be done, but it certainly is not a
state that is required and cannot be maintained across all configurations,
architectures and compiler versions. This is the real world.
> Besides, when testing *new* code, it's even more likely to have new
> warnings, and I'd like to see as many as possible, without -Werror
> getting in the way.
I have to say that I rather like -Werror. One thing that not a lot of people
are aware of is that you can selectively allow some warnings. -Wno-error=shadow
would allow shadow warnings to be reported without being treated as errors.
> So I still think -Werror is fundamentally wrong in some cases, and I
> would like to pursue some approach like in my original post.
>
> BTW, for a little more context: I realize the output of 'make W=[123]'
> may not be very useful on its own, sometimes, but it's actually pretty
> useful to quickly catch potential issues in new code, by diff'ing the
> warnings in the before/after build logs. In this case, it's not helpful
> at all if the first build "fails" due to dubious warnings. I'm doing
> this in the context of Aiaiai [1]. Right now, I have to keep around a
> few local patches to remove -Werror from arch/{mips,sh}.
The problem is that when a kernel build throws over 125,000 warnings, it just
becomes completely useless. That was what kind of set me off. I did wind up
pushing this rock further up the hill than I really meant to. Still, I got the
build under 1,400 warnings, and I now know how to address most of them in a
systematic way.
>> commit 7b9aace02b2405f0714bc08c424b72e6962f1c2e
>> Author: Mark Rustad
>> Date: Fri Aug 15 01:43:44 2014 -0700
>>
>>compiler: Add diagnostic control macros
>>
>>Add macros to control diagnostic messages where needed. These
>>are used to silence warning messages that are expected, normal
>>and do not indicate any sort of problem. Reducing the stream
>>of messages in this way helps possible problems to stand out.
>>
>>The macros provided are:
>>DIAG_PUSH() - to save diagnostic settings
>>DIAG_POP() - to restore diagnostic settings
>>DIAG_IGNORE(option) - to ignore a particular warning
>>DIAG_GCC_IGNORE(option) - DIAG_IGNORE for gcc only
>>DIAG_CLANG_IGNORE(option) - DIAG_IGNORE for clang only
>>
>>Signed-off-by: Mark Rustad
>>
>> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
>> index d1e49d5..039b112 100644
>> --- a/include/linux/compiler-clang.h
>> +++ b/include/linux/compiler-clang.h
>> @@ -10,3 +10,29 @@
>> #undef uninitialized_var
>> #define uninitialized_var(x) x = *(&(x))
>> #endif
>> +
>> +/*
>> + * Provide macros to manipulate diagnostic messages when possible.
>> + * DIAG_PUSH pushes the diagnostic settings
>> + * DIAG_POP pops the diagnostic settings
>> + * DIAG_IGNORE(x) changes the given diagnostic setting to ignore
>> + *
>> + * Example:
>> + *DIAG_PUSH DIAG_IGNORE(aggregate-return)
>> + *struct timespec ns_to_timespec(const s64 nsec)
>> + *{
>> + *...
>> + *}
>> + *DIAG_POP
>> + *
>> + * Will prevent the warning on compilation of the function. Other
>> + * steps are necessary to do the same thing for the call sites.
>> + */
>
> While I do not want to disparage your/Mark's work here, my first thought
> about this kind of annotation is that it seems to be a pretty big burden
> to have to annotate all code with these sorts of things.
I wouldn't suggest annotating everything. However note that the annotations can
serve as a notice that something has been analyzed and deemed ok.
Re: Overriding -Werror
Hi,
On Fri, Aug 15, 2014 at 02:30:49AM -0700, Jeff Kirsher wrote:
> Funny that you bring this up because I have ~60 patches in my queue to
> resolve several thousand of these warnings. Half of the patches
> actually resolve warnings that can be resolved and the other half
> implement compiler diagnostic control macros to help silence warnings.
> All these were the work of a co-worker Mark Rustad, below is the patch
> he put together to implement the compiler diagnostic control macros.
While fixing warnings is usually a good thing (at least when done right;
there are plenty of ways to fight with the compiler over silly things,
but that's another discussion), I think that my issue is still
orthogonal to the one you're addressing. In my estimation, it is
impossible to guarantee that the entire kernel (including drivers) will
build without any warnings, across all levels of warning verbosity.
Thus, even with a valiant effort to fix or annotate all warnings, we
still won't get to the point where I can build 'make ARCH=mips W=1', if
-Werror is active.
Besides, when testing *new* code, it's even more likely to have new
warnings, and I'd like to see as many as possible, without -Werror
getting in the way.
So I still think -Werror is fundamentally wrong in some cases, and I
would like to pursue some approach like in my original post.
BTW, for a little more context: I realize the output of 'make W=[123]'
may not be very useful on its own, sometimes, but it's actually pretty
useful to quickly catch potential issues in new code, by diff'ing the
warnings in the before/after build logs. In this case, it's not helpful
at all if the first build "fails" due to dubious warnings. I'm doing
this in the context of Aiaiai [1]. Right now, I have to keep around a
few local patches to remove -Werror from arch/{mips,sh}.
> commit 7b9aace02b2405f0714bc08c424b72e6962f1c2e
> Author: Mark Rustad
> Date: Fri Aug 15 01:43:44 2014 -0700
>
> compiler: Add diagnostic control macros
>
> Add macros to control diagnostic messages where needed. These
> are used to silence warning messages that are expected, normal
> and do not indicate any sort of problem. Reducing the stream
> of messages in this way helps possible problems to stand out.
>
> The macros provided are:
> DIAG_PUSH() - to save diagnostic settings
> DIAG_POP() - to restore diagnostic settings
> DIAG_IGNORE(option) - to ignore a particular warning
> DIAG_GCC_IGNORE(option) - DIAG_IGNORE for gcc only
> DIAG_CLANG_IGNORE(option) - DIAG_IGNORE for clang only
>
> Signed-off-by: Mark Rustad
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index d1e49d5..039b112 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -10,3 +10,29 @@
> #undef uninitialized_var
> #define uninitialized_var(x) x = *(&(x))
> #endif
> +
> +/*
> + * Provide macros to manipulate diagnostic messages when possible.
> + * DIAG_PUSH pushes the diagnostic settings
> + * DIAG_POP pops the diagnostic settings
> + * DIAG_IGNORE(x) changes the given diagnostic setting to ignore
> + *
> + * Example:
> + *DIAG_PUSH DIAG_IGNORE(aggregate-return)
> + *struct timespec ns_to_timespec(const s64 nsec)
> + *{
> + *...
> + *}
> + *DIAG_POP
> + *
> + * Will prevent the warning on compilation of the function. Other
> + * steps are necessary to do the same thing for the call sites.
> + */
While I do not want to disparage your/Mark's work here, my first thought
about this kind of annotation is that it seems to be a pretty big burden
to have to annotate all code with these sorts of things. While it could
help for some core parts of the kernel that can be closely scrutinized
and defended against false/useless warnings, from my perspective it
seems like people are bound to get it wrong when it comes to the long
tail of scattered device drivers.
But I'm not really the right voice for that topic. Feel free to send
your work and see what the rest of the community thinks.
Thanks,
Brian
[1] http://lwn.net/Articles/488992/
http://git.infradead.org/aiaiai.git
--
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: Overriding -Werror
On Fri, Aug 15, 2014 at 5:33 PM, Lennart Sorensen
wrote:
> On Thu, Aug 14, 2014 at 03:21:19PM -0700, Brian Norris wrote:
>> I'm interested in being able to build-test kernels on various
>> architectures while enabling extra warnings (make W=[123]). I'd like to
>> be able to finish the builds and see all warnings, rather than seeing a
>> failed build. However, GCC's -Werror is incompatible with this. There is
>> plenty of code that will produce at least one warning, when warning
>> verbosity is turned up. And GCC's -Werror is not guaranteed to remain
>> stable over time; new versions may develop new warnings that may or may
>> not be legitimate.
>
> What ever is wrong with using '-k' with your make command?
While -k helps to get more compiled, and thus see more warnings, you'll still
be missing the report of missing symbols in the final link/modpost stage.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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: Overriding -Werror
On Thu, Aug 14, 2014 at 03:21:19PM -0700, Brian Norris wrote: > I'm interested in being able to build-test kernels on various > architectures while enabling extra warnings (make W=[123]). I'd like to > be able to finish the builds and see all warnings, rather than seeing a > failed build. However, GCC's -Werror is incompatible with this. There is > plenty of code that will produce at least one warning, when warning > verbosity is turned up. And GCC's -Werror is not guaranteed to remain > stable over time; new versions may develop new warnings that may or may > not be legitimate. What ever is wrong with using '-k' with your make command? -- Len Sorensen -- 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: Overriding -Werror
On Thu, Aug 14, 2014 at 3:21 PM, Brian Norris
wrote:
> Hi all,
>
> I'm interested in being able to build-test kernels on various
> architectures while enabling extra warnings (make W=[123]). I'd like to
> be able to finish the builds and see all warnings, rather than seeing a
> failed build. However, GCC's -Werror is incompatible with this. There is
> plenty of code that will produce at least one warning, when warning
> verbosity is turned up. And GCC's -Werror is not guaranteed to remain
> stable over time; new versions may develop new warnings that may or may
> not be legitimate.
>
> It seems that there are a few problem ARCHes that enable -Werror by
> default: SuperH (orphaned), SPARC, and MIPS. There are also a few
> scattered Makefiles throughout the build tree. Developers have
> previously tried to remove some of the worst offenders [1], but were
> mostly rejected [2]. It doesn't seem like we can fully prevent
> maintainers from enabling -Werror on their code--or even on their entire
> ARCH build, as with MIPS--for better or worse, so I look to other
> alternatives.
>
> For the easiest approach, I considered how one might add -Wno-error to
> the CFLAGS. 'make KCFLAGS=-Wno-error' looked promising, but
> KBUILD_CFLAGS is applied before the sub-directory Makefiles add their
> own options to ccflags-y. So it seems like others have come to the same
> conclusion as me: that Kbuild doesn't seem to provide a way to override
> the -Werror behvaior from the top level. [3][4]
>
> So, how can we fix this? -Werror may be useful in some cases to
> encourage developers to fix up their code immediately, but it is
> decidedly unhelpful when running code through analysis tools.
>
> Possibilities include:
>
> 1. make -Werror be applied only when we do not have W=[123]. [5]
>
> 2. develop a top-level override for CFLAGS that is applied *after* all
>sub-directory modifications
>
> 3. make -Werror opt-in / configurable, like PPC's CONFIG_PPC_WERROR
>(maybe make it a generic CONFIG_WERROR?), and prevent its
>unconditional use in Makefiles
>
> 4. better ideas?
>
> Regards,
> Brian
>
> [1] http://www.linux-mips.org/archives/linux-mips/2012-04/msg00179.html
> http://patchwork.ozlabs.org/patch/146297/
>
> [2] http://www.linux-mips.org/archives/linux-mips/2012-05/msg00064.html
>
> [3]
> http://lists.linaro.org/pipermail/linaro-toolchain/2011-November/001869.html
>
> [4] http://lists.linaro.org/pipermail/linaro-dev/2011-December/008880.html
>
> [5] http://www.linux-mips.org/archives/linux-mips/2012-05/msg00070.html
> --
Funny that you bring this up because I have ~60 patches in my queue to
resolve several thousand of these warnings. Half of the patches
actually resolve warnings that can be resolved and the other half
implement compiler diagnostic control macros to help silence warnings.
All these were the work of a co-worker Mark Rustad, below is the patch
he put together to implement the compiler diagnostic control macros.
commit 7b9aace02b2405f0714bc08c424b72e6962f1c2e
Author: Mark Rustad
Date: Fri Aug 15 01:43:44 2014 -0700
compiler: Add diagnostic control macros
Add macros to control diagnostic messages where needed. These
are used to silence warning messages that are expected, normal
and do not indicate any sort of problem. Reducing the stream
of messages in this way helps possible problems to stand out.
The macros provided are:
DIAG_PUSH() - to save diagnostic settings
DIAG_POP() - to restore diagnostic settings
DIAG_IGNORE(option) - to ignore a particular warning
DIAG_GCC_IGNORE(option) - DIAG_IGNORE for gcc only
DIAG_CLANG_IGNORE(option) - DIAG_IGNORE for clang only
Signed-off-by: Mark Rustad
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index d1e49d5..039b112 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -10,3 +10,29 @@
#undef uninitialized_var
#define uninitialized_var(x) x = *(&(x))
#endif
+
+/*
+ * Provide macros to manipulate diagnostic messages when possible.
+ * DIAG_PUSH pushes the diagnostic settings
+ * DIAG_POP pops the diagnostic settings
+ * DIAG_IGNORE(x) changes the given diagnostic setting to ignore
+ *
+ * Example:
+ *DIAG_PUSH DIAG_IGNORE(aggregate-return)
+ *struct timespec ns_to_timespec(const s64 nsec)
+ *{
+ *...
+ *}
+ *DIAG_POP
+ *
+ * Will prevent the warning on compilation of the function. Other
+ * steps are necessary to do the same thing for the call sites.
+ */
+#define DIAG_STR(x) #x
+#define DIAG_CAT(x, y) DIAG_STR(x##y)
+#define DIAG_DO_PRAGMA(x) _Pragma(#x)
+#define DIAG_PRAGMA(x) DIAG_DO_PRAGMA(clang diagnostic x)
+#define DIAG_IGNORE(x) DIAG_PRAGMA(ignored DIAG_CAT(-W, x))
+#define DIAG_PUSH DIAG_PRAGMA(push)
+#define DIAG_POP DIAG_PRAGMA(pop)
+#define DIAG_CLANG_IGNORE(x) DIAG_IGNORE(x)
diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 2507fd2..78387f6 100644
Re: Overriding -Werror
On Thu, Aug 14, 2014 at 3:21 PM, Brian Norris
computersforpe...@gmail.com wrote:
Hi all,
I'm interested in being able to build-test kernels on various
architectures while enabling extra warnings (make W=[123]). I'd like to
be able to finish the builds and see all warnings, rather than seeing a
failed build. However, GCC's -Werror is incompatible with this. There is
plenty of code that will produce at least one warning, when warning
verbosity is turned up. And GCC's -Werror is not guaranteed to remain
stable over time; new versions may develop new warnings that may or may
not be legitimate.
It seems that there are a few problem ARCHes that enable -Werror by
default: SuperH (orphaned), SPARC, and MIPS. There are also a few
scattered Makefiles throughout the build tree. Developers have
previously tried to remove some of the worst offenders [1], but were
mostly rejected [2]. It doesn't seem like we can fully prevent
maintainers from enabling -Werror on their code--or even on their entire
ARCH build, as with MIPS--for better or worse, so I look to other
alternatives.
For the easiest approach, I considered how one might add -Wno-error to
the CFLAGS. 'make KCFLAGS=-Wno-error' looked promising, but
KBUILD_CFLAGS is applied before the sub-directory Makefiles add their
own options to ccflags-y. So it seems like others have come to the same
conclusion as me: that Kbuild doesn't seem to provide a way to override
the -Werror behvaior from the top level. [3][4]
So, how can we fix this? -Werror may be useful in some cases to
encourage developers to fix up their code immediately, but it is
decidedly unhelpful when running code through analysis tools.
Possibilities include:
1. make -Werror be applied only when we do not have W=[123]. [5]
2. develop a top-level override for CFLAGS that is applied *after* all
sub-directory modifications
3. make -Werror opt-in / configurable, like PPC's CONFIG_PPC_WERROR
(maybe make it a generic CONFIG_WERROR?), and prevent its
unconditional use in Makefiles
4. better ideas?
Regards,
Brian
[1] http://www.linux-mips.org/archives/linux-mips/2012-04/msg00179.html
http://patchwork.ozlabs.org/patch/146297/
[2] http://www.linux-mips.org/archives/linux-mips/2012-05/msg00064.html
[3]
http://lists.linaro.org/pipermail/linaro-toolchain/2011-November/001869.html
[4] http://lists.linaro.org/pipermail/linaro-dev/2011-December/008880.html
[5] http://www.linux-mips.org/archives/linux-mips/2012-05/msg00070.html
--
Funny that you bring this up because I have ~60 patches in my queue to
resolve several thousand of these warnings. Half of the patches
actually resolve warnings that can be resolved and the other half
implement compiler diagnostic control macros to help silence warnings.
All these were the work of a co-worker Mark Rustad, below is the patch
he put together to implement the compiler diagnostic control macros.
commit 7b9aace02b2405f0714bc08c424b72e6962f1c2e
Author: Mark Rustad mark.d.rus...@intel.com
Date: Fri Aug 15 01:43:44 2014 -0700
compiler: Add diagnostic control macros
Add macros to control diagnostic messages where needed. These
are used to silence warning messages that are expected, normal
and do not indicate any sort of problem. Reducing the stream
of messages in this way helps possible problems to stand out.
The macros provided are:
DIAG_PUSH() - to save diagnostic settings
DIAG_POP() - to restore diagnostic settings
DIAG_IGNORE(option) - to ignore a particular warning
DIAG_GCC_IGNORE(option) - DIAG_IGNORE for gcc only
DIAG_CLANG_IGNORE(option) - DIAG_IGNORE for clang only
Signed-off-by: Mark Rustad mark.d.rus...@intel.com
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index d1e49d5..039b112 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -10,3 +10,29 @@
#undef uninitialized_var
#define uninitialized_var(x) x = *((x))
#endif
+
+/*
+ * Provide macros to manipulate diagnostic messages when possible.
+ * DIAG_PUSH pushes the diagnostic settings
+ * DIAG_POP pops the diagnostic settings
+ * DIAG_IGNORE(x) changes the given diagnostic setting to ignore
+ *
+ * Example:
+ *DIAG_PUSH DIAG_IGNORE(aggregate-return)
+ *struct timespec ns_to_timespec(const s64 nsec)
+ *{
+ *...
+ *}
+ *DIAG_POP
+ *
+ * Will prevent the warning on compilation of the function. Other
+ * steps are necessary to do the same thing for the call sites.
+ */
+#define DIAG_STR(x) #x
+#define DIAG_CAT(x, y) DIAG_STR(x##y)
+#define DIAG_DO_PRAGMA(x) _Pragma(#x)
+#define DIAG_PRAGMA(x) DIAG_DO_PRAGMA(clang diagnostic x)
+#define DIAG_IGNORE(x) DIAG_PRAGMA(ignored DIAG_CAT(-W, x))
+#define DIAG_PUSH DIAG_PRAGMA(push)
+#define DIAG_POP DIAG_PRAGMA(pop)
+#define DIAG_CLANG_IGNORE(x) DIAG_IGNORE(x)
diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index
Re: Overriding -Werror
On Thu, Aug 14, 2014 at 03:21:19PM -0700, Brian Norris wrote: I'm interested in being able to build-test kernels on various architectures while enabling extra warnings (make W=[123]). I'd like to be able to finish the builds and see all warnings, rather than seeing a failed build. However, GCC's -Werror is incompatible with this. There is plenty of code that will produce at least one warning, when warning verbosity is turned up. And GCC's -Werror is not guaranteed to remain stable over time; new versions may develop new warnings that may or may not be legitimate. What ever is wrong with using '-k' with your make command? -- Len Sorensen -- 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: Overriding -Werror
On Fri, Aug 15, 2014 at 5:33 PM, Lennart Sorensen
lsore...@csclub.uwaterloo.ca wrote:
On Thu, Aug 14, 2014 at 03:21:19PM -0700, Brian Norris wrote:
I'm interested in being able to build-test kernels on various
architectures while enabling extra warnings (make W=[123]). I'd like to
be able to finish the builds and see all warnings, rather than seeing a
failed build. However, GCC's -Werror is incompatible with this. There is
plenty of code that will produce at least one warning, when warning
verbosity is turned up. And GCC's -Werror is not guaranteed to remain
stable over time; new versions may develop new warnings that may or may
not be legitimate.
What ever is wrong with using '-k' with your make command?
While -k helps to get more compiled, and thus see more warnings, you'll still
be missing the report of missing symbols in the final link/modpost stage.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
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: Overriding -Werror
Hi,
On Fri, Aug 15, 2014 at 02:30:49AM -0700, Jeff Kirsher wrote:
Funny that you bring this up because I have ~60 patches in my queue to
resolve several thousand of these warnings. Half of the patches
actually resolve warnings that can be resolved and the other half
implement compiler diagnostic control macros to help silence warnings.
All these were the work of a co-worker Mark Rustad, below is the patch
he put together to implement the compiler diagnostic control macros.
While fixing warnings is usually a good thing (at least when done right;
there are plenty of ways to fight with the compiler over silly things,
but that's another discussion), I think that my issue is still
orthogonal to the one you're addressing. In my estimation, it is
impossible to guarantee that the entire kernel (including drivers) will
build without any warnings, across all levels of warning verbosity.
Thus, even with a valiant effort to fix or annotate all warnings, we
still won't get to the point where I can build 'make ARCH=mips W=1', if
-Werror is active.
Besides, when testing *new* code, it's even more likely to have new
warnings, and I'd like to see as many as possible, without -Werror
getting in the way.
So I still think -Werror is fundamentally wrong in some cases, and I
would like to pursue some approach like in my original post.
BTW, for a little more context: I realize the output of 'make W=[123]'
may not be very useful on its own, sometimes, but it's actually pretty
useful to quickly catch potential issues in new code, by diff'ing the
warnings in the before/after build logs. In this case, it's not helpful
at all if the first build fails due to dubious warnings. I'm doing
this in the context of Aiaiai [1]. Right now, I have to keep around a
few local patches to remove -Werror from arch/{mips,sh}.
commit 7b9aace02b2405f0714bc08c424b72e6962f1c2e
Author: Mark Rustad mark.d.rus...@intel.com
Date: Fri Aug 15 01:43:44 2014 -0700
compiler: Add diagnostic control macros
Add macros to control diagnostic messages where needed. These
are used to silence warning messages that are expected, normal
and do not indicate any sort of problem. Reducing the stream
of messages in this way helps possible problems to stand out.
The macros provided are:
DIAG_PUSH() - to save diagnostic settings
DIAG_POP() - to restore diagnostic settings
DIAG_IGNORE(option) - to ignore a particular warning
DIAG_GCC_IGNORE(option) - DIAG_IGNORE for gcc only
DIAG_CLANG_IGNORE(option) - DIAG_IGNORE for clang only
Signed-off-by: Mark Rustad mark.d.rus...@intel.com
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index d1e49d5..039b112 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -10,3 +10,29 @@
#undef uninitialized_var
#define uninitialized_var(x) x = *((x))
#endif
+
+/*
+ * Provide macros to manipulate diagnostic messages when possible.
+ * DIAG_PUSH pushes the diagnostic settings
+ * DIAG_POP pops the diagnostic settings
+ * DIAG_IGNORE(x) changes the given diagnostic setting to ignore
+ *
+ * Example:
+ *DIAG_PUSH DIAG_IGNORE(aggregate-return)
+ *struct timespec ns_to_timespec(const s64 nsec)
+ *{
+ *...
+ *}
+ *DIAG_POP
+ *
+ * Will prevent the warning on compilation of the function. Other
+ * steps are necessary to do the same thing for the call sites.
+ */
While I do not want to disparage your/Mark's work here, my first thought
about this kind of annotation is that it seems to be a pretty big burden
to have to annotate all code with these sorts of things. While it could
help for some core parts of the kernel that can be closely scrutinized
and defended against false/useless warnings, from my perspective it
seems like people are bound to get it wrong when it comes to the long
tail of scattered device drivers.
But I'm not really the right voice for that topic. Feel free to send
your work and see what the rest of the community thinks.
Thanks,
Brian
[1] http://lwn.net/Articles/488992/
http://git.infradead.org/aiaiai.git
--
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: Overriding -Werror
Brian,
On Aug 15, 2014, at 12:33 PM, Brian Norris computersforpe...@gmail.com wrote:
Hi,
On Fri, Aug 15, 2014 at 02:30:49AM -0700, Jeff Kirsher wrote:
Funny that you bring this up because I have ~60 patches in my queue to
resolve several thousand of these warnings. Half of the patches
actually resolve warnings that can be resolved and the other half
implement compiler diagnostic control macros to help silence warnings.
All these were the work of a co-worker Mark Rustad, below is the patch
he put together to implement the compiler diagnostic control macros.
While fixing warnings is usually a good thing (at least when done right;
there are plenty of ways to fight with the compiler over silly things,
but that's another discussion),
I have said at some presentations on the subject that resolving warnings is not
something you want an intern to do.
I think that my issue is still
orthogonal to the one you're addressing. In my estimation, it is
impossible to guarantee that the entire kernel (including drivers) will
build without any warnings, across all levels of warning verbosity.
Thus, even with a valiant effort to fix or annotate all warnings, we
still won't get to the point where I can build 'make ARCH=mips W=1', if
-Werror is active.
Actually, some years ago, I got a MIPS Linux kernel to compile clean with even
more warnings than W=12 provides. It can be done, but it certainly is not a
state that is required and cannot be maintained across all configurations,
architectures and compiler versions. This is the real world.
Besides, when testing *new* code, it's even more likely to have new
warnings, and I'd like to see as many as possible, without -Werror
getting in the way.
I have to say that I rather like -Werror. One thing that not a lot of people
are aware of is that you can selectively allow some warnings. -Wno-error=shadow
would allow shadow warnings to be reported without being treated as errors.
So I still think -Werror is fundamentally wrong in some cases, and I
would like to pursue some approach like in my original post.
BTW, for a little more context: I realize the output of 'make W=[123]'
may not be very useful on its own, sometimes, but it's actually pretty
useful to quickly catch potential issues in new code, by diff'ing the
warnings in the before/after build logs. In this case, it's not helpful
at all if the first build fails due to dubious warnings. I'm doing
this in the context of Aiaiai [1]. Right now, I have to keep around a
few local patches to remove -Werror from arch/{mips,sh}.
The problem is that when a kernel build throws over 125,000 warnings, it just
becomes completely useless. That was what kind of set me off. I did wind up
pushing this rock further up the hill than I really meant to. Still, I got the
build under 1,400 warnings, and I now know how to address most of them in a
systematic way.
commit 7b9aace02b2405f0714bc08c424b72e6962f1c2e
Author: Mark Rustad mark.d.rus...@intel.com
Date: Fri Aug 15 01:43:44 2014 -0700
compiler: Add diagnostic control macros
Add macros to control diagnostic messages where needed. These
are used to silence warning messages that are expected, normal
and do not indicate any sort of problem. Reducing the stream
of messages in this way helps possible problems to stand out.
The macros provided are:
DIAG_PUSH() - to save diagnostic settings
DIAG_POP() - to restore diagnostic settings
DIAG_IGNORE(option) - to ignore a particular warning
DIAG_GCC_IGNORE(option) - DIAG_IGNORE for gcc only
DIAG_CLANG_IGNORE(option) - DIAG_IGNORE for clang only
Signed-off-by: Mark Rustad mark.d.rus...@intel.com
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index d1e49d5..039b112 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -10,3 +10,29 @@
#undef uninitialized_var
#define uninitialized_var(x) x = *((x))
#endif
+
+/*
+ * Provide macros to manipulate diagnostic messages when possible.
+ * DIAG_PUSH pushes the diagnostic settings
+ * DIAG_POP pops the diagnostic settings
+ * DIAG_IGNORE(x) changes the given diagnostic setting to ignore
+ *
+ * Example:
+ *DIAG_PUSH DIAG_IGNORE(aggregate-return)
+ *struct timespec ns_to_timespec(const s64 nsec)
+ *{
+ *...
+ *}
+ *DIAG_POP
+ *
+ * Will prevent the warning on compilation of the function. Other
+ * steps are necessary to do the same thing for the call sites.
+ */
While I do not want to disparage your/Mark's work here, my first thought
about this kind of annotation is that it seems to be a pretty big burden
to have to annotate all code with these sorts of things.
I wouldn't suggest annotating everything. However note that the annotations can
serve as a notice that something has been analyzed and deemed ok. That can be
useful as long as that is really true. I wouldn't take
Re: Overriding -Werror
Hi Mark,
(BTW, your mailer is creating some pretty long, unwrapped lines. I've
rewrapped them when quoting below.)
On Fri, Aug 15, 2014 at 08:36:07PM -0700, Mark D Rustad wrote:
On Aug 15, 2014, at 12:33 PM, Brian Norris computersforpe...@gmail.com
wrote:
On Fri, Aug 15, 2014 at 02:30:49AM -0700, Jeff Kirsher wrote:
Funny that you bring this up because I have ~60 patches in my queue to
resolve several thousand of these warnings. Half of the patches
actually resolve warnings that can be resolved and the other half
implement compiler diagnostic control macros to help silence warnings.
All these were the work of a co-worker Mark Rustad, below is the patch
he put together to implement the compiler diagnostic control macros.
While fixing warnings is usually a good thing (at least when done right;
there are plenty of ways to fight with the compiler over silly things,
but that's another discussion),
I have said at some presentations on the subject that resolving
warnings is not something you want an intern to do.
Nice.
I think that my issue is still
orthogonal to the one you're addressing. In my estimation, it is
impossible to guarantee that the entire kernel (including drivers) will
build without any warnings, across all levels of warning verbosity.
Thus, even with a valiant effort to fix or annotate all warnings, we
still won't get to the point where I can build 'make ARCH=mips W=1', if
-Werror is active.
Actually, some years ago, I got a MIPS Linux kernel to compile clean
with even more warnings than W=12 provides.
Congrats!
It can be done, but it certainly is not a state that is required and
cannot be maintained across all configurations, architectures and
compiler versions. This is the real world.
Right, and this is the crux of why I would like to have the option of
disabling -Werror systematically.
Besides, when testing *new* code, it's even more likely to have new
warnings, and I'd like to see as many as possible, without -Werror
getting in the way.
I have to say that I rather like -Werror.
It has its uses. I think it can be a pretty good option whenever the
compiler's warning level is kept to a reasonable level.
-Wextra, for instance, enables a lot of warnings that can be problematic
for little practical benefit (like -Wsign-compare, which notably is
explicitly overridden for x86).
One thing that not a lot of people are aware of is that you can
selectively allow some warnings. -Wno-error=shadow would allow shadow
warnings to be reported without being treated as errors.
That's interesting. I glazed over this option because I misinterpreted
the second sentence of this note in the gcc manpage:
Note that specifying -Werror=foo automatically implies -Wfoo.
However, -Wno-error=foo does not imply anything.
[ Really, -Wno-error=foo doesn't imply anything? So why does it exist? ;) ]
But now that you mention it, I think it could work as a hack for my
builds right now, since it isn't overridden by a blanket -Werror found
later on the command linie. So if I do
make ARCH=mips W=1 KCFLAGS=-Wno-error=...(long list of warnings that break
the build)...
then I can systematically perform the build-tests I'd like.
So I still think -Werror is fundamentally wrong in some cases, and I
would like to pursue some approach like in my original post.
BTW, for a little more context: I realize the output of 'make W=[123]'
may not be very useful on its own, sometimes, but it's actually pretty
useful to quickly catch potential issues in new code, by diff'ing the
warnings in the before/after build logs. In this case, it's not helpful
at all if the first build fails due to dubious warnings. I'm doing
this in the context of Aiaiai [1]. Right now, I have to keep around a
few local patches to remove -Werror from arch/{mips,sh}.
The problem is that when a kernel build throws over 125,000 warnings,
it just becomes completely useless. That was what kind of set me off.
Yeah that's bad. But it *still* can provide a helpful diff for tools
like Aiaiai. The difference between 125,000 and 125,001 warnings still
can be determined automatically for new code, although it's not helpful
if every new warning is actually just because you used a new core
header that causes a lot of warnings.
I did wind up pushing this rock further up the hill than I really
meant to. Still, I got the build under 1,400 warnings, and I now know
how to address most of them in a systematic way.
commit 7b9aace02b2405f0714bc08c424b72e6962f1c2e
Author: Mark Rustad mark.d.rus...@intel.com
Date: Fri Aug 15 01:43:44 2014 -0700
compiler: Add diagnostic control macros
Add macros to control diagnostic messages where needed. These
are used to silence warning messages that are expected, normal
and do not indicate any sort of problem. Reducing the stream
of messages in this way helps possible problems to stand out.
The macros
Overriding -Werror
Hi all, I'm interested in being able to build-test kernels on various architectures while enabling extra warnings (make W=[123]). I'd like to be able to finish the builds and see all warnings, rather than seeing a failed build. However, GCC's -Werror is incompatible with this. There is plenty of code that will produce at least one warning, when warning verbosity is turned up. And GCC's -Werror is not guaranteed to remain stable over time; new versions may develop new warnings that may or may not be legitimate. It seems that there are a few problem ARCHes that enable -Werror by default: SuperH (orphaned), SPARC, and MIPS. There are also a few scattered Makefiles throughout the build tree. Developers have previously tried to remove some of the worst offenders [1], but were mostly rejected [2]. It doesn't seem like we can fully prevent maintainers from enabling -Werror on their code--or even on their entire ARCH build, as with MIPS--for better or worse, so I look to other alternatives. For the easiest approach, I considered how one might add -Wno-error to the CFLAGS. 'make KCFLAGS=-Wno-error' looked promising, but KBUILD_CFLAGS is applied before the sub-directory Makefiles add their own options to ccflags-y. So it seems like others have come to the same conclusion as me: that Kbuild doesn't seem to provide a way to override the -Werror behvaior from the top level. [3][4] So, how can we fix this? -Werror may be useful in some cases to encourage developers to fix up their code immediately, but it is decidedly unhelpful when running code through analysis tools. Possibilities include: 1. make -Werror be applied only when we do not have W=[123]. [5] 2. develop a top-level override for CFLAGS that is applied *after* all sub-directory modifications 3. make -Werror opt-in / configurable, like PPC's CONFIG_PPC_WERROR (maybe make it a generic CONFIG_WERROR?), and prevent its unconditional use in Makefiles 4. better ideas? Regards, Brian [1] http://www.linux-mips.org/archives/linux-mips/2012-04/msg00179.html http://patchwork.ozlabs.org/patch/146297/ [2] http://www.linux-mips.org/archives/linux-mips/2012-05/msg00064.html [3] http://lists.linaro.org/pipermail/linaro-toolchain/2011-November/001869.html [4] http://lists.linaro.org/pipermail/linaro-dev/2011-December/008880.html [5] http://www.linux-mips.org/archives/linux-mips/2012-05/msg00070.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/
Overriding -Werror
Hi all, I'm interested in being able to build-test kernels on various architectures while enabling extra warnings (make W=[123]). I'd like to be able to finish the builds and see all warnings, rather than seeing a failed build. However, GCC's -Werror is incompatible with this. There is plenty of code that will produce at least one warning, when warning verbosity is turned up. And GCC's -Werror is not guaranteed to remain stable over time; new versions may develop new warnings that may or may not be legitimate. It seems that there are a few problem ARCHes that enable -Werror by default: SuperH (orphaned), SPARC, and MIPS. There are also a few scattered Makefiles throughout the build tree. Developers have previously tried to remove some of the worst offenders [1], but were mostly rejected [2]. It doesn't seem like we can fully prevent maintainers from enabling -Werror on their code--or even on their entire ARCH build, as with MIPS--for better or worse, so I look to other alternatives. For the easiest approach, I considered how one might add -Wno-error to the CFLAGS. 'make KCFLAGS=-Wno-error' looked promising, but KBUILD_CFLAGS is applied before the sub-directory Makefiles add their own options to ccflags-y. So it seems like others have come to the same conclusion as me: that Kbuild doesn't seem to provide a way to override the -Werror behvaior from the top level. [3][4] So, how can we fix this? -Werror may be useful in some cases to encourage developers to fix up their code immediately, but it is decidedly unhelpful when running code through analysis tools. Possibilities include: 1. make -Werror be applied only when we do not have W=[123]. [5] 2. develop a top-level override for CFLAGS that is applied *after* all sub-directory modifications 3. make -Werror opt-in / configurable, like PPC's CONFIG_PPC_WERROR (maybe make it a generic CONFIG_WERROR?), and prevent its unconditional use in Makefiles 4. better ideas? Regards, Brian [1] http://www.linux-mips.org/archives/linux-mips/2012-04/msg00179.html http://patchwork.ozlabs.org/patch/146297/ [2] http://www.linux-mips.org/archives/linux-mips/2012-05/msg00064.html [3] http://lists.linaro.org/pipermail/linaro-toolchain/2011-November/001869.html [4] http://lists.linaro.org/pipermail/linaro-dev/2011-December/008880.html [5] http://www.linux-mips.org/archives/linux-mips/2012-05/msg00070.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/
