Fix analyzer crash on 'StructuralValue'

[bolshakov-a]

OpaqueValueExpr doesn't necessarily contain a source expression. Particularly, after #78041, it is used to carry the type and the value kind of a non-type template argument of floating-point type or referring to a subobject (those are so called StructuralValue arguments).

This fixes #79575.

[llvmbot]

@llvm/pr-subscribers-clang

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

Author: Andrey Ali Khan Bolshakov (bolshakov-a)

Changes

OpaqueValueExpr doesn't necessarily contain a source expression. Particularly, after #78041, it is used to carry the type and the value kind of a non-type template argument of floating-point type or referring to a subobject (those are so called StructuralValue arguments).

This fixes #79575.

---

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

2 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Core/Environment.cpp (+6-3)
• (modified) clang/test/Analysis/templates.cpp (+13)

diff --git a/clang/lib/StaticAnalyzer/Core/Environment.cpp b/clang/lib/StaticAnalyzer/Core/Environment.cpp
index 4f989ed59bee38c..c77b28bc48fd674 100644
--- a/clang/lib/StaticAnalyzer/Core/Environment.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Environment.cpp
@@ -40,8 +40,12 @@ static const Expr *ignoreTransparentExprs(const Expr *E) {
 
   switch (E->getStmtClass()) {
   case Stmt::OpaqueValueExprClass:
-    E = cast<OpaqueValueExpr>(E)->getSourceExpr();
-    break;
+    if (const clang::Expr *SE = cast<OpaqueValueExpr>(E)->getSourceExpr()) {
+      E = SE;
+      break;
+    } else {
+      return E;
+    }
   case Stmt::ExprWithCleanupsClass:
     E = cast<ExprWithCleanups>(E)->getSubExpr();
     break;
@@ -98,7 +102,6 @@ SVal Environment::getSVal(const EnvironmentEntry &Entry,
   case Stmt::CXXBindTemporaryExprClass:
   case Stmt::ExprWithCleanupsClass:
   case Stmt::GenericSelectionExprClass:
-  case Stmt::OpaqueValueExprClass:
   case Stmt::ConstantExprClass:
   case Stmt::ParenExprClass:
   case Stmt::SubstNonTypeTemplateParmExprClass:
diff --git a/clang/test/Analysis/templates.cpp b/clang/test/Analysis/templates.cpp
index 061c19fe7e04451..6da1821b70f26fa 100644
--- a/clang/test/Analysis/templates.cpp
+++ b/clang/test/Analysis/templates.cpp
@@ -68,3 +68,16 @@ namespace rdar13954714 {
   // force instantiation
   template void blockWithStatic<true>();
 }
+
+namespace structural_value_crash {
+  constexpr char abc[] = "abc";
+
+  template <const char* in>
+  void use_template_param() {
+    const char *p = in;
+  }
+
+  void force_instantiate() {
+    use_template_param<abc>();
+  }
+}

[bolshakov-a]

@steakhal, @cor3ntin, @erichkeane

[cor3ntin]

Thanks!

[zmodem]

I think a similar fix may be needed in clang::IgnoreParensSingleStep, see my comment here: #78041 (comment)

[steakhal]

Approved with nits. This works around the crash.

[steakhal]

LGTM for the Static Analyzer.
Thanks for fixing this crash.

[steakhal]

LGTM for the Static Analyzer.

Actually, the other hunk also makes sense. LGTM.
Shouldn't we add a regression test for the crash on that part too?

[zmodem]

Shouldn't we add a regression test for the crash on that part too?

+1 that would be good to have.

[zmodem]

Thanks! It would be great to get this landed as soon as possible to unbreak trunk. (I believe we need it for the 18.x branch too?)

[steakhal]

Thanks! It would be great to get this landed as soon as possible to unbreak trunk. (I believe we need it for the 18.x branch too?)

I'll take care of the backport, after this PR is merged by @bolshakov-a

[bolshakov-a]

I'm just waiting for someone who would do it instead of me... I don't have commit access.

[bolshakov-a]

Thanks!

[steakhal]

Thanks!

Thanks for the quick workaround!

FYI Backport proposed in issue #79992, that refers to PR #79997 actually doing the backport.