Compilation warning with "arenas" option enabled for protobuf-3.1

[changchun...@gmail.com]

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?

[Adam Cozzette]

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.

--
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 protobuf+unsubscribe@googlegroups.com.
To post to this group, send email to prot...@googlegroups.com.
Visit this group at https://groups.google.com/group/protobuf.
For more options, visit https://groups.google.com/d/optout.

[changchun...@gmail.com]

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::MergePartialFromCodedStream(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, <changchun...@gmail.com> 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 protobuf+u...@googlegroups.com.

[Adam Cozzette]

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?

To unsubscribe from this group and stop receiving emails from it, send an email to protobuf+unsubscribe@googlegroups.com.

[changchun...@gmail.com]

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

[Adam Cozzette]

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.

To unsubscribe from this group and stop receiving emails from it, send an email to protobuf+unsubscribe@googlegroups.com.

[changchun...@gmail.com]

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

[Adam Cozzette]

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.

To unsubscribe from this group and stop receiving emails from it, send an email to protobuf+unsubscribe@googlegroups.com.

[Zhao, Changchun]

Thank you, Adam, if it’s confirmed just matter of warning no harm, then I can live with it for now. Once I can upgrade the compiler, then as you tested, it should go away.

 

Regards,

----

Changchun Zhao | Software Engineer

Transaction Network Services

10740 Parkridge Blvd | Suite 100 | Reston | VA | 20191 | USA

O: +1 703 667 4674 | M: +1 240 535 2615 | E: cz...@tnsi.com | www.tnsi.com

 

One Connection – A World of Opportunities

---

This e-mail message is for the sole use of the intended recipient(s)and may
contain confidential and privileged information of Transaction Network Services.
Any unauthorized review, use, disclosure or distribution is prohibited. If you
are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message.