Using DEFINE_string causes error in optimized compiles about gflags HOT 8 CLOSED

I guess gcc 4.4 turned on strict aliasing by default?  Or maybe the detection 
just 
got better; I'm not seeing any problems with gcc 4.2, even with 
-fstrict-aliasing 
explicitly enabled.

In any cased, I thought strict aliasing problems were a warning, not an error.  
Are 
you using -Werror as well?

I don't have gcc 4.4.1 handy.  Can you try compiling, say, the unittests (via 
'make 
check'), and attaching the full output?  With line numbers, it should be easy 
to 
diagnose the problem and, hopefully, fix it.

Yes, I'm using -Werror for my compiles - that's why I see the error. Otherwise, 
its a 
warning - which is still ugly.

Attached is a file that shows the complete output when a 'make' is done inside 
the 
gflags-1.2 source. You can see the relevant warnings when the unittests are 
being 
compiled. Here's an excerpt:

src/gflags_unittest.cc:84: warning: dereferencing type-punned pointer will 
break 
strict-aliasing rules
src/gflags_unittest.cc:85: warning: dereferencing type-punned pointer will 
break 
strict-aliasing rules
src/gflags_unittest.cc:94: warning: dereferencing type-punned pointer will 
break 
strict-aliasing rules
src/gflags_unittest.cc:104: warning: dereferencing type-punned pointer will 
break 
strict-aliasing rules
src/gflags_unittest.cc:105: warning: dereferencing type-punned pointer will 
break 
strict-aliasing rules
src/gflags_unittest.cc:106: warning: dereferencing type-punned pointer will 
break 
strict-aliasing rules
src/gflags_unittest.cc:109: warning: dereferencing type-punned pointer will 
break 
strict-aliasing rules
src/gflags_unittest.cc:123: warning: dereferencing type-punned pointer will 
break 
strict-aliasing rules
src/gflags_unittest.cc:134: warning: dereferencing type-punned pointer will 
break 
strict-aliasing rules

Just a little more information to help out with fixing this bug. The line 
numbers 
reported by the compiler while emitting the warning correspond to the location 
where 
the DEFINE_string macro is used. It doesn't give the precise line within the 
macro's 
implementation that's causing the warning to be emitted. 

So what I did was replace one of the uses of the macro DEFINE_string with its 
implementation (after fixing the implementation appropriately to make it valid 
outside a macro definition). The compiler reported the warning this time at the 
following line within the macro's implementation:

   std::string& FLAGS_##name = *(reinterpret_cast<std::string*>(s_##name[0].s));

I found that changing the offending line:
   std::string& FLAGS_##name = *(reinterpret_cast<std::string*>(s_##name[0].s));
to:
   std::string& FLAGS_##name = *const_cast<std::string*>(FLAGS_no##name);
fixes the issue. Given below is the complete macro implementation with the fix.


#define DEFINE_string(name, val, txt)                                     \
  namespace fLS {                                                         \
    static union { void* align; char s[sizeof(std::string)]; } s_##name[2]; \
    const std::string* const FLAGS_no##name = new (s_##name[0].s) std::string(val); \
    static ::google::FlagRegisterer o_##name(                \
      #name, "string", MAYBE_STRIPPED_HELP(txt), __FILE__,                \
      s_##name[0].s, new (s_##name[1].s) std::string(*FLAGS_no##name));   \
    extern std::string& FLAGS_##name;                                     \
    using fLS::FLAGS_##name;                                              \
    std::string& FLAGS_##name = *const_cast<std::string*>(FLAGS_no##name);   \
  }                                                                       \
  using fLS::FLAGS_##name

Nice fix!  But I'm wondering now why we'd bother to have the const at all in 
this 
case.  What if you changed it to this:
---
#define DEFINE_string(name, val, txt)                                     \
  namespace fLS {                                                         \
    static union { void* align; char s[sizeof(std::string)]; } s_##name[2]; \
    std::string* const FLAGS_no##name = new (s_##name[0].s) std::string(val); \
    static ::google::FlagRegisterer o_##name(                \
      #name, "string", MAYBE_STRIPPED_HELP(txt), __FILE__,                \
      s_##name[0].s, new (s_##name[1].s) std::string(*FLAGS_no##name));   \
    extern std::string& FLAGS_##name;                                     \
    using fLS::FLAGS_##name;                                              \
    std::string& FLAGS_##name = *FLAGS_no##name;   \
  }                                                                       \
  using fLS::FLAGS_##name
---

Does that still work for you?  If so, I'll make it part of the next release.

> Does that still work for you?  If so, I'll make it part of the next release.

That was actually my first fix and that does work too. The only reason I even 
suggested 
the other fix was because I wanted to keep the suggested changes to a minimum. 
:)

This should be fixed in gflags 1.4, just released.