Bug 1738

Summary:	strict aliasing compiler bug
Product:	ns-3	Reporter:	Tom Henderson <tomh>
Component:	core	Assignee:	Peter Barnes <pdbarnes>
Status:	RESOLVED FIXED
Severity:	normal	CC:	mathieu.lacage, ns-bugs, vedran
Priority:	P5
Version:	pre-release
Hardware:	All
OS:	All

Description Tom Henderson 2013-07-26 17:44:54 UTC

the hash-murmur3.cc code still reports strict aliasing problems, but only on gcc-4.4.3:

cc1plus: warnings being treated as errors
../src/core/model/hash-murmur3.cc: In member function ‘virtual uint64_t ns3::Hash::Function::Murmur3::GetHash64(const char*, size_t)’:
../src/core/model/hash-murmur3.cc:337: error: dereferencing pointer ‘res’ does break strict-aliasing rules
../src/core/model/hash-murmur3.cc:336: note: initialized from here
../src/core/model/hash-murmur3.cc:338: error: dereferencing pointer ‘<anonymous>’ does break strict-aliasing rules
../src/core/model/hash-murmur3.cc:338: note: initialized from here
../src/core/model/hash-murmur3.cc:339: error: dereferencing pointer ‘<anonymous>’ does break strict-aliasing rules
../src/core/model/hash-murmur3.cc:339: note: initialized from here
../src/core/model/hash-murmur3.cc:340: error: dereferencing pointer ‘<anonymous>’ does break strict-aliasing rules
../src/core/model/hash-murmur3.cc:340: note: initialized from here 

Peter traced this to a gcc bug:
http://gcc.gnu.org/bugzilla/﷒0﷓

and we verified that it is the culprit.

So, options seem to be:

1) tell users that if they want to run with this compiler, to disable -Werror as documented here:
http://www.nsnam.org/wiki/index.php/HOWTO_build_old_versions_of_ns-3_on_newer_compilers

2) try to fix this with compiler-specific macros, similar to how boost handles GCC_DIAG_OFF:

https://svn.boost.org/trac/boost/wiki/Guidelines/WarningsGuidelines

Comment 1 Peter Barnes 2013-07-26 18:14:39 UTC

Tom wrote:
> Initial reaction was "no", but after a bit more thought, it might make my maintenance a bit easier 
> (I wouldn't have to always build with custom CXXFLAGS on that system) and I guess it could be 
> removed over time.
> 
> would the macro be defined in something like a warnings.h file?

Sure, or some other options:

compiler.h
compiler-warnings.h
compiler-features.h

I lean toward warnings.h or compiler-features.h

Right now we are in danger of proliferating feature_n.h for every feature we want to cover, such as NS_UNUSED, NS_DEPRECATED, and now, NS_NO_STRICT_ALIAS or some such.

So, in addition to handling the gcc-4.4 issue with strict-aliasing, I guess I'm proposing we move NS_UNUSED* and NS_DEPRECATED into a single header to deal with compiler-specific issues.

BTW, all of these cases (strict aliasing, unused and deprecated) could/should be extended to MSVS, clang, ...

Comment 2 Vedran Miletić 2013-08-12 17:43:46 UTC

+1 for moving to common header.

Comment 3 Peter Barnes 2013-08-14 16:26:32 UTC

On Aug 12, 2013, at 2:33 PM, Tom Henderson <tomh@tomh.org>
wrote:
> 2) strict aliasing compiler bugs (bug 1738)
> I like your idea for moving this sort of stuff into compiler-features.h or warnings.h.  Do you think
> something could be turned around quickly to do that?  However, to just make progress at the 
> moment, would an explicit file-scoped pragma to disable this aliasing check work until this is 
> more uniformly supported?

I think the general compiler-features.h is the way to go, but more
involved than I have bandwidth for, until the time patch is done.

I'll add

 #pragma GCC diagnostic ignored "-Wstrict-aliasing"

to src/core/model/hash-murmur3.cc

Agreed?

Comment 4 Peter Barnes 2013-08-14 16:28:02 UTC

Done, commit 10144 (f14c652ad233)

http://code.nsnam.org/ns-3-dev/rev/f14c652ad233

Comment 5 Tom Henderson 2013-08-14 16:55:01 UTC

(In reply to comment #4)
> Done, commit 10144 (f14c652ad233)
> 
> http://code.nsnam.org/ns-3-dev/rev/f14c652ad233

this commit clears the compilation error that was experienced on Ubuntu 10.04 system (gcc-4.4.3) under './waf -d optimized --enable-static' configuration