[clang] 3f09273 - [C++20] [Modules] Fix crash when emitting module inits for duplicated modules

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-10-02T18:31:54+08:00
New Revision: 3f0927368285505ead516ba7572baaa7f52b01a9

URL: https://github.com/llvm/llvm-project/commit/3f0927368285505ead516ba7572baaa7f52b01a9
DIFF: https://github.com/llvm/llvm-project/commit/3f0927368285505ead516ba7572baaa7f52b01a9.diff

LOG: [C++20] [Modules] Fix crash when emitting module inits for duplicated modules

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

The root cause of the crash is an oversight that we missed the point
that the same module can be imported multiple times. And we should use
`SmallSetVector` instead of `SmallVector` to filter the case.

Added: 
    clang/test/Modules/module-init-duplicated-import.cppm
    clang/test/Modules/pr67893.cppm

Modified: 
    clang/lib/CodeGen/CGDeclCXX.cpp
    clang/lib/CodeGen/CodeGenModule.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 8a05e4893196ad2..30683ad474cd2c0 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -666,13 +666,13 @@ void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
   // Module initializers for imported modules are emitted first.
 
   // Collect all the modules that we import
-  SmallVector<Module *> AllImports;
+  llvm::SmallSetVector<Module *, 8> AllImports;
   // Ones that we export
   for (auto I : Primary->Exports)
-    AllImports.push_back(I.getPointer());
+    AllImports.insert(I.getPointer());
   // Ones that we only import.
   for (Module *M : Primary->Imports)
-    AllImports.push_back(M);
+    AllImports.insert(M);
   // Ones that we import in the global module fragment or the private module
   // fragment.
   llvm::for_each(Primary->submodules(), [&AllImports](Module *SubM) {
@@ -683,7 +683,7 @@ void CodeGenModule::EmitCXXModuleInitFunc(Module *Primary) {
            "The global mdoule fragments and the private module fragments are "
            "not allowed to export import modules.");
     for (Module *M : SubM->Imports)
-      AllImports.push_back(M);
+      AllImports.insert(M);
   });
 
   SmallVector<llvm::Function *, 8> ModuleInits;

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index c96523c52562582..925f5114e9108e0 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2938,6 +2938,9 @@ void CodeGenModule::EmitModuleInitializers(clang::Module *Primary) {
   // Third any associated with the Privat eMOdule Fragment, if present.
   if (auto PMF = Primary->getPrivateModuleFragment()) {
     for (Decl *D : getContext().getModuleInitializers(PMF)) {
+      // Skip import decls, the inits for those are called explicitly.
+      if (isa<ImportDecl>(D))
+        continue;
       assert(isa<VarDecl>(D) && "PMF initializer decl is not a var?");
       EmitTopLevelDecl(D);
     }

diff  --git a/clang/test/Modules/module-init-duplicated-import.cppm b/clang/test/Modules/module-init-duplicated-import.cppm
new file mode 100644
index 000000000000000..de0ce1962f10082
--- /dev/null
+++ b/clang/test/Modules/module-init-duplicated-import.cppm
@@ -0,0 +1,27 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/a.cppm \
+// RUN:      -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/m.cppm \
+// RUN:      -emit-module-interface -fmodule-file=a=%t/a.pcm -o %t/m.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/m.pcm  \
+// RUN:     -S -emit-llvm -o - | FileCheck %t/m.cppm
+
+//--- a.cppm
+export module a;
+export struct A {
+  A(){};
+};
+export A __dynamic_inited_a;
+
+//--- m.cppm
+export module m;
+import a;
+export import a;
+
+
+// CHECK: define void @_ZGIW1m
+// CHECK: store i8 1, ptr @_ZGIW1m__in_chrg
+// CHECK: call{{.*}}@_ZGIW1a

diff  --git a/clang/test/Modules/pr67893.cppm b/clang/test/Modules/pr67893.cppm
new file mode 100644
index 000000000000000..7d4e4c1dc5d843c
--- /dev/null
+++ b/clang/test/Modules/pr67893.cppm
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/a.cppm \
+// RUN:      -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/m.cppm \
+// RUN:      -emit-module-interface -fmodule-file=a=%t/a.pcm -o %t/m.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %t/m.pcm  \
+// RUN:     -S -emit-llvm -o - | FileCheck %t/m.cppm
+
+//--- a.cppm
+export module a;
+export struct A {
+  A(){};
+};
+export A __dynamic_inited_a;
+
+//--- m.cppm
+module;
+import a;
+export module m;
+import a;
+module :private;
+import a;
+
+// CHECK: define void @_ZGIW1m
+// CHECK: store i8 1, ptr @_ZGIW1m__in_chrg
+// CHECK: call{{.*}}@_ZGIW1a