Fix crash with lambda and variadic template

[rtrieu]

Summary

Inside Sema::tryCaptureVariable, there is an unconditional cast from a FunctionScopeInfo to a CapturingScopeInfo. With a mixture of lambdas and variadic template, we can arrive at this point with a FunctionScopeInfo that is not a CapturingScopeInfo. This is the simple fix of bailing out early with a default return value.

This fixes https://github.com/llvm/llvm-project/issues/56071 plus another test case reduction I found.

[aaron.ballman]

Adding additional reviewers for some additional perspective (perhaps my concerns are unfounded).

The changes should also have a release note about the fix.

clang/lib/Sema/SemaExpr.cpp

18875–18876

I'm not certain whether this is indicative of a deeper issue where we expect to get a capturing scope and we're not, but the early return seems surprising. I would have guessed that we'd want to continue so that we loop around again and continue to try to capture the variable.

[cor3ntin]

clang/lib/Sema/SemaExpr.cpp

18875–18876

Yes, i think there is something fishy here, and I am uncertain this is the right fix.

Either we are not currently in a lambda and if (VarDC == DC) return true; would prevent us from continuing, or there should only ever be lambda scopes between us and the variable declaration point.

Note that in either tests, the lambda don't capture anything.

My guess is that either:

• We are trying to capture something when we should not - unevaluated lambdas can't have captures. In which case bailing early in unevaluated scope may be a cleaner fix
• We are trying to capture in the the wrong context (ie we should do sdomethink like ActOnReenterFunctionContext but that would also be suspicious)

I think continuing would be wrong too. captures cannot skip a capturing scope.

[asbirlea]

Gentle ping. Can you propose an alternate patch to fix this crash? The github issue is 6 months old and similar crashes are reported due to this.

[rtrieu]

clang/lib/Sema/SemaExpr.cpp

18875–18876

I've been trying look into this more. From my understanding, FunctionScopeInfo FSI should correspond to DeclContext DC. When the crash happens, DC->dumpAsDecl() shows that the DeclContext is for a lambda (specifically, inner_lambda), but FSI->Kind evaluates to FunctionScopeInfo::SK_Function. I think this means that a function scope was never popped off FunctionScopes and replaced with a lambda scope.

[shafik]

It looks like this bug may also be related: https://github.com/llvm/llvm-project/issues/60155

[asbirlea]

Re-ping to current reviewers. Any thoughts on the above @cor3ntin? Is there someone who you recommend look into this issue?

[ayzhao]

clang/lib/Sema/SemaExpr.cpp

18875–18876

I took a look at this and it appears that in test2, in the following lines from inner_function():

char c;
[](auto param) -> decltype(c) { return param; }(0);

we try to capture char c because of the decltype.

I'm not entirely sure what the behavior is here - if we remove the template parameter from inner_function(), this function still tries to capture char c, and yet it FSI is always a CapturingScopeInfo.

[shafik]

It looks like we might have another similar bug: https://github.com/llvm/llvm-project/issues/61589

[shafik]

@cor3ntin is this still relevant? It looks like the test cases don't crash: https://godbolt.org/z/o3Y9req17

[cor3ntin]

Nope, the "deeper issue" seemed to have been fixed in the 17 cycle. But maybe we could keep the tests anyway?

[erichkeane]

Keeping the tests seems reasonable, please do so!

[shafik]

@cor3ntin can you do a new PR on github w/ just the tests or alternatively land the tests w/o the rest of the patch?

[cor3ntin]

I've added the tests to clang.
Thanks a lot of finding these tests cases and the work you did on this PR!

clang/lib/Sema/SemaExpr.cpp

if (!ParentDC) {
       if (IsGlobal) {
         FunctionScopesIndex = MaxFunctionScopesIndex - 1;
         break;
       }
       return true;
     }
 
     FunctionScopeInfo  *FSI = FunctionScopes[FunctionScopesIndex];
-    CapturingScopeInfo *CSI = cast<CapturingScopeInfo>(FSI);
-
+    CapturingScopeInfo *CSI = dyn_cast<CapturingScopeInfo>(FSI);
+    if (!CSI) return true;
 
     // Check whether we've already captured it.
     if (isVariableAlreadyCapturedInScopeInfo(CSI, Var, Nested, CaptureType,
                                              DeclRefType)) {
       CSI->getCapture(Var).markUsed(BuildAndDiagnose);
       break;
     }
     // If we are instantiating a generic lambda call operator body,

clang/test/SemaTemplate/lambda-variadic.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wno-unused-value
+// expected-no-diagnostics
+namespace test1 {
+
+template <int num> int return_num() { return num; }
+
+template <typename lambda> struct lambda_wrapper {
+  lambda &outer_lambda;
+  lambda_wrapper(lambda& outer_lambda) : outer_lambda(outer_lambda) {}
+  template <typename T> auto operator+(T t) { outer_lambda(t); return 1; }
+};
+
+template <int... nums, typename lambda>
+void bw(lambda& outer_lambda) {
+  (lambda_wrapper(outer_lambda) + ... + return_num<nums>());
+}
+
+template <typename lambda> auto check_return_type(lambda inner_lambda) {
+  using inner_lambda_return_type = decltype(inner_lambda(5));
+}
+
+void cs() {
+  auto outer_lambda = [](auto param) {
+    auto inner_lambda = [](auto o) -> decltype(param) {};
+    check_return_type(inner_lambda);
+  };
+  bw<1,2>(outer_lambda);
+}
+
+}  // namespace test1
+
+namespace test2 {
+
+template <typename lambda>
+auto run_lambda_with_zero(lambda l) {
+  l(0);
+}
+template <typename ... Ts, typename lambda>
+void run_lambda_once_per_type(lambda l) {
+  ((Ts{}, run_lambda_with_zero(l)), ...);
+}
+template <typename> void inner_function() {
+  char c;
+  [](auto param) -> decltype(c) { return param; }(0);
+}
+void run() {
+  auto x = [](auto) -> void { inner_function<int>(); };
+  run_lambda_once_per_type<int>(x);
+}
+
+}  // namespace test2