diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 241a570..b13ac01 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -5886,7 +5886,7 @@ public: private: void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr, - bool isSubscript=false); + bool isSubscript=false, bool AllowOnePastEnd=true); void CheckArrayAccess(const Expr *E); bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall); bool CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall); diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index f86133a..931924e 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -3363,6 +3363,8 @@ void Sema::CheckImplicitConversions(Expr *E, SourceLocation CC) { if (E->isTypeDependent() || E->isValueDependent()) return; + CheckArrayAccess(E); + // This is not the right CC for (e.g.) a variable initialization. AnalyzeImplicitConversions(*this, E, CC); } @@ -3477,7 +3479,7 @@ static const Type* getElementType(const Expr *BaseExpr) { } void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr, - bool isSubscript) { + bool isSubscript, bool AllowOnePastEnd) { const Type* EffectiveType = getElementType(BaseExpr); BaseExpr = BaseExpr->IgnoreParenCasts(); IndexExpr = IndexExpr->IgnoreParenCasts(); @@ -3499,21 +3501,17 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr, return; const Type* BaseType = getElementType(BaseExpr); - if (BaseType != EffectiveType) { + if (!isSubscript && BaseType != EffectiveType) { // Make sure we're comparing apples to apples when comparing index to size uint64_t ptrarith_typesize = Context.getTypeSize(EffectiveType); uint64_t array_typesize = Context.getTypeSize(BaseType); if (ptrarith_typesize != array_typesize) { // There's a cast to a different size type involved uint64_t ratio = array_typesize / ptrarith_typesize; - if (ptrarith_typesize * ratio != array_typesize) - // If the size of the array's base type is not a multiple of the - // casted-to pointee type, the results of the pointer arithmetic - // may or may not point to something consistently meaningful or within a - // valid reference... - return; - - size *= llvm::APInt(size.getBitWidth(), ratio); + // TODO: Be smarter about handling cases where array_typesize is not a + // multiple of ptrarith_typesize + if (ptrarith_typesize * ratio == array_typesize) + size *= llvm::APInt(size.getBitWidth(), ratio); } } @@ -3526,7 +3524,7 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr, // arithmetic also allow the index (offset) to be equal to size since // computing the next address after the end of the array is legal and // commonly done e.g. in C++ iterators and range-based for loops. - if (isSubscript ? index.slt(size) : index.sle(size)) + if (AllowOnePastEnd ? index.sle(size) : index.slt(size)) return; unsigned DiagID = diag::warn_ptr_arith_exceeds_bounds; @@ -3562,15 +3560,32 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr, } void Sema::CheckArrayAccess(const Expr *expr) { - while (true) { - expr = expr->IgnoreParens(); + int AllowOnePastEnd = 0; + while (expr) { + expr = expr->IgnoreParenImpCasts(); switch (expr->getStmtClass()) { case Stmt::ArraySubscriptExprClass: { const ArraySubscriptExpr *ASE = cast(expr); - CheckArrayAccess(ASE->getBase(), - ASE->getIdx(), true); + CheckArrayAccess(ASE->getBase(), ASE->getIdx(), true, + AllowOnePastEnd > 0); return; } + case Stmt::UnaryOperatorClass: { + // Only unwrap the * and & unary operators + const UnaryOperator *UO = cast(expr); + expr = UO->getSubExpr(); + switch (UO->getOpcode()) { + case UO_AddrOf: + AllowOnePastEnd++; + break; + case UO_Deref: + AllowOnePastEnd--; + break; + default: + return; + } + break; + } case Stmt::ConditionalOperatorClass: { const ConditionalOperator *cond = cast(expr); if (const Expr *lhs = cond->getLHS()) diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index bb7e3d8..c654157 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -268,7 +268,6 @@ ExprResult Sema::DefaultFunctionArrayConversion(Expr *E) { E = ImpCastExprToType(E, Context.getPointerType(Ty), CK_FunctionToPointerDecay).take(); else if (Ty->isArrayType()) { - CheckArrayAccess(E); // In C90 mode, arrays only promote to pointers if the array expression is // an lvalue. The relevant legalese is C90 6.2.2.1p3: "an lvalue that has // type 'array of type' is converted to an expression that has type 'pointer @@ -354,8 +353,6 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) { // type of the lvalue. if (T.hasQualifiers()) T = T.getUnqualifiedType(); - - CheckArrayAccess(E); return Owned(ImplicitCastExpr::Create(Context, T, CK_LValueToRValue, E, 0, VK_RValue)); @@ -387,14 +384,6 @@ ExprResult Sema::UsualUnaryConversions(Expr *E) { QualType Ty = E->getType(); assert(!Ty.isNull() && "UsualUnaryConversions - missing type"); - if (Ty->isPointerType() || Ty->isArrayType()) { - Expr *subE = E->IgnoreParenImpCasts(); - while (UnaryOperator *UO = dyn_cast(subE)) { - subE = UO->getSubExpr()->IgnoreParenImpCasts(); - } - if (subE) CheckArrayAccess(subE); - } - // Try to perform integral promotions if the object has a theoretically // promotable type. if (Ty->isIntegralOrUnscopedEnumerationType()) { @@ -3316,6 +3305,7 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, Arg = ArgExpr.takeAs(); } + CheckArrayAccess(Arg); AllArgs.push_back(Arg); } @@ -5822,6 +5812,7 @@ QualType Sema::CheckAdditionOperands( // C99 6.5.6 return QualType(); } + // Check array bounds for pointer arithemtic CheckArrayAccess(PExp, IExp); if (CompLHSTy) { @@ -5881,6 +5872,7 @@ QualType Sema::CheckSubtractionOperands(ExprResult &lex, ExprResult &rex, Expr *IExpr = rex.get()->IgnoreParenCasts(); UnaryOperator negRex(IExpr, UO_Minus, IExpr->getType(), VK_RValue, OK_Ordinary, IExpr->getExprLoc()); + // Check array bounds for pointer arithemtic CheckArrayAccess(lex.get()->IgnoreParenCasts(), &negRex); if (CompLHSTy) *CompLHSTy = lex.get()->getType(); @@ -6849,9 +6841,7 @@ QualType Sema::CheckAssignmentOperands(Expr *LHS, ExprResult &RHS, return QualType(); CheckForNullPointerDereference(*this, LHS); - // Check for trivial buffer overflows. - CheckArrayAccess(LHS->IgnoreParenCasts()); - + // C99 6.5.16p3: The type of an assignment expression is the type of the // left operand unless the left operand has qualified type, in which case // it is the unqualified version of the type of the left operand. @@ -7584,6 +7574,10 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, } if (ResultTy.isNull() || lhs.isInvalid() || rhs.isInvalid()) return ExprError(); + + CheckArrayAccess(lhs.get()); + CheckArrayAccess(rhs.get()); + if (CompResultTy.isNull()) return Owned(new (Context) BinaryOperator(lhs.take(), rhs.take(), Opc, ResultTy, VK, OK, OpLoc)); @@ -7932,6 +7926,8 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc, } if (resultType.isNull() || Input.isInvalid()) return ExprError(); + if (Opc != UO_AddrOf && Opc != UO_Deref) + CheckArrayAccess(Input.get()); return Owned(new (Context) UnaryOperator(Input.take(), Opc, resultType, VK, OK, OpLoc)); diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index 0225c66..151aea1 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -2241,9 +2241,6 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType, if (!From->isGLValue()) break; } - // Check for trivial buffer overflows. - CheckArrayAccess(From); - FromType = FromType.getUnqualifiedType(); From = ImplicitCastExpr::Create(Context, FromType, CK_LValueToRValue, From, 0, VK_RValue); diff --git a/test/SemaCXX/array-bounds.cpp b/test/SemaCXX/array-bounds.cpp index f7bb7dd..ac52ecd 100644 --- a/test/SemaCXX/array-bounds.cpp +++ b/test/SemaCXX/array-bounds.cpp @@ -35,11 +35,14 @@ void test() { s2.a[3] = 0; // no warning for 0-sized array union { - short a[2]; // expected-note 2 {{declared here}} + short a[2]; // expected-note 4 {{declared here}} char c[4]; } u; u.a[3] = 1; // expected-warning {{array index of '3' indexes past the end of an array (that contains 2 elements)}} u.c[3] = 1; // no warning + short *p = &u.a[2]; // no warning + p = &u.a[3]; // expected-warning {{array index of '3' indexes past the end of an array (that contains 2 elements)}} + *(&u.a[2]) = 1; // expected-warning {{array index of '2' indexes past the end of an array (that contains 2 elements)}} *(&u.a[3]) = 1; // expected-warning {{array index of '3' indexes past the end of an array (that contains 2 elements)}} *(&u.c[3]) = 1; // no warning @@ -174,3 +177,14 @@ void test_all_enums_covered(enum Values v) { } x[2] = 0; // no-warning } + +void bar(int x) {} +int test_more() { + int foo[5]; // expected-note 5 {{array 'foo' declared here}} + bar(foo[5]); // expected-warning {{array index of '5' indexes past the end of an array (that contains 5 elements)}} + ++foo[5]; // expected-warning {{array index of '5' indexes past the end of an array (that contains 5 elements)}} + if (foo[6]) // expected-warning {{array index of '6' indexes past the end of an array (that contains 5 elements)}} + return --foo[6]; // expected-warning {{array index of '6' indexes past the end of an array (that contains 5 elements)}} + else + return foo[5]; // expected-warning {{array index of '5' indexes past the end of an array (that contains 5 elements)}} +}