Skip to content

Conversation

tbaederr
Copy link
Contributor

The Expression here migth be value dependent, which makes us run into an assertion later on. Just bail out early.

Fixes #67690

@tbaederr tbaederr requested review from cor3ntin and shafik September 29, 2023 09:25
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 29, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 29, 2023

@llvm/pr-subscribers-clang

Changes

The Expression here migth be value dependent, which makes us run into an assertion later on. Just bail out early.

Fixes #67690


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

2 Files Affected:

  • (modified) clang/lib/AST/ExprConstant.cpp (+3)
  • (modified) clang/test/SemaCXX/constant-expression-cxx1z.cpp (+13)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fea06b97259fe31..8372d81234669fd 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -3357,6 +3357,9 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const Expr *E,
     return false;
   }
 
+  if (E->isValueDependent())
+    return false;
+
   // Dig out the initializer, and use the declaration which it's attached to.
   // FIXME: We should eventually check whether the variable has a reachable
   // initializing declaration.
diff --git a/clang/test/SemaCXX/constant-expression-cxx1z.cpp b/clang/test/SemaCXX/constant-expression-cxx1z.cpp
index 9335626a5c90a4f..a36b8f15f826f41 100644
--- a/clang/test/SemaCXX/constant-expression-cxx1z.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx1z.cpp
@@ -177,3 +177,16 @@ namespace LambdaCallOp {
     p();
   }
 }
+
+/// This used to crash due to an assertion failure,
+/// see gh#67690
+namespace {
+  struct C {
+    int x;
+  };
+
+  template <const C *p> void f() {
+    const auto &[c] = *p;
+    &c;
+  }
+}

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

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

This needs a release note, Otherwise LGTM

@tbaederr
Copy link
Contributor Author

Should we get crash fixes like this into clang 17 as well?

@tbaederr tbaederr force-pushed the 67690 branch 2 times, most recently from 8d771fa to 22339d9 Compare October 3, 2023 18:11
The Expression here migth be value dependent, which makes us run into an
assertion later on. Just bail out early.

Fixes llvm#67690
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang] Crash from constexpr-evaluation of structured-binding variable
4 participants