Skip to content

Commit e42dd5a

Browse files
authored
[ClangImporter] Protect against re-entrant bridging header loading (swiftlang#27045)
If, while loading a bridging header, we pick up a Clang module that claims to have an overlay Swift module, and that Swift module turns out to have a bridging header, we can end up reallocating the array of modules to process while we're looping over it. Be defensive against this occurrence. This just fixes a crash; it does not at all solve the problem of this being broken in several ways: - Accidentally naming your module the same as a system module shadows the latter (if the system module is a Swift module) or *makes your module into an overlay* (if the system module is a Clang module). - Bridging headers are only officially supported on executable targets and unit tests, but this isn't really enforced. - Implicit inclusion of a bridging header *when you import a Swift module* is a hack to begin with, and a hack that worsens when the main module also has a bridging header. (All the bridging headers get folded together into the "same" module, which leads to more visibility than desired as well as cycles in the import graph.) - Combining all of these can result in some pretty bizarre behavior. rdar://problem/54581756
1 parent 5cbfeb7 commit e42dd5a

File tree

33 files changed

+83
-2
lines changed

33 files changed

+83
-2
lines changed

lib/ClangImporter/ClangImporter.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,8 +1738,16 @@ void ClangImporter::Implementation::handleDeferredImports()
17381738
ImportedHeaderExports.push_back(R.getSubmodule(ID));
17391739
}
17401740
PCHImportedSubmodules.clear();
1741-
for (const clang::Module *M : ImportedHeaderExports)
1742-
(void)finishLoadingClangModule(M, /*preferOverlay=*/true);
1741+
1742+
// Avoid a for-in loop because in unusual situations we can end up pulling in
1743+
// another bridging header while we finish loading the modules that are
1744+
// already here. This is a brittle situation but it's outside what's
1745+
// officially supported with bridging headers: app targets and unit tests
1746+
// only. Unfortunately that's not enforced.
1747+
for (size_t i = 0; i < ImportedHeaderExports.size(); ++i) {
1748+
(void)finishLoadingClangModule(ImportedHeaderExports[i],
1749+
/*preferOverlay=*/true);
1750+
}
17431751
}
17441752

17451753
ModuleDecl *ClangImporter::getImportedHeaderModule() const {

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

Whitespace-only changes.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include <CoincidentalNameCollision.h>
2+
#include <A.h>
3+
#include <B.h>
4+
#include <C.h>
5+
#include <D.h>
6+
#include <E.h>
7+
#include <F.h>
8+
#include <G.h>
9+
#include <H.h>
10+
#include <I.h>
11+
12+
void appBridgingHeaderLoaded(void);

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

Whitespace-only changes.

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

Whitespace-only changes.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
void CNCTest(void);

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

0 commit comments

Comments
 (0)