Skip to content

Commit ad4aba8

Browse files
varungandhi-applevarungandhi-apple
authored
Merge pull request swiftlang#33377 from varungandhi-apple/vg-fix-module-trace-cycle
[ModuleTrace] Handle cycle detection edge case in module trace emission.
2 parents bf2ef39 + 0ca8458 commit ad4aba8

File tree

8 files changed

+305
-4
lines changed

8 files changed

+305
-4
lines changed

‎lib/FrontendTool/FrontendTool.cpp

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,31 @@ class ABIDependencyEvaluator {
342342
void reexposeImportedABI(ModuleDecl *module, ModuleDecl *importedModule,
343343
bool includeImportedModule = true);
344344

345+
/// Check if a Swift module is an overlay for some Clang module.
346+
///
347+
/// FIXME: Delete this hack once SR-13363 is fixed and ModuleDecl has the
348+
/// right API which we can use directly.
349+
bool isOverlayOfClangModule(ModuleDecl *swiftModule);
350+
351+
/// Check for cases where we have a fake cycle through an overlay.
352+
///
353+
/// \code
354+
/// Actual stack:
355+
/// sandwichedModule -> Overlay (Swift) -> ... -> sandwichedModule
356+
/// ^^--- wrong!
357+
/// Ideal stack:
358+
/// sandwichedModule -> Underlying (Clang)
359+
/// \endcode
360+
///
361+
/// This happens when we have a dependency like:
362+
/// \code
363+
/// Overlay (Swift) -> sandwichedModule -> Underlying (Clang)
364+
/// \endcode
365+
///
366+
/// We check this lazily because eagerly detecting if the dependency on an
367+
/// overlay is correct or not is difficult.
368+
bool isFakeCycleThroughOverlay(ModuleDecl **sandwichedModuleIter);
369+
345370
/// Recursive step in computing ABI dependencies.
346371
///
347372
/// Use this method instead of using the \c forClangModule/\c forSwiftModule
@@ -453,9 +478,42 @@ void ABIDependencyEvaluator::reexposeImportedABI(
453478
addToABIExportMap(module, reexportedModule);
454479
}
455480

481+
bool ABIDependencyEvaluator::isOverlayOfClangModule(ModuleDecl *swiftModule) {
482+
assert(!swiftModule->isNonSwiftModule());
483+
484+
llvm::SmallPtrSet<ModuleDecl *, 8> importList;
485+
::getImmediateImports(swiftModule, importList,
486+
{ModuleDecl::ImportFilterKind::Public});
487+
bool isOverlay =
488+
llvm::any_of(importList, [&](ModuleDecl *importedModule) -> bool {
489+
return isClangOverlayOf(swiftModule, importedModule);
490+
});
491+
return isOverlay;
492+
}
493+
494+
bool ABIDependencyEvaluator::isFakeCycleThroughOverlay(
495+
ModuleDecl **sandwichModuleIter) {
496+
assert(sandwichModuleIter >= searchStack.begin()
497+
&& sandwichModuleIter < searchStack.end()
498+
&& "sandwichModuleIter points to an element in searchStack");
499+
// The sandwichedModule must be a Clang module.
500+
if (!(*sandwichModuleIter)->isNonSwiftModule())
501+
return false;
502+
auto nextModuleIter = sandwichModuleIter + 1;
503+
if (nextModuleIter == searchStack.end())
504+
return false;
505+
// The next module must be a Swift overlay for a Clang module
506+
if ((*nextModuleIter)->isNonSwiftModule())
507+
return false;
508+
return isOverlayOfClangModule(*nextModuleIter);
509+
}
510+
456511
void ABIDependencyEvaluator::computeABIDependenciesForModule(
457512
ModuleDecl *module) {
458-
if (llvm::find(searchStack, module) != searchStack.end()) {
513+
auto moduleIter = llvm::find(searchStack, module);
514+
if (moduleIter != searchStack.end()) {
515+
if (isFakeCycleThroughOverlay(moduleIter))
516+
return;
459517
crashOnInvariantViolation([&](llvm::raw_string_ostream &os) {
460518
os << "unexpected cycle in import graph!\n";
461519
for (auto m: searchStack) {
@@ -510,6 +568,10 @@ void ABIDependencyEvaluator::computeABIDependenciesForClangModule(
510568
// C' imports S. This creates a cycle: S -> C' -> ... -> S.
511569
// In practice, this case is hit for
512570
// Darwin (Swift) -> SwiftOverlayShims (Clang) -> Darwin (Swift).
571+
// We may also hit this in a slightly different direction, in case
572+
// the module directly imports SwiftOverlayShims:
573+
// SwiftOverlayShims -> Darwin (Swift) -> SwiftOverlayShims
574+
// The latter is handled later by isFakeCycleThroughOverlay.
513575
// 3. [NOTE: Intra-module-leafwards-traversal]
514576
// Cycles within the same top-level module.
515577
// These don't matter for us, since we only care about the dependency
@@ -519,9 +581,8 @@ void ABIDependencyEvaluator::computeABIDependenciesForClangModule(
519581
if (import->isStdlibModule()) {
520582
continue;
521583
}
522-
if (!import->isNonSwiftModule()
523-
&& import->findUnderlyingClangModule() != nullptr
524-
&& llvm::find(searchStack, import) != searchStack.end()) {
584+
if (!import->isNonSwiftModule() && isOverlayOfClangModule(import) &&
585+
llvm::find(searchStack, import) != searchStack.end()) {
525586
continue;
526587
}
527588
if (import->isNonSwiftModule()

‎test/Driver/Inputs/imported_modules/ComplexModuleGraph2/CoreDaemon.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
typedef void Runner(int);
2+
3+
struct Daemon {
4+
Runner *run;
5+
};

‎test/Driver/Inputs/imported_modules/ComplexModuleGraph2/CoreMemory.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
struct MemoryMapRegion {
2+
void *start;
3+
void *end;
4+
};

‎test/Driver/Inputs/imported_modules/ComplexModuleGraph2/DaemonKit.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#import <CoreDaemon.h>
2+
3+
struct DaemonPair {
4+
struct Daemon daemons[2];
5+
};

‎test/Driver/Inputs/imported_modules/ComplexModuleGraph2/FilesystemKit.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#import <CoreFilesystem-Generated.h>
2+
3+
struct FileStorage {
4+
MyNode *nodes[4];
5+
};

‎test/Driver/Inputs/imported_modules/ComplexModuleGraph2/MemoryKit.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#import <CoreMemory.h>
2+
#import <CoreMemory-Generated.h>
3+
4+
struct MemoryMap {
5+
MemoryMapEntry *entries;
6+
unsigned count;
7+
};

‎test/Driver/Inputs/imported_modules/ComplexModuleGraph2/module.modulemap

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
module CoreFilesystem {
2+
header "CoreFilesystem-Generated.h"
3+
export *
4+
}
5+
6+
module FilesystemKit {
7+
header "FilesystemKit.h"
8+
export *
9+
}
10+
11+
module CoreDaemon {
12+
header "CoreDaemon.h"
13+
export *
14+
}
15+
16+
module DaemonKit {
17+
header "DaemonKit.h"
18+
export *
19+
}
20+
21+
module CoreMemory {
22+
header "CoreMemory.h"
23+
export *
24+
}
25+
26+
module MemoryKit {
27+
header "MemoryKit.h"
28+
export *
29+
}

‎test/Driver/loaded_module_trace_directness_2.swift

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: cp -r %S/Inputs/imported_modules/ComplexModuleGraph2 %t/include
3+
4+
// REQUIRES: objc_interop
5+
6+
// Dependency graph:
7+
//
8+
// * CoreFilesystem - Swift module with generated ObjC header
9+
// * FilesystemKit - ObjC module without overlay, has #import <CoreFileystem-Generated.h>
10+
// * TestFilesystem - Swift module, has import CoreFilesystem or FilesystemKit.
11+
//
12+
// * CoreDaemon - ObjC module with overlay, the overlay has import DaemonKit
13+
// * DaemonKit - ObjC module without overlay, has #import <CoreDaemon.h>
14+
// * TestDaemon - Swift module, has import CoreDaemon or DaemonKit.
15+
// NOTE: This mimics the Darwin -> SwiftOverlayShims -> Darwin dependency "cycle".
16+
//
17+
// * CoreMemory - ObjC module with overlay and generated header for overlay, the overlay has import MemoryKit
18+
// * MemoryKit - ObjC module without overlay, has #import <CoreMemory-Generated.h>
19+
// * TestMemory - Swift module, has import CoreMemory or MemoryKit.
20+
21+
// 1. CoreFilesystem - Build.
22+
23+
// RUN: %target-swift-frontend %s -emit-module -o %t/include/CoreFilesystem.swiftmodule -DCoreFilesystem -module-name CoreFilesystem -emit-objc-header-path %t/include/CoreFilesystem-Generated.h -disable-objc-attr-requires-foundation-module
24+
25+
#if CoreFilesystem
26+
@objc
27+
public class MyNode {
28+
public var number: Int
29+
public init(_ n: Int) { number = n }
30+
}
31+
#endif
32+
33+
// 2. FilesystemKit - Nothing to do (pure Clang module).
34+
35+
// 3. TestFilesystem - Check that CoreFilesystem and Filesytem can be imported.
36+
37+
// RUN: %target-swift-frontend %s -typecheck -DTestFilesystem -DV1 -module-name TestFilesystemV1 -emit-loaded-module-trace-path %t/TestFilesystemV1.trace.json -I %t/include
38+
// RUN: %FileCheck %s --check-prefix=TESTFILESYSTEM < %t/TestFilesystemV1.trace.json
39+
// RUN: %target-swift-frontend %s -typecheck -DTestFilesystem -DV2 -module-name TestFilesystemV2 -emit-loaded-module-trace-path %t/TestFilesystemV2.trace.json -I %t/include
40+
// RUN: %FileCheck %s --check-prefix=TESTFILESYSTEM < %t/TestFilesystemV2.trace.json
41+
// RUN: %target-swift-frontend %s -typecheck -DTestFilesystem -DV3 -module-name TestFilesystemV3 -emit-loaded-module-trace-path %t/TestFilesystemV3.trace.json -I %t/include
42+
// RUN: %FileCheck %s --check-prefix=TESTFILESYSTEM < %t/TestFilesystemV3.trace.json
43+
// RUN: %target-swift-frontend %s -typecheck -DTestFilesystem -DV4 -module-name TestFilesystemV4 -emit-loaded-module-trace-path %t/TestFilesystemV4.trace.json -I %t/include
44+
// RUN: %FileCheck %s --check-prefix=TESTFILESYSTEM < %t/TestFilesystemV4.trace.json
45+
46+
#if TestFilesystem
47+
#if V1
48+
import CoreFilesystem
49+
#endif
50+
#if V2
51+
import FilesystemKit
52+
public func noop(_: CoreFilesystem.MyNode) {}
53+
#endif
54+
#if V3
55+
import CoreFilesystem
56+
import FilesystemKit
57+
#endif
58+
#if V4
59+
import FilesystemKit
60+
import CoreFilesystem
61+
#endif
62+
#endif
63+
64+
// FilesystemKit has no overlay, so it shouldn't show up.
65+
// TESTFILESYSTEM: "swiftmodulesDetailedInfo":[
66+
// TESTFILESYSTEM-NOT: FilesystemKit
67+
// TESTFILESYSTEM-DAG: {"name":"CoreFilesystem","path":"{{[^"]*}}CoreFilesystem.swiftmodule","isImportedDirectly":true,
68+
// TESTFILESYSTEM: ]
69+
70+
// 4. CoreDaemon - Build.
71+
72+
// RUN: %target-swift-frontend %s -emit-module -o %t/include/CoreDaemon.swiftmodule -DCoreDaemon -module-name CoreDaemon -emit-loaded-module-trace-path %t/CoreDaemon.trace.json -I %t/include
73+
// RUN: %FileCheck %s --check-prefix=COREDAEMON < %t/CoreDaemon.trace.json
74+
75+
// * CoreDaemon - ObjC module with overlay, the overlay has import DaemonKit
76+
// * DaemonKit - ObjC module without overlay, has #import <CoreDaemon.h>
77+
// * TestDaemon - Swift module, has import CoreDaemon or DaemonKit.
78+
79+
#if CoreDaemon
80+
@_exported import CoreDaemon
81+
import DaemonKit
82+
83+
public func runBoth(_ pair: DaemonKit.DaemonPair) {
84+
let daemons : (CoreDaemon.Daemon, CoreDaemon.Daemon) = pair.daemons;
85+
daemons.0.run(0);
86+
daemons.1.run(1);
87+
}
88+
#endif
89+
90+
// COREDAEMON: "swiftmodulesDetailedInfo":[
91+
// COREDAEMON-NOT: CoreDaemon
92+
// COREDAEMON-NOT: DaemonKit
93+
// COREDAEMON: ]
94+
95+
// 5. DaemonKit - Nothing to do (pure Clang module).
96+
97+
// 6. TestCoreDaemon
98+
99+
// RUN: %target-swift-frontend %s -typecheck -DTestDaemon -DV1 -module-name TestDaemonV1 -emit-loaded-module-trace-path %t/TestDaemonV1.trace.json -I %t/include
100+
// RUN: %FileCheck %s --check-prefix=TESTDAEMON < %t/TestDaemonV1.trace.json
101+
102+
// RUN: %target-swift-frontend %s -typecheck -DTestDaemon -DV2 -module-name TestDaemonV2 -emit-loaded-module-trace-path %t/TestDaemonV2.trace.json -I %t/include
103+
// RUN: %FileCheck %s --check-prefix=TESTDAEMON < %t/TestDaemonV2.trace.json
104+
105+
// RUN: %target-swift-frontend %s -typecheck -DTestDaemon -DV3 -module-name TestDaemonV3 -emit-loaded-module-trace-path %t/TestDaemonV3.trace.json -I %t/include
106+
// RUN: %FileCheck %s --check-prefix=TESTDAEMON < %t/TestDaemonV3.trace.json
107+
108+
// RUN: %target-swift-frontend %s -typecheck -DTestDaemon -DV4 -module-name TestDaemonV4 -emit-loaded-module-trace-path %t/TestDaemonV4.trace.json -I %t/include
109+
// RUN: %FileCheck %s --check-prefix=TESTDAEMON < %t/TestDaemonV4.trace.json
110+
111+
#if TestDaemon
112+
#if V1
113+
import CoreDaemon
114+
#endif
115+
#if V2
116+
import DaemonKit
117+
public func noop(_: CoreDaemon.Daemon) {}
118+
#endif
119+
#if V3
120+
import CoreDaemon
121+
import DaemonKit
122+
#endif
123+
#if V4
124+
import DaemonKit
125+
import CoreDaemon
126+
#endif
127+
#endif
128+
129+
// DaemonKit has no overlay, so it shouldn't show up.
130+
// TESTDAEMON: "swiftmodulesDetailedInfo":[
131+
// TESTDAEMON-NOT: DaemonKit
132+
// TESTDAEMON-DAG: {"name":"CoreDaemon","path":"{{[^"]*}}CoreDaemon.swiftmodule","isImportedDirectly":true,
133+
// TESTDAEMON: ]
134+
135+
// 7. CoreMemory - Build.
136+
137+
// RUN: %target-swift-frontend %s -emit-module -o %t/include/CoreMemory.swiftmodule -DCoreMemory -module-name CoreMemory -emit-objc-header-path %t/include/CoreMemory-Generated.h -disable-objc-attr-requires-foundation-module -I %t/include
138+
139+
#if CoreMemory
140+
@_exported import CoreMemory
141+
142+
@objc
143+
public class MemoryMapEntry {
144+
public var region: MemoryMapRegion
145+
public var permissions: Int = 0
146+
public init(_ r: MemoryMapRegion) { region = r }
147+
}
148+
#endif
149+
150+
// 8. MemoryKit - Nothing to do (pure Clang module).
151+
152+
// 9. TestMemory - Check that CoreMemory and MemoryKit can be imported.
153+
154+
// RUN: %target-swift-frontend %s -typecheck -DTestMemory -DV1 -module-name TestMemoryV1 -emit-loaded-module-trace-path %t/TestMemoryV1.trace.json -I %t/include
155+
// RUN: %FileCheck %s --check-prefix=TESTMEMORY < %t/TestMemoryV1.trace.json
156+
// RUN: %target-swift-frontend %s -typecheck -DTestMemory -DV2 -module-name TestMemoryV2 -emit-loaded-module-trace-path %t/TestMemoryV2.trace.json -I %t/include
157+
// RUN: %FileCheck %s --check-prefix=TESTMEMORY < %t/TestMemoryV2.trace.json
158+
// RUN: %target-swift-frontend %s -typecheck -DTestMemory -DV3 -module-name TestMemoryV3 -emit-loaded-module-trace-path %t/TestMemoryV3.trace.json -I %t/include
159+
// RUN: %FileCheck %s --check-prefix=TESTMEMORY < %t/TestMemoryV3.trace.json
160+
// RUN: %target-swift-frontend %s -typecheck -DTestMemory -DV4 -module-name TestMemoryV4 -emit-loaded-module-trace-path %t/TestMemoryV4.trace.json -I %t/include
161+
// RUN: %FileCheck %s --check-prefix=TESTMEMORY < %t/TestMemoryV4.trace.json
162+
163+
#if TestMemory
164+
#if V1
165+
import CoreMemory
166+
#endif
167+
#if V2
168+
import MemoryKit
169+
public func noop(_: CoreMemory.MemoryMapRegion, _: CoreMemory.MemoryMapEntry) {}
170+
#endif
171+
#if V3
172+
import CoreMemory
173+
import MemoryKit
174+
#endif
175+
#if V4
176+
import MemoryKit
177+
import CoreMemory
178+
#endif
179+
#endif
180+
181+
// MemoryKit has no overlay, so it shouldn't show up.
182+
// TESTMEMORY: "swiftmodulesDetailedInfo":[
183+
// TESTMEMORY-NOT: MemoryKit
184+
// TESTMEMORY-DAG: {"name":"CoreMemory","path":"{{[^"]*}}CoreMemory.swiftmodule","isImportedDirectly":true,
185+
// TESTMEMORY: ]

0 commit comments

Comments
 (0)