From: Mike Stump Date: Tue, 3 Nov 2009 23:25:48 +0000 (+0000) Subject: Refine volatile handling, specifically, we must have the canonical X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=df317bf71653eeb235da8337b1e8e790f9653aa4;p=clang Refine volatile handling, specifically, we must have the canonical type to look at the volatile specifier. I found these all from just hand auditing the code. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@85967 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index 3a43cc0e63..27d2ba72f2 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -135,7 +135,7 @@ public: /// with location to warn on and the source range[s] to report with the /// warning. bool isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1, - SourceRange &R2) const; + SourceRange &R2, ASTContext &Ctx) const; /// isLvalue - C99 6.3.2.1: an lvalue is an expression with an object type or /// incomplete type other than void. Nonarray expressions that can be lvalues: diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index 265823e4eb..a8ea752a4a 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -698,7 +698,7 @@ Stmt *BlockExpr::getBody() { /// with location to warn on and the source range[s] to report with the /// warning. bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1, - SourceRange &R2) const { + SourceRange &R2, ASTContext &Ctx) const { // Don't warn if the expr is type dependent. The type could end up // instantiating to void. if (isTypeDependent()) @@ -711,7 +711,7 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1, return true; case ParenExprClass: return cast(this)->getSubExpr()-> - isUnusedResultAWarning(Loc, R1, R2); + isUnusedResultAWarning(Loc, R1, R2, Ctx); case UnaryOperatorClass: { const UnaryOperator *UO = cast(this); @@ -724,17 +724,18 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1, return false; // Not a warning. case UnaryOperator::Deref: // Dereferencing a volatile pointer is a side-effect. - if (getType().isVolatileQualified()) + if (Ctx.getCanonicalType(getType()).isVolatileQualified()) return false; break; case UnaryOperator::Real: case UnaryOperator::Imag: // accessing a piece of a volatile complex is a side-effect. - if (UO->getSubExpr()->getType().isVolatileQualified()) + if (Ctx.getCanonicalType(UO->getSubExpr()->getType()) + .isVolatileQualified()) return false; break; case UnaryOperator::Extension: - return UO->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2); + return UO->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx); } Loc = UO->getOperatorLoc(); R1 = UO->getSubExpr()->getSourceRange(); @@ -744,8 +745,8 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1, const BinaryOperator *BO = cast(this); // Consider comma to have side effects if the LHS or RHS does. if (BO->getOpcode() == BinaryOperator::Comma) - return BO->getRHS()->isUnusedResultAWarning(Loc, R1, R2) || - BO->getLHS()->isUnusedResultAWarning(Loc, R1, R2); + return (BO->getRHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx) || + BO->getLHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx)); if (BO->isAssignmentOp()) return false; @@ -762,15 +763,15 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1, // warning, warn about them. const ConditionalOperator *Exp = cast(this); if (Exp->getLHS() && - Exp->getLHS()->isUnusedResultAWarning(Loc, R1, R2)) + Exp->getLHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx)) return true; - return Exp->getRHS()->isUnusedResultAWarning(Loc, R1, R2); + return Exp->getRHS()->isUnusedResultAWarning(Loc, R1, R2, Ctx); } case MemberExprClass: // If the base pointer or element is to a volatile pointer/field, accessing // it is a side effect. - if (getType().isVolatileQualified()) + if (Ctx.getCanonicalType(getType()).isVolatileQualified()) return false; Loc = cast(this)->getMemberLoc(); R1 = SourceRange(Loc, Loc); @@ -780,7 +781,7 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1, case ArraySubscriptExprClass: // If the base pointer or element is to a volatile pointer/field, accessing // it is a side effect. - if (getType().isVolatileQualified()) + if (Ctx.getCanonicalType(getType()).isVolatileQualified()) return false; Loc = cast(this)->getRBracketLoc(); R1 = cast(this)->getLHS()->getSourceRange(); @@ -838,7 +839,7 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1, const CompoundStmt *CS = cast(this)->getSubStmt(); if (!CS->body_empty()) if (const Expr *E = dyn_cast(CS->body_back())) - return E->isUnusedResultAWarning(Loc, R1, R2); + return E->isUnusedResultAWarning(Loc, R1, R2, Ctx); Loc = cast(this)->getLParenLoc(); R1 = getSourceRange(); @@ -856,20 +857,20 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1, // If this is a cast to void, check the operand. Otherwise, the result of // the cast is unused. if (getType()->isVoidType()) - return cast(this)->getSubExpr() - ->isUnusedResultAWarning(Loc, R1, R2); + return (cast(this)->getSubExpr() + ->isUnusedResultAWarning(Loc, R1, R2, Ctx)); Loc = cast(this)->getTypeBeginLoc(); R1 = cast(this)->getSubExpr()->getSourceRange(); return true; case ImplicitCastExprClass: // Check the operand, since implicit casts are inserted by Sema - return cast(this) - ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2); + return (cast(this) + ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx)); case CXXDefaultArgExprClass: - return cast(this) - ->getExpr()->isUnusedResultAWarning(Loc, R1, R2); + return (cast(this) + ->getExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx)); case CXXNewExprClass: // FIXME: In theory, there might be new expressions that don't have side @@ -877,11 +878,11 @@ bool Expr::isUnusedResultAWarning(SourceLocation &Loc, SourceRange &R1, case CXXDeleteExprClass: return false; case CXXBindTemporaryExprClass: - return cast(this) - ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2); + return (cast(this) + ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx)); case CXXExprWithTemporariesClass: - return cast(this) - ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2); + return (cast(this) + ->getSubExpr()->isUnusedResultAWarning(Loc, R1, R2, Ctx)); } } diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp index c70d05e55b..7862c57c2d 100644 --- a/lib/AST/ExprConstant.cpp +++ b/lib/AST/ExprConstant.cpp @@ -167,7 +167,7 @@ public: bool VisitParenExpr(ParenExpr *E) { return Visit(E->getSubExpr()); } bool VisitDeclRefExpr(DeclRefExpr *E) { - if (E->getType().isVolatileQualified()) + if (Info.Ctx.getCanonicalType(E->getType()).isVolatileQualified()) return true; return false; } @@ -197,7 +197,7 @@ public: bool VisitUnaryPreDec(UnaryOperator *E) { return true; } bool VisitUnaryPostDec(UnaryOperator *E) { return true; } bool VisitUnaryDeref(UnaryOperator *E) { - if (E->getType().isVolatileQualified()) + if (Info.Ctx.getCanonicalType(E->getType()).isVolatileQualified()) return true; return Visit(E->getSubExpr()); } diff --git a/lib/CodeGen/CGCXX.cpp b/lib/CodeGen/CGCXX.cpp index 06a17ff6e7..95f073c570 100644 --- a/lib/CodeGen/CGCXX.cpp +++ b/lib/CodeGen/CGCXX.cpp @@ -71,16 +71,17 @@ void CodeGenFunction::EmitCXXGlobalVarDeclInit(const VarDecl &D, const Expr *Init = D.getInit(); QualType T = D.getType(); + bool isVolatile = getContext().getCanonicalType(T).isVolatileQualified(); if (T->isReferenceType()) { ErrorUnsupported(Init, "global variable that binds to a reference"); } else if (!hasAggregateLLVMType(T)) { llvm::Value *V = EmitScalarExpr(Init); - EmitStoreOfScalar(V, DeclPtr, T.isVolatileQualified(), T); + EmitStoreOfScalar(V, DeclPtr, isVolatile, T); } else if (T->isAnyComplexType()) { - EmitComplexExprIntoAddr(Init, DeclPtr, T.isVolatileQualified()); + EmitComplexExprIntoAddr(Init, DeclPtr, isVolatile); } else { - EmitAggExpr(Init, DeclPtr, T.isVolatileQualified()); + EmitAggExpr(Init, DeclPtr, isVolatile); if (const RecordType *RT = T->getAs()) { CXXRecordDecl *RD = cast(RT->getDecl()); diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp index 8e445de29b..4f8aef420d 100644 --- a/lib/CodeGen/CGDecl.cpp +++ b/lib/CodeGen/CGDecl.cpp @@ -417,17 +417,18 @@ void CodeGenFunction::EmitLocalBlockVarDecl(const VarDecl &D) { Loc = Builder.CreateStructGEP(DeclPtr, getByRefValueLLVMField(&D), D.getNameAsString()); + bool isVolatile = (getContext().getCanonicalType(D.getType()) + .isVolatileQualified()); if (Ty->isReferenceType()) { RValue RV = EmitReferenceBindingToExpr(Init, Ty, /*IsInitializer=*/true); EmitStoreOfScalar(RV.getScalarVal(), Loc, false, Ty); } else if (!hasAggregateLLVMType(Init->getType())) { llvm::Value *V = EmitScalarExpr(Init); - EmitStoreOfScalar(V, Loc, D.getType().isVolatileQualified(), - D.getType()); + EmitStoreOfScalar(V, Loc, isVolatile, D.getType()); } else if (Init->getType()->isAnyComplexType()) { - EmitComplexExprIntoAddr(Init, Loc, D.getType().isVolatileQualified()); + EmitComplexExprIntoAddr(Init, Loc, isVolatile); } else { - EmitAggExpr(Init, Loc, D.getType().isVolatileQualified()); + EmitAggExpr(Init, Loc, isVolatile); } } @@ -556,6 +557,7 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, llvm::Value *Arg) { assert((isa(D) || isa(D)) && "Invalid argument to EmitParmDecl"); QualType Ty = D.getType(); + CanQualType CTy = getContext().getCanonicalType(Ty); llvm::Value *DeclPtr; if (!Ty->isConstantSizeType()) { @@ -572,7 +574,7 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, llvm::Value *Arg) { DeclPtr->setName(Name.c_str()); // Store the initial value into the alloca. - EmitStoreOfScalar(Arg, DeclPtr, Ty.isVolatileQualified(), Ty); + EmitStoreOfScalar(Arg, DeclPtr, CTy.isVolatileQualified(), Ty); } else { // Otherwise, if this is an aggregate, just use the input pointer. DeclPtr = Arg; diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index 4bff8c44b3..96b58d8995 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -1648,9 +1648,10 @@ Value *ScalarExprEmitter::VisitBinComma(const BinaryOperator *E) { /// expression is cheap enough and side-effect-free enough to evaluate /// unconditionally instead of conditionally. This is used to convert control /// flow into selects in some cases. -static bool isCheapEnoughToEvaluateUnconditionally(const Expr *E) { +static bool isCheapEnoughToEvaluateUnconditionally(const Expr *E, + CodeGenFunction &CGF) { if (const ParenExpr *PE = dyn_cast(E)) - return isCheapEnoughToEvaluateUnconditionally(PE->getSubExpr()); + return isCheapEnoughToEvaluateUnconditionally(PE->getSubExpr(), CGF); // TODO: Allow anything we can constant fold to an integer or fp constant. if (isa(E) || isa(E) || @@ -1661,7 +1662,9 @@ static bool isCheapEnoughToEvaluateUnconditionally(const Expr *E) { // X and Y are local variables. if (const DeclRefExpr *DRE = dyn_cast(E)) if (const VarDecl *VD = dyn_cast(DRE->getDecl())) - if (VD->hasLocalStorage() && !VD->getType().isVolatileQualified()) + if (VD->hasLocalStorage() && !(CGF.getContext() + .getCanonicalType(VD->getType()) + .isVolatileQualified())) return true; return false; @@ -1690,8 +1693,9 @@ VisitConditionalOperator(const ConditionalOperator *E) { // If this is a really simple expression (like x ? 4 : 5), emit this as a // select instead of as control flow. We can only do this if it is cheap and // safe to evaluate the LHS and RHS unconditionally. - if (E->getLHS() && isCheapEnoughToEvaluateUnconditionally(E->getLHS()) && - isCheapEnoughToEvaluateUnconditionally(E->getRHS())) { + if (E->getLHS() && isCheapEnoughToEvaluateUnconditionally(E->getLHS(), + CGF) && + isCheapEnoughToEvaluateUnconditionally(E->getRHS(), CGF)) { llvm::Value *CondV = CGF.EvaluateExprAsBool(E->getCond()); llvm::Value *LHS = Visit(E->getLHS()); llvm::Value *RHS = Visit(E->getRHS()); diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index 893f1ed880..9bb219642a 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -517,7 +517,7 @@ public: //===--------------------------------------------------------------------===// Qualifiers MakeQualifiers(QualType T) { - Qualifiers Quals = T.getQualifiers(); + Qualifiers Quals = getContext().getCanonicalType(T).getQualifiers(); Quals.setObjCGCAttr(getContext().getObjCGCAttrKind(T)); return Quals; } diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index 1453424385..6a65bd1dd1 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -70,7 +70,7 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) { SourceLocation Loc; SourceRange R1, R2; - if (!E->isUnusedResultAWarning(Loc, R1, R2)) + if (!E->isUnusedResultAWarning(Loc, R1, R2, Context)) return; // Okay, we have an unused result. Depending on what the base expression is,