Bugzilla – Bug 2499
resolving -Wstrict-overflow warnings
Last modified: 2016-09-13 19:35:09 UTC
a recent changeset enabed -Wstrict-overflow=5 for all gcc versions >= 4.8.2. Previously, we had patched the codebase for all such warnings on gcc-4.8.2 (fedora core 20): http://code.nsnam.org/ns-3-dev/rev/2a65963e27ac We had set up a FC 20 buildslave to keep track of this, and disabled checking for gcc > 4.8.2 (since it was observed at the time that different compiler versions were detecting new warning issues). That is, checking was done only on gcc version == 4.8.2 (on a FC 20 buildslave). However, we are well past FC 20 now and are not checking gcc-4.8.2 any more. -Wstrict-overflow warning levels are documented here; there are 5 levels, with level 5 being the most strict. https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html For gcc-5.3.1, I am observing 473 current warnings at level 5, 408 at level 4, 407 at level 3, and none at level 2. Most of these warnings are not due to changes in our codebase, but changes to gcc to emit more warnings. For the immediate release, we could consider to disable this (revert the level back to 2).
Created attachment 2572 [details] warning list at level 5
Created attachment 2573 [details] warning list at level 4
The list of errors is certainly daunting. A few even seem intractable. For example, look at the error reported in src/core/examples/hash-example.cc. The error is ../src/core/examples/hash-example.cc: In member function ‘bool ns3::Hash::Example::DictFiles::Add(std::string)’: ../src/core/examples/hash-example.cc:465:8: error: assuming signed overflow does not occur when changing X +- C1 cmp C2 to X cmp C2 -+ C1 [-Werror=strict-overflow] bool Add (const std::string file) ^ The Add function is: bool Add (const std::string file) { if (std::find (m_files.begin (), m_files.end (), file) == m_files.end ()) { m_files.push_back (file); } return true; } m_files is a std::vector<std::string>. Where is the arithmetic in a comparison? I suspect it's in std::find. How am I supposed to fix that? A similar case, utils/print-introspected-doxygen.cc: template <typename T> void Uniquefy (T t) { std::sort (t.begin (), t.end ()); t.erase (std::unique (t.begin (), t.end ()), t.end ()); } Both std::sort and std::unique use comparisons, but I can't fix them.
This SO response is informative: http://stackoverflow.com/questions/23020208/why-is-gcc-4-8-2-complaining-about-addition-under-strict-overflow A function with a reasonable argument check: hi >= lo The function gets inlined, with hi = u + 3, lo = u GCC figures out the condition is now u + 3 >= u, but is worried u + 3 might overflow The answer quotes GCC docs section 3.8: "this warning can easily give a false positive: a warning about code that is not actually a problem."
Hi, We can use #pragma around code that comes from standard libraries, even if I don't know why it hasn't been reported to GCC devel yet (or it has, but I can't find the upstream bug)
BTW, I'm trying to reproduce these warnings. What configure params do you use? Thanks :)
(In reply to natale.patriciello from comment #6) > BTW, I'm trying to reproduce these warnings. What configure params do you > use? Thanks :) ./waf configure -d optimized --enable-examples --enable-tests
It is not clear we will ever want to enable level >2 without some suppressions in place, and we need to audit ~400 warnings, so I suggest that for now we try to reduce the warning level in the wscript to 2. We could leave this open to perhaps audit at a future date the list of warnings at levels 4 and 5.
Suggest we close 1868 to all current discussion here.
I'm disturbed by the warnings on STL code. The most helpful things I've found: The three LLVM blog posts beginning here: http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html The starting assumption, rarely stated, is that compiler optimizations *assume* you didn't mean to invoke undefined behavior, so they don't have to consider those cases in optimizing. Look at the "RNCE before DCE" example in part 2 of those posts. http://stackoverflow.com/questions/18521501 Works through a nice example, showing how to refactor a conditional. http://stackoverflow.com/questions/23020208 Another nice example, where function inlining enables optimizations. The error is reported on a perfectly sensible assert conditional, despite the fact that the addition which enables the optimization is in the caller site. This matches our STL cases exactly. Here's my question on SO: http://stackoverflow.com/questions/39421416 which got some mildly useful replies. Here's the pragma stanza to wrap a false positive: #ifdef GNUC #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wstrict-overflow" source line #pragma GCC diagnostic pop #endif
+1 to Tom's pragmatic immediate approach in comment #8: fix at level=2 for now, then revisit later To help us triage false positives in the future I suggest that, instead of the five lines of preprocessor foo, we add a standard comment to the line of code. (Do in line comments survive to the warning output?) source line // 2016-09-12 false positive strict-overflow
(In reply to Peter Barnes from comment #11) > +1 to Tom's pragmatic immediate approach in comment #8: fix at level=2 for > now, > then revisit later I pushed the change to level 2, and closed 1868 as RESOLVED, MOVED. One lingering comment from that report was to consider (in the future) to raise the warnings level here: diff -r 0114887f7644 wscript --- a/wscript Mon Sep 29 22:07:14 2014 +0100 +++ b/wscript Wed Oct 15 19:57:04 2014 -0700 @@ -45,7 +45,7 @@ cflags.profiles = { # profile name: [optimization_level, warnings_level, debug_level] - 'debug': [0, 2, 3], + 'debug': [0, 3, 3], 'optimized': [3, 2, 1], 'release': [3, 2, 0], } but that will trigger a new batch of warnings. > > To help us triage false positives in the future I suggest that, instead of > the > five lines of preprocessor foo, we add a standard comment to the line of > code. > (Do in line comments survive to the warning output?) > > source line // 2016-09-12 false positive strict-overflow In looking at the warnings report, it will typically preserve the line that it points to, but it may point to an unhelpful line number such as: ../src/flow-monitor/model/histogram.cc:149:1: error: assuming signed overflow does not occur when reducing constant in comparison [-Werror=strict-overflow] } // namespace ns3 ^