From: John McCall Date: Thu, 11 Nov 2010 03:21:53 +0000 (+0000) Subject: Extend the bitfield-truncation warning to initializations. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=15d7d12226f83de24f96f4bf4e27ebba30fef51e;p=clang Extend the bitfield-truncation warning to initializations. rdar://problem/8652606 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@118773 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 8e99ca1eb3..cae67dbe89 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -4539,6 +4539,9 @@ private: void CheckFloatComparison(SourceLocation loc, Expr* lex, Expr* rex); void CheckImplicitConversions(Expr *E, SourceLocation CC = SourceLocation()); + void CheckBitFieldInitialization(SourceLocation InitLoc, FieldDecl *Field, + Expr *Init); + /// \brief The parser's current scope. /// /// The parser maintains this state here. @@ -4546,6 +4549,7 @@ private: protected: friend class Parser; + friend class InitializationSequence; /// \brief Retrieve the parser's current scope. Scope *getCurScope() const { return CurScope; } diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 2f3abfcb3d..4fc16f51ee 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -2608,6 +2608,55 @@ void AnalyzeComparison(Sema &S, BinaryOperator *E) { << lex->getSourceRange() << rex->getSourceRange(); } +/// Analyzes an attempt to assign the given value to a bitfield. +/// +/// Returns true if there was something fishy about the attempt. +bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init, + SourceLocation InitLoc) { + assert(Bitfield->isBitField()); + if (Bitfield->isInvalidDecl()) + return false; + + Expr *OriginalInit = Init->IgnoreParenImpCasts(); + + llvm::APSInt Width(32); + Expr::EvalResult InitValue; + if (!Bitfield->getBitWidth()->isIntegerConstantExpr(Width, S.Context) || + !Init->Evaluate(InitValue, S.Context) || + !InitValue.Val.isInt()) + return false; + + const llvm::APSInt &Value = InitValue.Val.getInt(); + unsigned OriginalWidth = Value.getBitWidth(); + unsigned FieldWidth = Width.getZExtValue(); + + if (OriginalWidth <= FieldWidth) + return false; + + llvm::APSInt TruncatedValue = Value; + TruncatedValue.trunc(FieldWidth); + + // It's fairly common to write values into signed bitfields + // that, if sign-extended, would end up becoming a different + // value. We don't want to warn about that. + if (Value.isSigned() && Value.isNegative()) + TruncatedValue.sext(OriginalWidth); + else + TruncatedValue.zext(OriginalWidth); + + if (Value == TruncatedValue) + return false; + + std::string PrettyValue = Value.toString(10); + std::string PrettyTrunc = TruncatedValue.toString(10); + + S.Diag(InitLoc, diag::warn_impcast_bitfield_precision_constant) + << PrettyValue << PrettyTrunc << OriginalInit->getType() + << Init->getSourceRange(); + + return true; +} + /// Analyze the given simple or compound assignment for warning-worthy /// operations. void AnalyzeAssignment(Sema &S, BinaryOperator *E) { @@ -2617,44 +2666,11 @@ void AnalyzeAssignment(Sema &S, BinaryOperator *E) { // We want to recurse on the RHS as normal unless we're assigning to // a bitfield. if (FieldDecl *Bitfield = E->getLHS()->getBitField()) { - assert(Bitfield->isBitField()); - - Expr *RHS = E->getRHS()->IgnoreParenImpCasts(); - - llvm::APSInt Width(32); - Expr::EvalResult RHSValue; - if (!Bitfield->isInvalidDecl() && - Bitfield->getBitWidth()->isIntegerConstantExpr(Width, S.Context) && - RHS->Evaluate(RHSValue, S.Context) && RHSValue.Val.isInt()) { - const llvm::APSInt &Value = RHSValue.Val.getInt(); - unsigned OriginalWidth = Value.getBitWidth(); - unsigned FieldWidth = Width.getZExtValue(); - - if (OriginalWidth > FieldWidth) { - llvm::APSInt TruncatedValue = Value; - TruncatedValue.trunc(FieldWidth); - - // It's fairly common to write values into signed bitfields - // that, if sign-extended, would end up becoming a different - // value. We don't want to warn about that. - if (Value.isSigned() && Value.isNegative()) - TruncatedValue.sext(OriginalWidth); - else - TruncatedValue.zext(OriginalWidth); - - if (Value != TruncatedValue) { - std::string PrettyValue = Value.toString(10); - std::string PrettyTrunc = TruncatedValue.toString(10); - - S.Diag(E->getOperatorLoc(), - diag::warn_impcast_bitfield_precision_constant) - << PrettyValue << PrettyTrunc << RHS->getType() - << E->getRHS()->getSourceRange(); - - // Recurse, ignoring any implicit conversions on the RHS. - return AnalyzeImplicitConversions(S, RHS, E->getOperatorLoc()); - } - } + if (AnalyzeBitFieldAssignment(S, Bitfield, E->getRHS(), + E->getOperatorLoc())) { + // Recurse, ignoring any implicit conversions on the RHS. + return AnalyzeImplicitConversions(S, E->getRHS()->IgnoreParenImpCasts(), + E->getOperatorLoc()); } } @@ -2932,6 +2948,12 @@ void Sema::CheckImplicitConversions(Expr *E, SourceLocation CC) { AnalyzeImplicitConversions(*this, E, CC); } +void Sema::CheckBitFieldInitialization(SourceLocation InitLoc, + FieldDecl *BitField, + Expr *Init) { + (void) AnalyzeBitFieldAssignment(*this, BitField, Init, InitLoc); +} + /// CheckParmsForFunctionDef - Check that the parameters of the given /// function are appropriate for the definition of a function. This /// takes care of any checks that cannot be performed on the diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp index 999d1d24c6..ffbb76d8c5 100644 --- a/lib/Sema/SemaInit.cpp +++ b/lib/Sema/SemaInit.cpp @@ -4047,6 +4047,13 @@ InitializationSequence::Perform(Sema &S, break; } } + + // Diagnose non-fatal problems with the completed initialization. + if (Entity.getKind() == InitializedEntity::EK_Member && + cast(Entity.getDecl())->isBitField()) + S.CheckBitFieldInitialization(Kind.getLocation(), + cast(Entity.getDecl()), + CurInit.get()); return move(CurInit); } @@ -4534,7 +4541,7 @@ Sema::PerformCopyInitialization(const InitializedEntity &Entity, if (Init.isInvalid()) return ExprError(); - Expr *InitE = (Expr *)Init.get(); + Expr *InitE = Init.get(); assert(InitE && "No initialization expression?"); if (EqualLoc.isInvalid()) diff --git a/test/Sema/constant-conversion.c b/test/Sema/constant-conversion.c index b01813a685..af77741fc1 100644 --- a/test/Sema/constant-conversion.c +++ b/test/Sema/constant-conversion.c @@ -24,3 +24,16 @@ enum Test2 test2(enum Test2 *t) { *t = 20; return 10; // shouldn't warn } + +void test3() { + struct A { + unsigned int foo : 2; + int bar : 2; + }; + + struct A a = { 0, 10 }; // expected-warning {{implicit truncation from 'int' to bitfield changes value from 10 to 2}} + struct A b[] = { 0, 10, 0, 0 }; // expected-warning {{implicit truncation from 'int' to bitfield changes value from 10 to 2}} + struct A c[] = {{10, 0}}; // expected-warning {{implicit truncation from 'int' to bitfield changes value from 10 to 2}} + struct A d = (struct A) { 10, 0 }; // expected-warning {{implicit truncation from 'int' to bitfield changes value from 10 to 2}} + struct A e = { .foo = 10 }; // expected-warning {{implicit truncation from 'int' to bitfield changes value from 10 to 2}} +} diff --git a/test/SemaCXX/conversion.cpp b/test/SemaCXX/conversion.cpp index f648943807..fdda7ac76a 100644 --- a/test/SemaCXX/conversion.cpp +++ b/test/SemaCXX/conversion.cpp @@ -43,3 +43,10 @@ namespace test1 { return p == foo(); } } + +namespace test2 { + struct A { + unsigned int x : 2; + A() : x(10) {} // expected-warning {{implicit truncation from 'int' to bitfield changes value from 10 to 2}} + }; +}