Hi Ted,
I've gone ahead and attached the full patch to facilitate seeing all of the
changes in better context.
On Mon, Aug 1, 2011 at 2:18 PM, Ted Kremenek <[email protected]> wrote:
> Hi Kaelyn,
>
> Sorry for the delay in responding. I'm trying to understand the
> differences proposed between your last patch and the new one. When I look
> at the "delta from patch set" from 2 to 3, I see changes that appear
> unrelated to this feature, which I find confusing.
>
> What would help me is if I understood more of the high-level motivation of
> the changes. What I see is calls to "CheckArrayAccess" moved to various
> places, for example when checking the comma operator:
>
> 7578 CheckArrayAccess(lhs.get());
> 7579 CheckArrayAccess(rhs.get());
>
> This feels a little kludgy. I'm not saying it is wrong; I just don't
> understand the motivation. Could you provide some insight behind these
> changes other than they got the particular test cases working?
>
The changes to get everything working properly (from what I had committed to
svn and then reverted) are, except for the unary operator handling in
CheckArrayAccess, pretty much just shuffling around when CheckArrayAccess is
called. I agree that it feels a bit kludgy, but unfortunately it was the
only way I could get CheckArrayAccess called once and only once for each
array access for all the cases I could come up with (see the changes to
test/SemaCXX/array-bounds.cpp).
For example, with my original patch that had to be reverted "p = &u.a[3];",
"++foo[5];", and "return --foo[6];" from array-bounds.cpp (lines 44, 185,
and 187) do not trigger any warning. Additionally, "p == &u.a[2]" triggers a
false warning (where "a" is a 2-element array). There are a few places like
that where the warnings are triggered incorrectly, but even more places
where CheckArrayAccess is never called--such as on the right hand side of
assignments. It not being called at all in some cases is why e.g. line 43
"short *p = &u.a[2];" did not give a warning.
With just the additional changes I made within CheckArrayAccess and leaving
the CheckArrayAccess call sites alone, "p = &u.a[3];", "++foo[5];", and
"return --foo[6];" from array-bounds.cpp (lines 44, 185, and 187) still do
not trigger any warning. The noticeable change is that "*(&u.a[2]) = 1;" and
"*(&u.a[3]) = 1;" (lines 45 and 46) generate the warning twice. Likewise
just changing where CheckArrayAccess is called, without the additional
changes to CheckArrayAccess, leads to quite a few situations where the
warning is never properly emitted because CheckArrayAccess was called with
an unrecognized type of statement (15 instances in
test/SemaCXX/array-bounds.cpp spread over the entire file--first case is on
line 9 and the last on line 189).
Finally, without either my original or revised patches, there are quite a
few cases where the warning is never triggered, such as "int val = a[3];",
"p = &u.a[3];", "array[const_subscript] = 0;", "(*array_ptr)[3] = 1;", and
"++foo[5];" (lines 28, 44, 51, 67, and 185 of the version of
test/SemaCXX/array-bounds.cpp with my patch applied).
It took a few days of trial, error, tracing through call stacks, digging
through code paths, and general frustration to figure out where to place the
calls to CheckArrayAccess to get as much coverage as possible while avoiding
emitting the same warnings multiple times. Hope that helps shed some light
on the nature of my additional changes.
Cheers,
Kaelyn
> Thanks so much,
> Ted
>
> On Jul 27, 2011, at 4:18 PM, Kaelyn Uhrain wrote:
>
> Ted & Eric,
>
> After having to revert my array bounds enhancements (which had gone in as
> r136046) due to false positives in cases like &array[array_size], I've
> reworked the array bounds checking a bit and think I have it working
> correctly now. I've uploaded the full set of changes including my reverted
> changes to
> http://codereview.appspot.com/4675068 and attached a diff of the changes
> from what I'd committed and later rolled back.
>
> Cheers,
> Kaelyn
>
> On Mon, Jul 25, 2011 at 6:17 PM, Kaelyn Uhrain <[email protected]> wrote:
>
>> Just realized my patch missed the new unit test file. d'oh!
>>
>>
>> On Sat, Jul 23, 2011 at 11:35 AM, Ted Kremenek <[email protected]>wrote:
>>
>>> This all sounds good to me. I think the patch can go in as is.
>>>
>>> On Jul 21, 2011, at 4:04 PM, Kaelyn Uhrain wrote:
>>>
>>> Hi Ted,
>>>
>>> I've attached a new version of my patch that moves the pointer arithmetic
>>> portion of the -Warray-bounds improvements into a separate flag,
>>> -Warray-bounds-pointer-arithmetic. Since the remaining noise in the pointer
>>> arithmetic check is more a case of undefined behavior that does the right
>>> thing with most (all?) compilers than a case of being true false positives,
>>> the plan is to have -Warray-bounds-pointer-arithmetic initially be disabled
>>> by default and in later patches to enhance the warning by e.g. providing a
>>> fixit note to add parens to exprs like "ptr + sizeof("foo") - 1" so they
>>> become "ptr + (sizeof("foo")) - 1".
>>>
>>> Cheers,
>>> Kaelyn
>>>
>>> On Thu, Jul 21, 2011 at 1:18 PM, Ted Kremenek <[email protected]>wrote:
>>>
>>>> Hi Kaelyn,
>>>>
>>>> While I think there will be some contention here, I think this is still
>>>> too high of a false positive rate for this warning to be on by default in
>>>> the compiler. Users simply aren't going to tolerate it. I think it is
>>>> reasonable for a codebase to adopt a strict policy for this warning, but I
>>>> don't think a 17% false positive rate (or possibly higher) is acceptable
>>>> for
>>>> a default warning for all users of Clang.
>>>>
>>>> Unless you believe you think there are additional heuristics to drop the
>>>> false positive rate down below 5-10%, I think I'm fine with proceeding as
>>>> putting this in as an opt-in warning, and then refine from there.
>>>>
>>>> On Jul 20, 2011, at 11:39 AM, Kaelyn Uhrain wrote:
>>>>
>>>> I've attached an updated version of my patch that better handles cases
>>>> where pointer arithmetic is done after casting a constant-size array to a
>>>> pointer for a smaller base type (e.g. casting an int array to char*). Of
>>>> the
>>>> pointer arithmetic warnings, about 24% could be considered false positives;
>>>> however, the actual number of false positives is quite small and 2/3 of
>>>> them
>>>> stem from the use of a single macro--if you count those as a single warning
>>>> & false positive, the rate drops to 17%. Of the false positives most are
>>>> from semi-questionable pointer arithmetic where a constant greater than the
>>>> length of the array/pointer is being added to the pointer and some int > 1
>>>> being subtracted from it, e.g.:
>>>>
>>>> void foo(int n) {
>>>> char x[5];
>>>> if (n > 0) bar(x + 6 - n);
>>>> }
>>>>
>>>> Cheers,
>>>> Kaelyn
>>>>
>>>> On Tue, Jul 19, 2011 at 11:44 AM, Kaelyn Uhrain <[email protected]>wrote:
>>>>
>>>>> I'm still looking at the pointer arithmetic warnings to determine which
>>>>> are false positives, but I realized I screwed up my previous stats a bit
>>>>> as
>>>>> I forgot to account for duplicated/repeated warnings (e.g. the same header
>>>>> included in multiple compilation units and so generating the same warnings
>>>>> multiple times). For unique warnings, the stats are:
>>>>>
>>>>> - 16.5% increase in warnings from before my patch (originally reported
>>>>> a 24% increase)
>>>>> - 47% of those new warnings being about pointer arithmetic
>>>>> - 6.7% of all of the warnings emitted with my patch applied are
>>>>> concerning pointer arithmetic. (originally reported 8.3%)
>>>>> - The new pointer arithmetic warnings represent a 7.7% increase in
>>>>> warnings from before my patch, not 10%.
>>>>>
>>>>> I'll send another email once I have a feel for how noisy the pointer
>>>>> arithmetic warnings are.
>>>>>
>>>>> Cheers,
>>>>> Kaelyn
>>>>>
>>>>>
>>>>> On Mon, Jul 18, 2011 at 5:54 PM, Kaelyn Uhrain <[email protected]>wrote:
>>>>>
>>>>>> Ted,
>>>>>>
>>>>>> You're welcome. I'll try to figure out what fraction of the pointer
>>>>>> arithmetic warnings are false positives (requires a bit of manual
>>>>>> digging on
>>>>>> my part to determine if the code is indeed buggy or if it is valid /
>>>>>> intended). For the overall 24% increase in warnings, keep in mind that
>>>>>> over
>>>>>> half of that is the existing bounds checking now being applied to cases
>>>>>> where it wasn't before, i.e.:
>>>>>>
>>>>>> char *foo[5];
>>>>>> foo[77]; // -Warray-bounds already found
>>>>>> &foo[77]; // -Warray-bounds currently misses
>>>>>> *foo[77]; // -Warray-bounds currently misses
>>>>>>
>>>>>> The function that did the bounds checking would never catch the last
>>>>>> two cases because it would see a UnaryOperator (in the above cases for
>>>>>> the
>>>>>> '&' and the '*') and skip the expression instead of looking inside the
>>>>>> UnaryOperator expression for the array subscripting.
>>>>>>
>>>>>> The new pointer arithmetic bounds checking only represents a 10%
>>>>>> increase in warnings--and IMHO that is the only part where the number of
>>>>>> false positives introduced might be an issue.
>>>>>>
>>>>>> Cheers,
>>>>>> Kaelyn
>>>>>>
>>>>>>
>>>>>> On Mon, Jul 18, 2011 at 5:36 PM, Ted Kremenek <[email protected]>wrote:
>>>>>>
>>>>>>> Hi Kaelyn,
>>>>>>>
>>>>>>> Thanks for the statistics. What would be good to know is what
>>>>>>> fraction of these are false positives (i.e., are these all real bugs)?
>>>>>>> A
>>>>>>> small random sample might be helpful. A 24% increase in warnings is
>>>>>>> fairly
>>>>>>> substantial, and we don't want to do that unless there is a real
>>>>>>> benefit.
>>>>>>>
>>>>>>> Ted
>>>>>>>
>>>>>>> On Jul 18, 2011, at 5:18 PM, Kaelyn Uhrain wrote:
>>>>>>>
>>>>>>> Ted,
>>>>>>>
>>>>>>> On Thu, Jul 14, 2011 at 5:04 PM, Ted Kremenek <[email protected]>wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> The only other issue: should this be controlled under a separate
>>>>>>>> warning flag, at least initially so we can experiment with this new
>>>>>>>> warning
>>>>>>>> and see how noisy it is? E.g. "-Warray-bounds-pointer-arithmetic".
>>>>>>>>
>>>>>>>
>>>>>>> I've tested the -Warray-bounds changes against the Google codebase
>>>>>>> and my patch increases the number of warnings from -Warray-bounds by
>>>>>>> 24%. Of
>>>>>>> the new warnings, 57.33% are for array indexes that most likely weren't
>>>>>>> picked up before because of unary operators like & or * (11.1% of all
>>>>>>> the
>>>>>>> warnings now emitted), and the remaining 42.67% are from out-of-bounds
>>>>>>> pointer arithmetic (8.3% of all the warnings from -Warray-bounds).
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Kaelyn
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>> <array-bounds-enhancement3.diff>
>>>>
>>>>
>>>>
>>> <array-bounds-enhancement4.diff>
>>>
>>>
>>>
>>
> <array-bounds-enhancement-changes.diff>
> _______________________________________________
>
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 83f232e..3494371 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4081,11 +4081,17 @@ def err_out_of_line_default_deletes : Error<
def err_defaulted_move_unsupported : Error<
"defaulting move functions not yet supported">;
+def warn_ptr_arith_precedes_bounds : Warning<
+ "the pointer decremented by %0 refers before the beginning of the array">,
+ InGroup<DiagGroup<"array-bounds-pointer-arithmetic">>, DefaultIgnore;
+def warn_ptr_arith_exceeds_bounds : Warning<
+ "the pointer incremented by %0 refers past the end of the array (that contains %1 element%s2)">,
+ InGroup<DiagGroup<"array-bounds-pointer-arithmetic">>, DefaultIgnore;
def warn_array_index_precedes_bounds : Warning<
"array index of '%0' indexes before the beginning of the array">,
InGroup<DiagGroup<"array-bounds">>;
def warn_array_index_exceeds_bounds : Warning<
- "array index of '%0' indexes past the end of an array (that contains %1 elements)">,
+ "array index of '%0' indexes past the end of an array (that contains %1 element%s2)">,
InGroup<DiagGroup<"array-bounds">>;
def note_array_index_out_of_bounds : Note<
"array %0 declared here">;
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index cbb096d..07f6739 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -5916,6 +5916,8 @@ public:
unsigned ByteNo) const;
private:
+ void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
+ 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 28085ef..ff3f7cf 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);
}
@@ -3467,43 +3469,83 @@ void Sema::CheckCastAlign(Expr *Op, QualType T, SourceRange TRange) {
<< TRange << Op->getSourceRange();
}
-static void CheckArrayAccess_Check(Sema &S,
- const clang::ArraySubscriptExpr *E) {
- const Expr *BaseExpr = E->getBase()->IgnoreParenImpCasts();
+static const Type* getElementType(const Expr *BaseExpr) {
+ const Type* EltType = BaseExpr->getType().getTypePtr();
+ if (EltType->isAnyPointerType())
+ return EltType->getPointeeType().getTypePtr();
+ else if (EltType->isArrayType())
+ return EltType->getBaseElementTypeUnsafe();
+ return EltType;
+}
+
+void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
+ bool isSubscript, bool AllowOnePastEnd) {
+ const Type* EffectiveType = getElementType(BaseExpr);
+ BaseExpr = BaseExpr->IgnoreParenCasts();
+ IndexExpr = IndexExpr->IgnoreParenCasts();
+
const ConstantArrayType *ArrayTy =
- S.Context.getAsConstantArrayType(BaseExpr->getType());
+ Context.getAsConstantArrayType(BaseExpr->getType());
if (!ArrayTy)
return;
- const Expr *IndexExpr = E->getIdx();
if (IndexExpr->isValueDependent())
return;
llvm::APSInt index;
- if (!IndexExpr->isIntegerConstantExpr(index, S.Context))
+ if (!IndexExpr->isIntegerConstantExpr(index, Context))
return;
if (index.isUnsigned() || !index.isNegative()) {
llvm::APInt size = ArrayTy->getSize();
if (!size.isStrictlyPositive())
return;
+
+ const Type* BaseType = getElementType(BaseExpr);
+ 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;
+ // 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);
+ }
+ }
+
if (size.getBitWidth() > index.getBitWidth())
index = index.sext(size.getBitWidth());
else if (size.getBitWidth() < index.getBitWidth())
size = size.sext(index.getBitWidth());
- if (index.slt(size))
+ // For array subscripting the index must be less than size, but for pointer
+ // 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 (AllowOnePastEnd ? index.sle(size) : index.slt(size))
return;
- S.DiagRuntimeBehavior(E->getBase()->getLocStart(), BaseExpr,
- S.PDiag(diag::warn_array_index_exceeds_bounds)
- << index.toString(10, true)
- << size.toString(10, true)
- << IndexExpr->getSourceRange());
+ unsigned DiagID = diag::warn_ptr_arith_exceeds_bounds;
+ if (isSubscript)
+ DiagID = diag::warn_array_index_exceeds_bounds;
+
+ DiagRuntimeBehavior(BaseExpr->getLocStart(), BaseExpr,
+ PDiag(DiagID) << index.toString(10, true)
+ << size.toString(10, true)
+ << (unsigned)size.getLimitedValue(~0U)
+ << IndexExpr->getSourceRange());
} else {
- S.DiagRuntimeBehavior(E->getBase()->getLocStart(), BaseExpr,
- S.PDiag(diag::warn_array_index_precedes_bounds)
- << index.toString(10, true)
- << IndexExpr->getSourceRange());
+ unsigned DiagID = diag::warn_array_index_precedes_bounds;
+ if (!isSubscript) {
+ DiagID = diag::warn_ptr_arith_precedes_bounds;
+ if (index.isNegative()) index = -index;
+ }
+
+ DiagRuntimeBehavior(BaseExpr->getLocStart(), BaseExpr,
+ PDiag(DiagID) << index.toString(10, true)
+ << IndexExpr->getSourceRange());
}
const NamedDecl *ND = NULL;
@@ -3512,18 +3554,38 @@ static void CheckArrayAccess_Check(Sema &S,
if (const MemberExpr *ME = dyn_cast<MemberExpr>(BaseExpr))
ND = dyn_cast<NamedDecl>(ME->getMemberDecl());
if (ND)
- S.DiagRuntimeBehavior(ND->getLocStart(), BaseExpr,
- S.PDiag(diag::note_array_index_out_of_bounds)
- << ND->getDeclName());
+ DiagRuntimeBehavior(ND->getLocStart(), BaseExpr,
+ PDiag(diag::note_array_index_out_of_bounds)
+ << ND->getDeclName());
}
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:
- CheckArrayAccess_Check(*this, cast<ArraySubscriptExpr>(expr));
+ case Stmt::ArraySubscriptExprClass: {
+ const ArraySubscriptExpr *ASE = cast<ArraySubscriptExpr>(expr);
+ CheckArrayAccess(ASE->getBase(), ASE->getIdx(), true,
+ AllowOnePastEnd > 0);
return;
+ }
+ case Stmt::UnaryOperatorClass: {
+ // Only unwrap the * and & unary operators
+ const UnaryOperator *UO = cast<UnaryOperator>(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<ConditionalOperator>(expr);
if (const Expr *lhs = cond->getLHS())
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index bfaf546..b899ea0 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -311,6 +311,7 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) {
// A glvalue of a non-function, non-array type T can be
// converted to a prvalue.
if (!E->isGLValue()) return Owned(E);
+
QualType T = E->getType();
assert(!T.isNull() && "r-value conversion on typeless expression?");
@@ -353,8 +354,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));
@@ -3339,6 +3338,7 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc,
Arg = ArgExpr.takeAs<Expr>();
}
+ CheckArrayAccess(Arg);
AllArgs.push_back(Arg);
}
@@ -5845,6 +5845,9 @@ QualType Sema::CheckAdditionOperands( // C99 6.5.6
return QualType();
}
+ // Check array bounds for pointer arithemtic
+ CheckArrayAccess(PExp, IExp);
+
if (CompLHSTy) {
QualType LHSTy = Context.isPromotableBitField(lex.get());
if (LHSTy.isNull()) {
@@ -5899,6 +5902,12 @@ QualType Sema::CheckSubtractionOperands(ExprResult &lex, ExprResult &rex,
if (!checkArithmeticOpPointerOperand(*this, Loc, lex.get()))
return QualType();
+ 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();
return lex.get()->getType();
}
@@ -6865,9 +6874,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.
@@ -7600,6 +7607,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));
@@ -7948,6 +7959,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 0061db9..b09ed7e 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -2264,9 +2264,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-ptr-arith.cpp b/test/SemaCXX/array-bounds-ptr-arith.cpp
new file mode 100644
index 0000000..ce1ace6
--- /dev/null
+++ b/test/SemaCXX/array-bounds-ptr-arith.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -verify -Warray-bounds-pointer-arithmetic %s
+
+void swallow (const char *x) { (void)x; }
+void test_pointer_arithmetic(int n) {
+ const char hello[] = "Hello world!"; // expected-note 2 {{declared here}}
+ const char *helloptr = hello;
+
+ swallow("Hello world!" + 6); // no-warning
+ swallow("Hello world!" - 6); // expected-warning {{refers before the beginning of the array}}
+ swallow("Hello world!" + 14); // expected-warning {{refers past the end of the array}}
+ swallow("Hello world!" + 13); // no-warning
+
+ swallow(hello + 6); // no-warning
+ swallow(hello - 6); // expected-warning {{refers before the beginning of the array}}
+ swallow(hello + 14); // expected-warning {{refers past the end of the array}}
+ swallow(hello + 13); // no-warning
+
+ swallow(helloptr + 6); // no-warning
+ swallow(helloptr - 6); // no-warning
+ swallow(helloptr + 14); // no-warning
+ swallow(helloptr + 13); // no-warning
+
+ double numbers[2]; // expected-note {{declared here}}
+ swallow((char*)numbers + sizeof(double)); // no-warning
+ swallow((char*)numbers + 60); // expected-warning {{refers past the end of the array}}
+
+ char buffer[5]; // expected-note 2 {{declared here}}
+ // TODO: Add FixIt notes for adding parens around non-ptr part of arith expr
+ swallow(buffer + sizeof("Hello")-1); // expected-warning {{refers past the end of the array}}
+ swallow(buffer + (sizeof("Hello")-1)); // no-warning
+ if (n > 0 && n <= 6) swallow(buffer + 6 - n); // expected-warning {{refers past the end of the array}}
+ if (n > 0 && n <= 6) swallow(buffer + (6 - n)); // no-warning
+}
diff --git a/test/SemaCXX/array-bounds.cpp b/test/SemaCXX/array-bounds.cpp
index 3bd6c35..ac52ecd 100644
--- a/test/SemaCXX/array-bounds.cpp
+++ b/test/SemaCXX/array-bounds.cpp
@@ -25,7 +25,7 @@ void f1(int a[1]) {
}
void f2(const int (&a)[1]) { // expected-note {{declared here}}
- int val = a[3]; // expected-warning {{array index of '3' indexes past the end of an array (that contains 1 elements)}}
+ int val = a[3]; // expected-warning {{array index of '3' indexes past the end of an array (that contains 1 element)}}
}
void test() {
@@ -35,15 +35,20 @@ void test() {
s2.a[3] = 0; // no warning for 0-sized array
union {
- short a[2]; // expected-note {{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
const int const_subscript = 3;
int array[1]; // expected-note {{declared here}}
- array[const_subscript] = 0; // expected-warning {{array index of '3' indexes past the end of an array (that contains 1 elements)}}
+ array[const_subscript] = 0; // expected-warning {{array index of '3' indexes past the end of an array (that contains 1 element)}}
int *ptr;
ptr[3] = 0; // no warning for pointer references
@@ -59,7 +64,7 @@ void test() {
char c2 = str2[5]; // expected-warning {{array index of '5' indexes past the end of an array (that contains 4 elements)}}
int (*array_ptr)[1];
- (*array_ptr)[3] = 1; // expected-warning {{array index of '3' indexes past the end of an array (that contains 1 elements)}}
+ (*array_ptr)[3] = 1; // expected-warning {{array index of '3' indexes past the end of an array (that contains 1 element)}}
}
template <int I> struct S {
@@ -151,8 +156,7 @@ void test_switch() {
enum enumA { enumA_A, enumA_B, enumA_C, enumA_D, enumA_E };
enum enumB { enumB_X, enumB_Y, enumB_Z };
static enum enumB myVal = enumB_X;
-void test_nested_switch()
-{
+void test_nested_switch() {
switch (enumA_E) { // expected-warning {{no case matching constant}}
switch (myVal) { // expected-warning {{enumeration values 'enumB_X' and 'enumB_Z' not handled in switch}}
case enumB_Y: ;
@@ -173,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)}}
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
