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.