From 40da1ed572d60e9c7cc2fe1ca4175e30682f5a9d Mon Sep 17 00:00:00 2001 From: brian-peloton Date: Tue, 23 May 2017 16:22:57 -0700 Subject: 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. --- src/google/protobuf/arena.h | 6 ++-- src/google/protobuf/compiler/parser.cc | 2 +- src/google/protobuf/io/tokenizer_unittest.cc | 2 +- src/google/protobuf/message.h | 6 ++-- src/google/protobuf/stubs/port.h | 10 ++++-- src/google/protobuf/util/message_differencer.h | 42 +++++++++++++------------- src/google/protobuf/wire_format_lite.h | 2 +- 7 files changed, 38 insertions(+), 32 deletions(-) (limited to 'src/google') diff --git a/src/google/protobuf/arena.h b/src/google/protobuf/arena.h index b6a375ac..0ffc6004 100644 --- a/src/google/protobuf/arena.h +++ b/src/google/protobuf/arena.h @@ -829,13 +829,13 @@ class LIBPROTOBUF_EXPORT Arena { } template 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 static void RegisterDestructorInternal( - T* ptr, Arena* arena, google::protobuf::internal::true_type) {} + T* /* ptr */, Arena* /* arena */, google::protobuf::internal::true_type) {} template static void RegisterDestructorInternal( T* ptr, Arena* arena, google::protobuf::internal::false_type) { @@ -870,7 +870,7 @@ class LIBPROTOBUF_EXPORT Arena { } template 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 7a03d42b..1a409566 100644 --- a/src/google/protobuf/compiler/parser.cc +++ b/src/google/protobuf/compiler/parser.cc @@ -1373,7 +1373,7 @@ bool Parser::ParseOption(Message* options, value_location.AddPath( UninterpretedOption::kNegativeIntValueFieldNumber); uninterpreted_option->set_negative_int_value( - -static_cast(value)); + static_cast(-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 cadeb696..e55288e2 100644 --- a/src/google/protobuf/io/tokenizer_unittest.cc +++ b/src/google/protobuf/io/tokenizer_unittest.cc @@ -341,7 +341,7 @@ inline std::ostream& operator<<(std::ostream& out, 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 68acb5b1..c155cbd6 100644 --- a/src/google/protobuf/message.h +++ b/src/google/protobuf/message.h @@ -753,7 +753,7 @@ class LIBPROTOBUF_EXPORT Reflection { // 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 @@ class LIBPROTOBUF_EXPORT Reflection { // 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 @@ class LIBPROTOBUF_EXPORT Reflection { // 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 6ec5e001..0afd55d7 100644 --- a/src/google/protobuf/stubs/port.h +++ b/src/google/protobuf/stubs/port.h @@ -252,9 +252,15 @@ static const uint64 kuint64max = GOOGLE_ULONGLONG(0xFFFFFFFFFFFFFFFF); #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(_p)) #define GOOGLE_UNALIGNED_LOAD32(_p) (*reinterpret_cast(_p)) diff --git a/src/google/protobuf/util/message_differencer.h b/src/google/protobuf/util/message_differencer.h index d99223cb..192266b6 100644 --- a/src/google/protobuf/util/message_differencer.h +++ b/src/google/protobuf/util/message_differencer.h @@ -241,18 +241,18 @@ class LIBPROTOBUF_EXPORT MessageDifferencer { // 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& field_path) { } + const Message& /* message1 */, + const Message& /* message2 */, + const std::vector& /* 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& field_path) { } + const Message& /* message1 */, + const Message& /* message2 */, + const std::vector& /* 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 @@ class LIBPROTOBUF_EXPORT MessageDifferencer { // 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& field_path) { } + const Message& /* message1 */, + const Message& /* message2 */, + const std::vector& /* 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& field_path) {} + const Message& /* message1 */, const Message& /* message2 */, + const std::vector& /* field_path */) {} private: GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(Reporter); @@ -297,9 +297,9 @@ class LIBPROTOBUF_EXPORT MessageDifferencer { virtual ~MapKeyComparator(); virtual bool IsMatch( - const Message& message1, - const Message& message2, - const std::vector& parent_fields) const { + const Message& /* message1 */, + const Message& /* message2 */, + const std::vector& /* parent_fields */) const { GOOGLE_CHECK(false) << "IsMatch() is not implemented."; return false; } @@ -321,18 +321,18 @@ class LIBPROTOBUF_EXPORT MessageDifferencer { // Returns true if the field should be ignored. virtual bool IsIgnored( - const Message& message1, - const Message& message2, - const FieldDescriptor* field, - const std::vector& parent_fields) = 0; + const Message& /* message1 */, + const Message& /* message2 */, + const FieldDescriptor* /* field */, + const std::vector& /* 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& parent_fields) { + const Message& /* message1 */, const Message& /* message2 */, + const SpecificField& /* field */, + const std::vector& /* parent_fields */) { return false; } }; diff --git a/src/google/protobuf/wire_format_lite.h b/src/google/protobuf/wire_format_lite.h index 18b38eae..ce55f266 100644 --- a/src/google/protobuf/wire_format_lite.h +++ b/src/google/protobuf/wire_format_lite.h @@ -197,7 +197,7 @@ class LIBPROTOBUF_EXPORT WireFormatLite { // type-safe, though, so prefer it if possible. #define GOOGLE_PROTOBUF_WIRE_FORMAT_MAKE_TAG(FIELD_NUMBER, TYPE) \ static_cast( \ - ((FIELD_NUMBER) << ::google::protobuf::internal::WireFormatLite::kTagTypeBits) \ + (static_cast(FIELD_NUMBER) << ::google::protobuf::internal::WireFormatLite::kTagTypeBits) \ | (TYPE)) // These are the tags for the old MessageSet format, which was defined as: -- cgit v1.2.3