diff --git a/include/clang/AST/ASTContext.h b/include/clang/AST/ASTContext.h index 87ecdc632a2f..397059088c3a 100644 --- a/include/clang/AST/ASTContext.h +++ b/include/clang/AST/ASTContext.h @@ -17,6 +17,7 @@ #include "clang/AST/ASTTypeTraits.h" #include "clang/AST/CanonicalType.h" +#include "clang/AST/CanonBounds.h" #include "clang/AST/CommentCommandTraits.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclarationName.h" @@ -2515,7 +2516,8 @@ class ASTContext : public RefCountedBase { public: bool EquivalentAnnotations(const BoundsAnnotations &Annots1, const BoundsAnnotations &Annots2); - bool EquivalentBounds(const BoundsExpr *Expr1, const BoundsExpr *Expr2); + bool EquivalentBounds(const BoundsExpr *Expr1, const BoundsExpr *Expr2, + EquivExprSets *EquivExprs = nullptr); bool EquivalentInteropTypes(const InteropTypeExpr *Expr1, const InteropTypeExpr *Expr2); diff --git a/include/clang/AST/CanonBounds.h b/include/clang/AST/CanonBounds.h index d6659454fec4..3a785cd1e70a 100644 --- a/include/clang/AST/CanonBounds.h +++ b/include/clang/AST/CanonBounds.h @@ -35,12 +35,8 @@ namespace clang { class Expr; class VarDecl; - // Abstract base class that provides information what variables - // currently are equal to each other. - class EqualityRelation { - public: - virtual const VarDecl *getRepresentative(const VarDecl *V) = 0; - }; + // List of sets of equivalent expressions. + typedef SmallVector *, 4> EquivExprSets; class Lexicographic { public: @@ -52,7 +48,7 @@ namespace clang { private: ASTContext &Context; - EqualityRelation *EqualVars; + EquivExprSets *EquivExprs; bool Trace; template @@ -66,6 +62,10 @@ namespace clang { return Lexicographic::CompareImpl(E1, E2); } + // See if E1 and E2 are considered equivalent using EquivExprs. If + // they are not, return Current. + Result CheckEquivExprs(Result Current, const Expr *E1, const Expr *E2); + Result CompareInteger(signed I1, signed I2); Result CompareInteger(unsigned I1, unsigned I2); Result CompareRelativeBoundsClause(const RelativeBoundsClause *RC1, @@ -86,8 +86,7 @@ namespace clang { Result CompareImpl(const BinaryOperator *E1, const BinaryOperator *E2); Result CompareImpl(const CompoundAssignOperator *E1, const CompoundAssignOperator *E2); - Result CompareImpl(const ImplicitCastExpr *E1, const ImplicitCastExpr *E2); - Result CompareImpl(const CStyleCastExpr *E1, const CStyleCastExpr *E2); + Result CompareImpl(const CastExpr *E1, const CastExpr *E2); Result CompareImpl(const CompoundLiteralExpr *E1, const CompoundLiteralExpr *E2); Result CompareImpl(const GenericSelectionExpr *E1, @@ -105,7 +104,7 @@ namespace clang { public: - Lexicographic(ASTContext &Ctx, EqualityRelation *EV); + Lexicographic(ASTContext &Ctx, EquivExprSets *EquivExprs); /// \brief Lexicographic comparison of expressions that can occur in /// bounds expressions. @@ -115,6 +114,7 @@ namespace clang { /// or types. Result CompareDecl(const NamedDecl *D1, const NamedDecl *D2); Result CompareType(QualType T1, QualType T2); + Result CompareTypeIgnoreCheckedness(QualType QT1, QualType QT2); }; } // end namespace clang diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index 494eb189509b..a8d7cf878df2 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -893,7 +893,11 @@ def MicrosoftEndOfFile : DiagGroup<"microsoft-end-of-file">; def MicrosoftInaccessibleBase : DiagGroup<"microsoft-inaccessible-base">; // Checked C warnings for bounds declaration checking. -def CheckBoundsDecls : DiagGroup<"check-bounds-decls">; +def CheckBoundsDeclsUnchecked : DiagGroup<"check-bounds-decls-unchecked-scope">; +def CheckBoundsDeclsChecked : DiagGroup<"check-bounds-decls-checked-scope">; +def CheckBoundsDecls : DiagGroup<"check-bounds-decls", + [CheckBoundsDeclsUnchecked, + CheckBoundsDeclsChecked]>; // Checked C warnings about memory accesses determined to be out of // declared bounds. def CheckMemoryAccesses : DiagGroup<"check-memory-accesses">; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 9d42a1fc367f..23d3b9084634 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -9640,13 +9640,12 @@ def err_bounds_type_annotation_lost_checking : Error< def warn_bounds_declaration_invalid : Warning< "cannot prove declared bounds for %1 are valid after " "%select{assignment|initialization}0">, - DefaultIgnore, - InGroup; + InGroup; def warn_checked_scope_bounds_declaration_invalid : Warning< "cannot prove declared bounds for %1 are valid after " "%select{assignment|initialization}0">, - InGroup; + InGroup; def error_bounds_declaration_invalid : Error< "declared bounds for %1 are invalid after " @@ -9670,12 +9669,11 @@ def err_bounds_type_annotation_lost_checking : Error< def warn_argument_bounds_invalid : Warning< "cannot prove argument meets declared bounds for %ordinal0 parameter">, - DefaultIgnore, - InGroup; + InGroup; def warn_checked_scope_argument_bounds_invalid : Warning< "cannot prove argument meets declared bounds for %ordinal0 parameter">, - InGroup; + InGroup; def error_argument_bounds_invalid : Error< "argument does not meet declared bounds for %ordinal0 parameter">; @@ -9686,11 +9684,11 @@ def err_bounds_type_annotation_lost_checking : Error< def warn_static_cast_bounds_invalid : Warning< "cannot prove cast source bounds are wide enough for %0">, DefaultIgnore, - InGroup; + InGroup; def warn_checked_scopestatic_cast_bounds_invalid : Warning< "cannot prove cast source bounds are wide enough for %0">, - InGroup; + InGroup; def error_static_cast_bounds_invalid : Error< "cast source bounds are too narrow for %0">; diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 3dfd4440bd41..8eb352c51dd8 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -4587,7 +4587,7 @@ class Sema { //===---------------------------- Checked C Extension ----------------------===// private: - const Type *ValidateBoundsExprArgument(Expr *Arg); + QualType ValidateBoundsExprArgument(Expr *Arg); public: ExprResult ActOnNullaryBoundsExpr(SourceLocation BoundKWLoc, diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 0f4346da164a..bff8ace2e257 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -9047,9 +9047,10 @@ bool ASTContext::isNotAllowedForNoPrototypeFunction(QualType QT) const { return false; } -bool ASTContext::EquivalentBounds(const BoundsExpr *Expr1, const BoundsExpr *Expr2) { +bool ASTContext::EquivalentBounds(const BoundsExpr *Expr1, const BoundsExpr *Expr2, + EquivExprSets *EquivExprs) { if (Expr1 && Expr2) { - Lexicographic::Result Cmp = Lexicographic(*this, nullptr).CompareExpr(Expr1, Expr2); + Lexicographic::Result Cmp = Lexicographic(*this, EquivExprs).CompareExpr(Expr1, Expr2); return Cmp == Lexicographic::Result::Equal; } diff --git a/lib/AST/CanonBounds.cpp b/lib/AST/CanonBounds.cpp index 7084624798fc..d6defeabaebe 100644 --- a/lib/AST/CanonBounds.cpp +++ b/lib/AST/CanonBounds.cpp @@ -17,6 +17,7 @@ #include "llvm/ADT/APInt.h" #include "llvm/Support/raw_ostream.h" #include "clang/AST/CanonBounds.h" +#include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" #include "clang/AST/Stmt.h" @@ -24,8 +25,119 @@ using namespace clang; using Result = Lexicographic::Result; -Lexicographic::Lexicographic(ASTContext &Ctx, EqualityRelation *EV) : - Context(Ctx), EqualVars(EV), Trace(false) { +namespace { + // Return true if this cast preserve the bits of the value, + // false otherwise. + static bool IsValuePreserving(CastKind CK) { + switch (CK) { + case CK_BitCast: + case CK_LValueBitCast: + case CK_NoOp: + case CK_ArrayToPointerDecay: + case CK_FunctionToPointerDecay: + case CK_NullToPointer: + case CK_AssumePtrBounds: + case CK_DynamicPtrBounds: + return true; + default: + return false; + } + } + + static bool isPointerToArrayType(QualType Ty) { + if (const PointerType *T = Ty->getAs()) + return T->getPointeeType()->isArrayType(); + else + return false; + } + // Ignore operations that don't change runtime values: parens, some cast operations, + // array/function address-of and dereference operators, and address-of/dereference + // operators that cancel (&* and *&). + // + // The code for casts is adapted from Expr::IgnoreNoopCasts, which seems like doesn't + // do enough filtering (it'll ignore LValueToRValue casts for example). + // TODO: reconcile with CheckValuePreservingCast + static Expr *IgnoreValuePreservingOperations(ASTContext &Ctx, Expr *E) { + while (true) { + E = E->IgnoreParens(); + + if (CastExpr *P = dyn_cast(E)) { + CastKind CK = P->getCastKind(); + Expr *SE = P->getSubExpr(); + if (IsValuePreserving(CK)) { + E = SE; + continue; + } + + // Ignore integer <-> casts that are of the same width, ptr<->ptr + // and ptr<->int casts of the same width. + if (CK == CK_IntegralToPointer || CK == CK_PointerToIntegral || + CK == CK_IntegralCast) { + if (Ctx.hasSameUnqualifiedType(E->getType(), SE->getType())) { + E = SE; + continue; + } + + if ((E->getType()->isPointerType() || + E->getType()->isIntegralType(Ctx)) && + (SE->getType()->isPointerType() || + SE->getType()->isIntegralType(Ctx)) && + Ctx.getTypeSize(E->getType()) == Ctx.getTypeSize(SE->getType())) { + E = SE; + continue; + } + } + } else if (const UnaryOperator *UO = dyn_cast(E)) { + QualType ETy = UO->getType(); + Expr *SE = UO->getSubExpr(); + QualType SETy = SE->getType(); + + UnaryOperator::Opcode Op = UO->getOpcode(); + if (Op == UO_Deref) { + // This may be more conservative than necessary. + bool between_functions = ETy->isFunctionType() && SETy->isFunctionPointerType(); + bool between_arrays = ETy->isArrayType() && isPointerToArrayType(SETy); + if (between_functions || between_arrays) { + E = SE; + continue; + } + + // handle *&e, which reduces to e. + if (const UnaryOperator *Child = + dyn_cast(SE->IgnoreParens())) { + if (Child->getOpcode() == UO_AddrOf) { + E = Child->getSubExpr(); + continue; + } + } + + } else if (Op == UO_AddrOf) { + // This may be more conservative than necessary. + bool between_functions = ETy->isFunctionPointerType() && SETy->isFunctionType(); + bool between_arrays = isPointerToArrayType(ETy) && SETy->isArrayType(); + if (between_functions || between_arrays) { + E = SE; + continue; + } + + // handle &*e, which reduces to e. + if (const UnaryOperator *Child = + dyn_cast(SE->IgnoreParens())) { + if (Child->getOpcode() == UO_Deref) { + E = Child->getSubExpr(); + continue; + } + } + } + } + + return E; + } + } +} + +Lexicographic::Lexicographic(ASTContext &Ctx, EquivExprSets *EquivExprs) : + Context(Ctx), EquivExprs(EquivExprs), Trace(false) { } Result Lexicographic::CompareInteger(signed I1, signed I2) { @@ -188,24 +300,34 @@ Lexicographic::CompareDecl(const NamedDecl *D1Arg, const NamedDecl *D2Arg) { return Result::LessThan; } -Result Lexicographic::CompareExpr(const Expr *E1, const Expr *E2) { +Result Lexicographic::CompareExpr(const Expr *Arg1, const Expr *Arg2) { if (Trace) { raw_ostream &OS = llvm::outs(); OS << "Lexicographic comparing expressions\n"; OS << "E1:\n"; - E1->dump(OS); + Arg1->dump(OS); OS << "E2:\n"; - E2->dump(OS); + Arg2->dump(OS); } - if (E1 == E2) - return Result::Equal; + Expr *E1 = const_cast(Arg1); + Expr *E2 = const_cast(Arg2); + E1 = IgnoreValuePreservingOperations(Context, E1); + E2 = IgnoreValuePreservingOperations(Context, E2); + + if (E1 == E2) + return Result::Equal; + + // Commpare expressions structurally, recursively invoking + // comparison for subcomponents. If that fails, consult + // EquivExprs to see if the expressions are considered + // equivalent. Stmt::StmtClass E1Kind = E1->getStmtClass(); Stmt::StmtClass E2Kind = E2->getStmtClass(); Result Cmp = CompareInteger(E1Kind, E2Kind); - if (Cmp != Result::Equal) - return Cmp; + if (Cmp != Result::Equal && !(isa(E1) && isa(E2))) + return CheckEquivExprs(Cmp, E1, E2); Cmp = Result::Equal; switch (E1Kind) { @@ -234,8 +356,8 @@ Result Lexicographic::CompareExpr(const Expr *E1, const Expr *E2) { case Expr::CompoundAssignOperatorClass: Cmp = Compare(E1, E2); break; case Expr::BinaryConditionalOperatorClass: break; - case Expr::ImplicitCastExprClass: Cmp = Compare(E1, E2); break; - case Expr::CStyleCastExprClass: Cmp = Compare(E1, E2); break; + case Expr::ImplicitCastExprClass: Cmp = Compare(E1, E2); break; + case Expr::CStyleCastExprClass: Cmp = Compare(E1, E2); break; case Expr::CompoundLiteralExprClass: Cmp = Compare(E1, E2); break; // TODO: // case: ExtVectorElementExpr @@ -274,7 +396,7 @@ Result Lexicographic::CompareExpr(const Expr *E1, const Expr *E2) { } if (Cmp != Result::Equal) - return Cmp; + return CheckEquivExprs(Cmp, E1, E2); // Check children auto I1 = E1->child_begin(); @@ -287,7 +409,7 @@ Result Lexicographic::CompareExpr(const Expr *E1, const Expr *E2) { if (ordered) { if (Cmp == Result::Equal) continue; - return Cmp; + return CheckEquivExprs(Cmp, E1, E2); } assert(E1Child && E2Child); const Expr *E1ChildExpr = dyn_cast(E1Child); @@ -296,7 +418,20 @@ Result Lexicographic::CompareExpr(const Expr *E1, const Expr *E2) { if (E1ChildExpr && E2ChildExpr) { Cmp = CompareExpr(E1ChildExpr, E2ChildExpr); if (Cmp != Result::Equal) - return Cmp; + return CheckEquivExprs(Cmp, E1, E2); + // TODO: Github issue #475. We need to sort out typing rules + // for uses of variables with bounds-safe interfaces in bounds + // expressions. Then we can likely replace this with CompareType. + // TODO: consider treating operations whose types differ + // but that still produce the same value as being the + // same. For example: + // - Pointer arithmetic where the pointer referent types are the same + // size, checkedness is the same, and the integer types are the + // same size/signedness. + Cmp = CompareTypeIgnoreCheckedness(E1ChildExpr->getType(), + E2ChildExpr->getType()); + if (Cmp != Result::Equal) + return CheckEquivExprs(Cmp, E1, E2); } else // assert fired - return something return Result::LessThan; @@ -306,14 +441,58 @@ Result Lexicographic::CompareExpr(const Expr *E1, const Expr *E2) { // fewer children than E2, make it less than E2. If it has more // children, make it greater than E2. if (I1 == E1->child_end() && I2 != E2->child_end()) - return Result::LessThan; + return CheckEquivExprs(Result::LessThan, E1, E2); if (I1 != E1->child_end() && I2 == E2->child_end()) - return Result::GreaterThan; + return CheckEquivExprs(Result::GreaterThan, E1, E2); return Result::Equal; } +// See if the expressions are considered equivalent using the list of lists +// of equivalent expressions. +Result Lexicographic::CheckEquivExprs(Result Current, const Expr *E1, const Expr *E2) { + if (!EquivExprs) + return Current; + + // Important: compare expressions for equivalence without using equality facts. + // This keep the asymptotic complexity of this method linear in the number of AST nodes + // for E1, E2, and EquivExprs. It also avoid the complexities of having to avoid + // infinite recursions. + Lexicographic SimpleComparer = Lexicographic(Context, nullptr); + // Iterate over the list of sets. + for (auto OuterList = EquivExprs->begin(); OuterList != EquivExprs->end(); + ++OuterList) { + bool LHSAppears = false; + bool RHSAppears = false; + // See if the LHS expression appears in the set. + SmallVector *ExprList = *OuterList; + for (auto InnerList = ExprList->begin(); InnerList != ExprList->end(); ++InnerList) { + if (SimpleComparer.CompareExpr(E1, *InnerList) == Result::Equal) { + LHSAppears = true; + break; + } + } + if (!LHSAppears) + continue; + + // See if the RHS expression appears in the set. + for (auto InnerList = ExprList->begin(); InnerList != ExprList->end(); ++InnerList) { + if (SimpleComparer.CompareExpr(E2, *InnerList) == Result::Equal) { + RHSAppears = true; + break; + } + } + + // If both appear, consider them equivalent. + if (RHSAppears) + return Result::Equal; + } + + return Current; + } + + Result Lexicographic::CompareType(QualType QT1, QualType QT2) { QT1 = QT1.getCanonicalType(); @@ -325,6 +504,17 @@ Lexicographic::CompareType(QualType QT1, QualType QT2) { return Result::LessThan; } +Result +Lexicographic::CompareTypeIgnoreCheckedness(QualType QT1, QualType QT2) { + QT1 = QT1.getCanonicalType(); + QT2 = QT2.getCanonicalType(); + if (Context.isEqualIgnoringChecked(QT1, QT2)) + return Result::Equal; + else + // TODO: fill this in + return Result::LessThan; +} + Result Lexicographic::CompareImpl(const PredefinedExpr *E1, const PredefinedExpr *E2) { return CompareInteger(E1->getIdentType(), E2->getIdentType()); @@ -428,15 +618,15 @@ Lexicographic::CompareImpl(const CompoundAssignOperator *E1, } Result -Lexicographic::CompareImpl(const ImplicitCastExpr *E1, - const ImplicitCastExpr *E2) { - return CompareInteger(E1->getValueKind(), E2->getValueKind()); -} - -Result -Lexicographic::CompareImpl(const CStyleCastExpr *E1, - const CStyleCastExpr *E2) { - return CompareType(E1->getType(), E2->getType()); +Lexicographic::CompareImpl(const CastExpr *E1, + const CastExpr *E2) { + Result Cmp = CompareInteger(E1->getCastKind(), E2->getCastKind()); + if (Cmp != Result::Equal) + return Cmp; + // TODO: Github issue #475. We need to sort out typing rules + // for uses of variables with bounds-safe interfaces in bounds + // expressions. Then we can likely replace this with CompareType. + return CompareTypeIgnoreCheckedness(E1->getType(), E2->getType()); } Result diff --git a/lib/Sema/SemaBounds.cpp b/lib/Sema/SemaBounds.cpp index cc06681e0741..4f51fdb1eabe 100644 --- a/lib/Sema/SemaBounds.cpp +++ b/lib/Sema/SemaBounds.cpp @@ -48,94 +48,8 @@ class BoundsUtil { K == BoundsExpr::Kind::Range || K == BoundsExpr::Kind::Invalid); } - // Return true if this cast preserve the bits of the value, - // false otherwise. - static bool IsValuePreserving(CastKind CK) { - switch (CK) { - case CK_BitCast: - case CK_LValueBitCast: - case CK_NoOp: - case CK_ArrayToPointerDecay: - case CK_FunctionToPointerDecay: - case CK_NullToPointer: - case CK_AssumePtrBounds: - case CK_DynamicPtrBounds: - return true; - default: - return false; - } - } - - static bool isPointerToArrayType(QualType Ty) { - if (const PointerType *T = Ty->getAs()) - return T->getPointeeType()->isArrayType(); - else - return false; - } - // Ignore operations that don't change runtime values: parens, some cast operations, - // and array/function address-of and dereference operators. - // - // The code for casts is adapted from Expr::IgnoreNoopCasts, which seems like doesn't - // do enough filtering (it'll ignore LValueToRValue casts for example). - // TODO: reconcile with CheckValuePreservingCast - static Expr *IgnoreValuePreservingOperations(ASTContext &Ctx, Expr *E) { - while (true) { - E = E->IgnoreParens(); - - if (CastExpr *P = dyn_cast(E)) { - CastKind CK = P->getCastKind(); - Expr *SE = P->getSubExpr(); - if (IsValuePreserving(CK)) { - E = SE; - continue; - } - // Ignore integer <-> casts that are of the same width, ptr<->ptr - // and ptr<->int casts of the same width. - if (CK == CK_IntegralToPointer || CK == CK_PointerToIntegral || - CK == CK_IntegralCast) { - if (Ctx.hasSameUnqualifiedType(E->getType(), SE->getType())) { - E = SE; - continue; - } - if ((E->getType()->isPointerType() || - E->getType()->isIntegralType(Ctx)) && - (SE->getType()->isPointerType() || - SE->getType()->isIntegralType(Ctx)) && - Ctx.getTypeSize(E->getType()) == Ctx.getTypeSize(SE->getType())) { - E = SE; - continue; - } - } - } else if (const UnaryOperator *UO = dyn_cast(E)) { - QualType ETy = UO->getType(); - Expr *SE = UO->getSubExpr(); - QualType SETy = SE->getType(); - - UnaryOperator::Opcode Op = UO->getOpcode(); - if (Op == UO_Deref) { - // This may be more conservative than necessary. - bool between_functions = ETy->isFunctionType() && SETy->isFunctionPointerType(); - bool between_arrays = ETy->isArrayType() && BoundsUtil::isPointerToArrayType(SETy); - if (between_functions || between_arrays) { - E = SE; - continue; - } - } else if (Op == UO_AddrOf) { - // This may be more conservative than necessary. - bool between_functions = ETy->isFunctionPointerType() && SETy->isFunctionType(); - bool between_arrays = BoundsUtil::isPointerToArrayType(ETy) && SETy->isArrayType(); - if (between_functions || between_arrays) { - E = SE; - continue; - } - } - } - - return E; - } - } static Expr *IgnoreRedundantCast(ASTContext &Ctx, CastKind NewCK, Expr *E) { CastExpr *P = dyn_cast(E); @@ -1095,9 +1009,11 @@ namespace { case Expr::BoundsCastExprClass: { CastExpr *CE = cast(E); Expr *subExpr = CE->getSubExpr(); + Expr *subExprAtNewType = CreateExplicitCast(E->getType(), + CastKind::CK_BitCast, + subExpr, true); BoundsExpr *Bounds = CE->getBoundsExpr(); - - Bounds = ExpandToRange(subExpr, Bounds); + Bounds = ExpandToRange(subExprAtNewType, Bounds); return Bounds; } case Expr::ImplicitCastExprClass: @@ -1666,8 +1582,8 @@ namespace { } // Is R in range of this range? - ProofResult InRange(ConstantSizedRange &R, ProofFailure &Cause) { - if (EqualValue(S.Context, Base, R.Base)) { + ProofResult InRange(ConstantSizedRange &R, ProofFailure &Cause, EquivExprSets *EquivExprs) { + if (EqualValue(S.Context, Base, R.Base, EquivExprs)) { ProofResult Result = ProofResult::True; if (LowerOffset > R.LowerOffset) { Cause = CombineFailures(Cause, ProofFailure::LowerBound); @@ -1825,17 +1741,15 @@ namespace { Offset = llvm::APSInt(PointerWidth, false); } - static bool EqualValue(ASTContext &Ctx, Expr *E1, Expr *E2) { - const Expr *NormalizedE1 = BoundsUtil::IgnoreValuePreservingOperations(Ctx, E1); - const Expr *NormalizedE2 = BoundsUtil::IgnoreValuePreservingOperations(Ctx, E2); - Lexicographic::Result R = - Lexicographic(Ctx, nullptr).CompareExpr(NormalizedE1, NormalizedE2); + static bool EqualValue(ASTContext &Ctx, Expr *E1, Expr *E2, EquivExprSets *EquivExprs) { + Lexicographic::Result R = Lexicographic(Ctx, EquivExprs).CompareExpr(E1, E2); return R == Lexicographic::Result::Equal; } // Convert a bounds expression to a constant-sized range. Returns true if the // the bounds expression can be converted and false if it cannot be converted. - bool CreateConstantRange(const BoundsExpr *Bounds, ConstantSizedRange *R) { + bool CreateConstantRange(const BoundsExpr *Bounds, ConstantSizedRange *R, + EquivExprSets *EquivExprs) { switch (Bounds->getKind()) { case BoundsExpr::Kind::Invalid: case BoundsExpr::Kind::Unknown: @@ -1853,7 +1767,7 @@ namespace { llvm::APSInt LowerOffset, UpperOffset; SplitIntoBaseAndOffset(Lower, LowerBase, LowerOffset); SplitIntoBaseAndOffset(Upper, UpperBase, UpperOffset); - if (EqualValue(S.Context, LowerBase, UpperBase)) { + if (EqualValue(S.Context, LowerBase, UpperBase, EquivExprs)) { R->SetBase(LowerBase); R->SetLower(LowerOffset); R->SetUpper(UpperOffset); @@ -1872,6 +1786,7 @@ namespace { ProofResult ProveBoundsDeclValidity(const BoundsExpr *DeclaredBounds, const BoundsExpr *SrcBounds, ProofFailure &Cause, + EquivExprSets *EquivExprs, ProofStmtKind Kind = ProofStmtKind::BoundsDeclaration) { assert(BoundsUtil::IsStandardForm(DeclaredBounds) && @@ -1887,13 +1802,13 @@ namespace { if (DeclaredBounds->isUnknown()) return ProofResult::True; - if (S.Context.EquivalentBounds(DeclaredBounds, SrcBounds)) + if (S.Context.EquivalentBounds(DeclaredBounds, SrcBounds, EquivExprs)) return ProofResult::True; ConstantSizedRange DeclaredRange(S); ConstantSizedRange SrcRange(S); - if (CreateConstantRange(DeclaredBounds, &DeclaredRange) && - CreateConstantRange(SrcBounds, &SrcRange)) { + if (CreateConstantRange(DeclaredBounds, &DeclaredRange, EquivExprs) && + CreateConstantRange(SrcBounds, &SrcRange, EquivExprs)) { #ifdef TRACE_RANGE llvm::outs() << "Found constant ranges:\n"; llvm::outs() << "Declared bounds"; @@ -1905,7 +1820,7 @@ namespace { llvm::outs() << "\nSource range:"; SrcRange.Dump(llvm::outs()); #endif - ProofResult R = SrcRange.InRange(DeclaredRange, Cause); + ProofResult R = SrcRange.InRange(DeclaredRange, Cause, EquivExprs); if (R == ProofResult::True) return R; if (R == ProofResult::False || R == ProofResult::Maybe) { @@ -1944,7 +1859,7 @@ namespace { "bounds not in standard form"); Cause = ProofFailure::None; ConstantSizedRange ValidRange(S); - if (!CreateConstantRange(Bounds, &ValidRange)) + if (!CreateConstantRange(Bounds, &ValidRange, nullptr)) return ProofResult::Maybe; bool Overflow; @@ -1985,7 +1900,7 @@ namespace { llvm::outs() << "Valid range:\n"; ValidRange.Dump(llvm::outs()); #endif - ProofResult R = ValidRange.InRange(MemoryAccessRange, Cause); + ProofResult R = ValidRange.InRange(MemoryAccessRange, Cause, nullptr); if (R == ProofResult::True) return R; if (R == ProofResult::False || R == ProofResult::Maybe) { @@ -2033,9 +1948,21 @@ namespace { BoundsExpr *DeclaredBounds, Expr *Src, BoundsExpr *SrcBounds, bool InCheckedScope) { + // Record expression equality implied by assignment. + SmallVector *, 4> EquivExprs; + SmallVector EqualExpr; + // TODO: make sure assignment to lvalue doesn't modify value used in Src. + if (S.CheckIsNonModifying(Target, Sema::NonModifyingContext::NMC_Unknown, false) && + S.CheckIsNonModifying(Src, Sema::NonModifyingContext::NMC_Unknown, false)) { + Expr *TargetExpr = BoundsInference(S).CreateImplicitCast(Target->getType(), CK_LValueToRValue, Target); + EqualExpr.push_back(TargetExpr); + EqualExpr.push_back(Src); + EquivExprs.push_back(&EqualExpr); + } + ProofFailure Cause; ProofResult Result = ProveBoundsDeclValidity(DeclaredBounds, SrcBounds, - Cause); + Cause, &EquivExprs); if (Result != ProofResult::True) { unsigned DiagId = (Result == ProofResult::False) ? diag::error_bounds_declaration_invalid : @@ -2065,7 +1992,7 @@ namespace { SourceLocation ArgLoc = Arg->getLocStart(); ProofFailure Cause; ProofResult Result = ProveBoundsDeclValidity(ExpectedArgBounds, - ArgBounds, Cause); + ArgBounds, Cause, nullptr); if (Result != ProofResult::True) { unsigned DiagId = (Result == ProofResult::False) ? diag::error_argument_bounds_invalid : @@ -2088,9 +2015,37 @@ namespace { BoundsExpr *DeclaredBounds, Expr *Src, BoundsExpr *SrcBounds, bool InCheckedScope) { + // Record expression equality implied by initialization. + SmallVector *, 4> EquivExprs; + SmallVector EqualExpr; + if (S.CheckIsNonModifying(Src, Sema::NonModifyingContext::NMC_Unknown, false)) { + // TODO: make sure variable being initialized isn't read by Src. + DeclRefExpr *TargetDeclRef = + DeclRefExpr::Create(S.getASTContext(), NestedNameSpecifierLoc(), + SourceLocation(), D, false, SourceLocation(), + D->getType(), ExprValueKind::VK_LValue); + CastKind Kind; + QualType TargetTy; + if (D->getType()->isArrayType()) { + Kind = CK_ArrayToPointerDecay; + TargetTy = S.getASTContext().getArrayDecayedType(D->getType()); + } else { + Kind = CK_LValueToRValue; + TargetTy = D->getType(); + } + Expr *TargetExpr = BoundsInference(S).CreateImplicitCast(TargetTy, Kind, TargetDeclRef); + EqualExpr.push_back(TargetExpr); + EqualExpr.push_back(Src); + EquivExprs.push_back(&EqualExpr); + /* + llvm::outs() << "Dumping target/src equality relation"; + TargetExpr->dump(llvm::outs()); + Src->dump(llvm::outs()); + */ + } ProofFailure Cause; ProofResult Result = ProveBoundsDeclValidity(DeclaredBounds, - SrcBounds, Cause); + SrcBounds, Cause, &EquivExprs); if (Result != ProofResult::True) { unsigned DiagId = (Result == ProofResult::False) ? diag::error_bounds_declaration_invalid : @@ -2125,7 +2080,7 @@ namespace { ProofStmtKind Kind = IsStaticPtrCast ? ProofStmtKind::StaticBoundsCast : ProofStmtKind::BoundsDeclaration; ProofResult Result = - ProveBoundsDeclValidity(TargetBounds, SrcBounds, Cause, Kind); + ProveBoundsDeclValidity(TargetBounds, SrcBounds, Cause, nullptr, Kind); if (Result != ProofResult::True) { unsigned DiagId = (Result == ProofResult::False) ? diag::error_static_cast_bounds_invalid : diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index e6f9aeac18b7..9185c99f0ec5 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -13317,16 +13317,16 @@ ExprResult Sema::ActOnCountBoundsExpr(SourceLocation BoundsKWLoc, } /// \brief Validate the type of an argument expression to a bounds -/// expression. If valid, return the unqualified pointee type. Otherwise -/// return null. Assumes that the usual C conversions for expressions +/// expression. If valid, return the pointee type. Otherwise +/// return a null QualType Assumes that the usual C conversions for expressions /// have been applied already, including array->pointer conversions. -const Type *Sema::ValidateBoundsExprArgument(Expr *Arg) { +QualType Sema::ValidateBoundsExprArgument(Expr *Arg) { QualType ArgType = Arg->getType(); const Type *ArgPointerType = ArgType->getAs(); if (!ArgPointerType) { Diag(Arg->getLocStart(), diag::err_typecheck_pointer_type_expected) << ArgType; - return nullptr; + return QualType(); } QualType ArgTypePointee = ArgPointerType->getPointeeType(); // We return the unqualified type so that we can compare @@ -13346,11 +13346,11 @@ const Type *Sema::ValidateBoundsExprArgument(Expr *Arg) { if (!ArgTypePointee->isIncompleteOrObjectType()) { Diag(Arg->getLocStart(), diag::err_typecheck_bounds_expr) << ArgType; - return nullptr; + return QualType(); } - // Return the canonical unqualified pointee type. - return ArgTypePointee.getTypePtr(); + // Return the pointee type. + return ArgTypePointee; } ExprResult Sema::ActOnRangeBoundsExpr(SourceLocation BoundsKWLoc, @@ -13367,13 +13367,13 @@ ExprResult Sema::ActOnRangeBoundsExpr(SourceLocation BoundsKWLoc, return ExprError(); UpperBound = Result.get(); - const Type *LowerBoundPointee = ValidateBoundsExprArgument(LowerBound); - const Type *UpperBoundPointee = ValidateBoundsExprArgument(UpperBound); + QualType LowerBoundPointee = ValidateBoundsExprArgument(LowerBound); + QualType UpperBoundPointee = ValidateBoundsExprArgument(UpperBound); - if (!LowerBoundPointee || !UpperBoundPointee) + if (LowerBoundPointee.isNull() || UpperBoundPointee.isNull()) return ExprError(); - if (LowerBoundPointee != UpperBoundPointee && + if (!Context.typesAreCompatible(LowerBoundPointee, UpperBoundPointee, true) && !LowerBoundPointee->isVoidType() && !UpperBoundPointee->isVoidType()) { Diag(LowerBound->getLocStart(), diff --git a/test/CheckedC/inferred-bounds/basic.c b/test/CheckedC/inferred-bounds/basic.c index e779d0974012..d3c0e869c2e6 100644 --- a/test/CheckedC/inferred-bounds/basic.c +++ b/test/CheckedC/inferred-bounds/basic.c @@ -15,7 +15,7 @@ // The description uses AST dumps. // // This line is for the clang test infrastructure: -// RUN: %clang_cc1 -fcheckedc-extension -verify -verify-ignore-unexpected=warning -fdump-inferred-bounds %s | FileCheck %s +// RUN: %clang_cc1 -fcheckedc-extension -verify -verify-ignore-unexpected=warning -verify-ignore-unexpected=note -fdump-inferred-bounds %s | FileCheck %s //-------------------------------------------------------------------------// // Test assignment of integers to _Array_ptr variables. This covers both // diff --git a/test/CheckedC/inferred-bounds/member-arrow-reference.c b/test/CheckedC/inferred-bounds/member-arrow-reference.c index 87f46d6b89c6..e908ccfd6081 100644 --- a/test/CheckedC/inferred-bounds/member-arrow-reference.c +++ b/test/CheckedC/inferred-bounds/member-arrow-reference.c @@ -10,7 +10,6 @@ // // This line is for the clang test infrastructure: // RUN: %clang_cc1 -fcheckedc-extension -verify -fdump-inferred-bounds %s | FileCheck %s -// expected-no-diagnostics struct S1 { _Array_ptr p : count(len); @@ -97,7 +96,9 @@ void f1(struct S1 *a1, struct S2 *b2) { void f2(struct S1 *a3) { int local_arr1[5]; // TODO: need bundled block. - a3->p = local_arr1; + a3->p = local_arr1; // expected-warning {{cannot prove declared bounds for a3->p are valid after assignment}} \ + // expected-note {{(expanded) declared bounds are 'bounds(a3->p, a3->p + a3->len)'}} \ + // expected-note {{(expanded) inferred bounds are 'bounds(local_arr1, local_arr1 + 5)'}} // CHECK: BinaryOperator {{0x[0-9a-f]+}} '_Array_ptr' '=' // CHECK: |-MemberExpr {{0x[0-9a-f]+}} '_Array_ptr' lvalue ->p {{0x[0-9a-f]+}} diff --git a/test/CheckedC/static-checking/bounds-decl-challenges.c b/test/CheckedC/static-checking/bounds-decl-challenges.c new file mode 100644 index 000000000000..143220990a6d --- /dev/null +++ b/test/CheckedC/static-checking/bounds-decl-challenges.c @@ -0,0 +1,31 @@ +// Tests that the current static checking of bounds declaration can't +// handle. +// +// RUN: %clang -cc1 -fcheckedc-extension -Wcheck-bounds-decls -verify -verify-ignore-unexpected=note %s + +// A function call on the rhs that returns a value with bounds. + +extern _Array_ptr h4(void) : count(3) { + _Array_ptr p : bounds(p, p + 3) = 0; + return p; +} + +extern void f7(void *p) { + _Array_ptr r : count(3) = 0; + r = _Assume_bounds_cast<_Array_ptr>(h4(), count(3)); // expected-warning {{cannot prove declared bounds for r are valid after assignment}} +} + +// Deferencing an array of nt_checked arrays. +extern void check_assign(int val, int p[10], int q[], int r _Checked[10], int s _Checked[], + int s2d _Checked[10][10], int v _Nt_checked[10], int w _Nt_checked[], + int w2d _Checked[10]_Nt_checked[10]) { + int x2d _Checked[10]_Nt_checked[10]; + _Nt_array_ptr t13b = w2d[0]; // expected-warning {{cannot prove declared bounds for 't13b' are valid after initialization}} + _Nt_array_ptr t15b = x2d[0]; // expected-warning {{cannot prove declared bounds for 't15b' are valid after initialization}} +} + +// Creating a pointer with count bounds into an existing array. +void passing_test_1(void) { + int a _Checked[10] = { 9, 8, 7, 6, 5, 4, 3, 2, 1}; + _Array_ptr b : count(5) = &a[2]; // expected-warning {{cannot prove declared bounds for 'b' are valid after initialization}} +} \ No newline at end of file diff --git a/test/CheckedC/static-checking/bounds-decl-checking.c b/test/CheckedC/static-checking/bounds-decl-checking.c index 5c4c4e9742e9..5df222c325db 100644 --- a/test/CheckedC/static-checking/bounds-decl-checking.c +++ b/test/CheckedC/static-checking/bounds-decl-checking.c @@ -16,12 +16,24 @@ void f2(_Array_ptr p : bounds(p, p + x), int x) { _Array_ptr r : bounds(p, p + x) = p; } -// Initialization by expression without syntactically identical -// normalized bounds - warning expected. +// Initialization of a pointer by an array with syntactically identical +// normalized bounds. +void f2a(void) { + int a _Checked[10]; + _Array_ptr r : bounds(a, a + 10) = a; +} + +// Initialization by expression with bounds that are syntactically identical +// modulo expression equality implied by initialization - no warning. void f3(_Array_ptr p : bounds(p, p + x), int x) { - _Array_ptr r : count(x) = p; // expected-warning {{cannot prove declared bounds}} \ - // expected-note {{(expanded) declared bounds are 'bounds(r, r + x)'}} \ - // expected-note {{(expanded) inferred bounds are 'bounds(p, p + x)'}} + _Array_ptr r : count(x) = p; +} + +// Initialization by an array with bounds that syntactically identical +// modulo expression equality implied by initialization - no warning. +void f3a(void) { + int a _Checked[10]; + _Array_ptr r : count(10) = a; } @@ -38,13 +50,11 @@ void f5(_Array_ptr p : count(x), int x) { r = p; } -// Assignment of expression without syntactically identical normalized bounds - -// no warning. +// Assignment of expression with bounds that are syntactically identical +// modulo expression equality implied by assignment - no warning. void f6(_Array_ptr p : count(x), int x) { _Array_ptr r : count(x) = 0; - r = p; // expected-warning {{cannot prove declared bounds}} \ - // expected-note {{(expanded) declared bounds are 'bounds(r, r + x)'}} \ - // expected-note {{(expanded) inferred bounds are 'bounds(p, p + x)'}} + r = p; } // Parameter passing @@ -94,6 +104,15 @@ void f20(_Array_ptr p: count(5)) { // expected-note {{(expanded) inferred bounds are 'bounds(p, p + 5)'}} } +// Initialization of pointers using top-level arrays +int a0 _Checked[9] = { 0, 1, 2, 3, 4, 5, 6, 7, 8 }; +_Array_ptr a1 : count(3) = a0; +_Array_ptr a2 : count(11) = a0; // expected-error {{declared bounds for 'a2' are invalid after initialization}} \ + // expected-note {{destination bounds are wider than the source bounds}} \ + // expected-note {{destination upper bound is above source upper bound}} \ + // expected-note {{(expanded) declared bounds are 'bounds(a2, a2 + 11)'}} \ + // expected-note {{(expanded) inferred bounds are 'bounds(a0, a0 + 9)'}} + // Assignments void f21(_Array_ptr p: count(5)) { _Array_ptr q1 : bounds(p, p + 5) = 0;