[ClangImporter] Protect against re-entrant bridging header loading

lib/ClangImporter/ClangImporter.cpp

@@ -1737,8 +1737,16 @@ void ClangImporter::Implementation::handleDeferredImports()
     ImportedHeaderExports.push_back(R.getSubmodule(ID));
   }
   PCHImportedSubmodules.clear();
-  for (const clang::Module *M : ImportedHeaderExports)
-    (void)finishLoadingClangModule(M, /*preferOverlay=*/true);
+
+  // Avoid a for-in loop because in unusual situations we can end up pulling in
+  // another bridging header while we finish loading the modules that are
+  // already here. This is a brittle situation but it's outside what's
+  // officially supported with bridging headers: app targets and unit tests
+  // only. Unfortunately that's not enforced.
+  for (size_t i = 0; i < ImportedHeaderExports.size(); ++i) {
+    (void)finishLoadingClangModule(ImportedHeaderExports[i],
+                                   /*preferOverlay=*/true);
+  }
 }
 
 ModuleDecl *ClangImporter::getImportedHeaderModule() const {

validation-test/ClangImporter/Inputs/bridging-header-reentrancy/App-Bridging-Header.h

@@ -0,0 +1,12 @@
+#include <CoincidentalNameCollision.h>
+#include <A.h>
+#include <B.h>
+#include <C.h>
+#include <D.h>
+#include <E.h>
+#include <F.h>
+#include <G.h>
+#include <H.h>
+#include <I.h>
+
+void appBridgingHeaderLoaded(void);

validation-test/ClangImporter/Inputs/bridging-header-reentrancy/CoincidentalNameCollision.h

@@ -0,0 +1 @@
+void CNCTest(void);

validation-test/ClangImporter/Inputs/bridging-header-reentrancy/Main-Bridging-Header.h

@@ -0,0 +1,13 @@
+#include <CoincidentalNameCollision.h>
+#include <J.h>
+#include <K.h>
+#include <L.h>
+#include <M.h>
+#include <N.h>
+#include <O.h>
+#include <P.h>
+#include <Q.h>
+#include <R.h>
+
+void mainBridgingHeaderLoaded(void);
+

validation-test/ClangImporter/Inputs/bridging-header-reentrancy/module.modulemap

@@ -0,0 +1,30 @@
+module A { header "A.h" }
+module B { header "B.h" }
+module C { header "C.h" }
+module D { header "D.h" }
+module E { header "E.h" }
+module F { header "F.h" }
+module G { header "G.h" }
+module H { header "H.h" }
+module I { header "I.h" }
+module J { header "J.h" }
+module K { header "K.h" }
+module L { header "L.h" }
+module M { header "M.h" }
+module N { header "N.h" }
+module O { header "O.h" }
+module P { header "P.h" }
+module Q { header "Q.h" }
+module R { header "R.h" }
+module S { header "S.h" }
+module T { header "T.h" }
+module U { header "U.h" }
+module V { header "V.h" }
+module W { header "W.h" }
+module X { header "X.h" }
+module Y { header "Y.h" }
+module Z { header "Z.h" }
+
+module CoincidentalNameCollision {
+  header "CoincidentalNameCollision.h"
+}

validation-test/ClangImporter/bridging-header-reentrancy.swift

@@ -0,0 +1,17 @@
+// rdar://problem/54581756
+// RUN: %empty-directory(%t)
+
+// First set up an app-like target with the same name as a system module. Make
+// sure the bridging header has a bunch of extra imports to force the
+// SmallVector 'ImportedHeaderExports' in ClangImporter::Implementation to be
+// reallocated.
+// RUN: %target-swift-frontend -sdk "" -emit-module -o %t -import-objc-header %S/Inputs/bridging-header-reentrancy/App-Bridging-Header.h -I %S/Inputs/bridging-header-reentrancy -module-name CoincidentalNameCollision %S/Inputs/bridging-header-reentrancy/CoincidentalNameCollision.swift
+
+// Then import that app-like target by accident in another target that also has
+// a bridging header and a bunch of imports.
+// RUN: %target-typecheck-verify-swift -sdk "" -I %t -import-objc-header %S/Inputs/bridging-header-reentrancy/Main-Bridging-Header.h -I %S/Inputs/bridging-header-reentrancy -verify-ignore-unknown
+
+mainBridgingHeaderLoaded() // ok, expected
+appBridgingHeaderLoaded() // ok, accidentally loaded
+CNCTest() // error, accidentally shadowed
+// expected-error@-1 {{use of unresolved identifier 'CNCTest'}}