ilya-biryukov created this revision. ilya-biryukov added a reviewer: gribozavr. Herald added subscribers: llvm-commits, hiraditya, mgorny. Herald added projects: clang, LLVM.
This is a follow-up to 590f279c456bbde632eca8ef89a85c478f15a249 <https://reviews.llvm.org/rG590f279c456bbde632eca8ef89a85c478f15a249>, which moved some of the callers to use VFS. It turned out more code in Driver calls into real filesystem APIs and also needs an update. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D70440 Files: clang/lib/Basic/SanitizerSpecialCaseList.cpp clang/lib/Basic/XRayLists.cpp clang/lib/Driver/SanitizerArgs.cpp clang/lib/Driver/XRayArgs.cpp clang/unittests/Driver/CMakeLists.txt clang/unittests/Driver/SanitizerArgsTest.cpp llvm/include/llvm/Support/SpecialCaseList.h llvm/lib/Support/SpecialCaseList.cpp llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
Index: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp =================================================================== --- llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp +++ llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp @@ -88,6 +88,7 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/SpecialCaseList.h" +#include "llvm/Support/VirtualFileSystem.h" #include "llvm/Transforms/Instrumentation.h" #include "llvm/Transforms/Utils/BasicBlockUtils.h" #include "llvm/Transforms/Utils/Local.h" @@ -480,7 +481,9 @@ std::vector<std::string> AllABIListFiles(std::move(ABIListFiles)); AllABIListFiles.insert(AllABIListFiles.end(), ClABIListFiles.begin(), ClABIListFiles.end()); - ABIList.set(SpecialCaseList::createOrDie(AllABIListFiles)); + // FIXME: should we propagate vfs::FileSystem to this constructor? + ABIList.set( + SpecialCaseList::createOrDie(AllABIListFiles, *vfs::getRealFileSystem())); } FunctionType *DataFlowSanitizer::getArgsFunctionType(FunctionType *T) { Index: llvm/lib/Support/SpecialCaseList.cpp =================================================================== --- llvm/lib/Support/SpecialCaseList.cpp +++ llvm/lib/Support/SpecialCaseList.cpp @@ -18,6 +18,7 @@ #include "llvm/ADT/StringExtras.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Regex.h" +#include "llvm/Support/VirtualFileSystem.h" #include <string> #include <system_error> #include <utility> @@ -71,9 +72,9 @@ std::unique_ptr<SpecialCaseList> SpecialCaseList::create(const std::vector<std::string> &Paths, - std::string &Error) { + llvm::vfs::FileSystem &FS, std::string &Error) { std::unique_ptr<SpecialCaseList> SCL(new SpecialCaseList()); - if (SCL->createInternal(Paths, Error)) + if (SCL->createInternal(Paths, FS, Error)) return SCL; return nullptr; } @@ -87,15 +88,16 @@ } std::unique_ptr<SpecialCaseList> -SpecialCaseList::createOrDie(const std::vector<std::string> &Paths) { +SpecialCaseList::createOrDie(const std::vector<std::string> &Paths, + llvm::vfs::FileSystem &FS) { std::string Error; - if (auto SCL = create(Paths, Error)) + if (auto SCL = create(Paths, FS, Error)) return SCL; report_fatal_error(Error); } bool SpecialCaseList::createInternal(const std::vector<std::string> &Paths, - std::string &Error, vfs::FileSystem &VFS) { + vfs::FileSystem &VFS, std::string &Error) { StringMap<size_t> Sections; for (const auto &Path : Paths) { ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr = Index: llvm/include/llvm/Support/SpecialCaseList.h =================================================================== --- llvm/include/llvm/Support/SpecialCaseList.h +++ llvm/include/llvm/Support/SpecialCaseList.h @@ -69,7 +69,8 @@ /// Parses the special case list entries from files. On failure, returns /// 0 and writes an error message to string. static std::unique_ptr<SpecialCaseList> - create(const std::vector<std::string> &Paths, std::string &Error); + create(const std::vector<std::string> &Paths, llvm::vfs::FileSystem &FS, + std::string &Error); /// Parses the special case list from a memory buffer. On failure, returns /// 0 and writes an error message to string. static std::unique_ptr<SpecialCaseList> create(const MemoryBuffer *MB, @@ -77,7 +78,7 @@ /// Parses the special case list entries from files. On failure, reports a /// fatal error. static std::unique_ptr<SpecialCaseList> - createOrDie(const std::vector<std::string> &Paths); + createOrDie(const std::vector<std::string> &Paths, llvm::vfs::FileSystem &FS); ~SpecialCaseList(); @@ -103,8 +104,8 @@ protected: // Implementations of the create*() functions that can also be used by derived // classes. - bool createInternal(const std::vector<std::string> &Paths, std::string &Error, - vfs::FileSystem &VFS = *vfs::getRealFileSystem()); + bool createInternal(const std::vector<std::string> &Paths, + vfs::FileSystem &VFS, std::string &Error); bool createInternal(const MemoryBuffer *MB, std::string &Error); SpecialCaseList() = default; Index: clang/unittests/Driver/SanitizerArgsTest.cpp =================================================================== --- /dev/null +++ clang/unittests/Driver/SanitizerArgsTest.cpp @@ -0,0 +1,132 @@ +//===- unittests/Driver/SanitizerArgsTest.cpp -----------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticIDs.h" +#include "clang/Basic/DiagnosticOptions.h" +#include "clang/Driver/Compilation.h" +#include "clang/Driver/Driver.h" +#include "clang/Driver/Job.h" +#include "clang/Frontend/TextDiagnosticPrinter.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Host.h" +#include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/VirtualFileSystem.h" +#include "llvm/Support/raw_ostream.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include <cstdlib> +#include <memory> +#include <string> +using namespace clang; +using namespace clang::driver; + +using ::testing::Contains; +using ::testing::StrEq; + +namespace { + +static constexpr const char *ClangPath = "/opt/llvm/bin/clang"; +static constexpr const char *InputFile = "/sources/foo.c"; + +class SanitizerArgsTest : public ::testing::Test { +protected: + const Command &emulateSingleCompilation(std::vector<std::string> ExtraArgs, + std::vector<std::string> ExtraFiles) { + assert(!Driver && "Running emulateCompilation() twice is not allowed"); + + llvm::IntrusiveRefCntPtr<DiagnosticOptions> Opts = new DiagnosticOptions; + DiagnosticsEngine Diags( + new DiagnosticIDs, Opts, + new TextDiagnosticPrinter(llvm::errs(), Opts.get())); + Driver.emplace(ClangPath, "x86_64-unknown-linux-gnu", Diags, + prepareFS(ExtraFiles)); + + std::vector<const char *> Args = {ClangPath, "-c", InputFile}; + for (const auto &A : ExtraArgs) + Args.push_back(A.c_str()); + + Compilation.reset(Driver->BuildCompilation(Args)); + + if (Diags.hasErrorOccurred()) + ADD_FAILURE() << "Error occurred while parsing compilation arguments. " + "See stderr for details."; + + const auto &Commands = Compilation->getJobs().getJobs(); + assert(Commands.size() == 1); + return *Commands.front(); + } + +private: + llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> + prepareFS(llvm::ArrayRef<std::string> ExtraFiles) { + + llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS = + new llvm::vfs::InMemoryFileSystem; + FS->addFile(ClangPath, time_t(), llvm::MemoryBuffer::getMemBuffer("")); + FS->addFile(InputFile, time_t(), llvm::MemoryBuffer::getMemBuffer("")); + for (llvm::StringRef F : ExtraFiles) + FS->addFile(F, time_t(), llvm::MemoryBuffer::getMemBuffer("")); + + return FS; + } + + llvm::Optional<Driver> Driver; + std::unique_ptr<driver::Compilation> Compilation; +}; + +TEST_F(SanitizerArgsTest, Blacklists) { + const char *ResourceDir = "/opt/llvm/lib/resources"; + const char *ASanBlacklist = + "/opt/llvm/lib/resources/share/asan_blacklist.txt"; + const char *UserBlacklist = "/source/my_blacklist.txt"; + + auto &Command = emulateSingleCompilation( + /*ExtraArgs=*/{"-fsanitize=address", "-resource-dir", ResourceDir, + std::string("-fsanitize-blacklist=") + UserBlacklist}, + /*ExtraFiles=*/{ASanBlacklist, UserBlacklist}); + + // System blacklists are added based on resource-dir. + EXPECT_THAT(Command.getArguments(), + Contains(StrEq(std::string("-fsanitize-system-blacklist=") + + ASanBlacklist))); + // User blacklists should also be added. + EXPECT_THAT( + Command.getArguments(), + Contains(StrEq(std::string("-fsanitize-blacklist=") + UserBlacklist))); +} + +TEST_F(SanitizerArgsTest, XRayLists) { + const std::string XRayWhitelist = "/source/xray_whitelist.txt"; + const std::string XRayBlacklist = "/source/xray_blacklist.txt"; + const std::string XRayAttrList = "/source/xray_attr_list.txt"; + + auto &Command = emulateSingleCompilation( + /*ExtraArgs=*/ + { + "-fxray-instrument", + "-fxray-always-instrument=" + XRayWhitelist, + "-fxray-never-instrument=" + XRayBlacklist, + "-fxray-attr-list=" + XRayAttrList, + }, + /*ExtraFiles=*/{XRayWhitelist, XRayBlacklist, XRayAttrList}); + + // Blacklists exist in the filesystem, so they should be added to the + // compilation command, produced by the driver. + EXPECT_THAT(Command.getArguments(), + Contains(StrEq("-fxray-always-instrument=" + XRayWhitelist))); + EXPECT_THAT(Command.getArguments(), + Contains(StrEq("-fxray-never-instrument=" + XRayBlacklist))); + EXPECT_THAT(Command.getArguments(), + Contains(StrEq("-fxray-attr-list=" + XRayAttrList))); +} + +} // namespace \ No newline at end of file Index: clang/unittests/Driver/CMakeLists.txt =================================================================== --- clang/unittests/Driver/CMakeLists.txt +++ clang/unittests/Driver/CMakeLists.txt @@ -9,10 +9,12 @@ ToolChainTest.cpp ModuleCacheTest.cpp MultilibTest.cpp + SanitizerArgsTest.cpp ) clang_target_link_libraries(ClangDriverTests PRIVATE clangDriver clangBasic + clangFrontend # For TextDiagnosticPrinter. ) Index: clang/lib/Driver/XRayArgs.cpp =================================================================== --- clang/lib/Driver/XRayArgs.cpp +++ clang/lib/Driver/XRayArgs.cpp @@ -129,7 +129,7 @@ // are treated as actual dependencies. for (const auto &Filename : Args.getAllArgValues(options::OPT_fxray_always_instrument)) { - if (llvm::sys::fs::exists(Filename)) { + if (D.getVFS().exists(Filename)) { AlwaysInstrumentFiles.push_back(Filename); ExtraDeps.push_back(Filename); } else @@ -138,7 +138,7 @@ for (const auto &Filename : Args.getAllArgValues(options::OPT_fxray_never_instrument)) { - if (llvm::sys::fs::exists(Filename)) { + if (D.getVFS().exists(Filename)) { NeverInstrumentFiles.push_back(Filename); ExtraDeps.push_back(Filename); } else @@ -147,7 +147,7 @@ for (const auto &Filename : Args.getAllArgValues(options::OPT_fxray_attr_list)) { - if (llvm::sys::fs::exists(Filename)) { + if (D.getVFS().exists(Filename)) { AttrListFiles.push_back(Filename); ExtraDeps.push_back(Filename); } else Index: clang/lib/Driver/SanitizerArgs.cpp =================================================================== --- clang/lib/Driver/SanitizerArgs.cpp +++ clang/lib/Driver/SanitizerArgs.cpp @@ -141,7 +141,7 @@ clang::SmallString<64> Path(D.ResourceDir); llvm::sys::path::append(Path, "share", BL.File); - if (llvm::sys::fs::exists(Path)) + if (D.getVFS().exists(Path)) BlacklistFiles.push_back(Path.str()); else if (BL.Mask == SanitizerKind::CFI) // If cfi_blacklist.txt cannot be found in the resource dir, driver @@ -563,7 +563,7 @@ if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) { Arg->claim(); std::string BLPath = Arg->getValue(); - if (llvm::sys::fs::exists(BLPath)) { + if (D.getVFS().exists(BLPath)) { UserBlacklistFiles.push_back(BLPath); } else { D.Diag(clang::diag::err_drv_no_such_file) << BLPath; @@ -578,14 +578,14 @@ { std::string BLError; std::unique_ptr<llvm::SpecialCaseList> SCL( - llvm::SpecialCaseList::create(UserBlacklistFiles, BLError)); + llvm::SpecialCaseList::create(UserBlacklistFiles, D.getVFS(), BLError)); if (!SCL.get()) D.Diag(clang::diag::err_drv_malformed_sanitizer_blacklist) << BLError; } { std::string BLError; - std::unique_ptr<llvm::SpecialCaseList> SCL( - llvm::SpecialCaseList::create(SystemBlacklistFiles, BLError)); + std::unique_ptr<llvm::SpecialCaseList> SCL(llvm::SpecialCaseList::create( + SystemBlacklistFiles, D.getVFS(), BLError)); if (!SCL.get()) D.Diag(clang::diag::err_drv_malformed_sanitizer_blacklist) << BLError; } Index: clang/lib/Basic/XRayLists.cpp =================================================================== --- clang/lib/Basic/XRayLists.cpp +++ clang/lib/Basic/XRayLists.cpp @@ -17,10 +17,13 @@ ArrayRef<std::string> AlwaysInstrumentPaths, ArrayRef<std::string> NeverInstrumentPaths, ArrayRef<std::string> AttrListPaths, SourceManager &SM) - : AlwaysInstrument( - llvm::SpecialCaseList::createOrDie(AlwaysInstrumentPaths)), - NeverInstrument(llvm::SpecialCaseList::createOrDie(NeverInstrumentPaths)), - AttrList(llvm::SpecialCaseList::createOrDie(AttrListPaths)), SM(SM) {} + : AlwaysInstrument(llvm::SpecialCaseList::createOrDie( + AlwaysInstrumentPaths, SM.getFileManager().getVirtualFileSystem())), + NeverInstrument(llvm::SpecialCaseList::createOrDie( + NeverInstrumentPaths, SM.getFileManager().getVirtualFileSystem())), + AttrList(llvm::SpecialCaseList::createOrDie( + AttrListPaths, SM.getFileManager().getVirtualFileSystem())), + SM(SM) {} XRayFunctionFilter::ImbueAttribute XRayFunctionFilter::shouldImbueFunction(StringRef FunctionName) const { Index: clang/lib/Basic/SanitizerSpecialCaseList.cpp =================================================================== --- clang/lib/Basic/SanitizerSpecialCaseList.cpp +++ clang/lib/Basic/SanitizerSpecialCaseList.cpp @@ -20,7 +20,7 @@ std::string &Error) { std::unique_ptr<clang::SanitizerSpecialCaseList> SSCL( new SanitizerSpecialCaseList()); - if (SSCL->createInternal(Paths, Error, VFS)) { + if (SSCL->createInternal(Paths, VFS, Error)) { SSCL->createSanitizerSections(); return SSCL; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits