diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 5f36ffa926b26..434e1d8dca227 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -439,8 +439,16 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) { dyn_cast(Node.getBase()->IgnoreParenImpCasts()); uint64_t size; - if (!BaseDRE && !SLiteral) - return false; + if (!BaseDRE && !SLiteral) { + // Try harder to find something that looks like a DeclRefExpr + const auto *Member = dyn_cast(Node.getBase()->IgnoreParenImpCasts()); + if (!Member) return false; + + const auto *Value = Finder->getASTContext().getAsConstantArrayType(Member->getMemberDecl()->getType()); + if (!Value) return false; + + size = Value->getLimitedSize(); + } if (BaseDRE) { if (!BaseDRE->getDecl()) @@ -455,12 +463,9 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) { size = SLiteral->getLength() + 1; } - if (const auto *IdxLit = dyn_cast(Node.getIdx())) { - const APInt ArrIdx = IdxLit->getValue(); - // FIXME: ArrIdx.isNegative() we could immediately emit an error as that's a - // bug - if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size) - return true; + const auto IntConst = Node.getIdx()->getIntegerConstantExpr(Finder->getASTContext()); + if (IntConst && 0 <= *IntConst && *IntConst < size) { + return true; } return false; diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp index c6c93a27e4b96..0ae5b1b06b5c2 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp @@ -39,6 +39,28 @@ void constant_idx_unsafe(unsigned idx) { buffer[10] = 0; // expected-note{{used in buffer access here}} } +enum FooEnum { + A = 0, + B = 1, + C = 2, + D +}; + +void constant_enum_safe() { + int buffer[FooEnum::D] = { 0, 1, 2 }; // expected-warning{{'buffer' is an unsafe buffer that does not perform bounds checks}} + // expected-note@-1{{change type of 'buffer' to 'std::array' to label it for hardening}} + buffer[A] = 0; // no-warning + buffer[C] = 0; // no-warning + buffer[D] = 0; // expected-note{{used in buffer access here}} +} + +void constant_enum_unsafe(FooEnum e) { + int buffer[FooEnum::D] = { 0, 1, 2 }; // expected-warning{{'buffer' is an unsafe buffer that does not perform bounds checks}} + // expected-note@-1{{change type of 'buffer' to 'std::array' to label it for hardening}} + + buffer[e] = 0; // expected-note{{used in buffer access here}} +} + void constant_id_string(unsigned idx) { char safe_char = "abc"[1]; // no-warning safe_char = ""[0]; diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp index 0ba605475925b..1636c948da075 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp @@ -96,7 +96,6 @@ void test_attribute_multiple_fields (D d) { int v = d.buf[0]; //expected-warning{{field 'buf' prone to unsafe buffer manipulation}} - //expected-warning@+1{{unsafe buffer access}} v = d.buf[5]; //expected-warning{{field 'buf' prone to unsafe buffer manipulation}} } diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp index 642db0e9d3c63..41194a8e3f522 100644 --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp @@ -128,25 +128,25 @@ T_t funRetT(); T_t * funRetTStar(); void testStructMembers(struct T * sp, struct T s, T_t * sp2, T_t s2) { - foo(sp->a[1], // expected-warning{{unsafe buffer access}} + foo(sp->a[1], sp->b[1], // expected-warning{{unsafe buffer access}} - sp->c.a[1], // expected-warning{{unsafe buffer access}} + sp->c.a[1], sp->c.b[1], // expected-warning{{unsafe buffer access}} - s.a[1], // expected-warning{{unsafe buffer access}} + s.a[1], s.b[1], // expected-warning{{unsafe buffer access}} - s.c.a[1], // expected-warning{{unsafe buffer access}} + s.c.a[1], s.c.b[1], // expected-warning{{unsafe buffer access}} - sp2->a[1], // expected-warning{{unsafe buffer access}} + sp2->a[1], sp2->b[1], // expected-warning{{unsafe buffer access}} - sp2->c.a[1], // expected-warning{{unsafe buffer access}} + sp2->c.a[1], sp2->c.b[1], // expected-warning{{unsafe buffer access}} - s2.a[1], // expected-warning{{unsafe buffer access}} + s2.a[1], s2.b[1], // expected-warning{{unsafe buffer access}} - s2.c.a[1], // expected-warning{{unsafe buffer access}} + s2.c.a[1], s2.c.b[1], // expected-warning{{unsafe buffer access}} - funRetT().a[1], // expected-warning{{unsafe buffer access}} + funRetT().a[1], funRetT().b[1], // expected-warning{{unsafe buffer access}} - funRetTStar()->a[1], // expected-warning{{unsafe buffer access}} + funRetTStar()->a[1], funRetTStar()->b[1] // expected-warning{{unsafe buffer access}} ); } @@ -213,7 +213,6 @@ void testTypedefs(T_ptr_t p) { // expected-warning@-1{{'p' is an unsafe pointer used for buffer access}} foo(p[1], // expected-note{{used in buffer access here}} p[1].a[1], // expected-note{{used in buffer access here}} - // expected-warning@-1{{unsafe buffer access}} p[1].b[1] // expected-note{{used in buffer access here}} // expected-warning@-1{{unsafe buffer access}} );