Bug 2499

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
 ^