Removing undefined behavior and compiler warnings (#1315) * Comment out unused arguments. These last few are all that's needed to compile with -Wunused-arguments. * Fix missing struct field initializer. With this fix, everything compiles with -Wmissing-field-initializers. * Add support for disabling unaligned memory accesses on x86 too. ubsan doesn't like these because they are technically undefined behavior, so -DGOOGLE_PROTOBUF_DONT_USE_UNALIGNED will disable them easily. * Avoid undefined integer overflow. ubsan catches all of these.
diff --git a/src/google/protobuf/arena.h b/src/google/protobuf/arena.h index b6a375a..0ffc600 100644 --- a/src/google/protobuf/arena.h +++ b/src/google/protobuf/arena.h
@@ -829,13 +829,13 @@ } template <typename T> static void CreateInArenaStorageInternal( - T* ptr, Arena* arena, google::protobuf::internal::false_type) { + T* ptr, Arena* /* arena */, google::protobuf::internal::false_type) { new (ptr) T(); } template <typename T> static void RegisterDestructorInternal( - T* ptr, Arena* arena, google::protobuf::internal::true_type) {} + T* /* ptr */, Arena* /* arena */, google::protobuf::internal::true_type) {} template <typename T> static void RegisterDestructorInternal( T* ptr, Arena* arena, google::protobuf::internal::false_type) { @@ -870,7 +870,7 @@ } template<typename T> GOOGLE_ATTRIBUTE_ALWAYS_INLINE - static ::google::protobuf::Arena* GetArenaInternal(const T* value, ...) { + static ::google::protobuf::Arena* GetArenaInternal(const T* /* value */, ...) { return NULL; }
diff --git a/src/google/protobuf/compiler/parser.cc b/src/google/protobuf/compiler/parser.cc index 7a03d42..1a40956 100644 --- a/src/google/protobuf/compiler/parser.cc +++ b/src/google/protobuf/compiler/parser.cc
@@ -1373,7 +1373,7 @@ value_location.AddPath( UninterpretedOption::kNegativeIntValueFieldNumber); uninterpreted_option->set_negative_int_value( - -static_cast<int64>(value)); + static_cast<int64>(-value)); } else { value_location.AddPath( UninterpretedOption::kPositiveIntValueFieldNumber);
diff --git a/src/google/protobuf/io/tokenizer_unittest.cc b/src/google/protobuf/io/tokenizer_unittest.cc index cadeb69..e55288e 100644 --- a/src/google/protobuf/io/tokenizer_unittest.cc +++ b/src/google/protobuf/io/tokenizer_unittest.cc
@@ -341,7 +341,7 @@ MultiTokenCase kMultiTokenCases[] = { // Test empty input. { "", { - { Tokenizer::TYPE_END , "" , 0, 0 }, + { Tokenizer::TYPE_END , "" , 0, 0, 0 }, }}, // Test all token types at the same time.
diff --git a/src/google/protobuf/message.h b/src/google/protobuf/message.h index 68acb5b..c155cbd 100644 --- a/src/google/protobuf/message.h +++ b/src/google/protobuf/message.h
@@ -753,7 +753,7 @@ // TODO(tmarek): Make virtual after all subclasses have been // updated. virtual void AddAllocatedMessage(Message* /* message */, - const FieldDescriptor* /*field */, + const FieldDescriptor* /* field */, Message* /* new_entry */) const {} @@ -961,7 +961,7 @@ // TODO(jieluo) - make the map APIs pure virtual after updating // all the subclasses. // Returns true if key is in map. Returns false if key is not in map field. - virtual bool ContainsMapKey(const Message& /* message*/, + virtual bool ContainsMapKey(const Message& /* message */, const FieldDescriptor* /* field */, const MapKey& /* key */) const { return false; @@ -979,7 +979,7 @@ // Delete and returns true if key is in the map field. Returns false // otherwise. - virtual bool DeleteMapValue(Message* /* mesage */, + virtual bool DeleteMapValue(Message* /* message */, const FieldDescriptor* /* field */, const MapKey& /* key */) const { return false;
diff --git a/src/google/protobuf/stubs/port.h b/src/google/protobuf/stubs/port.h index 6ec5e00..0afd55d 100644 --- a/src/google/protobuf/stubs/port.h +++ b/src/google/protobuf/stubs/port.h
@@ -252,9 +252,15 @@ #define GOOGLE_GUARDED_BY(x) #define GOOGLE_ATTRIBUTE_COLD +#ifdef GOOGLE_PROTOBUF_DONT_USE_UNALIGNED +# define GOOGLE_PROTOBUF_USE_UNALIGNED 0 +#else // x86 and x86-64 can perform unaligned loads/stores directly. -#if defined(_M_X64) || defined(__x86_64__) || \ - defined(_M_IX86) || defined(__i386__) +# define GOOGLE_PROTOBUF_USE_UNALIGNED defined(_M_X64) || \ + defined(__x86_64__) || defined(_M_IX86) || defined(__i386__) +#endif + +#if GOOGLE_PROTOBUF_USE_UNALIGNED #define GOOGLE_UNALIGNED_LOAD16(_p) (*reinterpret_cast<const uint16 *>(_p)) #define GOOGLE_UNALIGNED_LOAD32(_p) (*reinterpret_cast<const uint32 *>(_p))
diff --git a/src/google/protobuf/util/message_differencer.h b/src/google/protobuf/util/message_differencer.h index d99223c..192266b 100644 --- a/src/google/protobuf/util/message_differencer.h +++ b/src/google/protobuf/util/message_differencer.h
@@ -241,18 +241,18 @@ // mutually exclusive. If a field has been both moved and modified, then // only ReportModified will be called. virtual void ReportMoved( - const Message& message1, - const Message& message2, - const std::vector<SpecificField>& field_path) { } + const Message& /* message1 */, + const Message& /* message2 */, + const std::vector<SpecificField>& /* field_path */) { } // Reports that two fields match. Useful for doing side-by-side diffs. // This function is mutually exclusive with ReportModified and ReportMoved. // Note that you must call set_report_matches(true) before calling Compare // to make use of this function. virtual void ReportMatched( - const Message& message1, - const Message& message2, - const std::vector<SpecificField>& field_path) { } + const Message& /* message1 */, + const Message& /* message2 */, + const std::vector<SpecificField>& /* field_path */) { } // Reports that two fields would have been compared, but the // comparison has been skipped because the field was marked as @@ -274,16 +274,16 @@ // the fields are equal or not (perhaps with a second call to // Compare()), if it cares. virtual void ReportIgnored( - const Message& message1, - const Message& message2, - const std::vector<SpecificField>& field_path) { } + const Message& /* message1 */, + const Message& /* message2 */, + const std::vector<SpecificField>& /* field_path */) { } // Report that an unknown field is ignored. (see comment above). // Note this is a different function since the last SpecificField in field // path has a null field. This could break existing Reporter. virtual void ReportUnknownFieldIgnored( - const Message& message1, const Message& message2, - const std::vector<SpecificField>& field_path) {} + const Message& /* message1 */, const Message& /* message2 */, + const std::vector<SpecificField>& /* field_path */) {} private: GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(Reporter); @@ -297,9 +297,9 @@ virtual ~MapKeyComparator(); virtual bool IsMatch( - const Message& message1, - const Message& message2, - const std::vector<SpecificField>& parent_fields) const { + const Message& /* message1 */, + const Message& /* message2 */, + const std::vector<SpecificField>& /* parent_fields */) const { GOOGLE_CHECK(false) << "IsMatch() is not implemented."; return false; } @@ -321,18 +321,18 @@ // Returns true if the field should be ignored. virtual bool IsIgnored( - const Message& message1, - const Message& message2, - const FieldDescriptor* field, - const std::vector<SpecificField>& parent_fields) = 0; + const Message& /* message1 */, + const Message& /* message2 */, + const FieldDescriptor* /* field */, + const std::vector<SpecificField>& /* parent_fields */) = 0; // Returns true if the unknown field should be ignored. // Note: This will be called for unknown fields as well in which case // field.field will be null. virtual bool IsUnknownFieldIgnored( - const Message& message1, const Message& message2, - const SpecificField& field, - const std::vector<SpecificField>& parent_fields) { + const Message& /* message1 */, const Message& /* message2 */, + const SpecificField& /* field */, + const std::vector<SpecificField>& /* parent_fields */) { return false; } };
diff --git a/src/google/protobuf/wire_format_lite.h b/src/google/protobuf/wire_format_lite.h index 18b38ea..ce55f26 100644 --- a/src/google/protobuf/wire_format_lite.h +++ b/src/google/protobuf/wire_format_lite.h
@@ -197,7 +197,7 @@ // type-safe, though, so prefer it if possible. #define GOOGLE_PROTOBUF_WIRE_FORMAT_MAKE_TAG(FIELD_NUMBER, TYPE) \ static_cast<uint32>( \ - ((FIELD_NUMBER) << ::google::protobuf::internal::WireFormatLite::kTagTypeBits) \ + (static_cast<uint32>(FIELD_NUMBER) << ::google::protobuf::internal::WireFormatLite::kTagTypeBits) \ | (TYPE)) // These are the tags for the old MessageSet format, which was defined as: