From 1ba8b6d35a7ae480c4a91c099dd52f02199fab54 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 8 Dec 2022 15:57:57 +0100 Subject: [PATCH 1/3] Swift: fix extraction of sources from `..` --- swift/extractor/SwiftExtractor.cpp | 24 ++++++++----------- swift/extractor/infra/Path.h | 7 ------ .../infra/SwiftLocationExtractor.cpp | 6 ++--- swift/extractor/infra/{ => file}/Path.cpp | 6 ++--- swift/extractor/infra/file/Path.h | 7 ++++++ .../frontend-invocations/Files.expected | 1 + .../posix-only/frontend-invocations/G.swift | 0 .../posix-only/frontend-invocations/build.sh | 1 + 8 files changed, 25 insertions(+), 27 deletions(-) delete mode 100644 swift/extractor/infra/Path.h rename swift/extractor/infra/{ => file}/Path.cpp (87%) create mode 100644 swift/extractor/infra/file/Path.h create mode 100644 swift/integration-tests/posix-only/frontend-invocations/G.swift diff --git a/swift/extractor/SwiftExtractor.cpp b/swift/extractor/SwiftExtractor.cpp index c9410d2f1f3..2cee41cb0e3 100644 --- a/swift/extractor/SwiftExtractor.cpp +++ b/swift/extractor/SwiftExtractor.cpp @@ -12,7 +12,7 @@ #include "swift/extractor/translators/SwiftVisitor.h" #include "swift/extractor/TargetTrapFile.h" #include "swift/extractor/SwiftBuiltinSymbols.h" -#include "swift/extractor/infra/Path.h" +#include "swift/extractor/infra/file/Path.h" using namespace codeql; using namespace std::string_literals; @@ -28,27 +28,23 @@ static void ensureDirectory(const char* label, const fs::path& dir) { } static void archiveFile(const SwiftExtractorConfiguration& config, swift::SourceFile& file) { - ensureDirectory("TRAP", config.trapDir); - ensureDirectory("source archive", config.sourceArchiveDir); + auto source = codeql::resolvePath(file.getFilename()); + auto destination = config.sourceArchiveDir / source.relative_path(); - fs::path srcFilePath = codeql::getCodeQLPath(file.getFilename()); - auto dstFilePath = config.sourceArchiveDir; - dstFilePath += srcFilePath; - - ensureDirectory("source archive destination", dstFilePath.parent_path()); + ensureDirectory("source archive destination", destination.parent_path()); std::error_code ec; - fs::copy(srcFilePath, dstFilePath, fs::copy_options::overwrite_existing, ec); + fs::copy(source, destination, fs::copy_options::overwrite_existing, ec); if (ec) { - std::cerr << "Cannot archive source file " << srcFilePath << " -> " << dstFilePath << ": " + std::cerr << "Cannot archive source file " << source << " -> " << destination << ": " << ec.message() << "\n"; } } static fs::path getFilename(swift::ModuleDecl& module, swift::SourceFile* primaryFile) { if (primaryFile) { - return primaryFile->getFilename().str(); + return resolvePath(primaryFile->getFilename()); } // PCM clang module if (module.isNonSwiftModule()) { @@ -57,7 +53,7 @@ static fs::path getFilename(swift::ModuleDecl& module, swift::SourceFile* primar // Moreover, pcm files may come from caches located in different directories, but are // unambiguously identified by the base file name, so we can discard the absolute directory fs::path filename = "/pcms"; - filename /= getCodeQLPath(module.getModuleFilename()).filename(); + filename /= fs::path{std::string_view{module.getModuleFilename()}}.filename(); filename += "-"; filename += module.getName().str(); return filename; @@ -66,13 +62,13 @@ static fs::path getFilename(swift::ModuleDecl& module, swift::SourceFile* primar // The Builtin module has an empty filename, let's fix that return "/__Builtin__"; } - auto filename = getCodeQLPath(module.getModuleFilename()); + std::string_view filename = module.getModuleFilename(); // there is a special case of a module without an actual filename reporting ``: in this // case we want to avoid the `<>` characters, in case a dirty DB is imported on Windows if (filename == "") { return "/__imports__"; } - return filename; + return resolvePath(filename); } /* The builtin module is special, as it does not publish any top-level declaration diff --git a/swift/extractor/infra/Path.h b/swift/extractor/infra/Path.h deleted file mode 100644 index 09ab7ed1773..00000000000 --- a/swift/extractor/infra/Path.h +++ /dev/null @@ -1,7 +0,0 @@ -#pragma once - -#include - -namespace codeql { -std::filesystem::path getCodeQLPath(std::string_view path); -} diff --git a/swift/extractor/infra/SwiftLocationExtractor.cpp b/swift/extractor/infra/SwiftLocationExtractor.cpp index a003519f4ce..7c98294804a 100644 --- a/swift/extractor/infra/SwiftLocationExtractor.cpp +++ b/swift/extractor/infra/SwiftLocationExtractor.cpp @@ -5,7 +5,7 @@ #include "swift/extractor/trap/generated/TrapEntries.h" #include "swift/extractor/trap/generated/TrapClasses.h" #include "swift/extractor/infra/SwiftLocationExtractor.h" -#include "swift/extractor/infra/Path.h" +#include "swift/extractor/infra/file/Path.h" using namespace codeql; @@ -17,7 +17,7 @@ void SwiftLocationExtractor::attachLocation(const swift::SourceManager& sourceMa // invalid locations seem to come from entities synthesized by the compiler return; } - auto file = getCodeQLPath(sourceManager.getDisplayNameForLoc(start)); + auto file = resolvePath(sourceManager.getDisplayNameForLoc(start)); DbLocation entry{{}}; entry.file = fetchFileLabel(file); std::tie(entry.start_line, entry.start_column) = sourceManager.getLineAndColumnInBuffer(start); @@ -30,7 +30,7 @@ void SwiftLocationExtractor::attachLocation(const swift::SourceManager& sourceMa } void SwiftLocationExtractor::emitFile(llvm::StringRef path) { - fetchFileLabel(getCodeQLPath(path)); + fetchFileLabel(resolvePath(path)); } TrapLabel SwiftLocationExtractor::fetchFileLabel(const std::filesystem::path& file) { diff --git a/swift/extractor/infra/Path.cpp b/swift/extractor/infra/file/Path.cpp similarity index 87% rename from swift/extractor/infra/Path.cpp rename to swift/extractor/infra/file/Path.cpp index 37b4d13fd0c..f114bfdd41b 100644 --- a/swift/extractor/infra/Path.cpp +++ b/swift/extractor/infra/file/Path.cpp @@ -1,4 +1,4 @@ -#include "swift/extractor/infra/Path.h" +#include "swift/extractor/infra/file/Path.h" #include #include @@ -16,7 +16,7 @@ static bool shouldCanonicalize() { return true; } -std::filesystem::path getCodeQLPath(std::string_view path) { +std::filesystem::path resolvePath(std::string_view path) { std::error_code ec; std::filesystem::path ret = {}; static const auto canonicalize = shouldCanonicalize(); @@ -28,7 +28,7 @@ std::filesystem::path getCodeQLPath(std::string_view path) { if (ec) { std::cerr << "Cannot get " << (canonicalize ? "canonical" : "absolute") << " path: " << std::quoted(path) << ": " << ec.message() << "\n"; - return {}; + return path; } return ret; } diff --git a/swift/extractor/infra/file/Path.h b/swift/extractor/infra/file/Path.h new file mode 100644 index 00000000000..c1a31786454 --- /dev/null +++ b/swift/extractor/infra/file/Path.h @@ -0,0 +1,7 @@ +#pragma once + +#include + +namespace codeql { +std::filesystem::path resolvePath(std::string_view path); +} diff --git a/swift/integration-tests/posix-only/frontend-invocations/Files.expected b/swift/integration-tests/posix-only/frontend-invocations/Files.expected index f13e45d89a5..3bc0826c1f2 100644 --- a/swift/integration-tests/posix-only/frontend-invocations/Files.expected +++ b/swift/integration-tests/posix-only/frontend-invocations/Files.expected @@ -5,4 +5,5 @@ | E.swift:0:0:0:0 | E.swift | | F1.swift:0:0:0:0 | F1.swift | | F2.swift:0:0:0:0 | F2.swift | +| G.swift:0:0:0:0 | G.swift | | file://:0:0:0:0 | | diff --git a/swift/integration-tests/posix-only/frontend-invocations/G.swift b/swift/integration-tests/posix-only/frontend-invocations/G.swift new file mode 100644 index 00000000000..e69de29bb2d diff --git a/swift/integration-tests/posix-only/frontend-invocations/build.sh b/swift/integration-tests/posix-only/frontend-invocations/build.sh index 03f1148ca05..31cf6e39969 100755 --- a/swift/integration-tests/posix-only/frontend-invocations/build.sh +++ b/swift/integration-tests/posix-only/frontend-invocations/build.sh @@ -16,3 +16,4 @@ $FRONTEND -frontend -c -primary-file E.swift Esup.swift -o E.o $SDK $FRONTEND -frontend -emit-module -primary-file F1.swift F2.swift -module-name F -o F1.swiftmodule $SDK $FRONTEND -frontend -emit-module F1.swift -primary-file F2.swift -module-name F -o F2.swiftmodule $SDK $FRONTEND -merge-modules F1.swiftmodule F2.swiftmodule -o F.swiftmodule $SDK +( cd dir; $FRONTEND -frontend -c ../G.swift $SDK ) From 935e264f2472cbc6a58aee4c35821d0456d66a48 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 8 Dec 2022 17:04:32 +0100 Subject: [PATCH 2/3] Swift: add empty directory marker --- .../integration-tests/posix-only/frontend-invocations/dir/.empty | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 swift/integration-tests/posix-only/frontend-invocations/dir/.empty diff --git a/swift/integration-tests/posix-only/frontend-invocations/dir/.empty b/swift/integration-tests/posix-only/frontend-invocations/dir/.empty new file mode 100644 index 00000000000..e69de29bb2d From 7645d4d92802b38f7614fc5f3270aee88c03c431 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Thu, 8 Dec 2022 17:31:48 +0100 Subject: [PATCH 3/3] Swift: remove `ModuleDecl` from `PrintAst` test --- swift/ql/test/library-tests/ast/PrintAst.expected | 6 ------ swift/ql/test/library-tests/ast/PrintAst.ql | 4 +++- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/swift/ql/test/library-tests/ast/PrintAst.expected b/swift/ql/test/library-tests/ast/PrintAst.expected index 2ec81962d30..570065b9db1 100644 --- a/swift/ql/test/library-tests/ast/PrintAst.expected +++ b/swift/ql/test/library-tests/ast/PrintAst.expected @@ -1,9 +1,3 @@ -#-----| [ModuleDecl] __ObjC -#-----| [ModuleDecl] cfg -#-----| [ModuleDecl] declarations -#-----| [ModuleDecl] expressions -#-----| [ModuleDecl] patterns -#-----| [ModuleDecl] statements cfg.swift: # 1| [TopLevelCodeDecl] { ... } # 1| getBody(): [BraceStmt] { ... } diff --git a/swift/ql/test/library-tests/ast/PrintAst.ql b/swift/ql/test/library-tests/ast/PrintAst.ql index 245e9ebdb12..05b5fec5a17 100644 --- a/swift/ql/test/library-tests/ast/PrintAst.ql +++ b/swift/ql/test/library-tests/ast/PrintAst.ql @@ -10,5 +10,7 @@ import TestUtils * The hook to customize the entities printed by this query. */ class PrintAstConfigurationOverride extends PrintAstConfiguration { - override predicate shouldPrint(Locatable e) { super.shouldPrint(e) and toBeTested(e) } + override predicate shouldPrint(Locatable e) { + super.shouldPrint(e) and toBeTested(e) and not e instanceof ModuleDecl + } }