Skip to content

Conversation

steakhal
Copy link
Contributor

@steakhal steakhal commented Aug 2, 2024

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

(cherry picked from commit d96569e)

@steakhal steakhal added this to the LLVM 19.X Release milestone Aug 2, 2024
@steakhal steakhal requested a review from Xazax-hun August 2, 2024 15:10
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Aug 2, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 2, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Balazs Benics (steakhal)

Changes

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

(cherry picked from commit d96569e)


Full diff: https://github.com/llvm/llvm-project/pull/101684.diff

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+3)
  • (modified) clang/lib/StaticAnalyzer/Core/Store.cpp (+11-1)
  • (modified) clang/test/Analysis/exercise-ps.c (+18-2)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c42cb9932f3f7..5cd398c22c946 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1372,6 +1372,9 @@ Crash and bug fixes
 - Fixed a crash when storing through an address that refers to the address of
   a label. (#GH89185)
 
+- Fixed a crash when using ``__builtin_bitcast(type, array)`` as an array
+  subscript. (#GH94496)
+
 - Z3 crosschecking (aka. Z3 refutation) is now bounded, and can't consume
   more total time than the eymbolic execution itself. (#GH97298)
 
diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp
index 67ca61bb56ba2..b436dd746d21f 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -472,7 +472,17 @@ 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..50643d5b04687 100644
--- a/clang/test/Analysis/exercise-ps.c
+++ b/clang/test/Analysis/exercise-ps.c
@@ -1,10 +1,13 @@
-// RUN: %clang_analyze_cc1 %s -verify -Wno-error=implicit-function-declaration \
-// RUN:   -analyzer-checker=core,unix.Malloc \
+// RUN: %clang_analyze_cc1 %s -triple=x86_64-unknown-linux \
+// RUN:   -verify -Wno-error=implicit-function-declaration \
+// RUN:   -analyzer-checker=core,unix.Malloc,debug.ExprInspection \
 // RUN:   -analyzer-config core.CallAndMessage:ArgPointeeInitializedness=true
 //
 // Just exercise the analyzer on code that has at one point caused issues
 // (i.e., no assertions or crashes).
 
+void clang_analyzer_dump_int(int);
+
 static void f1(const char *x, char *y) {
   while (*x != 0) {
     *y++ = *x++;
@@ -30,3 +33,16 @@ 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};
+
+  _Static_assert(sizeof(int) == 4, "Wrong triple for the test");
+
+  clang_analyzer_dump_int(__builtin_bit_cast(int, b)); // expected-warning {{lazyCompoundVal}}
+  clang_analyzer_dump_int(array[__builtin_bit_cast(int, b)]); // expected-warning {{Unknown}}
+
+  array[__builtin_bit_cast(int, b)] = 0x10; // no crash
+}

@steakhal
Copy link
Contributor Author

steakhal commented Aug 2, 2024

Cherrypick + added release notes as this crash was present in clang-18 too.

Copy link
Collaborator

@Xazax-hun Xazax-hun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, it makes sense to me to backport this crash fix.

…script (#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 #94496

(cherry picked from commit d96569e)
@tru tru merged commit 56fa019 into llvm:release/19.x Aug 4, 2024
Copy link

github-actions bot commented Aug 4, 2024

@steakhal (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

@steakhal steakhal deleted the backport-csa-crash-fix branch August 5, 2024 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:static analyzer clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

5 participants