-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[analyzer] Fix crash on using bitcast(<type>, <array>)
as array subscript
#101647
Conversation
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Pavel Skripkin (pskrgag) ChangesCurrent CSA logic does not expect Catch such cases and return Closes #94496 Full diff: https://github.com/llvm/llvm-project/pull/101647.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp
index 67ca61bb56ba2..72587ef31a17c 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -472,7 +472,19 @@ SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
const auto *ElemR = dyn_cast<ElementRegion>(BaseRegion);
// Convert the offset to the appropriate size and signedness.
- Offset = svalBuilder.convertToArrayIndex(Offset).castAs<NonLoc>();
+ auto Off = svalBuilder.convertToArrayIndex(Offset).getAs<NonLoc>();
+ if (!Off) {
+ // Handle cases when LazyCompoundVal is used for an array index.
+ // Such case is possible if code does:
+ //
+ // char b[4];
+ // a[__builtin_bitcast(int, b)];
+ //
+ // Return UnknownVal, since we cannot model it.
+ return UnknownVal();
+ }
+
+ Offset = Off.value();
if (!ElemR) {
// If the base region is not an ElementRegion, create one.
diff --git a/clang/test/Analysis/exercise-ps.c b/clang/test/Analysis/exercise-ps.c
index d1e1771afddb5..9bba16c282967 100644
--- a/clang/test/Analysis/exercise-ps.c
+++ b/clang/test/Analysis/exercise-ps.c
@@ -30,3 +30,10 @@ void f3(void *dest) {
void *src = __builtin_alloca(5);
memcpy(dest, src, 1); // expected-warning{{2nd function call argument is a pointer to uninitialized value}}
}
+
+// Reproduce crash from GH#94496. When array is used as subcript to another array, CSA cannot model it
+// and should just assume it's unknown and do not crash.
+void f4(char *array) {
+ char b[4] = {0};
+ array[__builtin_bit_cast(int, b)] = 0x10; // no crash
+}
|
@llvm/pr-subscribers-clang Author: Pavel Skripkin (pskrgag) ChangesCurrent CSA logic does not expect Catch such cases and return Closes #94496 Full diff: https://github.com/llvm/llvm-project/pull/101647.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp
index 67ca61bb56ba2..72587ef31a17c 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -472,7 +472,19 @@ SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
const auto *ElemR = dyn_cast<ElementRegion>(BaseRegion);
// Convert the offset to the appropriate size and signedness.
- Offset = svalBuilder.convertToArrayIndex(Offset).castAs<NonLoc>();
+ auto Off = svalBuilder.convertToArrayIndex(Offset).getAs<NonLoc>();
+ if (!Off) {
+ // Handle cases when LazyCompoundVal is used for an array index.
+ // Such case is possible if code does:
+ //
+ // char b[4];
+ // a[__builtin_bitcast(int, b)];
+ //
+ // Return UnknownVal, since we cannot model it.
+ return UnknownVal();
+ }
+
+ Offset = Off.value();
if (!ElemR) {
// If the base region is not an ElementRegion, create one.
diff --git a/clang/test/Analysis/exercise-ps.c b/clang/test/Analysis/exercise-ps.c
index d1e1771afddb5..9bba16c282967 100644
--- a/clang/test/Analysis/exercise-ps.c
+++ b/clang/test/Analysis/exercise-ps.c
@@ -30,3 +30,10 @@ void f3(void *dest) {
void *src = __builtin_alloca(5);
memcpy(dest, src, 1); // expected-warning{{2nd function call argument is a pointer to uninitialized value}}
}
+
+// Reproduce crash from GH#94496. When array is used as subcript to another array, CSA cannot model it
+// and should just assume it's unknown and do not crash.
+void f4(char *array) {
+ char b[4] = {0};
+ array[__builtin_bit_cast(int, b)] = 0x10; // no crash
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM already. You may decide to fix add the extra dump or not.
bitcast(<type>, <array>)
as array subscriptbitcast(<type>, <array>)
as array subscript
Backporting to clang-19 in #101684 |
…script (llvm#101647) Current CSA logic does not expect `LazyCompoundValKind` as array index. This may happen if array is used as subscript to another, in case of bitcast to integer type. Catch such cases and return `UnknownVal`, since CSA cannot model array -> int casts. Closes llvm#94496 (cherry picked from commit d96569e)
Thanks! Appreciate the fix. |
…script (llvm#101647) Current CSA logic does not expect `LazyCompoundValKind` as array index. This may happen if array is used as subscript to another, in case of bitcast to integer type. Catch such cases and return `UnknownVal`, since CSA cannot model array -> int casts. Closes llvm#94496
Current CSA logic does not expect
LazyCompoundValKind
as array index. This may happen if array is used as subscript to another, in case of bitcast to integer type.Catch such cases and return
UnknownVal
, since CSA cannot model array ->int casts.
Closes #94496