From ad425626d237c3746c8de8d02c04f0ee6334f7e0 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Wed, 16 Nov 2016 23:40:00 +0000 Subject: [PATCH] Add warning when assigning enums to bitfields without an explicit unsigned underlying type Summary: Add a warning when assigning enums to bitfields without an explicit unsigned underlying type. This is to prevent problems with MSVC compatibility, since the Microsoft ABI defaults to storing enums with a signed type, causing inconsistencies with saving to/reading from bitfields. Also disabled the warning in the dr0xx.cpp test which throws the error, and added a test for the warning. The warning can be disabled with -Wno-signed-enum-bitfield. Patch by Sasha Bermeister! Reviewers: rnk, aaron.ballman Subscribers: mehdi_amini, aaron.ballman, cfe-commits, thakis, dcheng Differential Revision: https://reviews.llvm.org/D24289 llvm-svn: 287177 --- clang/include/clang/Basic/DiagnosticGroups.td | 2 ++ clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 +++ clang/lib/Sema/SemaChecking.cpp | 20 +++++++++++- clang/test/SemaCXX/warn-msvc-enum-bitfield.cpp | 40 ++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 clang/test/SemaCXX/warn-msvc-enum-bitfield.cpp diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 35a3473..ad92b4d 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -543,6 +543,7 @@ def GCCWriteStrings : DiagGroup<"write-strings" , [WritableStrings]>; def CharSubscript : DiagGroup<"char-subscripts">; def LargeByValueCopy : DiagGroup<"large-by-value-copy">; def DuplicateArgDecl : DiagGroup<"duplicate-method-arg">; +def SignedEnumBitfield : DiagGroup<"signed-enum-bitfield">; // Unreachable code warning groups. // @@ -661,6 +662,7 @@ def Most : DiagGroup<"most", [ ReturnType, SelfAssignment, SelfMove, + SignedEnumBitfield, SizeofArrayArgument, SizeofArrayDecay, StringPlusInt, diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 85aa1c5..32c9ca0 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3020,6 +3020,10 @@ def warn_int_to_void_pointer_cast : Warning< "cast to %1 from smaller integer type %0">, InGroup; +def warn_no_underlying_type_specified_for_enum_bitfield : Warning< + "enums in the Microsoft ABI are signed integers by default; consider giving " + "the enum %0 an unsigned underlying type to make this code portable">, + InGroup>, DefaultIgnore; def warn_attribute_packed_for_bitfield : Warning< "'packed' attribute was ignored on bit-fields with single-byte alignment " "in older versions of GCC and Clang">, diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 56bc3ac..c1db062 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -8518,6 +8518,24 @@ bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init, return false; // White-list bool bitfields. + QualType BitfieldType = Bitfield->getType(); + if (BitfieldType->isBooleanType()) + return false; + + if (BitfieldType->isEnumeralType()) { + EnumDecl *BitfieldEnumDecl = BitfieldType->getAs()->getDecl(); + // If the underlying enum type was not explicitly specified as an unsigned + // type and the enum contain only positive values, MSVC++ will cause an + // inconsistency by storing this as a signed type. + if (S.getLangOpts().CPlusPlus11 && + !BitfieldEnumDecl->getIntegerTypeSourceInfo() && + BitfieldEnumDecl->getNumPositiveBits() > 0 && + BitfieldEnumDecl->getNumNegativeBits() == 0) { + S.Diag(InitLoc, diag::warn_no_underlying_type_specified_for_enum_bitfield) + << BitfieldEnumDecl->getNameAsString(); + } + } + if (Bitfield->getType()->isBooleanType()) return false; @@ -8547,7 +8565,7 @@ bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init, // Compute the value which the bitfield will contain. llvm::APSInt TruncatedValue = Value.trunc(FieldWidth); - TruncatedValue.setIsSigned(Bitfield->getType()->isSignedIntegerType()); + TruncatedValue.setIsSigned(BitfieldType->isSignedIntegerType()); // Check whether the stored value is equal to the original value. TruncatedValue = TruncatedValue.extend(OriginalWidth); diff --git a/clang/test/SemaCXX/warn-msvc-enum-bitfield.cpp b/clang/test/SemaCXX/warn-msvc-enum-bitfield.cpp new file mode 100644 index 0000000..99e1669 --- /dev/null +++ b/clang/test/SemaCXX/warn-msvc-enum-bitfield.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -fsyntax-only -Wsigned-enum-bitfield -verify %s --std=c++11 + +// Enums used in bitfields with no explicitly specified underlying type. +void test0() { + enum E { E1, E2 }; + enum F { F1, F2 }; + struct { E e1 : 1; E e2; F f1 : 1; F f2; } s; + + s.e1 = E1; // expected-warning {{enums in the Microsoft ABI are signed integers by default; consider giving the enum E an unsigned underlying type to make this code portable}} + s.f1 = F1; // expected-warning {{enums in the Microsoft ABI are signed integers by default; consider giving the enum F an unsigned underlying type to make this code portable}} + + s.e2 = E2; + s.f2 = F2; +} + +// Enums used in bitfields with an explicit signed underlying type. +void test1() { + enum E : signed { E1, E2 }; + enum F : long { F1, F2 }; + struct { E e1 : 1; E e2; F f1 : 1; F f2; } s; + + s.e1 = E1; + s.f1 = F1; + + s.e2 = E2; + s.f2 = F2; +} + +// Enums used in bitfields with an explicitly unsigned underlying type. +void test3() { + enum E : unsigned { E1, E2 }; + enum F : unsigned long { F1, F2 }; + struct { E e1 : 1; E e2; F f1 : 1; F f2; } s; + + s.e1 = E1; + s.f1 = F1; + + s.e2 = E2; + s.f2 = F2; +} -- 2.7.4