| Message ID | CAL3LdT7-Ai7Dv93vrDP1o2BSVrmFAyTN=UAwE8n5+PVjpN1h0Q@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.rustad@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.rustad@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-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Brian, On Aug 15, 2014, at 12:33 PM, Brian Norris <computersforpeace@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.rustad@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.rustad@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 new code from a new developer that included such annotations. > 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. Yes, it is VERY valuable for addressing warnings thrown by core kernel includes. Just a few static inlines with warnings produced about 50,000 of those 125,000 warnings. > 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. I look forward to the discussion. I have no illusion about all my patches going in. In fact there are instances where I used the diagnostic control macros to silence a bunch of warnings because it made for a much smaller patch - a patch to resolve them would have had to replace an entire table, so I guess I hope to get a request to do that. The exchanges I have had with the few patches that have been sent thus far have been productive, so I hope for more like that. I do think that the diagnostic control macros serve a useful purpose if used well. I agree that they can be abused, like anything else, but some rules about their use should help. Maybe checkpatch should warn on patches that add them...
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 <computersforpeace@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.rustad@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.rustad@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 new code from a new developer that included such > annotations. Good points. And I'd recommend framing your argument as such if/when you post your patches for real. Maybe even add comments near the macro definitions; "with great power comes great responsibility." Thanks for your thoughts, Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 15, 2014, at 9:34 PM, Brian Norris <computersforpeace@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 <computersforpeace@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 it's not helpful > if every "new" warning is actually just because you used a new core > header that causes a lot of warnings. Yes, but it is still a bad situation to have so many. And a lot of them are trivial things like nested externs in static inline functions in include files. I'm not wild about the nested externs - it means that there are multiple sources for the prototype of those functions - but since that practice has now been used to disentangle include files, we may as well silence the warnings for those usages. The annotations might also be a bit of an ongoing reminder that there are issues with having done that. >> 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.rustad@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.rustad@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 new code from a new developer that included such >> annotations. > > Good points. And I'd recommend framing your argument as such if/when you > post your patches for real. Maybe even add comments near the macro > definitions; "with great power comes great responsibility." That is not a bad idea. And I do think that checkpatch ought to warn when they are added. It certainly is a bigger deal than a too long line! I am contemplating adding a DIAG_ERROR macro to enable an error on a warning. The use case I have come up with is switch-enum. Then, switch statements that are expected to itemize every value in an enum can force that error on for just those switch statements, because generally that is not needed. At least that is the best possible use I have come up with for that kind of macro. Haven't tried it yet, but that seems potentially useful to me. > Thanks for your thoughts, > Brian
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 --- a/include/linux/compiler-gcc4.h +++ b/include/linux/compiler-gcc4.h @@ -86,3 +86,34 @@ #define __HAVE_BUILTIN_BSWAP16__ #endif #endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP */ + +/* + * 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. + */ +#if GCC_VERSION >= 40600 +#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(GCC diagnostic x) +#define DIAG_IGNORE(x) DIAG_PRAGMA(ignored DIAG_CAT(-W, x)) +/* GCC 4.8 - 4.8.2 has issues with pop, so do not define push or pop */ +#if GCC_VERSION < 40800 || GCC_VERSION >= 40803 +#define DIAG_PUSH DIAG_PRAGMA(push) +#define DIAG_POP DIAG_PRAGMA(pop) +#endif /* GCC_VERSION < 40800 || GCC_VERSION >= 40803 */ +#define DIAG_GCC_IGNORE(x) DIAG_IGNORE(x) +#endif /* GCC_VERSION >= 40600 */ diff --git a/include/linux/compiler.h b/include/linux/compiler.h index d5ad7b1..fde7b21 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -94,6 +94,26 @@ struct ftrace_branch_data { }; /* + * Provide null definitions for diagnostic manipulation macros not provided + * for a particular compiler. + */ +#ifndef DIAG_PUSH +#define DIAG_PUSH +#endif +#ifndef DIAG_POP +#define DIAG_POP +#endif +#ifndef DIAG_IGNORE +#define DIAG_IGNORE(x) +#endif +#ifndef DIAG_CLANG_IGNORE +#define DIAG_CLANG_IGNORE(x) +#endif +#ifndef DIAG_GCC_IGNORE +#define DIAG_GCC_IGNORE(x) +#endif + +/* * Note: DISABLE_BRANCH_PROFILING can be used by special lowlevel code * to disable branch tracing on a per file basis. */