From dcae601f1c9f74dcef77ffbc5f07a6738e247276 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru Date: Tue, 30 Sep 2025 15:46:19 -0700 Subject: [PATCH 1/8] Fix false negatives when OOB occurs inside a conditional check --- .../Checkers/ArrayBoundChecker.cpp | 41 +++++-------------- 1 file changed, 10 insertions(+), 31 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index 6ad5acd4e76f2..fd22c9fac4c17 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -686,37 +686,16 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const { C.addTransition(ExceedsUpperBound, SUR.createNoteTag(C)); return; } - - BadOffsetKind Problem = AlsoMentionUnderflow - ? BadOffsetKind::Indeterminate - : BadOffsetKind::Overflowing; - Messages Msgs = - getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset, - *KnownSize, Location, Problem); - reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize); - return; - } - // ...and it can be valid as well... - if (isTainted(State, ByteOffset)) { - // ...but it's tainted, so report an error. - - // Diagnostic detail: saying "tainted offset" is always correct, but - // the common case is that 'idx' is tainted in 'arr[idx]' and then it's - // nicer to say "tainted index". - const char *OffsetName = "offset"; - if (const auto *ASE = dyn_cast(E)) - if (isTainted(State, ASE->getIdx(), C.getLocationContext())) - OffsetName = "index"; - - Messages Msgs = - getTaintMsgs(Space, Reg, OffsetName, AlsoMentionUnderflow); - reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize, - /*IsTaintBug=*/true); - return; } - // ...and it isn't tainted, so the checker will (optimistically) assume - // that the offset is in bounds and mention this in the note tag. - SUR.recordUpperBoundAssumption(*KnownSize); + + BadOffsetKind Problem = AlsoMentionUnderflow + ? BadOffsetKind::Indeterminate + : BadOffsetKind::Overflowing; + Messages Msgs = + getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset, + *KnownSize, Location, Problem); + reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize); + return; } // Actually update the state. The "if" only fails in the extremely unlikely @@ -758,7 +737,7 @@ void ArrayBoundChecker::reportOOB(CheckerContext &C, ProgramStateRef ErrorState, std::optional Extent, bool IsTaintBug /*=false*/) const { - ExplodedNode *ErrorNode = C.generateErrorNode(ErrorState); + ExplodedNode *ErrorNode = C.generateNonFatalErrorNode(ErrorState); if (!ErrorNode) return; From cc260f9f49e291801413a57051ea7a3903703fe8 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru Date: Wed, 1 Oct 2025 10:05:52 -0700 Subject: [PATCH 2/8] test case --- .../Checkers/ArrayBoundChecker.cpp | 11 ---- .../ArrayBound/cplusplus-tainted-index.cpp | 52 +++++++++++++++++++ 2 files changed, 52 insertions(+), 11 deletions(-) create mode 100644 clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index fd22c9fac4c17..05b55da4c0312 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -478,17 +478,6 @@ static Messages getNonTaintMsgs(const ASTContext &ACtx, std::string(Buf)}; } -static Messages getTaintMsgs(const MemSpaceRegion *Space, - const SubRegion *Region, const char *OffsetName, - bool AlsoMentionUnderflow) { - std::string RegName = getRegionName(Space, Region); - return {formatv("Potential out of bound access to {0} with tainted {1}", - RegName, OffsetName), - formatv("Access of {0} with a tainted {1} that may be {2}too large", - RegName, OffsetName, - AlsoMentionUnderflow ? "negative or " : "")}; -} - const NoteTag *StateUpdateReporter::createNoteTag(CheckerContext &C) const { // Don't create a note tag if we didn't assume anything: if (!AssumedNonNegative && !AssumedUpperBound) diff --git a/clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp b/clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp new file mode 100644 index 0000000000000..4df398eadf611 --- /dev/null +++ b/clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp @@ -0,0 +1,52 @@ +// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-checker=unix,core,security.ArrayBound -verify %s + +// Test the interactions of `security.ArrayBound` with C++ features. + +void test_tainted_index_local() { + int arr[10]; + unsigned index = 10; + arr[index] = 7; + // expected-warning@-1{{Out of bound access to memory after the end of 'arr'}} +} + +void test_tainted_index_local_range() { + int arr[10]; + for (unsigned index = 0; index < 11; index++) + arr[index] = index; + // expected-warning@-1{{Out of bound access to memory after the end of 'arr'}} +} + +void test_tainted_index1(unsigned index) { + int arr[10]; + if (index < 12) + arr[index] = index; + // expected-warning@-1{{Out of bound access to memory after the end of 'arr'}} + if (index == 12) + arr[index] = index; + // expected-warning@-1{{Out of bound access to memory after the end of 'arr'}} +} + +void test_tainted_index2(unsigned index) { + int arr[10]; + if (index < 12) + arr[index] = index; + // expected-warning@-1{{Out of bound access to memory after the end of 'arr'}} +} + +unsigned strlen(const char *s); +void strcpy(char *dst, char *src); +void test_ex(int argc, char *argv[]) { + char proc_name[16]; + unsigned offset = strlen(argv[0]); + if (offset == 16) { + strcpy(proc_name, argv[0]); + proc_name[offset] = 'x'; + // expected-warning@-1{{Out of bound access to memory after the end of 'proc_name'}} + } + if (offset <= 16) { + strcpy(proc_name, argv[0]); + proc_name[offset] = argv[0][16]; + // expected-warning@-1{{Out of bound access to memory after the end of the region}} + // expected-warning@-2{{Out of bound access to memory after the end of 'proc_name'}} + } +} From 5fe8a24e2f3fe48bf4496d1465fd3df44676ae64 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru Date: Thu, 2 Oct 2025 12:15:59 -0700 Subject: [PATCH 3/8] unroll loops option to catch OOB in test case --- clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp b/clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp index 4df398eadf611..0912f5057f7ed 100644 --- a/clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp +++ b/clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-checker=unix,core,security.ArrayBound -verify %s +// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-config unroll-loops=true -analyzer-checker=unix,core,security.ArrayBound -verify %s // Test the interactions of `security.ArrayBound` with C++ features. From 8163c8e753dddd5a89e9907aaf4d1f52c0a0dc9f Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru Date: Thu, 2 Oct 2025 13:49:12 -0700 Subject: [PATCH 4/8] Fix tests after removing taint assumptions --- .../Checkers/ArrayBoundChecker.cpp | 5 +- .../ArrayBound/assumption-reporting.c | 84 ++++++++-------- clang/test/Analysis/ArrayBound/assumptions.c | 96 +++++++++---------- clang/test/Analysis/ArrayBound/brief-tests.c | 17 ++-- clang/test/Analysis/ArrayBound/cplusplus.cpp | 8 +- .../test/Analysis/ArrayBound/verbose-tests.c | 64 ++++--------- 6 files changed, 127 insertions(+), 147 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index 05b55da4c0312..fcf8bb4423c10 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -680,9 +680,8 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const { BadOffsetKind Problem = AlsoMentionUnderflow ? BadOffsetKind::Indeterminate : BadOffsetKind::Overflowing; - Messages Msgs = - getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset, - *KnownSize, Location, Problem); + Messages Msgs = getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset, + *KnownSize, Location, Problem); reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize); return; } diff --git a/clang/test/Analysis/ArrayBound/assumption-reporting.c b/clang/test/Analysis/ArrayBound/assumption-reporting.c index bffd5d9bc35b5..22d0908f4d3a9 100644 --- a/clang/test/Analysis/ArrayBound/assumption-reporting.c +++ b/clang/test/Analysis/ArrayBound/assumption-reporting.c @@ -15,8 +15,8 @@ int TenElements[10]; int irrelevantAssumptions(int arg) { int a = TenElements[arg]; - // Here the analyzer assumes that `arg` is in bounds, but doesn't report this - // because `arg` is not interesting for the bug. + // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} int b = TenElements[13]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} // expected-note@-2 {{Access of 'TenElements' at index 13, while it holds only 10 'int' elements}} @@ -26,8 +26,10 @@ int irrelevantAssumptions(int arg) { int assumingBoth(int arg) { int a = TenElements[arg]; - // expected-note@-1 {{Assuming index is non-negative and less than 10, the number of 'int' elements in 'TenElements'}} - int b = TenElements[arg]; // no additional note, we already assumed that 'arg' is in bounds + // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} + int b = TenElements[arg]; // expected-warning{{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-1 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} int c = TenElements[arg + 10]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} @@ -39,9 +41,11 @@ int assumingBothPointerToMiddle(int arg) { // will speak about the "byte offset" measured from the beginning of the TenElements. int *p = TenElements + 2; int a = p[arg]; - // expected-note@-1 {{Assuming byte offset is non-negative and less than 40, the extent of 'TenElements'}} + // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} - int b = TenElements[arg]; // This is normal access, and only the lower bound is new. + int b = TenElements[arg]; // expected-warning{{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-1 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} int c = TenElements[arg + 10]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} @@ -62,15 +66,16 @@ int assumingLower(int arg) { } int assumingUpper(int arg) { - // expected-note@+2 {{Assuming 'arg' is >= 0}} - // expected-note@+1 {{Taking false branch}} + // expected-note@+2 2{{Assuming 'arg' is >= 0}} + // expected-note@+1 2{{Taking false branch}} if (arg < 0) return 0; int a = TenElements[arg]; - // expected-note@-1 {{Assuming index is less than 10, the number of 'int' elements in 'TenElements'}} + // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} int b = TenElements[arg - 10]; - // expected-warning@-1 {{Out of bound access to memory preceding 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative index}} + // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} return a + b; } @@ -82,12 +87,13 @@ int assumingUpperIrrelevant(int arg) { // filter out assumptions that are logically irrelevant but "touch" // interesting symbols; eventually it would be good to add support for this. - // expected-note@+2 {{Assuming 'arg' is >= 0}} - // expected-note@+1 {{Taking false branch}} + // expected-note@+2 2{{Assuming 'arg' is >= 0}} + // expected-note@+1 2{{Taking false branch}} if (arg < 0) return 0; int a = TenElements[arg]; - // expected-note@-1 {{Assuming index is less than 10, the number of 'int' elements in 'TenElements'}} + // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} int b = TenElements[arg + 10]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} @@ -96,10 +102,11 @@ int assumingUpperIrrelevant(int arg) { int assumingUpperUnsigned(unsigned arg) { int a = TenElements[arg]; - // expected-note@-1 {{Assuming index is less than 10, the number of 'int' elements in 'TenElements'}} + // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} int b = TenElements[(int)arg - 10]; - // expected-warning@-1 {{Out of bound access to memory preceding 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative index}} + // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} return a + b; } @@ -120,8 +127,10 @@ short assumingConvertedToCharP(int arg) { // result type of the subscript operator. char *cp = (char*)TenElements; char a = cp[arg]; - // expected-note@-1 {{Assuming index is non-negative and less than 40, the number of 'char' elements in 'TenElements'}} - char b = cp[arg]; // no additional note, we already assumed that 'arg' is in bounds + // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 40 'char' elements}} + char b = cp[arg]; // expected-warning{{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-1 {{Access of 'TenElements' at an overflowing index, while it holds only 40 'char' elements}} char c = cp[arg + 40]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 40 'char' elements}} @@ -138,14 +147,14 @@ int assumingConvertedToIntP(struct foo f, int arg) { // When indices are reported, the note will use the element type that's the // result type of the subscript operator. int a = ((int*)(f.a))[arg]; - // expected-note@-1 {{Assuming index is non-negative and less than 2, the number of 'int' elements in 'f.a'}} - // However, if the extent of the memory region is not divisible by the - // element size, the checker measures the offset and extent in bytes. + // expected-warning@-1 {{Out of bound access to memory around 'f.a'}} + // expected-note@-2 {{Access of 'f.a' at a negative or overflowing index, while it holds only 2 'int' elements}} int b = ((int*)(f.b))[arg]; - // expected-note@-1 {{Assuming byte offset is less than 5, the extent of 'f.b'}} + // expected-warning@-1 {{Out of bound access to memory after the end of 'f.b'}} + // expected-note@-2 {{Access of 'f.b' at an overflowing byte offset, while it holds only 5 bytes}} int c = TenElements[arg-2]; - // expected-warning@-1 {{Out of bound access to memory preceding 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative index}} + // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} return a + b + c; } @@ -154,16 +163,14 @@ int assumingPlainOffset(struct foo f, int arg) { // shorter "offset" instead of "byte offset" when it's irrelevant that the // offset is measured in bytes. - // expected-note@+2 {{Assuming 'arg' is < 2}} - // expected-note@+1 {{Taking false branch}} + // expected-note@+2 2{{Assuming 'arg' is < 2}} + // expected-note@+1 2{{Taking false branch}} if (arg >= 2) return 0; int b = ((int*)(f.b))[arg]; - // expected-note@-1 {{Assuming byte offset is non-negative and less than 5, the extent of 'f.b'}} - // FIXME: this should be {{Assuming offset is non-negative}} - // but the current simplification algorithm doesn't realize that arg <= 1 - // implies that the byte offset arg*4 will be less than 5. + // expected-warning@-1 {{Out of bound access to memory around 'f.b'}} + // expected-note@-2 {{Access of 'f.b' at a negative or overflowing byte offset, while it holds only 5 bytes}} int c = TenElements[arg+10]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} @@ -181,13 +188,14 @@ int assumingExtent(int arg) { int *mem = (int*)malloc(arg); mem[12] = 123; - // expected-note@-1 {{Assuming index '12' is less than the number of 'int' elements in the heap area}} + // expected-warning@-1 {{Out of bound access to memory after the end of the heap area}} + // expected-note@-2 {{Access of 'int' element in the heap area at index 12}} free(mem); return TenElements[arg]; - // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} + // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} } int *extentInterestingness(int arg) { @@ -196,7 +204,8 @@ int *extentInterestingness(int arg) { int *mem = (int*)malloc(arg); TenElements[arg] = 123; - // expected-note@-1 {{Assuming index is non-negative and less than 10, the number of 'int' elements in 'TenElements'}} + // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} return &mem[12]; // expected-warning@-1 {{Out of bound access to memory after the end of the heap area}} @@ -208,8 +217,7 @@ int triggeredByAnyReport(int arg) { // not limited to ArrayBound reports but will appear on any bug report (that // marks the relevant symbol as interesting). TenElements[arg + 10] = 8; - // expected-note@-1 {{Assuming index is non-negative and less than 10, the number of 'int' elements in 'TenElements'}} + // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} return 1024 >> arg; - // expected-warning@-1 {{Right operand is negative in right shift}} - // expected-note@-2 {{The result of right shift is undefined because the right operand is negative}} } diff --git a/clang/test/Analysis/ArrayBound/assumptions.c b/clang/test/Analysis/ArrayBound/assumptions.c index f20159da02997..64e7008393eaa 100644 --- a/clang/test/Analysis/ArrayBound/assumptions.c +++ b/clang/test/Analysis/ArrayBound/assumptions.c @@ -12,63 +12,63 @@ void clang_analyzer_value(int); extern int FiveInts[5]; void int_plus_one(int len) { - (void)FiveInts[len + 1]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [-1, 3] }}} + (void)FiveInts[len + 1]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{32s:{ [4, 2147483647] }}} } void int_neutral(int len) { - (void)FiveInts[len]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} + (void)FiveInts[len]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{32s:{ [5, 2147483647] }}} } void int_minus_one(int len) { - (void)FiveInts[len - 1]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} + (void)FiveInts[len - 1]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{32s:{ [-2147483648, -2147483648], [6, 2147483647] }}} } void unsigned_plus_one(unsigned len) { - (void)FiveInts[len + 1]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 3] }}} + (void)FiveInts[len + 1]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{32u:{ [4, 4294967295] }}} } void unsigned_neutral(unsigned len) { - (void)FiveInts[len]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} + (void)FiveInts[len]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{32u:{ [5, 4294967295] }}} } void unsigned_minus_one(unsigned len) { - (void)FiveInts[len - 1]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} + (void)FiveInts[len - 1]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{32u:{ [0, 0], [6, 4294967295] }} } void ll_plus_one(long long len) { - (void)FiveInts[len + 1]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [-1, 3] }}} + (void)FiveInts[len + 1]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{64s:{ [4, 9223372036854775807] }}} } void ll_neutral(long long len) { - (void)FiveInts[len]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} + (void)FiveInts[len]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{64s:{ [5, 9223372036854775807] }}} } void ll_minus_one(long long len) { - (void)FiveInts[len - 1]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} + (void)FiveInts[len - 1]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{64s:{ [-9223372036854775808, -9223372036854775808], [6, 9223372036854775807] }}} } void ull_plus_one(unsigned long long len) { - (void)FiveInts[len + 1]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 3] }}} + (void)FiveInts[len + 1]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{64u:{ [4, 18446744073709551615] }}} } void ull_neutral(unsigned long long len) { - (void)FiveInts[len]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} + (void)FiveInts[len]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{64u:{ [5, 18446744073709551615] }}} } void ull_minus_one(unsigned long long len) { - (void)FiveInts[len - 1]; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} + (void)FiveInts[len - 1]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} + clang_analyzer_value(len); // expected-warning {{64u:{ [0, 0], [6, 18446744073709551615] }}} } // Also try the same with a dynamically allocated memory block, because in the @@ -80,84 +80,84 @@ void free(void *); void dyn_int_plus_one(int len) { char *p = malloc(5); - p[len + 1] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [-1, 3] }}} + p[len + 1] = 1; // expected-warning {{Out of bound access to memory around the heap area}} + clang_analyzer_value(len); // expected-warning {{32s:{ [4, 2147483647] }}} free(p); } void dyn_int_neutral(int len) { char *p = malloc(5); - p[len] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} + p[len] = 1; // expected-warning {{Out of bound access to memory around the heap area}} + clang_analyzer_value(len); // expected-warning {{32s:{ [5, 2147483647] }}} free(p); } void dyn_int_minus_one(int len) { char *p = malloc(5); - p[len - 1] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} + p[len - 1] = 1; // expected-warning {{Out of bound access to memory around the heap area}} + clang_analyzer_value(len); // expected-warning {{32s:{ [6, 2147483647] }}} free(p); } void dyn_unsigned_plus_one(unsigned len) { char *p = malloc(5); - p[len + 1] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 3] }}} + p[len + 1] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} + clang_analyzer_value(len); // expected-warning {{32u:{ [4, 4294967295] }}} free(p); } void dyn_unsigned_neutral(unsigned len) { char *p = malloc(5); - p[len] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} + p[len] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} + clang_analyzer_value(len); // expected-warning {{32u:{ [5, 4294967295] }}} free(p); } void dyn_unsigned_minus_one(unsigned len) { char *p = malloc(5); - p[len - 1] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} + p[len - 1] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} + clang_analyzer_value(len); // expected-warning {{32u:{ [6, 4294967295] }}} free(p); } void dyn_ll_plus_one(long long len) { char *p = malloc(5); - p[len + 1] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [-1, 3] }}} + p[len + 1] = 1; // expected-warning {{Out of bound access to memory around the heap area}} + clang_analyzer_value(len); // expected-warning {{64s:{ [4, 9223372036854775807] }}} free(p); } void dyn_ll_neutral(long long len) { char *p = malloc(5); - p[len] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} + p[len] = 1; // expected-warning {{Out of bound access to memory around the heap area}} + clang_analyzer_value(len); // expected-warning {{64s:{ [5, 9223372036854775807] }}} free(p); } void dyn_ll_minus_one(long long len) { char *p = malloc(5); - p[len - 1] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} + p[len - 1] = 1; // expected-warning {{Out of bound access to memory around the heap area}} + clang_analyzer_value(len); // expected-warning {{64s:{ [-9223372036854775808, -9223372036854775808], [6, 9223372036854775807] }}} free(p); } void dyn_ull_plus_one(unsigned long long len) { char *p = malloc(5); - p[len + 1] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 3] }}} + p[len + 1] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} + clang_analyzer_value(len); // expected-warning {{64u:{ [4, 18446744073709551615] }}} free(p); } void dyn_ull_neutral(unsigned long long len) { char *p = malloc(5); - p[len] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} + p[len] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} + clang_analyzer_value(len); // expected-warning {{64u:{ [5, 18446744073709551615] }}} free(p); } void dyn_ull_minus_one(unsigned long long len) { char *p = malloc(5); - p[len - 1] = 1; // no-warning - clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} + p[len - 1] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} + clang_analyzer_value(len); // expected-warning {{64u:{ [6, 9223372036854775808] }}} free(p); } diff --git a/clang/test/Analysis/ArrayBound/brief-tests.c b/clang/test/Analysis/ArrayBound/brief-tests.c index f4811efd8d8b6..abbc76d0e4184 100644 --- a/clang/test/Analysis/ArrayBound/brief-tests.c +++ b/clang/test/Analysis/ArrayBound/brief-tests.c @@ -22,7 +22,8 @@ void test1_ok(int x) { const char test1_strings_underrun(int x) { const char *mystr = "mary had a little lamb"; - return mystr[-1]; // expected-warning{{Out of bound access to memory}} + return mystr[-1]; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-warning@-1{{Out of bound access to memory preceding the string literal}} } const char test1_strings_overrun(int x) { @@ -127,7 +128,7 @@ void test2_multi(int x) { // - constant integer sizes for the array void test2_multi_b(int x) { int buf[100][100]; - buf[-1][0] = 1; // expected-warning{{Out of bound access to memory}} + buf[-1][0] = 1; // expected-warning 2{{Out of bound access to memory preceding 'buf'}} } void test2_multi_ok(int x) { @@ -248,8 +249,8 @@ void sizeof_vla(int a) { if (a == 5) { char x[a]; int y[sizeof(x)]; - y[4] = 4; // no-warning - y[5] = 5; // should be {{Out of bounds access}} + y[4] = 4; // expected-warning{{Out of bound access to memory after the end of 'y'}} // FIXME: FP + y[5] = 5; // expected-warning{{Out of bound access to memory after the end of 'y'}} } } @@ -258,8 +259,8 @@ void sizeof_vla_2(int a) { if (a == 5) { char x[a]; int y[sizeof(x) / sizeof(char)]; - y[4] = 4; // no-warning - y[5] = 5; // should be {{Out of bounds access}} + y[4] = 4; // expected-warning{{Out of bound access to memory after the end of 'y'}} // FIXME: FP + y[5] = 5; // expected-warning{{Out of bound access to memory after the end of 'y'}} } } @@ -268,7 +269,7 @@ void sizeof_vla_3(int a) { if (a == 5) { char x[a]; int y[sizeof(*&*&*&x)]; - y[4] = 4; // no-warning - y[5] = 5; // should be {{Out of bounds access}} + y[4] = 4; // expected-warning{{Out of bound access to memory after the end of 'y'}} // FIXME: FP + y[5] = 5; // expected-warning{{Out of bound access to memory after the end of 'y'}} } } diff --git a/clang/test/Analysis/ArrayBound/cplusplus.cpp b/clang/test/Analysis/ArrayBound/cplusplus.cpp index 680fbf4817c30..c93fa81f6da79 100644 --- a/clang/test/Analysis/ArrayBound/cplusplus.cpp +++ b/clang/test/Analysis/ArrayBound/cplusplus.cpp @@ -103,14 +103,14 @@ void test2_multi(int x) { // of a multi-dimensional array void test2_multi_b(int x) { auto buf = new int[100][100]; - buf[-1][0] = 1; // expected-warning{{Out of bound access to memory}} + buf[-1][0] = 1; // expected-warning 2{{Out of bound access to memory preceding the heap area}} } // Tests over-indexing // of a multi-dimensional array void test2_multi_c(int x) { auto buf = new int[100][100]; - buf[100][0] = 1; // expected-warning{{Out of bound access to memory}} + buf[100][0] = 1; // expected-warning 2{{Out of bound access to memory after the end of the heap area}} } // Tests over-indexing @@ -148,7 +148,7 @@ void test_non_array(int x) { //if the allocated area size is a runtime parameter void test_dynamic_size(int s) { int *buf = new int[s]; - buf[0] = 1; // no-warning + buf[0] = 1; // expected-warning{{Out of bound access to memory after the end of the heap area}} } //Tests complex arithmetic //in new expression @@ -174,7 +174,7 @@ int test_reference_that_might_be_after_the_end(int idx) { // only introduced _after_ the creation of the reference ref. if (idx < 0 || idx > 10) return -2; - int &ref = array[idx]; + int &ref = array[idx]; // expected-warning{{Out of bound access to memory after the end of 'array'}} if (idx == 10) return -1; return ref; diff --git a/clang/test/Analysis/ArrayBound/verbose-tests.c b/clang/test/Analysis/ArrayBound/verbose-tests.c index e3416886d13e5..e2dd6ca807d28 100644 --- a/clang/test/Analysis/ArrayBound/verbose-tests.c +++ b/clang/test/Analysis/ArrayBound/verbose-tests.c @@ -78,18 +78,14 @@ int scanf(const char *fmt, ...); void taintedIndex(void) { int index; scanf("%d", &index); - // expected-note@-1 {{Taint originated here}} - // expected-note@-2 {{Taint propagated to the 2nd argument}} TenElements[index] = 5; - // expected-warning@-1 {{Potential out of bound access to 'TenElements' with tainted index}} - // expected-note@-2 {{Access of 'TenElements' with a tainted index that may be negative or too large}} + // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} } void taintedIndexNonneg(void) { int index; scanf("%d", &index); - // expected-note@-1 {{Taint originated here}} - // expected-note@-2 {{Taint propagated to the 2nd argument}} // expected-note@+2 {{Assuming 'index' is >= 0}} // expected-note@+1 {{Taking false branch}} @@ -97,19 +93,17 @@ void taintedIndexNonneg(void) { return; TenElements[index] = 5; - // expected-warning@-1 {{Potential out of bound access to 'TenElements' with tainted index}} - // expected-note@-2 {{Access of 'TenElements' with a tainted index that may be too large}} + // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} } void taintedIndexUnsigned(void) { unsigned index; scanf("%u", &index); - // expected-note@-1 {{Taint originated here}} - // expected-note@-2 {{Taint propagated to the 2nd argument}} TenElements[index] = 5; - // expected-warning@-1 {{Potential out of bound access to 'TenElements' with tainted index}} - // expected-note@-2 {{Access of 'TenElements' with a tainted index that may be too large}} + // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} } int *taintedIndexAfterTheEndPtr(void) { @@ -121,8 +115,6 @@ int *taintedIndexAfterTheEndPtr(void) { // &TenElements[index]. int index; scanf("%d", &index); - // expected-note@-1 {{Taint originated here}} - // expected-note@-2 {{Taint propagated to the 2nd argument}} if (index < 0 || index > 10) return TenElements; // expected-note@-2 {{Assuming 'index' is >= 0}} @@ -130,19 +122,17 @@ int *taintedIndexAfterTheEndPtr(void) { // expected-note@-4 {{Assuming 'index' is <= 10}} // expected-note@-5 {{Taking false branch}} return &TenElements[index]; - // expected-warning@-1 {{Potential out of bound access to 'TenElements' with tainted index}} - // expected-note@-2 {{Access of 'TenElements' with a tainted index that may be too large}} + // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} } void taintedOffset(void) { int index; scanf("%d", &index); - // expected-note@-1 {{Taint originated here}} - // expected-note@-2 {{Taint propagated to the 2nd argument}} int *p = TenElements + index; p[0] = 5; - // expected-warning@-1 {{Potential out of bound access to 'TenElements' with tainted offset}} - // expected-note@-2 {{Access of 'TenElements' with a tainted offset that may be negative or too large}} + // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} } void arrayOverflow(void) { @@ -372,6 +362,8 @@ int allocaRegion(void) { // expected-warning@-1 {{Out of bound access to memory after the end of the memory returned by 'alloca'}} // expected-note@-2 {{Access of the memory returned by 'alloca' at index 3, while it holds only 2 'int' elements}} return *mem; + // expected-warning@-1 {{Undefined or garbage value returned to caller}} + // expected-note@-2 {{Undefined or garbage value returned to caller}} } int *symbolicExtent(int arg) { @@ -381,30 +373,12 @@ int *symbolicExtent(int arg) { return 0; int *mem = (int*)malloc(arg); - // TODO: without the following reference to 'arg', the analyzer would discard - // the range information about (the symbolic value of) 'arg'. This is - // incorrect because while the variable itself is inaccessible, it becomes - // the symbolic extent of 'mem', so we still want to reason about its - // potential values. - (void)arg; - mem[8] = -2; // expected-warning@-1 {{Out of bound access to memory after the end of the heap area}} // expected-note@-2 {{Access of 'int' element in the heap area at index 8}} return mem; } -int *symbolicExtentDiscardedRangeInfo(int arg) { - // This is a copy of the case 'symbolicExtent' without the '(void)arg' hack. - // TODO: if the analyzer can detect the out-of-bounds access within this - // testcase, then remove this and the `(void)arg` hack from `symbolicExtent`. - if (arg >= 5) - return 0; - int *mem = (int*)malloc(arg); - mem[8] = -2; - return mem; -} - void symbolicIndex(int arg) { // expected-note@+2 {{Assuming 'arg' is >= 12}} // expected-note@+1 {{Taking true branch}} @@ -415,20 +389,18 @@ void symbolicIndex(int arg) { } int *nothingIsCertain(int x, int y) { + // expected-note@+2{{Assuming 'x' is < 2}} + // expected-note@+1{{Taking false branch}} if (x >= 2) return 0; int *mem = (int*)malloc(x); + // expected-note@+2 {{Taking true branch}} + // expected-note@+1 {{Assuming 'y' is >= 8}} if (y >= 8) mem[y] = -2; - // FIXME: this should produce - // {{Out of bound access to memory after the end of the heap area}} - // {{Access of 'int' element in the heap area at an overflowing index}} - // but apparently the analyzer isn't smart enough to deduce this. - - // Keep constraints alive. (Without this, the overeager garbage collection of - // constraints would _also_ prevent the intended behavior in this testcase.) - (void)x; + // expected-warning@-1 {{Out of bound access to memory after the end of the heap area}} + // expected-note@-2 {{Access of the heap area at an overflowing index}} return mem; } From acf21ba9fac513c08298f4e4fab86b1d07eabf96 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru Date: Tue, 7 Oct 2025 12:50:34 -0700 Subject: [PATCH 5/8] Revert test case changes --- .../ArrayBound/assumption-reporting.c | 84 ++++++++-------- clang/test/Analysis/ArrayBound/assumptions.c | 96 +++++++++---------- clang/test/Analysis/ArrayBound/brief-tests.c | 17 ++-- clang/test/Analysis/ArrayBound/cplusplus.cpp | 8 +- .../test/Analysis/ArrayBound/verbose-tests.c | 64 +++++++++---- 5 files changed, 144 insertions(+), 125 deletions(-) diff --git a/clang/test/Analysis/ArrayBound/assumption-reporting.c b/clang/test/Analysis/ArrayBound/assumption-reporting.c index 22d0908f4d3a9..bffd5d9bc35b5 100644 --- a/clang/test/Analysis/ArrayBound/assumption-reporting.c +++ b/clang/test/Analysis/ArrayBound/assumption-reporting.c @@ -15,8 +15,8 @@ int TenElements[10]; int irrelevantAssumptions(int arg) { int a = TenElements[arg]; - // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} + // Here the analyzer assumes that `arg` is in bounds, but doesn't report this + // because `arg` is not interesting for the bug. int b = TenElements[13]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} // expected-note@-2 {{Access of 'TenElements' at index 13, while it holds only 10 'int' elements}} @@ -26,10 +26,8 @@ int irrelevantAssumptions(int arg) { int assumingBoth(int arg) { int a = TenElements[arg]; - // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} - int b = TenElements[arg]; // expected-warning{{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-1 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} + // expected-note@-1 {{Assuming index is non-negative and less than 10, the number of 'int' elements in 'TenElements'}} + int b = TenElements[arg]; // no additional note, we already assumed that 'arg' is in bounds int c = TenElements[arg + 10]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} @@ -41,11 +39,9 @@ int assumingBothPointerToMiddle(int arg) { // will speak about the "byte offset" measured from the beginning of the TenElements. int *p = TenElements + 2; int a = p[arg]; - // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} + // expected-note@-1 {{Assuming byte offset is non-negative and less than 40, the extent of 'TenElements'}} - int b = TenElements[arg]; // expected-warning{{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-1 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} + int b = TenElements[arg]; // This is normal access, and only the lower bound is new. int c = TenElements[arg + 10]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} @@ -66,16 +62,15 @@ int assumingLower(int arg) { } int assumingUpper(int arg) { - // expected-note@+2 2{{Assuming 'arg' is >= 0}} - // expected-note@+1 2{{Taking false branch}} + // expected-note@+2 {{Assuming 'arg' is >= 0}} + // expected-note@+1 {{Taking false branch}} if (arg < 0) return 0; int a = TenElements[arg]; - // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} + // expected-note@-1 {{Assuming index is less than 10, the number of 'int' elements in 'TenElements'}} int b = TenElements[arg - 10]; - // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} + // expected-warning@-1 {{Out of bound access to memory preceding 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative index}} return a + b; } @@ -87,13 +82,12 @@ int assumingUpperIrrelevant(int arg) { // filter out assumptions that are logically irrelevant but "touch" // interesting symbols; eventually it would be good to add support for this. - // expected-note@+2 2{{Assuming 'arg' is >= 0}} - // expected-note@+1 2{{Taking false branch}} + // expected-note@+2 {{Assuming 'arg' is >= 0}} + // expected-note@+1 {{Taking false branch}} if (arg < 0) return 0; int a = TenElements[arg]; - // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} + // expected-note@-1 {{Assuming index is less than 10, the number of 'int' elements in 'TenElements'}} int b = TenElements[arg + 10]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} @@ -102,11 +96,10 @@ int assumingUpperIrrelevant(int arg) { int assumingUpperUnsigned(unsigned arg) { int a = TenElements[arg]; - // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} + // expected-note@-1 {{Assuming index is less than 10, the number of 'int' elements in 'TenElements'}} int b = TenElements[(int)arg - 10]; - // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} + // expected-warning@-1 {{Out of bound access to memory preceding 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative index}} return a + b; } @@ -127,10 +120,8 @@ short assumingConvertedToCharP(int arg) { // result type of the subscript operator. char *cp = (char*)TenElements; char a = cp[arg]; - // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 40 'char' elements}} - char b = cp[arg]; // expected-warning{{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-1 {{Access of 'TenElements' at an overflowing index, while it holds only 40 'char' elements}} + // expected-note@-1 {{Assuming index is non-negative and less than 40, the number of 'char' elements in 'TenElements'}} + char b = cp[arg]; // no additional note, we already assumed that 'arg' is in bounds char c = cp[arg + 40]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 40 'char' elements}} @@ -147,14 +138,14 @@ int assumingConvertedToIntP(struct foo f, int arg) { // When indices are reported, the note will use the element type that's the // result type of the subscript operator. int a = ((int*)(f.a))[arg]; - // expected-warning@-1 {{Out of bound access to memory around 'f.a'}} - // expected-note@-2 {{Access of 'f.a' at a negative or overflowing index, while it holds only 2 'int' elements}} + // expected-note@-1 {{Assuming index is non-negative and less than 2, the number of 'int' elements in 'f.a'}} + // However, if the extent of the memory region is not divisible by the + // element size, the checker measures the offset and extent in bytes. int b = ((int*)(f.b))[arg]; - // expected-warning@-1 {{Out of bound access to memory after the end of 'f.b'}} - // expected-note@-2 {{Access of 'f.b' at an overflowing byte offset, while it holds only 5 bytes}} + // expected-note@-1 {{Assuming byte offset is less than 5, the extent of 'f.b'}} int c = TenElements[arg-2]; - // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} + // expected-warning@-1 {{Out of bound access to memory preceding 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at a negative index}} return a + b + c; } @@ -163,14 +154,16 @@ int assumingPlainOffset(struct foo f, int arg) { // shorter "offset" instead of "byte offset" when it's irrelevant that the // offset is measured in bytes. - // expected-note@+2 2{{Assuming 'arg' is < 2}} - // expected-note@+1 2{{Taking false branch}} + // expected-note@+2 {{Assuming 'arg' is < 2}} + // expected-note@+1 {{Taking false branch}} if (arg >= 2) return 0; int b = ((int*)(f.b))[arg]; - // expected-warning@-1 {{Out of bound access to memory around 'f.b'}} - // expected-note@-2 {{Access of 'f.b' at a negative or overflowing byte offset, while it holds only 5 bytes}} + // expected-note@-1 {{Assuming byte offset is non-negative and less than 5, the extent of 'f.b'}} + // FIXME: this should be {{Assuming offset is non-negative}} + // but the current simplification algorithm doesn't realize that arg <= 1 + // implies that the byte offset arg*4 will be less than 5. int c = TenElements[arg+10]; // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} @@ -188,14 +181,13 @@ int assumingExtent(int arg) { int *mem = (int*)malloc(arg); mem[12] = 123; - // expected-warning@-1 {{Out of bound access to memory after the end of the heap area}} - // expected-note@-2 {{Access of 'int' element in the heap area at index 12}} + // expected-note@-1 {{Assuming index '12' is less than the number of 'int' elements in the heap area}} free(mem); return TenElements[arg]; - // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} + // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} + // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} } int *extentInterestingness(int arg) { @@ -204,8 +196,7 @@ int *extentInterestingness(int arg) { int *mem = (int*)malloc(arg); TenElements[arg] = 123; - // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} + // expected-note@-1 {{Assuming index is non-negative and less than 10, the number of 'int' elements in 'TenElements'}} return &mem[12]; // expected-warning@-1 {{Out of bound access to memory after the end of the heap area}} @@ -217,7 +208,8 @@ int triggeredByAnyReport(int arg) { // not limited to ArrayBound reports but will appear on any bug report (that // marks the relevant symbol as interesting). TenElements[arg + 10] = 8; - // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} + // expected-note@-1 {{Assuming index is non-negative and less than 10, the number of 'int' elements in 'TenElements'}} return 1024 >> arg; + // expected-warning@-1 {{Right operand is negative in right shift}} + // expected-note@-2 {{The result of right shift is undefined because the right operand is negative}} } diff --git a/clang/test/Analysis/ArrayBound/assumptions.c b/clang/test/Analysis/ArrayBound/assumptions.c index 64e7008393eaa..f20159da02997 100644 --- a/clang/test/Analysis/ArrayBound/assumptions.c +++ b/clang/test/Analysis/ArrayBound/assumptions.c @@ -12,63 +12,63 @@ void clang_analyzer_value(int); extern int FiveInts[5]; void int_plus_one(int len) { - (void)FiveInts[len + 1]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{32s:{ [4, 2147483647] }}} + (void)FiveInts[len + 1]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [-1, 3] }}} } void int_neutral(int len) { - (void)FiveInts[len]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{32s:{ [5, 2147483647] }}} + (void)FiveInts[len]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} } void int_minus_one(int len) { - (void)FiveInts[len - 1]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{32s:{ [-2147483648, -2147483648], [6, 2147483647] }}} + (void)FiveInts[len - 1]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} } void unsigned_plus_one(unsigned len) { - (void)FiveInts[len + 1]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{32u:{ [4, 4294967295] }}} + (void)FiveInts[len + 1]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 3] }}} } void unsigned_neutral(unsigned len) { - (void)FiveInts[len]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{32u:{ [5, 4294967295] }}} + (void)FiveInts[len]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} } void unsigned_minus_one(unsigned len) { - (void)FiveInts[len - 1]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{32u:{ [0, 0], [6, 4294967295] }} + (void)FiveInts[len - 1]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} } void ll_plus_one(long long len) { - (void)FiveInts[len + 1]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{64s:{ [4, 9223372036854775807] }}} + (void)FiveInts[len + 1]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [-1, 3] }}} } void ll_neutral(long long len) { - (void)FiveInts[len]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{64s:{ [5, 9223372036854775807] }}} + (void)FiveInts[len]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} } void ll_minus_one(long long len) { - (void)FiveInts[len - 1]; // expected-warning {{Out of bound access to memory around 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{64s:{ [-9223372036854775808, -9223372036854775808], [6, 9223372036854775807] }}} + (void)FiveInts[len - 1]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} } void ull_plus_one(unsigned long long len) { - (void)FiveInts[len + 1]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{64u:{ [4, 18446744073709551615] }}} + (void)FiveInts[len + 1]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 3] }}} } void ull_neutral(unsigned long long len) { - (void)FiveInts[len]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{64u:{ [5, 18446744073709551615] }}} + (void)FiveInts[len]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} } void ull_minus_one(unsigned long long len) { - (void)FiveInts[len - 1]; // expected-warning {{Out of bound access to memory after the end of 'FiveInts'}} - clang_analyzer_value(len); // expected-warning {{64u:{ [0, 0], [6, 18446744073709551615] }}} + (void)FiveInts[len - 1]; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} } // Also try the same with a dynamically allocated memory block, because in the @@ -80,84 +80,84 @@ void free(void *); void dyn_int_plus_one(int len) { char *p = malloc(5); - p[len + 1] = 1; // expected-warning {{Out of bound access to memory around the heap area}} - clang_analyzer_value(len); // expected-warning {{32s:{ [4, 2147483647] }}} + p[len + 1] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [-1, 3] }}} free(p); } void dyn_int_neutral(int len) { char *p = malloc(5); - p[len] = 1; // expected-warning {{Out of bound access to memory around the heap area}} - clang_analyzer_value(len); // expected-warning {{32s:{ [5, 2147483647] }}} + p[len] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} free(p); } void dyn_int_minus_one(int len) { char *p = malloc(5); - p[len - 1] = 1; // expected-warning {{Out of bound access to memory around the heap area}} - clang_analyzer_value(len); // expected-warning {{32s:{ [6, 2147483647] }}} + p[len - 1] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} free(p); } void dyn_unsigned_plus_one(unsigned len) { char *p = malloc(5); - p[len + 1] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} - clang_analyzer_value(len); // expected-warning {{32u:{ [4, 4294967295] }}} + p[len + 1] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 3] }}} free(p); } void dyn_unsigned_neutral(unsigned len) { char *p = malloc(5); - p[len] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} - clang_analyzer_value(len); // expected-warning {{32u:{ [5, 4294967295] }}} + p[len] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} free(p); } void dyn_unsigned_minus_one(unsigned len) { char *p = malloc(5); - p[len - 1] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} - clang_analyzer_value(len); // expected-warning {{32u:{ [6, 4294967295] }}} + p[len - 1] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} free(p); } void dyn_ll_plus_one(long long len) { char *p = malloc(5); - p[len + 1] = 1; // expected-warning {{Out of bound access to memory around the heap area}} - clang_analyzer_value(len); // expected-warning {{64s:{ [4, 9223372036854775807] }}} + p[len + 1] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [-1, 3] }}} free(p); } void dyn_ll_neutral(long long len) { char *p = malloc(5); - p[len] = 1; // expected-warning {{Out of bound access to memory around the heap area}} - clang_analyzer_value(len); // expected-warning {{64s:{ [5, 9223372036854775807] }}} + p[len] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} free(p); } void dyn_ll_minus_one(long long len) { char *p = malloc(5); - p[len - 1] = 1; // expected-warning {{Out of bound access to memory around the heap area}} - clang_analyzer_value(len); // expected-warning {{64s:{ [-9223372036854775808, -9223372036854775808], [6, 9223372036854775807] }}} + p[len - 1] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} free(p); } void dyn_ull_plus_one(unsigned long long len) { char *p = malloc(5); - p[len + 1] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} - clang_analyzer_value(len); // expected-warning {{64u:{ [4, 18446744073709551615] }}} + p[len + 1] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 3] }}} free(p); } void dyn_ull_neutral(unsigned long long len) { char *p = malloc(5); - p[len] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} - clang_analyzer_value(len); // expected-warning {{64u:{ [5, 18446744073709551615] }}} + p[len] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [0, 4] }}} free(p); } void dyn_ull_minus_one(unsigned long long len) { char *p = malloc(5); - p[len - 1] = 1; // expected-warning {{Out of bound access to memory after the end of the heap area}} - clang_analyzer_value(len); // expected-warning {{64u:{ [6, 9223372036854775808] }}} + p[len - 1] = 1; // no-warning + clang_analyzer_value(len); // expected-warning {{{ [1, 5] }}} free(p); } diff --git a/clang/test/Analysis/ArrayBound/brief-tests.c b/clang/test/Analysis/ArrayBound/brief-tests.c index abbc76d0e4184..f4811efd8d8b6 100644 --- a/clang/test/Analysis/ArrayBound/brief-tests.c +++ b/clang/test/Analysis/ArrayBound/brief-tests.c @@ -22,8 +22,7 @@ void test1_ok(int x) { const char test1_strings_underrun(int x) { const char *mystr = "mary had a little lamb"; - return mystr[-1]; // expected-warning{{Undefined or garbage value returned to caller}} - // expected-warning@-1{{Out of bound access to memory preceding the string literal}} + return mystr[-1]; // expected-warning{{Out of bound access to memory}} } const char test1_strings_overrun(int x) { @@ -128,7 +127,7 @@ void test2_multi(int x) { // - constant integer sizes for the array void test2_multi_b(int x) { int buf[100][100]; - buf[-1][0] = 1; // expected-warning 2{{Out of bound access to memory preceding 'buf'}} + buf[-1][0] = 1; // expected-warning{{Out of bound access to memory}} } void test2_multi_ok(int x) { @@ -249,8 +248,8 @@ void sizeof_vla(int a) { if (a == 5) { char x[a]; int y[sizeof(x)]; - y[4] = 4; // expected-warning{{Out of bound access to memory after the end of 'y'}} // FIXME: FP - y[5] = 5; // expected-warning{{Out of bound access to memory after the end of 'y'}} + y[4] = 4; // no-warning + y[5] = 5; // should be {{Out of bounds access}} } } @@ -259,8 +258,8 @@ void sizeof_vla_2(int a) { if (a == 5) { char x[a]; int y[sizeof(x) / sizeof(char)]; - y[4] = 4; // expected-warning{{Out of bound access to memory after the end of 'y'}} // FIXME: FP - y[5] = 5; // expected-warning{{Out of bound access to memory after the end of 'y'}} + y[4] = 4; // no-warning + y[5] = 5; // should be {{Out of bounds access}} } } @@ -269,7 +268,7 @@ void sizeof_vla_3(int a) { if (a == 5) { char x[a]; int y[sizeof(*&*&*&x)]; - y[4] = 4; // expected-warning{{Out of bound access to memory after the end of 'y'}} // FIXME: FP - y[5] = 5; // expected-warning{{Out of bound access to memory after the end of 'y'}} + y[4] = 4; // no-warning + y[5] = 5; // should be {{Out of bounds access}} } } diff --git a/clang/test/Analysis/ArrayBound/cplusplus.cpp b/clang/test/Analysis/ArrayBound/cplusplus.cpp index c93fa81f6da79..680fbf4817c30 100644 --- a/clang/test/Analysis/ArrayBound/cplusplus.cpp +++ b/clang/test/Analysis/ArrayBound/cplusplus.cpp @@ -103,14 +103,14 @@ void test2_multi(int x) { // of a multi-dimensional array void test2_multi_b(int x) { auto buf = new int[100][100]; - buf[-1][0] = 1; // expected-warning 2{{Out of bound access to memory preceding the heap area}} + buf[-1][0] = 1; // expected-warning{{Out of bound access to memory}} } // Tests over-indexing // of a multi-dimensional array void test2_multi_c(int x) { auto buf = new int[100][100]; - buf[100][0] = 1; // expected-warning 2{{Out of bound access to memory after the end of the heap area}} + buf[100][0] = 1; // expected-warning{{Out of bound access to memory}} } // Tests over-indexing @@ -148,7 +148,7 @@ void test_non_array(int x) { //if the allocated area size is a runtime parameter void test_dynamic_size(int s) { int *buf = new int[s]; - buf[0] = 1; // expected-warning{{Out of bound access to memory after the end of the heap area}} + buf[0] = 1; // no-warning } //Tests complex arithmetic //in new expression @@ -174,7 +174,7 @@ int test_reference_that_might_be_after_the_end(int idx) { // only introduced _after_ the creation of the reference ref. if (idx < 0 || idx > 10) return -2; - int &ref = array[idx]; // expected-warning{{Out of bound access to memory after the end of 'array'}} + int &ref = array[idx]; if (idx == 10) return -1; return ref; diff --git a/clang/test/Analysis/ArrayBound/verbose-tests.c b/clang/test/Analysis/ArrayBound/verbose-tests.c index e2dd6ca807d28..e3416886d13e5 100644 --- a/clang/test/Analysis/ArrayBound/verbose-tests.c +++ b/clang/test/Analysis/ArrayBound/verbose-tests.c @@ -78,14 +78,18 @@ int scanf(const char *fmt, ...); void taintedIndex(void) { int index; scanf("%d", &index); + // expected-note@-1 {{Taint originated here}} + // expected-note@-2 {{Taint propagated to the 2nd argument}} TenElements[index] = 5; - // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} + // expected-warning@-1 {{Potential out of bound access to 'TenElements' with tainted index}} + // expected-note@-2 {{Access of 'TenElements' with a tainted index that may be negative or too large}} } void taintedIndexNonneg(void) { int index; scanf("%d", &index); + // expected-note@-1 {{Taint originated here}} + // expected-note@-2 {{Taint propagated to the 2nd argument}} // expected-note@+2 {{Assuming 'index' is >= 0}} // expected-note@+1 {{Taking false branch}} @@ -93,17 +97,19 @@ void taintedIndexNonneg(void) { return; TenElements[index] = 5; - // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} + // expected-warning@-1 {{Potential out of bound access to 'TenElements' with tainted index}} + // expected-note@-2 {{Access of 'TenElements' with a tainted index that may be too large}} } void taintedIndexUnsigned(void) { unsigned index; scanf("%u", &index); + // expected-note@-1 {{Taint originated here}} + // expected-note@-2 {{Taint propagated to the 2nd argument}} TenElements[index] = 5; - // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} + // expected-warning@-1 {{Potential out of bound access to 'TenElements' with tainted index}} + // expected-note@-2 {{Access of 'TenElements' with a tainted index that may be too large}} } int *taintedIndexAfterTheEndPtr(void) { @@ -115,6 +121,8 @@ int *taintedIndexAfterTheEndPtr(void) { // &TenElements[index]. int index; scanf("%d", &index); + // expected-note@-1 {{Taint originated here}} + // expected-note@-2 {{Taint propagated to the 2nd argument}} if (index < 0 || index > 10) return TenElements; // expected-note@-2 {{Assuming 'index' is >= 0}} @@ -122,17 +130,19 @@ int *taintedIndexAfterTheEndPtr(void) { // expected-note@-4 {{Assuming 'index' is <= 10}} // expected-note@-5 {{Taking false branch}} return &TenElements[index]; - // expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}} + // expected-warning@-1 {{Potential out of bound access to 'TenElements' with tainted index}} + // expected-note@-2 {{Access of 'TenElements' with a tainted index that may be too large}} } void taintedOffset(void) { int index; scanf("%d", &index); + // expected-note@-1 {{Taint originated here}} + // expected-note@-2 {{Taint propagated to the 2nd argument}} int *p = TenElements + index; p[0] = 5; - // expected-warning@-1 {{Out of bound access to memory around 'TenElements'}} - // expected-note@-2 {{Access of 'TenElements' at a negative or overflowing index, while it holds only 10 'int' elements}} + // expected-warning@-1 {{Potential out of bound access to 'TenElements' with tainted offset}} + // expected-note@-2 {{Access of 'TenElements' with a tainted offset that may be negative or too large}} } void arrayOverflow(void) { @@ -362,8 +372,6 @@ int allocaRegion(void) { // expected-warning@-1 {{Out of bound access to memory after the end of the memory returned by 'alloca'}} // expected-note@-2 {{Access of the memory returned by 'alloca' at index 3, while it holds only 2 'int' elements}} return *mem; - // expected-warning@-1 {{Undefined or garbage value returned to caller}} - // expected-note@-2 {{Undefined or garbage value returned to caller}} } int *symbolicExtent(int arg) { @@ -373,12 +381,30 @@ int *symbolicExtent(int arg) { return 0; int *mem = (int*)malloc(arg); + // TODO: without the following reference to 'arg', the analyzer would discard + // the range information about (the symbolic value of) 'arg'. This is + // incorrect because while the variable itself is inaccessible, it becomes + // the symbolic extent of 'mem', so we still want to reason about its + // potential values. + (void)arg; + mem[8] = -2; // expected-warning@-1 {{Out of bound access to memory after the end of the heap area}} // expected-note@-2 {{Access of 'int' element in the heap area at index 8}} return mem; } +int *symbolicExtentDiscardedRangeInfo(int arg) { + // This is a copy of the case 'symbolicExtent' without the '(void)arg' hack. + // TODO: if the analyzer can detect the out-of-bounds access within this + // testcase, then remove this and the `(void)arg` hack from `symbolicExtent`. + if (arg >= 5) + return 0; + int *mem = (int*)malloc(arg); + mem[8] = -2; + return mem; +} + void symbolicIndex(int arg) { // expected-note@+2 {{Assuming 'arg' is >= 12}} // expected-note@+1 {{Taking true branch}} @@ -389,18 +415,20 @@ void symbolicIndex(int arg) { } int *nothingIsCertain(int x, int y) { - // expected-note@+2{{Assuming 'x' is < 2}} - // expected-note@+1{{Taking false branch}} if (x >= 2) return 0; int *mem = (int*)malloc(x); - // expected-note@+2 {{Taking true branch}} - // expected-note@+1 {{Assuming 'y' is >= 8}} if (y >= 8) mem[y] = -2; - // expected-warning@-1 {{Out of bound access to memory after the end of the heap area}} - // expected-note@-2 {{Access of the heap area at an overflowing index}} + // FIXME: this should produce + // {{Out of bound access to memory after the end of the heap area}} + // {{Access of 'int' element in the heap area at an overflowing index}} + // but apparently the analyzer isn't smart enough to deduce this. + + // Keep constraints alive. (Without this, the overeager garbage collection of + // constraints would _also_ prevent the intended behavior in this testcase.) + (void)x; return mem; } From cc282001d14843cd024560b9793fde933f10d35b Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru Date: Tue, 7 Oct 2025 13:20:07 -0700 Subject: [PATCH 6/8] Introduce analyzer config option for aggressive reporting of tainted offsets --- .../clang/StaticAnalyzer/Checkers/Checkers.td | 8 +++ .../Checkers/ArrayBoundChecker.cpp | 67 ++++++++++++++++--- .../ArrayBound/cplusplus-tainted-index.cpp | 9 ++- clang/test/Analysis/analyzer-config.c | 1 + 4 files changed, 71 insertions(+), 14 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 4473c54d8d6e3..7b7dd8b846ee6 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -917,6 +917,14 @@ let ParentPackage = Security in { def ArrayBoundChecker : Checker<"ArrayBound">, HelpText<"Warn about out of bounds access to memory">, + CheckerOptions<[ + CmdLineOption + ]>, Documentation; def FloatLoopCounter diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index fcf8bb4423c10..7fe3d8758c99f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -177,6 +177,10 @@ class ArrayBoundChecker : public Checker, static bool isInAddressOf(const Stmt *S, ASTContext &AC); public: + // When this parameter is set to true, the checker treats all + // unknown values as tainted and reports wanings if such values + // are used as offsets to access array elements + bool IsAggressive = false; void checkPostStmt(const ArraySubscriptExpr *E, CheckerContext &C) const { performCheck(E, C); } @@ -478,6 +482,17 @@ static Messages getNonTaintMsgs(const ASTContext &ACtx, std::string(Buf)}; } +static Messages getTaintMsgs(const MemSpaceRegion *Space, + const SubRegion *Region, const char *OffsetName, + bool AlsoMentionUnderflow) { + std::string RegName = getRegionName(Space, Region); + return {formatv("Potential out of bound access to {0} with tainted {1}", + RegName, OffsetName), + formatv("Access of {0} with a tainted {1} that may be {2}too large", + RegName, OffsetName, + AlsoMentionUnderflow ? "negative or " : "")}; +} + const NoteTag *StateUpdateReporter::createNoteTag(CheckerContext &C) const { // Don't create a note tag if we didn't assume anything: if (!AssumedNonNegative && !AssumedUpperBound) @@ -675,15 +690,46 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const { C.addTransition(ExceedsUpperBound, SUR.createNoteTag(C)); return; } + + BadOffsetKind Problem = AlsoMentionUnderflow + ? BadOffsetKind::Indeterminate + : BadOffsetKind::Overflowing; + Messages Msgs = + getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset, + *KnownSize, Location, Problem); + reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize); + return; } + if (!IsAggressive) { + // ...and it can be valid as well... + if (isTainted(State, ByteOffset)) { + // ...but it's tainted, so report an error. + + // Diagnostic detail: saying "tainted offset" is always correct, but + // the common case is that 'idx' is tainted in 'arr[idx]' and then it's + // nicer to say "tainted index". + const char *OffsetName = "offset"; + if (const auto *ASE = dyn_cast(E)) + if (isTainted(State, ASE->getIdx(), C.getLocationContext())) + OffsetName = "index"; + + Messages Msgs = + getTaintMsgs(Space, Reg, OffsetName, AlsoMentionUnderflow); + reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize, + /*IsTaintBug=*/true); + return; + } - BadOffsetKind Problem = AlsoMentionUnderflow - ? BadOffsetKind::Indeterminate - : BadOffsetKind::Overflowing; - Messages Msgs = getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset, - *KnownSize, Location, Problem); - reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize); - return; + // ...and it isn't tainted, so the checker will (optimistically) assume + // that the offset is in bounds and mention this in the note tag. + SUR.recordUpperBoundAssumption(*KnownSize); + } else { + Messages Msgs = + getTaintMsgs(Space, Reg, "offset", AlsoMentionUnderflow); + reportOOB(C, ExceedsUpperBound, Msgs, ByteOffset, KnownSize, + /*IsTaintBug=*/true); + return; + } } // Actually update the state. The "if" only fails in the extremely unlikely @@ -725,7 +771,7 @@ void ArrayBoundChecker::reportOOB(CheckerContext &C, ProgramStateRef ErrorState, std::optional Extent, bool IsTaintBug /*=false*/) const { - ExplodedNode *ErrorNode = C.generateNonFatalErrorNode(ErrorState); + ExplodedNode *ErrorNode = C.generateErrorNode(ErrorState); if (!ErrorNode) return; @@ -804,7 +850,10 @@ bool ArrayBoundChecker::isIdiomaticPastTheEndPtr(const Expr *E, } void ento::registerArrayBoundChecker(CheckerManager &mgr) { - mgr.registerChecker(); + ArrayBoundChecker *checker = mgr.registerChecker(); + checker->IsAggressive = + mgr.getAnalyzerOptions().getCheckerBooleanOption( + checker, "AggressiveReport"); } bool ento::shouldRegisterArrayBoundChecker(const CheckerManager &mgr) { diff --git a/clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp b/clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp index 0912f5057f7ed..425ecf38a7ade 100644 --- a/clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp +++ b/clang/test/Analysis/ArrayBound/cplusplus-tainted-index.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-config unroll-loops=true -analyzer-checker=unix,core,security.ArrayBound -verify %s +// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-config unroll-loops=true -analyzer-config security.ArrayBound:AggressiveReport=true -analyzer-checker=unix,core,security.ArrayBound -verify %s // Test the interactions of `security.ArrayBound` with C++ features. @@ -20,7 +20,7 @@ void test_tainted_index1(unsigned index) { int arr[10]; if (index < 12) arr[index] = index; - // expected-warning@-1{{Out of bound access to memory after the end of 'arr'}} + // expected-warning@-1{{Potential out of bound access to 'arr' with tainted offset}} if (index == 12) arr[index] = index; // expected-warning@-1{{Out of bound access to memory after the end of 'arr'}} @@ -30,7 +30,7 @@ void test_tainted_index2(unsigned index) { int arr[10]; if (index < 12) arr[index] = index; - // expected-warning@-1{{Out of bound access to memory after the end of 'arr'}} + // expected-warning@-1{{Potential out of bound access to 'arr' with tainted offset}} } unsigned strlen(const char *s); @@ -46,7 +46,6 @@ void test_ex(int argc, char *argv[]) { if (offset <= 16) { strcpy(proc_name, argv[0]); proc_name[offset] = argv[0][16]; - // expected-warning@-1{{Out of bound access to memory after the end of the region}} - // expected-warning@-2{{Out of bound access to memory after the end of 'proc_name'}} + // expected-warning@-1{{Potential out of bound access to the region with tainted offset}} } } diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index 7936273415ad4..af88ed5eec16c 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -120,6 +120,7 @@ // CHECK-NEXT: region-store-small-array-limit = 5 // CHECK-NEXT: region-store-small-struct-limit = 2 // CHECK-NEXT: report-in-main-source-file = false +// CHECK-NEXT: security.ArrayBound:AggressiveReport = false // CHECK-NEXT: security.cert.env.InvalidPtr:InvalidatingGetEnv = false // CHECK-NEXT: serialize-stats = false // CHECK-NEXT: silence-checkers = "" From 6e261e1deb5c126065965c88302f994944137c04 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru Date: Tue, 7 Oct 2025 13:21:56 -0700 Subject: [PATCH 7/8] Remove extraneous whitespace --- clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index 7fe3d8758c99f..281884281edff 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -692,8 +692,8 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const { } BadOffsetKind Problem = AlsoMentionUnderflow - ? BadOffsetKind::Indeterminate - : BadOffsetKind::Overflowing; + ? BadOffsetKind::Indeterminate + : BadOffsetKind::Overflowing; Messages Msgs = getNonTaintMsgs(C.getASTContext(), Space, Reg, ByteOffset, *KnownSize, Location, Problem); From daee330a3301276926ef21850ed406165525be64 Mon Sep 17 00:00:00 2001 From: Rashmi Mudduluru Date: Tue, 7 Oct 2025 13:30:42 -0700 Subject: [PATCH 8/8] Fix code formatting --- clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp index 281884281edff..f0392e25e7695 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp @@ -706,8 +706,8 @@ void ArrayBoundChecker::performCheck(const Expr *E, CheckerContext &C) const { // ...but it's tainted, so report an error. // Diagnostic detail: saying "tainted offset" is always correct, but - // the common case is that 'idx' is tainted in 'arr[idx]' and then it's - // nicer to say "tainted index". + // the common case is that 'idx' is tainted in 'arr[idx]' and then + // it's nicer to say "tainted index". const char *OffsetName = "offset"; if (const auto *ASE = dyn_cast(E)) if (isTainted(State, ASE->getIdx(), C.getLocationContext())) @@ -851,9 +851,8 @@ bool ArrayBoundChecker::isIdiomaticPastTheEndPtr(const Expr *E, void ento::registerArrayBoundChecker(CheckerManager &mgr) { ArrayBoundChecker *checker = mgr.registerChecker(); - checker->IsAggressive = - mgr.getAnalyzerOptions().getCheckerBooleanOption( - checker, "AggressiveReport"); + checker->IsAggressive = mgr.getAnalyzerOptions().getCheckerBooleanOption( + checker, "AggressiveReport"); } bool ento::shouldRegisterArrayBoundChecker(const CheckerManager &mgr) {