Changchun, actually I have looked at this a little bit more and I haven't been able to convince myself for sure of whether there is a bug or not. It seems suspicious to call a method on the inactive member of a union, but on the other hand both members are POD types and the UnsafeSetDefault() method does not actually read from the raw union data (it just writes to it to reinitialize it). The C++14 standard says this in 9.5.4 (and similarly for C++11):
--- [ Note: In general, one must use explicit destructor calls and placement new operators to change the active member of a union. — end note ] [Example: Consider an object u of a union type U having non-static data members m of type M and n of type N. If M has a non-trivial destructor and N has a non-trivial constructor (for instance, if they declare or inherit virtual functions), the active member of u can be safely switched from m to n using the destructor and placement new operator as follows: u.m.~M(); new (&u.n) N; — end example ] --- It's not entirely clear but I think it hints that if the members are POD types then you might not need to explicitly call the destructor and placement new operator in order to change the active member of the union--their example suggests that you have to do this only if the types have non-trivial constructors and/or destructors. So I think the code *might* be ok, and that makes me hesitate to spend much time trying to fix it unless it's causing a real problem. If you would like to get the warning to go away, would it be possible for you to try a newer version of g++? With g++ 4.8.4 I can't reproduce the warning. On Fri, Nov 18, 2016 at 8:00 PM, <[email protected]> wrote: > Adam, thank you for debugging and quick response! > > So is any fix planned in next 3.1 patch? Or need to wait little bit > further? > > Regards. > Changchun > > On Friday, November 18, 2016 at 8:18:36 PM UTC-5, Adam Cozzette wrote: >> >> I think I see what's going on. Your oneof field gets stored as a union >> that looks like this in the generated code: >> >> union MsgDataUnion { >> MsgDataUnion() {} >> ::google::protobuf::internal::ArenaStringPtr first_; >> ::google::protobuf::uint32 second_; >> } msg_data_; >> >> In DemoMessage::mutable_first() we call msg_data_.first_.UnsafeSetDefault(), >> but this is technically incorrect because we are calling a method on one of >> the types in the union without first initializing it. For example it could >> be that the union is actually in the uint32 form and yet we are attempting >> to call an ArenaStringPtr method on the data. I think this works out OK in >> practice but is probably technically undefined behavior and so we should >> fix it. >> >> On Mon, Nov 14, 2016 at 10:05 AM, <[email protected]> wrote: >> >>> Adam, yes, forgot providing my setup, here it is: >>> >>> $ g++ --version >>> g++ (GCC) 4.4.7 20120313 (Red Hat 4.4.7-3) >>> Copyright (C) 2010 Free Software Foundation, Inc. >>> This is free software; see the source for copying conditions. There is >>> NO >>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR >>> PURPOSE. >>> >>> $ uname -a >>> Linux demo-host 2.6.32-358.el6.x86_64 #1 SMP Tue Jan 29 11:47:41 EST >>> 2013 x86_64 x86_64 x86_64 GNU/Linux >>> >>> *And generated code around the problematic line*: >>> 262 inline ::std::string* DemoMessage::mutable_first() { >>> 263 if (!has_first()) { >>> 264 clear_msg_data(); >>> 265 set_has_first(); >>> 266 msg_data_.first_.UnsafeSetDefault(&::google::protobuf::internal >>> ::GetEmptyStringAlreadyInited()); >>> 267 } >>> >>> I have also attached the generated files from my system for your >>> reference. >>> >>> Thanks again! >>> >>> Regards, >>> Changchun >>> >>> On Monday, November 14, 2016 at 12:50:56 PM UTC-5, Adam Cozzette wrote: >>>> >>>> Thanks, Changchun. I have just a few more questions to try to debug the >>>> problem: >>>> - What version of g++ are you using? (g++ --version should tell you) >>>> - What architecture are you building for? >>>> - Could you send the generated code around line 266 in demo.pb.h just >>>> so that it's clear what lines the compiler is warning about? >>>> >>>> On Mon, Nov 14, 2016 at 7:55 AM, <[email protected]> wrote: >>>> >>>>> Adam, Thanks for your reply. >>>>> >>>>> Here I attached a simple .proto file to demonstrate this warning >>>>> message: >>>>> >>>>> $ protobuf-3.1/bin/protoc --cpp_out=. demo.proto >>>>> >>>>> $ /usr/bin/g++ -Iprotobuf-3.1/include -c -Wall -O2 -Wno-long-long demo >>>>> .pb.cc -o demo.pb.o >>>>> In member function 'virtual bool demo::DemoMessage::MergePartia >>>>> lFromCodedStream(google::protobuf::io::CodedInputStream*)': >>>>> cc1plus: warning: dereferencing pointer 'prephitmp.1630' does break >>>>> strict-aliasing rules >>>>> demo.pb.h:266: note: initialized from here >>>>> cc1plus: note: initialized from here >>>>> >>>>> Please let me know if anything else you need to debug further. >>>>> >>>>> Thanks, >>>>> Changchun >>>>> >>>>> On Friday, November 11, 2016 at 4:45:03 PM UTC-5, Adam Cozzette wrote: >>>>>> >>>>>> What version of g++ are you using? Also, do you have a way of >>>>>> determining what actual line of code the compiler is warning about there? >>>>>> >>>>>> I tried building with -Wstrict-aliasing=2 at commit df8390790a and >>>>>> saw a few warnings like this: >>>>>> >>>>>> ./google/protobuf/generated_message_util.h:94:66: warning: >>>>>> dereferencing type-punned pointer might break strict-aliasing rules >>>>>> [-Wstrict-aliasing] >>>>>> const T& get() const { return reinterpret_cast<const T&>(union_); } >>>>>> >>>>>> But that doesn't look like it's quite the same warning as what you >>>>>> are seeing. >>>>>> >>>>>> On Fri, Nov 11, 2016 at 12:39 PM, <[email protected]> wrote: >>>>>> >>>>>>> I have below option in my .proto file: >>>>>>> >>>>>>> *option cc_enable_arenas = true;* >>>>>>> >>>>>>> But g++ for Linux gives below warning: >>>>>>> >>>>>>> *cc1plus: warning: dereferencing pointer 'prephitmp.4925' does break >>>>>>> strict-aliasing rules* >>>>>>> >>>>>>> Will this cause issue and how to solve it? >>>>>>> >>>>>>> -- >>>>>>> You received this message because you are subscribed to the Google >>>>>>> Groups "Protocol Buffers" group. >>>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>>> send an email to [email protected]. >>>>>>> To post to this group, send email to [email protected]. >>>>>>> Visit this group at https://groups.google.com/group/protobuf. >>>>>>> For more options, visit https://groups.google.com/d/optout. >>>>>>> >>>>>> >>>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "Protocol Buffers" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to [email protected]. >>>>> To post to this group, send email to [email protected]. >>>>> Visit this group at https://groups.google.com/group/protobuf. >>>>> For more options, visit https://groups.google.com/d/optout. >>>>> >>>> >>>> -- >>> You received this message because you are subscribed to the Google >>> Groups "Protocol Buffers" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected]. >>> To post to this group, send email to [email protected]. >>> Visit this group at https://groups.google.com/group/protobuf. >>> For more options, visit https://groups.google.com/d/optout. >>> >> >> -- > You received this message because you are subscribed to the Google Groups > "Protocol Buffers" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at https://groups.google.com/group/protobuf. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Protocol Buffers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/protobuf. For more options, visit https://groups.google.com/d/optout.
