[NFC][analyzer] Split [[assume]] tests to a separate file #130763
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Recently commit e5821bae80db3f3f0fe0d5f8ce62f79e548eed5 (which is a re-application of 2b9abf0) added some tests to out-of-bonunds-new.cpp, which use a very simple out of bounds report to reveal the internal state of the analyzer, but are otherwise completely unrelated to the checker security.ArrayBound. (Instead, they test handling of `__builtin_assume` and `[[assume()]]` annotations.) I'm reverting out-of-bounds-new.cpp to its previous state and moving the new tests to a separate test file.
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: Donát Nagy (NagyDonat) ChangesRecently commit e5821bae80db3f3f0fe0d5f8ce62f79e548eed5 (which is a re-application of 2b9abf0) added some tests to out-of-bonunds-new.cpp, which use a very simple out of bounds report to reveal the internal state of the analyzer, but are otherwise completely unrelated to the checker security.ArrayBound. (Instead, they test handling of I'm reverting out-of-bounds-new.cpp to its previous state and moving the new tests to a separate test file. Full diff: https://github.com/llvm/llvm-project/pull/130763.diff 2 Files Affected:
diff --git a/clang/test/Analysis/builtin_assume.cpp b/clang/test/Analysis/builtin_assume.cpp
new file mode 100644
index 0000000000000..7158306be2b82
--- /dev/null
+++ b/clang/test/Analysis/builtin_assume.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -verify %s \
+// RUN: -triple=x86_64-unknown-linux-gnu \
+// RUN: -analyzer-checker=core,security.ArrayBound,debug.ExprInspection
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_value(int);
+void clang_analyzer_dump(int);
+
+// From: https://github.com/llvm/llvm-project/issues/100762
+extern int arrOf10[10];
+void using_builtin(int x) {
+ __builtin_assume(x > 101); // CallExpr
+ arrOf10[x] = 404; // expected-warning {{Out of bound access to memory}}
+}
+
+void using_assume_attr(int ax) {
+ [[assume(ax > 100)]]; // NullStmt with an "assume" attribute.
+ arrOf10[ax] = 405; // expected-warning {{Out of bound access to memory}}
+}
+
+void using_many_assume_attr(int yx) {
+ [[assume(yx > 104), assume(yx > 200), assume(yx < 300)]]; // NullStmt with an attribute
+ arrOf10[yx] = 406; // expected-warning{{Out of bound access to memory}}
+}
+
+int using_assume_attr_has_no_sideeffects(int y) {
+ int orig_y = y;
+ clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+
+ // We should not apply sideeffects of the argument of [[assume(...)]].
+ // "y" should not get incremented;
+ [[assume(++y == 43)]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
+
+ clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_eval(y == orig_y); // expected-warning {{TRUE}} Good.
+
+ return y;
+}
+
+int using_builtin_assume_has_no_sideeffects(int y) {
+ int orig_y = y;
+ clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+
+ // We should not apply sideeffects of the argument of __builtin_assume(...)
+ // "u" should not get incremented;
+ __builtin_assume(++y == 43); // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
+
+ clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_eval(y == orig_y); // expected-warning {{TRUE}} Good.
+
+ return y;
+}
diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp
index 9ae371819714d..4e5442422bff4 100644
--- a/clang/test/Analysis/out-of-bounds-new.cpp
+++ b/clang/test/Analysis/out-of-bounds-new.cpp
@@ -1,10 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -verify %s \
-// RUN: -triple=x86_64-unknown-linux-gnu \
-// RUN: -analyzer-checker=unix,core,security.ArrayBound,debug.ExprInspection
-
-void clang_analyzer_eval(bool);
-void clang_analyzer_value(int);
-void clang_analyzer_dump(int);
+// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-checker=unix,core,security.ArrayBound -verify %s
// Tests doing an out-of-bounds access after the end of an array using:
// - constant integer index
@@ -186,59 +180,3 @@ int test_reference_that_might_be_after_the_end(int idx) {
return ref;
}
-// From: https://github.com/llvm/llvm-project/issues/100762
-extern int arrOf10[10];
-void using_builtin(int x) {
- __builtin_assume(x > 101); // CallExpr
- arrOf10[x] = 404; // expected-warning {{Out of bound access to memory}}
-}
-
-void using_assume_attr(int ax) {
- [[assume(ax > 100)]]; // NullStmt with an "assume" attribute.
- arrOf10[ax] = 405; // expected-warning {{Out of bound access to memory}}
-}
-
-void using_many_assume_attr(int yx) {
- [[assume(yx > 104), assume(yx > 200), assume(yx < 300)]]; // NullStmt with an attribute
- arrOf10[yx] = 406; // expected-warning{{Out of bound access to memory}}
-}
-
-int using_assume_attr_has_no_sideeffects(int y) {
- int orig_y = y;
- clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
-
- // We should not apply sideeffects of the argument of [[assume(...)]].
- // "y" should not get incremented;
- [[assume(++y == 43)]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
-
- clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_eval(y == orig_y); // expected-warning {{TRUE}} Good.
-
- return y;
-}
-
-int using_builtin_assume_has_no_sideeffects(int y) {
- int orig_y = y;
- clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
-
- // We should not apply sideeffects of the argument of __builtin_assume(...)
- // "u" should not get incremented;
- __builtin_assume(++y == 43); // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
-
- clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_eval(y == orig_y); // expected-warning {{TRUE}} Good.
-
- return y;
-}
|
Member
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Donát Nagy (NagyDonat) ChangesRecently commit e5821bae80db3f3f0fe0d5f8ce62f79e548eed5 (which is a re-application of 2b9abf0) added some tests to out-of-bonunds-new.cpp, which use a very simple out of bounds report to reveal the internal state of the analyzer, but are otherwise completely unrelated to the checker security.ArrayBound. (Instead, they test handling of I'm reverting out-of-bounds-new.cpp to its previous state and moving the new tests to a separate test file. Full diff: https://github.com/llvm/llvm-project/pull/130763.diff 2 Files Affected:
diff --git a/clang/test/Analysis/builtin_assume.cpp b/clang/test/Analysis/builtin_assume.cpp
new file mode 100644
index 0000000000000..7158306be2b82
--- /dev/null
+++ b/clang/test/Analysis/builtin_assume.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -verify %s \
+// RUN: -triple=x86_64-unknown-linux-gnu \
+// RUN: -analyzer-checker=core,security.ArrayBound,debug.ExprInspection
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_value(int);
+void clang_analyzer_dump(int);
+
+// From: https://github.com/llvm/llvm-project/issues/100762
+extern int arrOf10[10];
+void using_builtin(int x) {
+ __builtin_assume(x > 101); // CallExpr
+ arrOf10[x] = 404; // expected-warning {{Out of bound access to memory}}
+}
+
+void using_assume_attr(int ax) {
+ [[assume(ax > 100)]]; // NullStmt with an "assume" attribute.
+ arrOf10[ax] = 405; // expected-warning {{Out of bound access to memory}}
+}
+
+void using_many_assume_attr(int yx) {
+ [[assume(yx > 104), assume(yx > 200), assume(yx < 300)]]; // NullStmt with an attribute
+ arrOf10[yx] = 406; // expected-warning{{Out of bound access to memory}}
+}
+
+int using_assume_attr_has_no_sideeffects(int y) {
+ int orig_y = y;
+ clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+
+ // We should not apply sideeffects of the argument of [[assume(...)]].
+ // "y" should not get incremented;
+ [[assume(++y == 43)]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
+
+ clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_eval(y == orig_y); // expected-warning {{TRUE}} Good.
+
+ return y;
+}
+
+int using_builtin_assume_has_no_sideeffects(int y) {
+ int orig_y = y;
+ clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+
+ // We should not apply sideeffects of the argument of __builtin_assume(...)
+ // "u" should not get incremented;
+ __builtin_assume(++y == 43); // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
+
+ clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
+ clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
+ clang_analyzer_eval(y == orig_y); // expected-warning {{TRUE}} Good.
+
+ return y;
+}
diff --git a/clang/test/Analysis/out-of-bounds-new.cpp b/clang/test/Analysis/out-of-bounds-new.cpp
index 9ae371819714d..4e5442422bff4 100644
--- a/clang/test/Analysis/out-of-bounds-new.cpp
+++ b/clang/test/Analysis/out-of-bounds-new.cpp
@@ -1,10 +1,4 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -verify %s \
-// RUN: -triple=x86_64-unknown-linux-gnu \
-// RUN: -analyzer-checker=unix,core,security.ArrayBound,debug.ExprInspection
-
-void clang_analyzer_eval(bool);
-void clang_analyzer_value(int);
-void clang_analyzer_dump(int);
+// RUN: %clang_analyze_cc1 -std=c++11 -Wno-array-bounds -analyzer-checker=unix,core,security.ArrayBound -verify %s
// Tests doing an out-of-bounds access after the end of an array using:
// - constant integer index
@@ -186,59 +180,3 @@ int test_reference_that_might_be_after_the_end(int idx) {
return ref;
}
-// From: https://github.com/llvm/llvm-project/issues/100762
-extern int arrOf10[10];
-void using_builtin(int x) {
- __builtin_assume(x > 101); // CallExpr
- arrOf10[x] = 404; // expected-warning {{Out of bound access to memory}}
-}
-
-void using_assume_attr(int ax) {
- [[assume(ax > 100)]]; // NullStmt with an "assume" attribute.
- arrOf10[ax] = 405; // expected-warning {{Out of bound access to memory}}
-}
-
-void using_many_assume_attr(int yx) {
- [[assume(yx > 104), assume(yx > 200), assume(yx < 300)]]; // NullStmt with an attribute
- arrOf10[yx] = 406; // expected-warning{{Out of bound access to memory}}
-}
-
-int using_assume_attr_has_no_sideeffects(int y) {
- int orig_y = y;
- clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
-
- // We should not apply sideeffects of the argument of [[assume(...)]].
- // "y" should not get incremented;
- [[assume(++y == 43)]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
-
- clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_eval(y == orig_y); // expected-warning {{TRUE}} Good.
-
- return y;
-}
-
-int using_builtin_assume_has_no_sideeffects(int y) {
- int orig_y = y;
- clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
-
- // We should not apply sideeffects of the argument of __builtin_assume(...)
- // "u" should not get incremented;
- __builtin_assume(++y == 43); // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
-
- clang_analyzer_dump(y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_dump(orig_y); // expected-warning-re {{{{^}}reg_${{[0-9]+}}<int y> [debug.ExprInspection]{{$}}}}
- clang_analyzer_value(y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_value(orig_y); // expected-warning {{32s:{ [-2147483648, 2147483647] }}}
- clang_analyzer_eval(y == orig_y); // expected-warning {{TRUE}} Good.
-
- return y;
-}
|
steakhal
changed the title
Contributor
Author
|
No problem, in git we can eventually track down and resolve everything. However, let's try to pay attention to putting logically separate tests into separate files -- our tests are badly disorganized, but we can gradually improve the average quality if we spend some attention on this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recently commit 7e5821b (which is a re-application of 89da344) added some tests to
out-of-bounds-new.cpp, which use a very simple out of bounds report to reveal the internal state of the analyzer, but are otherwise completely unrelated to the checkersecurity.ArrayBound, which is tested inout-of-bounds-new.cpp.(Instead, they test handling of
__builtin_assumeand[[assume()]]annotations.)This commit reverts
out-of-bounds-new.cppto its previous state and moves the new tests to a separate test file.