[clang] 9f3fdb0 - Revert "[Driver] Use VFS to check if sanitizer blacklists exist"

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 21 02:32:10 PST 2019


Author: Ilya Biryukov
Date: 2019-11-21T11:31:14+01:00
New Revision: 9f3fdb0d7fab73083e354768eb5808597474e1b8

URL: https://github.com/llvm/llvm-project/commit/9f3fdb0d7fab73083e354768eb5808597474e1b8
DIFF: https://github.com/llvm/llvm-project/commit/9f3fdb0d7fab73083e354768eb5808597474e1b8.diff

LOG: Revert "[Driver] Use VFS to check if sanitizer blacklists exist"

This reverts commit ba6f906854263375cff3257d22d241a8a259cf77.
Commit caused compilation errors on llvm tests. Will fix and re-land.

Added: 
    

Modified: 
    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
    llvm/include/llvm/Support/SpecialCaseList.h
    llvm/lib/Support/SpecialCaseList.cpp
    llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp

Removed: 
    clang/unittests/Driver/SanitizerArgsTest.cpp


################################################################################
diff  --git a/clang/lib/Basic/SanitizerSpecialCaseList.cpp b/clang/lib/Basic/SanitizerSpecialCaseList.cpp
index 5bf8d39ffd95..7a820d4bef86 100644
--- a/clang/lib/Basic/SanitizerSpecialCaseList.cpp
+++ b/clang/lib/Basic/SanitizerSpecialCaseList.cpp
@@ -20,7 +20,7 @@ SanitizerSpecialCaseList::create(const std::vector<std::string> &Paths,
                                  std::string &Error) {
   std::unique_ptr<clang::SanitizerSpecialCaseList> SSCL(
       new SanitizerSpecialCaseList());
-  if (SSCL->createInternal(Paths, VFS, Error)) {
+  if (SSCL->createInternal(Paths, Error, VFS)) {
     SSCL->createSanitizerSections();
     return SSCL;
   }

diff  --git a/clang/lib/Basic/XRayLists.cpp b/clang/lib/Basic/XRayLists.cpp
index 222a28f79cc5..eb549436710a 100644
--- a/clang/lib/Basic/XRayLists.cpp
+++ b/clang/lib/Basic/XRayLists.cpp
@@ -17,13 +17,10 @@ XRayFunctionFilter::XRayFunctionFilter(
     ArrayRef<std::string> AlwaysInstrumentPaths,
     ArrayRef<std::string> NeverInstrumentPaths,
     ArrayRef<std::string> AttrListPaths, SourceManager &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) {}
+    : AlwaysInstrument(
+          llvm::SpecialCaseList::createOrDie(AlwaysInstrumentPaths)),
+      NeverInstrument(llvm::SpecialCaseList::createOrDie(NeverInstrumentPaths)),
+      AttrList(llvm::SpecialCaseList::createOrDie(AttrListPaths)), SM(SM) {}
 
 XRayFunctionFilter::ImbueAttribute
 XRayFunctionFilter::shouldImbueFunction(StringRef FunctionName) const {

diff  --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp
index ac9a294ee3fa..8937197c253c 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -141,7 +141,7 @@ static void addDefaultBlacklists(const Driver &D, SanitizerMask Kinds,
 
     clang::SmallString<64> Path(D.ResourceDir);
     llvm::sys::path::append(Path, "share", BL.File);
-    if (D.getVFS().exists(Path))
+    if (llvm::sys::fs::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 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
     if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) {
       Arg->claim();
       std::string BLPath = Arg->getValue();
-      if (D.getVFS().exists(BLPath)) {
+      if (llvm::sys::fs::exists(BLPath)) {
         UserBlacklistFiles.push_back(BLPath);
       } else {
         D.Diag(clang::diag::err_drv_no_such_file) << BLPath;
@@ -578,14 +578,14 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   {
     std::string BLError;
     std::unique_ptr<llvm::SpecialCaseList> SCL(
-        llvm::SpecialCaseList::create(UserBlacklistFiles, D.getVFS(), BLError));
+        llvm::SpecialCaseList::create(UserBlacklistFiles, 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, D.getVFS(), BLError));
+    std::unique_ptr<llvm::SpecialCaseList> SCL(
+        llvm::SpecialCaseList::create(SystemBlacklistFiles, BLError));
     if (!SCL.get())
       D.Diag(clang::diag::err_drv_malformed_sanitizer_blacklist) << BLError;
   }

diff  --git a/clang/lib/Driver/XRayArgs.cpp b/clang/lib/Driver/XRayArgs.cpp
index 6011deaccc1f..16e7c7ecf36b 100644
--- a/clang/lib/Driver/XRayArgs.cpp
+++ b/clang/lib/Driver/XRayArgs.cpp
@@ -129,7 +129,7 @@ XRayArgs::XRayArgs(const ToolChain &TC, const ArgList &Args) {
     // are treated as actual dependencies.
     for (const auto &Filename :
          Args.getAllArgValues(options::OPT_fxray_always_instrument)) {
-      if (D.getVFS().exists(Filename)) {
+      if (llvm::sys::fs::exists(Filename)) {
         AlwaysInstrumentFiles.push_back(Filename);
         ExtraDeps.push_back(Filename);
       } else
@@ -138,7 +138,7 @@ XRayArgs::XRayArgs(const ToolChain &TC, const ArgList &Args) {
 
     for (const auto &Filename :
          Args.getAllArgValues(options::OPT_fxray_never_instrument)) {
-      if (D.getVFS().exists(Filename)) {
+      if (llvm::sys::fs::exists(Filename)) {
         NeverInstrumentFiles.push_back(Filename);
         ExtraDeps.push_back(Filename);
       } else
@@ -147,7 +147,7 @@ XRayArgs::XRayArgs(const ToolChain &TC, const ArgList &Args) {
 
     for (const auto &Filename :
          Args.getAllArgValues(options::OPT_fxray_attr_list)) {
-      if (D.getVFS().exists(Filename)) {
+      if (llvm::sys::fs::exists(Filename)) {
         AttrListFiles.push_back(Filename);
         ExtraDeps.push_back(Filename);
       } else

diff  --git a/clang/unittests/Driver/CMakeLists.txt b/clang/unittests/Driver/CMakeLists.txt
index d0ab87bd9ea8..55b8a74830f4 100644
--- a/clang/unittests/Driver/CMakeLists.txt
+++ b/clang/unittests/Driver/CMakeLists.txt
@@ -9,12 +9,10 @@ add_clang_unittest(ClangDriverTests
   ToolChainTest.cpp
   ModuleCacheTest.cpp
   MultilibTest.cpp
-  SanitizerArgsTest.cpp
   )
 
 clang_target_link_libraries(ClangDriverTests
   PRIVATE
   clangDriver
   clangBasic
-  clangFrontend # For TextDiagnosticPrinter.
   )

diff  --git a/clang/unittests/Driver/SanitizerArgsTest.cpp b/clang/unittests/Driver/SanitizerArgsTest.cpp
deleted file mode 100644
index 164bc68051f6..000000000000
--- a/clang/unittests/Driver/SanitizerArgsTest.cpp
+++ /dev/null
@@ -1,141 +0,0 @@
-//===- 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/SmallString.h"
-#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Host.h"
-#include "llvm/Support/MemoryBuffer.h"
-#include "llvm/Support/Path.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 *ClangBinary = "clang";
-static constexpr const char *InputFile = "/sources/foo.c";
-
-std::string concatPaths(llvm::ArrayRef<StringRef> Components) {
-  llvm::SmallString<128> P;
-  for (StringRef C : Components)
-    llvm::sys::path::append(P, C);
-  return P.str().str();
-}
-
-class SanitizerArgsTest : public ::testing::Test {
-protected:
-  const Command &emulateSingleCompilation(std::vector<std::string> ExtraArgs,
-                                          std::vector<std::string> ExtraFiles) {
-    assert(!Driver && "Running twice is not allowed");
-
-    llvm::IntrusiveRefCntPtr<DiagnosticOptions> Opts = new DiagnosticOptions;
-    DiagnosticsEngine Diags(
-        new DiagnosticIDs, Opts,
-        new TextDiagnosticPrinter(llvm::errs(), Opts.get()));
-    Driver.emplace(ClangBinary, "x86_64-unknown-linux-gnu", Diags,
-                   prepareFS(ExtraFiles));
-
-    std::vector<const char *> Args = {ClangBinary};
-    for (const auto &A : ExtraArgs)
-      Args.push_back(A.c_str());
-    Args.push_back("-c");
-    Args.push_back(InputFile);
-
-    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(ClangBinary, 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 std::string ResourceDir = "/opt/llvm/lib/resources";
-  const std::string UserBlacklist = "/source/my_blacklist.txt";
-  const std::string ASanBlacklist =
-      concatPaths({ResourceDir, "share", "asan_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

diff  --git a/llvm/include/llvm/Support/SpecialCaseList.h b/llvm/include/llvm/Support/SpecialCaseList.h
index 5b5b7f6124d6..74a7a45312aa 100644
--- a/llvm/include/llvm/Support/SpecialCaseList.h
+++ b/llvm/include/llvm/Support/SpecialCaseList.h
@@ -69,8 +69,7 @@ class SpecialCaseList {
   /// 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, llvm::vfs::FileSystem &FS,
-         std::string &Error);
+  create(const std::vector<std::string> &Paths, 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,
@@ -78,7 +77,7 @@ class SpecialCaseList {
   /// 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, llvm::vfs::FileSystem &FS);
+  createOrDie(const std::vector<std::string> &Paths);
 
   ~SpecialCaseList();
 
@@ -104,8 +103,8 @@ class SpecialCaseList {
 protected:
   // Implementations of the create*() functions that can also be used by derived
   // classes.
-  bool createInternal(const std::vector<std::string> &Paths,
-                      vfs::FileSystem &VFS, std::string &Error);
+  bool createInternal(const std::vector<std::string> &Paths, std::string &Error,
+                      vfs::FileSystem &VFS = *vfs::getRealFileSystem());
   bool createInternal(const MemoryBuffer *MB, std::string &Error);
 
   SpecialCaseList() = default;

diff  --git a/llvm/lib/Support/SpecialCaseList.cpp b/llvm/lib/Support/SpecialCaseList.cpp
index d1ff44cefb08..5812f075aa47 100644
--- a/llvm/lib/Support/SpecialCaseList.cpp
+++ b/llvm/lib/Support/SpecialCaseList.cpp
@@ -18,7 +18,6 @@
 #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>
@@ -72,9 +71,9 @@ unsigned SpecialCaseList::Matcher::match(StringRef Query) const {
 
 std::unique_ptr<SpecialCaseList>
 SpecialCaseList::create(const std::vector<std::string> &Paths,
-                        llvm::vfs::FileSystem &FS, std::string &Error) {
+                        std::string &Error) {
   std::unique_ptr<SpecialCaseList> SCL(new SpecialCaseList());
-  if (SCL->createInternal(Paths, FS, Error))
+  if (SCL->createInternal(Paths, Error))
     return SCL;
   return nullptr;
 }
@@ -88,16 +87,15 @@ std::unique_ptr<SpecialCaseList> SpecialCaseList::create(const MemoryBuffer *MB,
 }
 
 std::unique_ptr<SpecialCaseList>
-SpecialCaseList::createOrDie(const std::vector<std::string> &Paths,
-                             llvm::vfs::FileSystem &FS) {
+SpecialCaseList::createOrDie(const std::vector<std::string> &Paths) {
   std::string Error;
-  if (auto SCL = create(Paths, FS, Error))
+  if (auto SCL = create(Paths, Error))
     return SCL;
   report_fatal_error(Error);
 }
 
 bool SpecialCaseList::createInternal(const std::vector<std::string> &Paths,
-                                     vfs::FileSystem &VFS, std::string &Error) {
+                                     std::string &Error, vfs::FileSystem &VFS) {
   StringMap<size_t> Sections;
   for (const auto &Path : Paths) {
     ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr =

diff  --git a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
index cf9a6a321c7a..6f5ec3d08527 100644
--- a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -88,7 +88,6 @@
 #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"
@@ -481,9 +480,7 @@ DataFlowSanitizer::DataFlowSanitizer(
   std::vector<std::string> AllABIListFiles(std::move(ABIListFiles));
   AllABIListFiles.insert(AllABIListFiles.end(), ClABIListFiles.begin(),
                          ClABIListFiles.end());
-  // FIXME: should we propagate vfs::FileSystem to this constructor?
-  ABIList.set(
-      SpecialCaseList::createOrDie(AllABIListFiles, *vfs::getRealFileSystem()));
+  ABIList.set(SpecialCaseList::createOrDie(AllABIListFiles));
 }
 
 FunctionType *DataFlowSanitizer::getArgsFunctionType(FunctionType *T) {


        


More information about the cfe-commits mailing list