Status: New
Owner: [email protected]
Labels: Type-Defect Priority-Medium

New issue 539 by [email protected]: Undefined behavior in src/google/protobuf/compiler/cpp/cpp_helpers.cc
http://code.google.com/p/protobuf/issues/detail?id=539

While trying to update protobuf in Chrome to r428 I've encountered the following warning reported by Visual Studio 10 (http://build.chromium.org/p/tryserver.chromium/builders/win_rel/builds/181826/steps/compile/logs/stdio):

FAILED: ninja -t msvc -e environment.x86 -- E:\b\build\goma\gomacc.exe "C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin\cl.exe" /nologo /showIncludes /FC @obj\third_party\protobuf\src\google\protobuf\compiler\cpp\protoc.cpp_helpers.obj.rsp /c ..\..\third_party\protobuf\src\google\protobuf\compiler\cpp\cpp_helpers.cc /Foobj\third_party\protobuf\src\google\protobuf\compiler\cpp\protoc.cpp_helpers.obj /Fdobj\third_party\protobuf\protoc.pdb e:\b\build\slave\win\build\src\third_party\protobuf\src\google\protobuf\compiler\cpp\cpp_helpers.cc(265) : error C2220: warning treated as error - no 'object' file generated e:\b\build\slave\win\build\src\third_party\protobuf\src\google\protobuf\compiler\cpp\cpp_helpers.cc(265) : warning C4146: unary minus operator applied to unsigned type, result still unsigned e:\b\build\slave\win\build\src\third_party\protobuf\src\google\protobuf\compiler\cpp\cpp_helpers.cc(265) : warning C4146: unary minus operator applied to unsigned type, result still unsigned

The corresponding lines are:

259 string DefaultValue(const FieldDescriptor* field) {
260   switch (field->cpp_type()) {
261     case FieldDescriptor::CPPTYPE_INT32:
262       // gcc rejects the decimal form of kint32min and kint64min.
263       if (field->default_value_int32() == kint32min) {
264         // Make sure we are in a 2's complement system.
265 GOOGLE_COMPILE_ASSERT(kint32min == -0x80000000, kint32min_value_error);
266         return "-0x80000000";
267       }

This code attempts to cast -0x80000000 to a signed int32 type, in which this value cannot be represented. This is undefined behavior in C++.

The following patch shall fix the problem by casting to unsigned types, for which the overflow is defined.
I'm also unsure if we should keep the "-" sign in front of the return value.

Index: /usr/local/google/protobuf-read-only/src/google/protobuf/compiler/cpp/cpp_helpers.cc
===================================================================
--- /usr/local/google/protobuf-read-only/src/google/protobuf/compiler/cpp/cpp_helpers.cc (revision 504) +++ /usr/local/google/protobuf-read-only/src/google/protobuf/compiler/cpp/cpp_helpers.cc (working copy)
@@ -262,7 +262,9 @@
       // gcc rejects the decimal form of kint32min and kint64min.
       if (field->default_value_int32() == kint32min) {
         // Make sure we are in a 2's complement system.
- GOOGLE_COMPILE_ASSERT(kint32min == -0x80000000, kint32min_value_error);
+        GOOGLE_COMPILE_ASSERT(
+            (uint32)kint32min == (uint32)0 - (uint32)0x80000000,
+            kint32min_value_error);
         return "-0x80000000";
       }
       return SimpleItoa(field->default_value_int32());
@@ -272,8 +274,10 @@
       // See the comments for CPPTYPE_INT32.
       if (field->default_value_int64() == kint64min) {
         // Make sure we are in a 2's complement system.
- GOOGLE_COMPILE_ASSERT(kint64min == GOOGLE_LONGLONG(-0x8000000000000000),
-                       kint64min_value_error);
+        GOOGLE_COMPILE_ASSERT(
+            (uint64)kint64min ==
+                (uint64)0 - (uint64)GOOGLE_LONGLONG(0x8000000000000000),
+            kint64min_value_error);
         return "GOOGLE_LONGLONG(-0x8000000000000000)";
       }
return "GOOGLE_LONGLONG(" + SimpleItoa(field->default_value_int64()) + ")";


--
You received this message because this project is configured to send all issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

--
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 http://groups.google.com/group/protobuf.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to