Skip to content

Commit 08b0760

Browse files
committed
Validate MODULE_DEPENDENCIES using tracing from Clang
This currently only checks Clang compilations with modules because there were some issues with introducing a new task action for non-modular Clang compilations. rdar://150313957
1 parent 572505d commit 08b0760

File tree

7 files changed

+329
-5
lines changed

7 files changed

+329
-5
lines changed

Sources/SWBCore/Dependencies.swift

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,48 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
8080
self.init(validate: validate, moduleDependencies: settings.moduleDependencies, fixItContext: fixItContext)
8181
}
8282

83+
// For Clang imports, we do not get the import locations and do not know whether it was a public
84+
// import (which depends on whether the import is in an installed header file).
85+
public func makeDiagnostics(files: [Path]) -> [Diagnostic] {
86+
guard validate != .no else { return [] }
87+
88+
// The following is a provisional/incomplete mechanism for resolving a module dependency from a file path.
89+
// For now, just grab the framework name and assume there is a module with the same name.
90+
func findFrameworkName(_ file: Path) -> String? {
91+
if file.fileExtension == "framework" {
92+
return file.basenameWithoutSuffix
93+
}
94+
return file.dirname.isEmpty || file.dirname.isRoot ? nil : findFrameworkName(file.dirname)
95+
}
96+
97+
let moduleDependencyNames = moduleDependencies.map { $0.name }
98+
let fileNames = files.compactMap { findFrameworkName($0) }
99+
let missingDeps = fileNames.filter {
100+
return !moduleDependencyNames.contains($0)
101+
}.map {
102+
ModuleDependency(name: $0, accessLevel: .Private)
103+
}
104+
105+
guard !missingDeps.isEmpty else { return [] }
106+
107+
let behavior: Diagnostic.Behavior = validate == .yesError ? .error : .warning
108+
109+
let fixIt = fixItContext?.makeFixIt(newModules: missingDeps)
110+
let fixIts = fixIt.map { [$0] } ?? []
111+
112+
let message = "Missing entries in \(BuiltinMacros.MODULE_DEPENDENCIES.name): \(missingDeps.map { $0.asBuildSettingEntryQuotedIfNeeded }.sorted().joined(separator: " "))"
113+
114+
let location: Diagnostic.Location = fixIt.map {
115+
Diagnostic.Location.path($0.sourceRange.path, line: $0.sourceRange.endLine, column: $0.sourceRange.endColumn)
116+
} ?? Diagnostic.Location.buildSetting(BuiltinMacros.MODULE_DEPENDENCIES)
117+
118+
return [Diagnostic(
119+
behavior: behavior,
120+
location: location,
121+
data: DiagnosticData(message),
122+
fixIts: fixIts)]
123+
}
124+
83125
/// Nil `imports` means the current toolchain doesn't have the features to gather imports. This is temporarily required to support running against older toolchains.
84126
public func makeDiagnostics(imports: [(ModuleDependency, importLocations: [Diagnostic.Location])]?) -> [Diagnostic] {
85127
guard validate != .no else { return [] }
@@ -181,4 +223,9 @@ public struct ModuleDependenciesContext: Sendable, SerializableCodable {
181223
return Diagnostic.FixIt(sourceRange: sourceRange, newText: newText)
182224
}
183225
}
226+
227+
func signatureData() -> String {
228+
let moduleNames = moduleDependencies.map { $0.name }
229+
return "validate:\(validate),modules:\(moduleNames.joined(separator: ":"))"
230+
}
184231
}

Sources/SWBCore/SpecImplementations/Tools/CCompiler.swift

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,10 @@ public struct ClangTaskPayload: ClangModuleVerifierPayloadType, DependencyInfoEd
432432

433433
public let fileNameMapPath: Path?
434434

435-
fileprivate init(serializedDiagnosticsPath: Path?, indexingPayload: ClangIndexingPayload?, explicitModulesPayload: ClangExplicitModulesPayload? = nil, outputObjectFilePath: Path? = nil, fileNameMapPath: Path? = nil, developerPathString: String? = nil) {
435+
public let moduleDependenciesContext: ModuleDependenciesContext?
436+
public let traceFile: Path?
437+
438+
fileprivate init(serializedDiagnosticsPath: Path?, indexingPayload: ClangIndexingPayload?, explicitModulesPayload: ClangExplicitModulesPayload? = nil, outputObjectFilePath: Path? = nil, fileNameMapPath: Path? = nil, developerPathString: String? = nil, moduleDependenciesContext: ModuleDependenciesContext? = nil, traceFile: Path? = nil) {
436439
if let developerPathString, explicitModulesPayload == nil {
437440
self.dependencyInfoEditPayload = .init(removablePaths: [], removableBasenames: [], developerPath: Path(developerPathString))
438441
} else {
@@ -443,27 +446,33 @@ public struct ClangTaskPayload: ClangModuleVerifierPayloadType, DependencyInfoEd
443446
self.explicitModulesPayload = explicitModulesPayload
444447
self.outputObjectFilePath = outputObjectFilePath
445448
self.fileNameMapPath = fileNameMapPath
449+
self.moduleDependenciesContext = moduleDependenciesContext
450+
self.traceFile = traceFile
446451
}
447452

448453
public func serialize<T: Serializer>(to serializer: T) {
449-
serializer.serializeAggregate(6) {
454+
serializer.serializeAggregate(8) {
450455
serializer.serialize(serializedDiagnosticsPath)
451456
serializer.serialize(indexingPayload)
452457
serializer.serialize(explicitModulesPayload)
453458
serializer.serialize(outputObjectFilePath)
454459
serializer.serialize(fileNameMapPath)
455460
serializer.serialize(dependencyInfoEditPayload)
461+
serializer.serialize(moduleDependenciesContext)
462+
serializer.serialize(traceFile)
456463
}
457464
}
458465

459466
public init(from deserializer: any Deserializer) throws {
460-
try deserializer.beginAggregate(6)
467+
try deserializer.beginAggregate(8)
461468
self.serializedDiagnosticsPath = try deserializer.deserialize()
462469
self.indexingPayload = try deserializer.deserialize()
463470
self.explicitModulesPayload = try deserializer.deserialize()
464471
self.outputObjectFilePath = try deserializer.deserialize()
465472
self.fileNameMapPath = try deserializer.deserialize()
466473
self.dependencyInfoEditPayload = try deserializer.deserialize()
474+
self.moduleDependenciesContext = try deserializer.deserialize()
475+
self.traceFile = try deserializer.deserialize()
467476
}
468477
}
469478

@@ -1156,6 +1165,22 @@ public class ClangCompilerSpec : CompilerSpec, SpecIdentifierType, GCCCompatible
11561165
dependencyData = nil
11571166
}
11581167

1168+
let moduleDependenciesContext = cbc.producer.moduleDependenciesContext
1169+
let traceFile: Path?
1170+
if clangInfo?.hasFeature("print-headers-direct-per-file") ?? false,
1171+
(moduleDependenciesContext?.validate ?? .defaultValue) != .no {
1172+
let file = Path(outputNode.path.str + ".trace.json")
1173+
commandLine += [
1174+
"-Xclang", "-header-include-file",
1175+
"-Xclang", file.str,
1176+
"-Xclang", "-header-include-filtering=direct-per-file",
1177+
"-Xclang", "-header-include-format=json"
1178+
]
1179+
traceFile = file
1180+
} else {
1181+
traceFile = nil
1182+
}
1183+
11591184
// Add the diagnostics serialization flag. We currently place the diagnostics file right next to the output object file.
11601185
let diagFilePath: Path?
11611186
if let serializedDiagnosticsOptions = self.serializedDiagnosticsOptions(scope: cbc.scope, outputPath: outputNode.path) {
@@ -1266,7 +1291,9 @@ public class ClangCompilerSpec : CompilerSpec, SpecIdentifierType, GCCCompatible
12661291
explicitModulesPayload: explicitModulesPayload,
12671292
outputObjectFilePath: shouldGenerateRemarks ? outputNode.path : nil,
12681293
fileNameMapPath: verifierPayload?.fileNameMapPath,
1269-
developerPathString: recordSystemHeaderDepsOutsideSysroot ? cbc.scope.evaluate(BuiltinMacros.DEVELOPER_DIR).str : nil
1294+
developerPathString: recordSystemHeaderDepsOutsideSysroot ? cbc.scope.evaluate(BuiltinMacros.DEVELOPER_DIR).str : nil,
1295+
moduleDependenciesContext: moduleDependenciesContext,
1296+
traceFile: traceFile
12701297
)
12711298

12721299
var inputNodes: [any PlannedNode] = inputDeps.map { delegate.createNode($0) }
@@ -1316,8 +1343,12 @@ public class ClangCompilerSpec : CompilerSpec, SpecIdentifierType, GCCCompatible
13161343
extraInputs = []
13171344
}
13181345

1346+
if let moduleDependenciesContext {
1347+
additionalSignatureData += moduleDependenciesContext.signatureData()
1348+
}
1349+
13191350
// Finally, create the task.
1320-
delegate.createTask(type: self, dependencyData: dependencyData, payload: payload, ruleInfo: ruleInfo, additionalSignatureData: additionalSignatureData, commandLine: commandLine, additionalOutput: additionalOutput, environment: environmentBindings, workingDirectory: compilerWorkingDirectory(cbc), inputs: inputNodes + extraInputs, outputs: [outputNode], action: action ?? delegate.taskActionCreationDelegate.createDeferredExecutionTaskActionIfRequested(userPreferences: cbc.producer.userPreferences), execDescription: resolveExecutionDescription(cbc, delegate), enableSandboxing: enableSandboxing, additionalTaskOrderingOptions: [.compilationForIndexableSourceFile], usesExecutionInputs: usesExecutionInputs, showEnvironment: true, priority: .preferred)
1351+
delegate.createTask(type: self, dependencyData: dependencyData, payload: payload, ruleInfo: ruleInfo, additionalSignatureData: additionalSignatureData, commandLine: commandLine, additionalOutput: additionalOutput, environment: environmentBindings, workingDirectory: compilerWorkingDirectory(cbc), inputs: inputNodes + extraInputs, outputs: [outputNode], action: action ?? delegate.taskActionCreationDelegate.createClangNonModularCompileTaskAction(), execDescription: resolveExecutionDescription(cbc, delegate), enableSandboxing: enableSandboxing, additionalTaskOrderingOptions: [.compilationForIndexableSourceFile], usesExecutionInputs: usesExecutionInputs, showEnvironment: true, priority: .preferred)
13211352

13221353
// If the object file verifier is enabled and we are building with explicit modules, also create a job to produce adjacent objects using implicit modules, then compare the results.
13231354
if cbc.scope.evaluate(BuiltinMacros.CLANG_ENABLE_EXPLICIT_MODULES_OBJECT_FILE_VERIFIER) && action != nil {

Sources/SWBCore/ToolInfo/ClangToolInfo.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ public struct DiscoveredClangToolSpecInfo: DiscoveredCommandLineToolSpecInfo {
3838
case wSystemHeadersInModule = "Wsystem-headers-in-module"
3939
case extractAPISupportsCPlusPlus = "extract-api-supports-cpp"
4040
case deploymentTargetEnvironmentVariables = "deployment-target-environment-variables"
41+
case printHeadersDirectPerFile = "print-headers-direct-per-file"
4142
}
4243
public var toolFeatures: ToolFeatures<FeatureFlag>
4344
public func hasFeature(_ feature: String) -> Bool {
@@ -46,6 +47,10 @@ public struct DiscoveredClangToolSpecInfo: DiscoveredCommandLineToolSpecInfo {
4647
if feature == FeatureFlag.extractAPISupportsCPlusPlus.rawValue {
4748
return clangVersion > Version(17)
4849
}
50+
// FIXME: Remove once the feature flag is added to clang.
51+
if feature == FeatureFlag.printHeadersDirectPerFile.rawValue, let clangVersion {
52+
return clangVersion >= Version(1700, 3, 10, 2)
53+
}
4954
return toolFeatures.has(feature)
5055
}
5156

Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,20 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
248248
casDBs = nil
249249
}
250250

251+
// Check if verifying dependencies from trace data is enabled.
252+
var traceFile: Path? = nil
253+
var moduleDependenciesContext: ModuleDependenciesContext? = nil
254+
if let payload = task.payload as? ClangTaskPayload {
255+
traceFile = payload.traceFile
256+
moduleDependenciesContext = payload.moduleDependenciesContext
257+
}
258+
if let traceFile {
259+
// Remove the trace output file if it already exists.
260+
if executionDelegate.fs.exists(traceFile) {
261+
try executionDelegate.fs.remove(traceFile)
262+
}
263+
}
264+
251265
var lastResult: CommandResult? = nil
252266
for command in dependencyInfo.commands {
253267
if let casDBs {
@@ -304,6 +318,24 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
304318
return lastResult ?? .failed
305319
}
306320
}
321+
322+
if let moduleDependenciesContext, let traceFile, lastResult == .succeeded {
323+
// Verify the dependencies from the trace data.
324+
let fs = executionDelegate.fs
325+
let traceData = try JSONDecoder().decode(Array<TraceData>.self, from: fs.readMemoryMapped(traceFile))
326+
327+
var allFiles = Set<Path>()
328+
traceData.forEach { allFiles.formUnion(Set($0.includes)) }
329+
let diagnostics = moduleDependenciesContext.makeDiagnostics(files: Array(allFiles))
330+
for diagnostic in diagnostics {
331+
outputDelegate.emit(diagnostic)
332+
}
333+
334+
if diagnostics.contains(where: { $0.behavior == .error }) {
335+
return .failed
336+
}
337+
}
338+
307339
return lastResult ?? .failed
308340
} catch {
309341
outputDelegate.emitError("\(error)")
@@ -431,3 +463,10 @@ public final class ClangCompileTaskAction: TaskAction, BuildValueValidatingTaskA
431463
)
432464
}
433465
}
466+
467+
// Results from tracing header includes with "direct-per-file" filtering.
468+
// This is used to validate dependencies.
469+
fileprivate struct TraceData: Decodable {
470+
let source: Path
471+
let includes: [Path]
472+
}

Sources/SWBTestSupport/CoreBasedTests.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,10 @@ extension CoreBasedTests {
147147
if clangInfo.clangVersion > Version(17) {
148148
realToolFeatures.insert(.extractAPISupportsCPlusPlus)
149149
}
150+
if let clangVersion = clangInfo.clangVersion, clangVersion >= Version(1700, 3, 10, 2) {
151+
realToolFeatures.insert(.printHeadersDirectPerFile)
152+
}
153+
150154
return ToolFeatures(realToolFeatures)
151155
}
152156
}
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift open source project
4+
//
5+
// Copyright (c) 2025 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See http://swift.org/LICENSE.txt for license information
9+
// See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import Testing
14+
15+
import SWBCore
16+
import SWBTestSupport
17+
@_spi(Testing) import SWBUtil
18+
19+
import SWBTaskExecution
20+
import SWBProtocol
21+
22+
@Suite(.requireSDKs(.macOS))
23+
fileprivate struct DependencyVerificationBuildOperationTests: CoreBasedTests {
24+
25+
func canVerifyDeclaredDependencies() async throws {
26+
try await withTemporaryDirectory { tmpDirPath async throws -> Void in
27+
let testWorkspace = TestWorkspace(
28+
"Test",
29+
sourceRoot: tmpDirPath.join("Test"),
30+
projects: [
31+
TestProject(
32+
"aProject",
33+
groupTree: TestGroup(
34+
"Sources", path: "Sources",
35+
children: [
36+
TestFile("CoreFoo.m")
37+
]),
38+
buildConfigurations: [
39+
TestBuildConfiguration(
40+
"Debug",
41+
buildSettings: [
42+
"PRODUCT_NAME": "$(TARGET_NAME)",
43+
"CLANG_ENABLE_MODULES": "YES",
44+
"GENERATE_INFOPLIST_FILE": "YES",
45+
"MODULE_DEPENDENCIES": "Foundation",
46+
"VALIDATE_MODULE_DEPENDENCIES": "YES_ERROR",
47+
// Disable the SetOwnerAndGroup action by setting them to empty values.
48+
"INSTALL_GROUP": "",
49+
"INSTALL_OWNER": "",
50+
]
51+
)
52+
],
53+
targets: [
54+
TestStandardTarget(
55+
"CoreFoo", type: .framework,
56+
buildPhases: [
57+
TestSourcesBuildPhase(["CoreFoo.m"]),
58+
TestFrameworksBuildPhase()
59+
])
60+
])
61+
]
62+
)
63+
64+
let tester = try await BuildOperationTester(getCore(), testWorkspace, simulated: false)
65+
let SRCROOT = testWorkspace.sourceRoot.join("aProject")
66+
67+
// Write the source files.
68+
try await tester.fs.writeFileContents(SRCROOT.join("Sources/CoreFoo.m")) { contents in
69+
contents <<< """
70+
#include <Foundation/Foundation.h>
71+
#include <Accelerate/Accelerate.h>
72+
73+
void f0(void) { };
74+
"""
75+
}
76+
77+
func parameters(_ overrides: [String: String] = [:]) -> BuildParameters {
78+
return BuildParameters(
79+
action: .install, configuration: "Debug",
80+
overrides: [
81+
"DSTROOT": tmpDirPath.join("dst").str
82+
].merging(overrides, uniquingKeysWith: { _, new in new })
83+
)
84+
}
85+
86+
if try await clangFeatures.has(.printHeadersDirectPerFile) {
87+
// Expect complaint about undeclared dependency
88+
try await tester.checkBuild(parameters: parameters(), runDestination: .macOS, persistent: true) { results in
89+
results.checkError(.contains("Missing entries in MODULE_DEPENDENCIES: Accelerate"))
90+
}
91+
92+
// Declaring dependencies resolves the problem
93+
try await tester.checkBuild(parameters: parameters(["MODULE_DEPENDENCIES": "Foundation Accelerate"]), runDestination: .macOS, persistent: true) { results in
94+
results.checkNoErrors()
95+
}
96+
}
97+
}
98+
}
99+
}

0 commit comments

Comments
 (0)