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.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#include <CoincidentalNameCollision.h>
2+
#include <J.h>
3+
#include <K.h>
4+
#include <L.h>
5+
#include <M.h>
6+
#include <N.h>
7+
#include <O.h>
8+
#include <P.h>
9+
#include <Q.h>
10+
#include <R.h>
11+
12+
void mainBridgingHeaderLoaded(void);
13+

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.

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

Whitespace-only changes.
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
module A { header "A.h" }
2+
module B { header "B.h" }
3+
module C { header "C.h" }
4+
module D { header "D.h" }
5+
module E { header "E.h" }
6+
module F { header "F.h" }
7+
module G { header "G.h" }
8+
module H { header "H.h" }
9+
module I { header "I.h" }
10+
module J { header "J.h" }
11+
module K { header "K.h" }
12+
module L { header "L.h" }
13+
module M { header "M.h" }
14+
module N { header "N.h" }
15+
module O { header "O.h" }
16+
module P { header "P.h" }
17+
module Q { header "Q.h" }
18+
module R { header "R.h" }
19+
module S { header "S.h" }
20+
module T { header "T.h" }
21+
module U { header "U.h" }
22+
module V { header "V.h" }
23+
module W { header "W.h" }
24+
module X { header "X.h" }
25+
module Y { header "Y.h" }
26+
module Z { header "Z.h" }
27+
28+
module CoincidentalNameCollision {
29+
header "CoincidentalNameCollision.h"
30+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// rdar://problem/54581756
2+
// RUN: %empty-directory(%t)
3+
4+
// First set up an app-like target with the same name as a system module. Make
5+
// sure the bridging header has a bunch of extra imports to force the
6+
// SmallVector 'ImportedHeaderExports' in ClangImporter::Implementation to be
7+
// reallocated.
8+
// 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
9+
10+
// Then import that app-like target by accident in another target that also has
11+
// a bridging header and a bunch of imports.
12+
// 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
13+
14+
mainBridgingHeaderLoaded() // ok, expected
15+
appBridgingHeaderLoaded() // ok, accidentally loaded
16+
CNCTest() // error, accidentally shadowed
17+
// expected-error@-1 {{use of unresolved identifier 'CNCTest'}}

0 commit comments

Comments
 (0)