Skip to content

Commit

Permalink
[clang][Sema] Use correct array size for diagnostic
Browse files Browse the repository at this point in the history
The diagnostic was confusing and reporting that an array contains far
more elements than it is defined to have, due to casting.

For example, this code:

  double foo[4096];
  ((char*)foo)[sizeof(foo)];

warns that the "index 32768 is past the end of the array (which contains
32768 elements)."

Reviewed By: serge-sans-paille, aaron.ballman

Differential Revision: https://reviews.llvm.org/D135920
  • Loading branch information
bwendling committed Oct 20, 2022
1 parent 9895447 commit b76219b
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 70 deletions.
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Expand Up @@ -321,6 +321,10 @@ Improvements to Clang's diagnostics
on the previous undocumented behavior. As a side effect of this change,
constructor calls outside of initializer expressions are no longer ignored,
which can result in new warnings (or make existing warnings disappear).
- The wording of diagnostics regarding arithmetic on fixed-sized arrays and
pointers is improved to include the type of the array and whether it's cast
to another type. This should improve comprehension for why an index is
out-of-bounds.

Non-comprehensive list of changes in this release
-------------------------------------------------
Expand Down
7 changes: 3 additions & 4 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Expand Up @@ -9401,15 +9401,14 @@ def warn_ptr_arith_precedes_bounds : Warning<
"the pointer decremented by %0 refers before the beginning of the array">,
InGroup<ArrayBoundsPointerArithmetic>, 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)">,
"the pointer incremented by %0 refers past the end of the array (that has type %1)">,
InGroup<ArrayBoundsPointerArithmetic>, DefaultIgnore;
def warn_array_index_precedes_bounds : Warning<
"array index %0 is before the beginning of the array">,
InGroup<ArrayBounds>;
def warn_array_index_exceeds_bounds : Warning<
"array index %0 is past the end of the array (which contains %1 "
"element%s2)">, InGroup<ArrayBounds>;
"array index %0 is past the end of the array (that has type %1%select{|, cast to %3}2)">,
InGroup<ArrayBounds>;
def warn_ptr_arith_exceeds_max_addressable_bounds : Warning<
"the pointer incremented by %0 refers past the last possible element for an array in %1-bit "
"address space containing %2-bit (%3-byte) elements (max possible %4 element%s5)">,
Expand Down
11 changes: 6 additions & 5 deletions clang/lib/Sema/SemaChecking.cpp
Expand Up @@ -16069,12 +16069,13 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,

unsigned DiagID = ASE ? diag::warn_array_index_exceeds_bounds
: diag::warn_ptr_arith_exceeds_bounds;
unsigned CastMsg = (!ASE || BaseType == EffectiveType) ? 0 : 1;
QualType CastMsgTy = ASE ? ASE->getLHS()->getType() : QualType();

DiagRuntimeBehavior(BaseExpr->getBeginLoc(), BaseExpr,
PDiag(DiagID) << toString(index, 10, true)
<< toString(size, 10, true)
<< (unsigned)size.getLimitedValue(~0U)
<< IndexExpr->getSourceRange());
DiagRuntimeBehavior(
BaseExpr->getBeginLoc(), BaseExpr,
PDiag(DiagID) << toString(index, 10, true) << ArrayTy->desugar()
<< CastMsg << CastMsgTy << IndexExpr->getSourceRange());
} else {
unsigned DiagID = diag::warn_array_index_precedes_bounds;
if (!ASE) {
Expand Down
4 changes: 2 additions & 2 deletions clang/test/Sema/array-bounds-ptr-arith.c
Expand Up @@ -11,7 +11,7 @@ void* ext2_statfs (struct ext2_super_block *es,int a) {
return (void *)es->s_uuid + sizeof(int); // no-warning
}
void* broken (struct ext2_super_block *es,int a) {
return (void *)es->s_uuid + 9; // expected-warning {{the pointer incremented by 9 refers past the end of the array}}
return (void *)es->s_uuid + 9; // expected-warning {{the pointer incremented by 9 refers past the end of the array (that has type 'unsigned char[8]')}}
}

// Test case reduced from PR11594
Expand Down Expand Up @@ -40,7 +40,7 @@ typedef struct RDar11387038_B RDar11387038_B;

void radar11387038(void) {
RDar11387038_B *pRDar11387038_B;
struct RDar11387038 *y = &(*pRDar11387038_B->x)->z[4]; // strict-warning {{array index 4 is past the end of the array (which contains 1 element)}}
struct RDar11387038 *y = &(*pRDar11387038_B->x)->z[4]; // strict-warning {{array index 4 is past the end of the array (that has type 'struct RDar11387038[1]')}}
}

void pr51682(void) {
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Sema/integer-overflow.c
Expand Up @@ -142,7 +142,7 @@ uint64_t check_integer_overflows(int i) {
(__imag__ x) = 4608 * 1024 * 1024;

// expected-warning@+4 {{overflow in expression; result is 536870912 with type 'int'}}
// expected-warning@+3 {{array index 536870912 is past the end of the array (which contains 10 elements)}}
// expected-warning@+3 {{array index 536870912 is past the end of the array (that has type 'uint64_t[10]' (aka 'unsigned long long[10]'))}}
// expected-note@+1 {{array 'a' declared here}}
uint64_t a[10];
a[4608 * 1024 * 1024] = 1i;
Expand Down
14 changes: 7 additions & 7 deletions clang/test/SemaCXX/array-bounds-ptr-arith.cpp
Expand Up @@ -6,13 +6,13 @@ void test_pointer_arithmetic(int n) {
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!" - 6); // expected-warning {{the pointer decremented by 6 refers before the beginning of the array}}
swallow("Hello world!" + 14); // expected-warning {{the pointer incremented by 14 refers past the end of the array (that has type 'const char[13]')}}
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 - 6); // expected-warning {{the pointer decremented by 6 refers before the beginning of the array}}
swallow(hello + 14); // expected-warning {{the pointer incremented by 14 refers past the end of the array (that has type 'const char[13]')}}
swallow(hello + 13); // no-warning

swallow(helloptr + 6); // no-warning
Expand All @@ -22,12 +22,12 @@ void test_pointer_arithmetic(int n) {

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}}
swallow((char*)numbers + 60); // expected-warning {{the pointer incremented by 60 refers past the end of the array (that has type 'double[2]')}}

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); // expected-warning {{the pointer incremented by 6 refers past the end of the array (that has type 'char[5]')}}
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); // expected-warning {{the pointer incremented by 6 refers past the end of the array (that has type 'char[5]')}}
if (n > 0 && n <= 6) swallow(buffer + (6 - n)); // no-warning
}
4 changes: 2 additions & 2 deletions clang/test/SemaCXX/array-bounds-strict-flex-arrays.cpp
Expand Up @@ -16,7 +16,7 @@ struct t0 {
int a[10]; // relaxed-note {{array 'a' declared here}}
};
void test0(t0 *s2) {
s2->a[12] = 0; // relaxed-warning {{array index 12 is past the end of the array (which contains 10 elements)}}
s2->a[12] = 0; // relaxed-warning {{array index 12 is past the end of the array (that has type 'int[10]')}}
}


Expand All @@ -26,7 +26,7 @@ struct t1 {
int a[1]; // strict-note {{array 'a' declared here}}
};
void test1(t1 *s2) {
s2->a[2] = 0; // strict-warning {{array index 2 is past the end of the array (which contains 1 element)}}
s2->a[2] = 0; // strict-warning {{array index 2 is past the end of the array (that has type 'int[1]')}}
}

// Under -fstrict-flex-arrays={1,2} `a` is a flexible array.
Expand Down

0 comments on commit b76219b

Please sign in to comment.