[analyzer] Support parenthesized list initialization (CXXParenListInitExpr)

[a-tarasyuk]

Fixes #148875

---

This patch addresses the lack of support for parenthesized initialization in the Clang Static Analyzer's ExprEngine. Previously, initializations such as V v(1, 2); were not modeled properly, which could lead to false negatives in analyses like DivideZero.

struct A {
  int x;
  A(int v) : x(v) {}
};

int t() {
  A a(42);
  return 1 / (a.x - 42); // expected-warning {{Division by zero}}
}

[llvmbot]

@llvm/pr-subscribers-clang

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

Author: Oleksandr T. (a-tarasyuk)

Changes

Fixes #148875

---

This patch addresses the lack of support for parenthesized initialization in the Clang Static Analyzer's ExprEngine. Previously, initializations such as V v(1, 2); were not modeled properly, which could lead to false negatives in analyses like DivideZero.

struct A {
  int x;
  A(int v) : x(v) {}
};

int t() {
  A a(42);
  return 1 / (a.x - 42); // expected-warning {{Division by zero}}
}

---

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

5 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+2)
• (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (+3)
• (modified) clang/lib/StaticAnalyzer/Core/ExprEngine.cpp (+6-1)
• (modified) clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (+31)
• (modified) clang/test/Analysis/div-zero.cpp (+44-6)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1eb3e369a302e..06a41700081a9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1199,6 +1199,8 @@ Static Analyzer
 ---------------
 - Fixed a crash when C++20 parenthesized initializer lists are used. This issue
   was causing a crash in clang-tidy. (#GH136041)
+- The Clang Static Analyzer now handles parenthesized initialization.
+  (#GH148875)
 
 New features
 ^^^^^^^^^^^^
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 6370586e218ef..79d86aef8a0c6 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -586,6 +586,9 @@ class ExprEngine {
   void VisitCXXDeleteExpr(const CXXDeleteExpr *CDE, ExplodedNode *Pred,
                           ExplodedNodeSet &Dst);
 
+  void VisitCXXParenListInitExpr(const CXXParenListInitExpr *PLIE,
+                                 ExplodedNode *Pred, ExplodedNodeSet &Dst);
+
   /// Create a C++ temporary object for an rvalue.
   void CreateCXXTemporaryObject(const MaterializeTemporaryExpr *ME,
                                 ExplodedNode *Pred,
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index c77ef26da568d..8f0cdd46045d0 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1941,7 +1941,6 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
     case Stmt::ConceptSpecializationExprClass:
     case Stmt::CXXRewrittenBinaryOperatorClass:
     case Stmt::RequiresExprClass:
-    case Expr::CXXParenListInitExprClass:
     case Stmt::EmbedExprClass:
       // Fall through.
 
@@ -2321,6 +2320,12 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       Bldr.addNodes(Dst);
       break;
 
+    case Expr::CXXParenListInitExprClass:
+      Bldr.takeNodes(Pred);
+      VisitCXXParenListInitExpr(cast<CXXParenListInitExpr>(S), Pred, Dst);
+      Bldr.addNodes(Dst);
+      break;
+
     case Stmt::MemberExprClass:
       Bldr.takeNodes(Pred);
       VisitMemberExpr(cast<MemberExpr>(S), Pred, Dst);
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 85353848aa124..059a435bd3e9e 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -1233,3 +1233,34 @@ void ExprEngine::VisitAttributedStmt(const AttributedStmt *A,
 
   getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, A, *this);
 }
+
+void ExprEngine::VisitCXXParenListInitExpr(const CXXParenListInitExpr *E,
+                                           ExplodedNode *Pred,
+                                           ExplodedNodeSet &Dst) {
+  StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+
+  ProgramStateRef S = Pred->getState();
+  QualType T = getContext().getCanonicalType(E->getType());
+
+  const LocationContext *LCtx = Pred->getLocationContext();
+
+  SmallVector<SVal, 4> ArgVals;
+  for (Expr *Arg : E->getInitExprs())
+    ArgVals.push_back(S->getSVal(Arg, LCtx));
+
+  if (!E->isGLValue() && (T->isRecordType() || T->isArrayType())) {
+    llvm::ImmutableList<SVal> ArgList = getBasicVals().getEmptySValList();
+
+    for (const SVal &V : llvm::reverse(ArgVals))
+      ArgList = getBasicVals().prependSVal(V, ArgList);
+
+    Bldr.generateNode(
+        E, Pred, S->BindExpr(E, LCtx, svalBuilder.makeCompoundVal(T, ArgList)));
+  } else {
+    Bldr.generateNode(E, Pred,
+                      S->BindExpr(E, LCtx,
+                                  ArgVals.empty()
+                                      ? getSValBuilder().makeZeroVal(T)
+                                      : ArgVals.front()));
+  }
+}
diff --git a/clang/test/Analysis/div-zero.cpp b/clang/test/Analysis/div-zero.cpp
index 063450d8883b0..2a44ad132d4a5 100644
--- a/clang/test/Analysis/div-zero.cpp
+++ b/clang/test/Analysis/div-zero.cpp
@@ -1,13 +1,51 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core.DivideZero -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core.DivideZero -std=c++20 -verify %s
 
-int fooPR10616 (int qX ) {
+namespace GH10616 {
+int foo(int qX) {
   int a, c, d;
 
-  d = (qX-1);
-  while ( d != 0 ) {
-    d = c - (c/d) * d;
+  d = (qX - 1);
+  while (d != 0) {
+    d = c - (c / d) * d;
   }
 
-  return (a % (qX-1)); // expected-warning {{Division by zero}}
+  return (a % (qX - 1)); // expected-warning {{Division by zero}}
+}
+} // namespace GH10616
+
+namespace GH148875 {
+struct A {
+  int x;
+  A(int v) : x(v) {}
+};
+
+struct B {
+  int x;
+  B() : x(0) {}
+};
+
+struct C {
+  int x, y;
+  C(int a, int b) : x(a), y(b) {}
+};
+
+int t1() {
+  A a(42);
+  return 1 / (a.x - 42); // expected-warning {{Division by zero}}
+}
+
+int t2() {
+  B b;
+  return 1 / b.x; // expected-warning {{Division by zero}}
+}
+
+int t3() {
+  C c1(1, -1);
+  return 1 / (c1.x + c1.y); // expected-warning {{Division by zero}}
+}
 
+int t4() {
+  C c2(0, 0);
+  return 1 / (c2.x + c2.y); // expected-warning {{Division by zero}}
 }
+} // namespace GH148875

[erichkeane]

I don't know much about the static analyzer, but perhaps Aaron can do a better review? I didn't see anything questionable however.

[steakhal]

I don't know much about the static analyzer, but perhaps Aaron can do a better review? I didn't see anything questionable however.

Moved this to the Static Analyzer folks.

[steakhal]

Looks good on the technical side. I have some questions mostly about code reuse.

[necto]

Thank you for tackling the issue! And so quickly!
I like the spirit, although I have not looked into the actual implementation in detail

[necto]

Given that this PR closes #148988, I think it will make it more clear to include the code snippet from the issue verbatim

[steakhal]

Given that this PR closes #148988, I think it will make it more clear to include the code snippet from the issue verbatim

I think you wanted to link #148875 actually.
Yes, having a verbatim copy of the original issue makes sense it it looks remotely what a human would write - which is this case.

The patch looks correct to me.

[steakhal]

Looks really good. Thanks for going the extra mile with the refactor.

[steakhal]

@a-tarasyuk Could you also add the test case from the issue as-is?

[steakhal]

Lets land this. Thank you!