[analyzer] Support pack indexing expressions

[lbonn]

Analyzer used to crash when visiting the unsubstituted DeclRefExpr nodes here. Instead we skip them and process them in a dedicated pack indexing transfer function.

Fixes #154042

[llvmbot]

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

@llvm/pr-subscribers-clang

Author: None (lbonn)

Changes

Analyzer used to crash when visiting the unsubstituted DeclRefExpr nodes here. Instead we skip them and process them in a dedicated pack indexing transfer function.

Fixes #154042

---

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

3 Files Affected:

• (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (+4)
• (modified) clang/lib/StaticAnalyzer/Core/ExprEngine.cpp (+31-1)
• (added) clang/test/Analysis/pack_indexing.cpp (+25)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index d184986cda15d..2d96d668d9f7e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -506,6 +506,10 @@ class ExprEngine {
   void VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
                      ExplodedNodeSet &Dst);
 
+  /// VisitPackIndexingExpr - Transfer function logic for C++26 pack indexing
+  void VisitPackIndexingExpr(const PackIndexingExpr *E, ExplodedNode *Pred,
+                             ExplodedNodeSet &Dst);
+
   /// VisitGuardedExpr - Transfer function logic for ?, __builtin_choose
   void VisitGuardedExpr(const Expr *Ex, const Expr *L, const Expr *R,
                         ExplodedNode *Pred, ExplodedNodeSet &Dst);
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index c8dc5b6e81b16..48e026faaa49f 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1740,7 +1740,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
     case Stmt::RecoveryExprClass:
     case Stmt::CXXNoexceptExprClass:
     case Stmt::PackExpansionExprClass:
-    case Stmt::PackIndexingExprClass:
     case Stmt::SubstNonTypeTemplateParmPackExprClass:
     case Stmt::FunctionParmPackExprClass:
     case Stmt::CoroutineBodyStmtClass:
@@ -2292,6 +2291,13 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       Bldr.addNodes(Dst);
       break;
 
+    case Stmt::PackIndexingExprClass: {
+      Bldr.takeNodes(Pred);
+      VisitPackIndexingExpr(cast<PackIndexingExpr>(S), Pred, Dst);
+      Bldr.addNodes(Dst);
+      break;
+    }
+
     case Stmt::ImplicitCastExprClass:
     case Stmt::CStyleCastExprClass:
     case Stmt::CXXStaticCastExprClass:
@@ -3296,6 +3302,14 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
 
     SVal V = UnknownVal();
 
+    // For pack indexing expressions. Binding is not available here but through
+    // the expanded expressions in the PackIndexingExpr node.
+    if (BD->isParameterPack()) {
+      Bldr.generateNode(Ex, Pred, state->BindExpr(Ex, LCtx, V), nullptr,
+                        ProgramPoint::PostLValueKind);
+      return;
+    }
+
     // Handle binding to data members
     if (const auto *ME = dyn_cast<MemberExpr>(BD->getBinding())) {
       const auto *Field = cast<FieldDecl>(ME->getMemberDecl());
@@ -3447,6 +3461,22 @@ void ExprEngine::VisitArrayInitLoopExpr(const ArrayInitLoopExpr *Ex,
   getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, Ex, *this);
 }
 
+void ExprEngine::VisitPackIndexingExpr(const PackIndexingExpr *E,
+                                       ExplodedNode *Pred,
+                                       ExplodedNodeSet &Dst) {
+  if (!E->isFullySubstituted()) {
+    llvm_unreachable("Unsubstituted pack indexing expression");
+  }
+
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(E->getSelectedExpr())) {
+    VisitCommonDeclRefExpr(E, DRE->getDecl(), Pred, Dst);
+    return;
+  }
+
+  StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+  Bldr.generateSink(E, Pred, Pred->getState());
+}
+
 /// VisitArraySubscriptExpr - Transfer function for array accesses
 void ExprEngine::VisitArraySubscriptExpr(const ArraySubscriptExpr *A,
                                              ExplodedNode *Pred,
diff --git a/clang/test/Analysis/pack_indexing.cpp b/clang/test/Analysis/pack_indexing.cpp
new file mode 100644
index 0000000000000..0c3f1280c1529
--- /dev/null
+++ b/clang/test/Analysis/pack_indexing.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -std=c++26 -verify %s
+
+void clang_analyzer_eval(bool);
+
+template <class T>
+constexpr decltype(auto) get0(const T& val) noexcept {
+    auto& [...members] = val;
+    auto&& r = members...[0]; // no-crash
+    return r;
+}
+
+struct A {
+    int a;
+};
+
+void no_crash_negative() {
+    const int& x = get0(A{1});
+    clang_analyzer_eval(x == 1);
+}
+
+void uninitialized() {
+    A a;
+    const int& x = get0(a);
+    clang_analyzer_eval(x == 0); // expected-warning{{The left operand of '==' is a garbage value}}
+}

[isuckatcs]

Suggested change

	if (!E->isFullySubstituted()) {
	llvm_unreachable("Unsubstituted pack indexing expression");
	}
	assert(E->isFullySubstituted() && "unsubstituted pack indexing expression");

WDYT?

[isuckatcs]

A sink node usually indicates an error. How about we add a SimpleProgramPointTag to the sink node that explains what the error was?

Also is it guaranteed that the selected expression is a DeclRefExpr? If it is, this sink should probably be an assertion, if it isn't we should indicate in a FIXME that other cases need to be handled as well.

[isuckatcs]

We should probably add a FIXME here instead of just binding UnknownVal to the bind expression. This should eventually be handled.

In this case the binding is a sub-region of the pack (e.g.: only the first/last N elements), right? Can't we model this somehow?