From 133a6caaa386f17770f3b202d45fda55c787ed8a Mon Sep 17 00:00:00 2001 From: Alex Denisov Date: Thu, 30 Jun 2022 12:03:53 +0200 Subject: [PATCH] Swift: cleanup output rewriting code --- swift/extractor/SwiftExtractor.cpp | 2 +- swift/extractor/SwiftExtractorConfiguration.h | 31 +++++----- swift/extractor/SwiftOutputRewrite.cpp | 60 +++++++++---------- swift/extractor/main.cpp | 5 -- 4 files changed, 45 insertions(+), 53 deletions(-) diff --git a/swift/extractor/SwiftExtractor.cpp b/swift/extractor/SwiftExtractor.cpp index 175975be8ed..91d25ce9c1b 100644 --- a/swift/extractor/SwiftExtractor.cpp +++ b/swift/extractor/SwiftExtractor.cpp @@ -76,7 +76,7 @@ static void extractDeclarations(const SwiftExtractorConfiguration& config, auto name = getTrapFilename(module, primaryFile); llvm::StringRef filename(name); std::string tempTrapName = filename.str() + '.' + std::to_string(getpid()) + ".trap"; - llvm::SmallString tempTrapPath(config.tempTrapDir); + llvm::SmallString tempTrapPath(config.getTempTrapDir()); llvm::sys::path::append(tempTrapPath, tempTrapName); llvm::StringRef tempTrapParent = llvm::sys::path::parent_path(tempTrapPath); diff --git a/swift/extractor/SwiftExtractorConfiguration.h b/swift/extractor/SwiftExtractorConfiguration.h index 230ee661ac2..3365da5f268 100644 --- a/swift/extractor/SwiftExtractorConfiguration.h +++ b/swift/extractor/SwiftExtractorConfiguration.h @@ -12,26 +12,25 @@ struct SwiftExtractorConfiguration { // A temporary directory that exists during database creation, but is deleted once the DB is // finalized. std::string scratchDir; - // A temporary directory that contains TRAP files before they are moved into their final destination. - // Subdirectory of the scratchDir. - std::string tempTrapDir; - - // VFS (virtual file system) support. - // A temporary directory that contains VFS files used during extraction. - // Subdirectory of the scratchDir. - std::string VFSDir; - // A temporary directory that contains temp VFS files before they moved into VFSDir. - // Subdirectory of the scratchDir. - std::string tempVFSDir; - - // A temporary directory that contains build artifacts generated by the extractor during the - // overall extraction process. - // Subdirectory of the scratchDir. - std::string tempArtifactDir; // The original arguments passed to the extractor. Used for debugging. std::vector frontendOptions; // The patched arguments passed to the swift::performFrontend/ Used for debugging. std::vector patchedFrontendOptions; + + // A temporary directory that contains TRAP files before they are moved into their final + // destination. + std::string getTempTrapDir() const { return scratchDir + "/swift-trap-temp"; } + + // VFS (virtual file system) support. + // A temporary directory that contains VFS files used during extraction. + std::string getVFSDir() const { return scratchDir + "/swift-vfs"; } + + // A temporary directory that contains temp VFS files before they moved into VFSDir. + std::string getTempVFSDir() const { return scratchDir + "/swift-vfs-temp"; } + + // A temporary directory that contains build artifacts generated by the extractor during the + // overall extraction process. + std::string getTempArtifactDir() const { return scratchDir + "/swift-extraction-artifacts"; } }; } // namespace codeql diff --git a/swift/extractor/SwiftOutputRewrite.cpp b/swift/extractor/SwiftOutputRewrite.cpp index 978f8417f81..35a38512ff8 100644 --- a/swift/extractor/SwiftOutputRewrite.cpp +++ b/swift/extractor/SwiftOutputRewrite.cpp @@ -19,11 +19,12 @@ static std::optional rewriteOutputFileMap( const std::string& outputFileMapPath, const std::vector& inputs, std::unordered_map& remapping) { - auto newPath = config.tempArtifactDir + '/' + outputFileMapPath; + auto newMapPath = config.getTempArtifactDir() + '/' + outputFileMapPath; // TODO: do not assume absolute path for the second parameter auto outputMapOrError = swift::OutputFileMap::loadFromPath(outputFileMapPath, ""); if (!outputMapOrError) { + std::cerr << "Cannot load output map: '" << outputFileMapPath << "'\n"; return std::nullopt; } auto oldOutputMap = outputMapOrError.get(); @@ -39,13 +40,13 @@ static std::optional rewriteOutputFileMap( newMap.copyFrom(*oldMap); for (auto& entry : newMap) { auto oldPath = entry.getSecond(); - auto newPath = config.tempArtifactDir + '/' + oldPath; + auto newPath = config.getTempArtifactDir() + '/' + oldPath; entry.getSecond() = newPath; remapping[oldPath] = newPath; } } std::error_code ec; - llvm::SmallString filepath(newPath); + llvm::SmallString filepath(newMapPath); llvm::StringRef parent = llvm::sys::path::parent_path(filepath); if (std::error_code ec = llvm::sys::fs::create_directories(parent)) { std::cerr << "Cannot create relocated output map dir: '" << parent.str() @@ -53,9 +54,9 @@ static std::optional rewriteOutputFileMap( return std::nullopt; } - llvm::raw_fd_ostream fd(newPath, ec, llvm::sys::fs::OF_None); + llvm::raw_fd_ostream fd(newMapPath, ec, llvm::sys::fs::OF_None); newOutputMap.write(fd, keys); - return newPath; + return newMapPath; } // This is an Xcode-specific workaround to produce alias names for an existing .swiftmodule file. @@ -132,30 +133,27 @@ static std::vector computeModuleAliases(llvm::StringRef modulePath, relocatedModulePath += "/Products/"; relocatedModulePath += destinationDir + '/'; - std::vector moduleLocations; + // clang-format off + std::vector moduleLocations = { + // First case + relocatedModulePath + moduleNameWithExt.str() + '/', + // Second case + relocatedModulePath + moduleName.str() + '/' + moduleNameWithExt.str() + '/', + // Third case + relocatedModulePath + moduleName.str() + '/' + moduleName.str() + ".framework/Modules/" + moduleNameWithExt.str() + '/', + }; + // clang-format on - std::string firstCase = relocatedModulePath; - firstCase += moduleNameWithExt.str() + '/'; - moduleLocations.push_back(firstCase); - - std::string secondCase = relocatedModulePath; - secondCase += moduleName.str() + '/'; - secondCase += moduleNameWithExt.str() + '/'; - moduleLocations.push_back(secondCase); - - std::string thirdCase = relocatedModulePath; - thirdCase += moduleName.str() + '/'; - thirdCase += moduleName.str() + ".framework/Modules/"; - thirdCase += moduleNameWithExt.str() + '/'; - moduleLocations.push_back(thirdCase); + std::vector archs({arch}); + if (!targetTriple.empty()) { + llvm::Triple triple(targetTriple); + archs.push_back(swift::getTargetSpecificModuleTriple(triple).normalize()); + } std::vector aliases; for (auto& location : moduleLocations) { - aliases.push_back(location + arch + ".swiftmodule"); - if (!targetTriple.empty()) { - llvm::Triple triple(targetTriple); - auto moduleTriple = swift::getTargetSpecificModuleTriple(triple); - aliases.push_back(location + moduleTriple.normalize() + ".swiftmodule"); + for (auto& a : archs) { + aliases.push_back(location + a + ".swiftmodule"); } } @@ -195,7 +193,7 @@ std::unordered_map rewriteOutputsInPlace( for (size_t i = 0; i < CLIArgs.size(); i++) { if (pathRewriteOptions.count(CLIArgs[i])) { auto oldPath = CLIArgs[i + 1]; - auto newPath = config.tempArtifactDir + '/' + oldPath; + auto newPath = config.getTempArtifactDir() + '/' + oldPath; CLIArgs[++i] = newPath; newLocations.push_back(newPath); @@ -261,12 +259,12 @@ void storeRemappingForVFS(const SwiftExtractorConfiguration& config, return; } - if (std::error_code ec = llvm::sys::fs::create_directories(config.tempVFSDir)) { + if (std::error_code ec = llvm::sys::fs::create_directories(config.getTempVFSDir())) { std::cerr << "Cannot create temp VFS directory: " << ec.message() << "\n"; return; } - if (std::error_code ec = llvm::sys::fs::create_directories(config.VFSDir)) { + if (std::error_code ec = llvm::sys::fs::create_directories(config.getVFSDir())) { std::cerr << "Cannot create VFS directory: " << ec.message() << "\n"; return; } @@ -274,7 +272,7 @@ void storeRemappingForVFS(const SwiftExtractorConfiguration& config, // Constructing the VFS yaml file in a temp folder so that the other process doesn't read it // while it is not complete // TODO: Pick a more robust way to not collide with files from other processes - auto tempVfsPath = config.tempVFSDir + '/' + std::to_string(getpid()) + "-vfs.yaml"; + auto tempVfsPath = config.getTempVFSDir() + '/' + std::to_string(getpid()) + "-vfs.yaml"; std::error_code ec; llvm::raw_fd_ostream fd(tempVfsPath, ec, llvm::sys::fs::OF_None); if (ec) { @@ -299,7 +297,7 @@ void storeRemappingForVFS(const SwiftExtractorConfiguration& config, fd << "}\n"; fd.flush(); - auto vfsPath = config.VFSDir + '/' + std::to_string(getpid()) + "-vfs.yaml"; + auto vfsPath = config.getVFSDir() + '/' + std::to_string(getpid()) + "-vfs.yaml"; if (std::error_code ec = llvm::sys::fs::rename(tempVfsPath, vfsPath)) { std::cerr << "Cannot move temp VFS file '" << tempVfsPath << "' -> '" << vfsPath << "': " << ec.message() << "\n"; @@ -308,7 +306,7 @@ void storeRemappingForVFS(const SwiftExtractorConfiguration& config, } std::vector collectVFSFiles(const SwiftExtractorConfiguration& config) { - auto vfsDir = config.VFSDir + '/'; + auto vfsDir = config.getVFSDir() + '/'; if (!llvm::sys::fs::exists(vfsDir)) { return {}; } diff --git a/swift/extractor/main.cpp b/swift/extractor/main.cpp index 59bbfa690f0..bde37b0ccb5 100644 --- a/swift/extractor/main.cpp +++ b/swift/extractor/main.cpp @@ -58,11 +58,6 @@ int main(int argc, char** argv) { configuration.sourceArchiveDir = getenv_or("CODEQL_EXTRACTOR_SWIFT_SOURCE_ARCHIVE_DIR", "."); configuration.scratchDir = getenv_or("CODEQL_EXTRACTOR_SWIFT_SCRATCH_DIR", "."); - configuration.tempTrapDir = configuration.scratchDir + "/swift-trap-temp"; - configuration.VFSDir = configuration.scratchDir + "/swift-vfs"; - configuration.tempVFSDir = configuration.scratchDir + "/swift-vfs-temp"; - configuration.tempArtifactDir = configuration.scratchDir + "/swift-extraction-artifacts"; - configuration.frontendOptions.reserve(argc - 1); for (int i = 1; i < argc; i++) { configuration.frontendOptions.push_back(argv[i]);