[clang] 4f8916c - [C++20] [Modules] Exit early if export decl is not valid

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-01-14T10:21:42+08:00
New Revision: 4f8916cfdd941db2a4c4cdad6e5bc549532277a2

URL: https://github.com/llvm/llvm-project/commit/4f8916cfdd941db2a4c4cdad6e5bc549532277a2
DIFF: https://github.com/llvm/llvm-project/commit/4f8916cfdd941db2a4c4cdad6e5bc549532277a2.diff

LOG: [C++20] [Modules] Exit early if export decl is not valid

This patch fixes a crash due to following simple program:
> export struct Unit {
>    bool operator<(const Unit&);
> };

It would crash since the compiler would set the module ownership for
Unit. And the declaration with a module ownership is assumed to own a
module. But here isn't one. So here is the crash.

This patch fixes this by exiting early if it finds the export decl is
already invalid.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D117093

Added: 
    clang/test/Modules/export-in-non-modules.cpp

Modified: 
    clang/lib/Sema/SemaModule.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index a4b9f3c242c1c..996063f83e946 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -527,21 +527,30 @@ Decl *Sema::ActOnStartExportDecl(Scope *S, SourceLocation ExportLoc,
   // Set this temporarily so we know the export-declaration was braced.
   D->setRBraceLoc(LBraceLoc);
 
+  CurContext->addDecl(D);
+  PushDeclContext(S, D);
+
   // C++2a [module.interface]p1:
   //   An export-declaration shall appear only [...] in the purview of a module
   //   interface unit. An export-declaration shall not appear directly or
   //   indirectly within [...] a private-module-fragment.
   if (ModuleScopes.empty() || !ModuleScopes.back().Module->isModulePurview()) {
     Diag(ExportLoc, diag::err_export_not_in_module_interface) << 0;
+    D->setInvalidDecl();
+    return D;
   } else if (!ModuleScopes.back().ModuleInterface) {
     Diag(ExportLoc, diag::err_export_not_in_module_interface) << 1;
     Diag(ModuleScopes.back().BeginLoc,
          diag::note_not_module_interface_add_export)
         << FixItHint::CreateInsertion(ModuleScopes.back().BeginLoc, "export ");
+    D->setInvalidDecl();
+    return D;
   } else if (ModuleScopes.back().Module->Kind ==
              Module::PrivateModuleFragment) {
     Diag(ExportLoc, diag::err_export_in_private_module_fragment);
     Diag(ModuleScopes.back().BeginLoc, diag::note_private_module_fragment);
+    D->setInvalidDecl();
+    return D;
   }
 
   for (const DeclContext *DC = CurContext; DC; DC = DC->getLexicalParent()) {
@@ -553,7 +562,7 @@ Decl *Sema::ActOnStartExportDecl(Scope *S, SourceLocation ExportLoc,
         Diag(ND->getLocation(), diag::note_anonymous_namespace);
         // Don't diagnose internal-linkage declarations in this region.
         D->setInvalidDecl();
-        break;
+        return D;
       }
 
       //   A declaration is exported if it is [...] a namespace-definition
@@ -572,10 +581,10 @@ Decl *Sema::ActOnStartExportDecl(Scope *S, SourceLocation ExportLoc,
     Diag(ExportLoc, diag::err_export_within_export);
     if (ED->hasBraces())
       Diag(ED->getLocation(), diag::note_export);
+    D->setInvalidDecl();
+    return D;
   }
 
-  CurContext->addDecl(D);
-  PushDeclContext(S, D);
   D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::VisibleWhenImported);
   return D;
 }

diff  --git a/clang/test/Modules/export-in-non-modules.cpp b/clang/test/Modules/export-in-non-modules.cpp
new file mode 100644
index 0000000000000..c10d863d27005
--- /dev/null
+++ b/clang/test/Modules/export-in-non-modules.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
+export struct Unit { // expected-error {{export declaration can only be used within a module interface unit after the module declaration}}
+  bool operator<(const Unit &);
+};