ChuanqiXu created this revision.
ChuanqiXu added reviewers: erichkeane, royjacobson, shafik, clang-language-wg.
ChuanqiXu added a project: clang.
Herald added a project: All.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.

Close https://github.com/llvm/llvm-project/issues/60275

The root cause of issue 60275 is the imbalance of 
PushExpressionEvaluationContext() and PopExpressionEvaluationContext().

See 
https://github.com/llvm/llvm-project/blob/f1c4f927f7c15b5efdc3589c050fd0513bf6b303/clang/lib/Parse/Parser.cpp#L1396-L1437

We will  PushExpressionEvaluationContext() in ActOnStartOfFunctionDef() in line 
1396 and we should pop it in ActOnFinishFunctionBody later. However if we skip 
the function body in line 1402, the expression evaluation context will not be 
popped. Then here is the issue report. I fix the issue by inserting codes to 
pop the expression evaluation context  explicitly if the function body is 
skipped. Maybe this looks like an ad-hoc fix. But if we want to fix this in a 
pretty way, we should refactor the current framework for pushing and popping 
expression evaluation contexts. Currently there are 23 
PushExpressionEvaluationContext() callsities and 21 
PopExpressionEvaluationContext() callsites in the code. And it seems not easy 
to balance them well and fast. So I suggest to land this fix first. At least it 
can prevent the crash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143053

Files:
  clang/lib/Parse/Parser.cpp
  clang/test/Modules/pr60275.cppm


Index: clang/test/Modules/pr60275.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/pr60275.cppm
@@ -0,0 +1,40 @@
+// Address: https://github.com/llvm/llvm-project/issues/60275
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple 
-emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cpp 
-fmodule-file=%t/a.pcm -emit-llvm -o - | FileCheck %t/b.cpp
+//--- foo.h
+
+consteval void global() {}
+
+//--- a.cppm
+module;
+#include "foo.h"
+export module a;
+
+//--- b.cpp
+#include "foo.h"
+import a;
+
+consteval int b() {
+       return 0;
+}
+
+struct bb {
+       int m = b();
+};
+
+void bbb() {
+       bb x;
+}
+
+// CHECK: define{{.*}}_ZN2bbC2Ev({{.*}}[[THIS:%.+]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   [[THIS_ADDR:%.*]] = alloca ptr
+// CHECK-NEXT:   store ptr [[THIS]], ptr [[THIS_ADDR]]
+// CHECK-NEXT:   [[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]]
+// CHECK-NEXT:   [[M_ADDR:%.*]] = getelementptr{{.*}}%struct.bb, ptr 
[[THIS1]], i32 0, i32 0
+// CHECK-NEXT:   store i32 0, ptr [[M_ADDR]]
Index: clang/lib/Parse/Parser.cpp
===================================================================
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/ASTLambda.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
@@ -1404,6 +1405,17 @@
     if (BodyKind == Sema::FnBodyKind::Other)
       SkipFunctionBody();
 
+    // ExpressionEvaluationContext is pushed in ActOnStartOfFunctionDef
+    // and it would be popped in ActOnFinishFunctionBody.
+    // We pop it explcitly here since ActOnFinishFunctionBody won't get called.
+    //
+    // Do not call PopExpressionEvaluationContext() if it is a lambda because
+    // one is already popped when finishing the lambda in BuildLambdaExpr().
+    //
+    // FIXME: It looks not easy to balance PushExpressionEvaluationContext()
+    // and PopExpressionEvaluationContext().
+    if (!isLambdaCallOperator(dyn_cast_if_present<FunctionDecl>(Res)))
+      Actions.PopExpressionEvaluationContext();
     return Res;
   }
 


Index: clang/test/Modules/pr60275.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/pr60275.cppm
@@ -0,0 +1,40 @@
+// Address: https://github.com/llvm/llvm-project/issues/60275
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/b.cpp -fmodule-file=%t/a.pcm -emit-llvm -o - | FileCheck %t/b.cpp
+//--- foo.h
+
+consteval void global() {}
+
+//--- a.cppm
+module;
+#include "foo.h"
+export module a;
+
+//--- b.cpp
+#include "foo.h"
+import a;
+
+consteval int b() {
+	return 0;
+}
+
+struct bb {
+	int m = b();
+};
+
+void bbb() {
+	bb x;
+}
+
+// CHECK: define{{.*}}_ZN2bbC2Ev({{.*}}[[THIS:%.+]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   [[THIS_ADDR:%.*]] = alloca ptr
+// CHECK-NEXT:   store ptr [[THIS]], ptr [[THIS_ADDR]]
+// CHECK-NEXT:   [[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]]
+// CHECK-NEXT:   [[M_ADDR:%.*]] = getelementptr{{.*}}%struct.bb, ptr [[THIS1]], i32 0, i32 0
+// CHECK-NEXT:   store i32 0, ptr [[M_ADDR]]
Index: clang/lib/Parse/Parser.cpp
===================================================================
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/ASTLambda.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
@@ -1404,6 +1405,17 @@
     if (BodyKind == Sema::FnBodyKind::Other)
       SkipFunctionBody();
 
+    // ExpressionEvaluationContext is pushed in ActOnStartOfFunctionDef
+    // and it would be popped in ActOnFinishFunctionBody.
+    // We pop it explcitly here since ActOnFinishFunctionBody won't get called.
+    //
+    // Do not call PopExpressionEvaluationContext() if it is a lambda because
+    // one is already popped when finishing the lambda in BuildLambdaExpr().
+    //
+    // FIXME: It looks not easy to balance PushExpressionEvaluationContext()
+    // and PopExpressionEvaluationContext().
+    if (!isLambdaCallOperator(dyn_cast_if_present<FunctionDecl>(Res)))
+      Actions.PopExpressionEvaluationContext();
     return Res;
   }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to