Allow to specify multiple -fsanitize-blacklist= arguments.

[samsonov]

Summary

Allow user to provide multiple blacklists by passing several
-fsanitize-blacklist= options. These options now don't override
default blacklist from Clang resource directory, which is always
applied (which fixes PR22431).

-fno-sanitize-blacklist option now disables all blacklists that
were specified earlier in the command line (including the default
one).

Expand the test case to check that both -fsanitize-blacklist= values are handled.

This change depends on http://reviews.llvm.org/D7367.

[rsmith]

LGTM, though I'd like a bit more thorough testing. The current tests would pass, for instance, if we just stopped after the first -fsanitize-blacklist= argument.

[timurrrr]

I agree more tests would be welcome, especially a test like

-fsanitize-blacklist=1.txt -fno-sanitize-blacklist -fsanitize-blacklist=2.txt

that documents the [rather strange to my taste] behavior of -fno-sanitize-blacklist

lib/Driver/SanitizerArgs.cpp

300 ↗

(On Diff #19208)

nit: I find

if (llvm::sys::fs::exists(BLPath))
  BlacklistFiles.push_back(BLPath);
else
  D.Diag(clang::diag::err_drv_no_such_file) << BLPath;

shorter and easier to understand

305 ↗

(On Diff #19208)

very nitty nit: you can fit into 80 chars if you shorten BLArg to say Arg?

[samsonov]

• Address reviewer's comments and add more tests.

[samsonov]

lib/Driver/SanitizerArgs.cpp

300 ↗

(On Diff #19208)

Done

305 ↗

(On Diff #19208)

Done

[timurrrr]

LGTM, thanks!

cfe/trunk/include/clang/Basic/LangOptions.h

 #define LLVM_CLANG_BASIC_LANGOPTIONS_H
 
 #include "clang/Basic/CommentOptions.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Sanitizers.h"
 #include "clang/Basic/Visibility.h"
 #include <string>
+#include <vector>
 
 namespace clang {
 
 /// Bitfields of LangOptions, split out from LangOptions in order to ensure that
 /// this large collection of bitfields is a trivial class type.
 class LangOptionsBase {
 public:
   // Define simple language options (with no accessors).
public:
   };
 
   enum AddrSpaceMapMangling { ASMM_Target, ASMM_On, ASMM_Off };
 
 public:
   /// \brief Set of enabled sanitizers.
   SanitizerSet Sanitize;
 
-  /// \brief Path to blacklist file specifying which objects
+  /// \brief Paths to blacklist files specifying which objects
   /// (files, functions, variables) should not be instrumented.
-  std::string SanitizerBlacklistFile;
+  std::vector<std::string> SanitizerBlacklistFiles;
 
   clang::ObjCRuntime ObjCRuntime;
 
   std::string ObjCConstantStringClass;
   
   /// \brief The name of the handler function to be called when -ftrapv is
   /// specified.
   ///

cfe/trunk/include/clang/Basic/SanitizerBlacklist.h

 
 namespace clang {
 
 class SanitizerBlacklist {
   std::unique_ptr<llvm::SpecialCaseList> SCL;
   SourceManager &SM;
 
 public:
-  SanitizerBlacklist(StringRef BlacklistPath, SourceManager &SM);
+  SanitizerBlacklist(const std::vector<std::string> &BlacklistPaths,
+                     SourceManager &SM);
   bool isBlacklistedGlobal(StringRef GlobalName,
                            StringRef Category = StringRef()) const;
   bool isBlacklistedType(StringRef MangledTypeName,
                          StringRef Category = StringRef()) const;
   bool isBlacklistedFunction(StringRef FunctionName) const;
   bool isBlacklistedFile(StringRef FileName,
                          StringRef Category = StringRef()) const;
   bool isBlacklistedLocation(SourceLocation Loc,
                              StringRef Category = StringRef()) const;
 };
 
 }  // end namespace clang
 
 #endif

cfe/trunk/include/clang/Driver/SanitizerArgs.h

 //===--- SanitizerArgs.h - Arguments for sanitizer tools  -------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_DRIVER_SANITIZERARGS_H
 #define LLVM_CLANG_DRIVER_SANITIZERARGS_H
 
 #include "clang/Basic/Sanitizers.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include <string>
+#include <vector>
 
 namespace clang {
 namespace driver {
 
 class Driver;
 class ToolChain;
 
 class SanitizerArgs {
   SanitizerSet Sanitizers;
   SanitizerSet RecoverableSanitizers;
 
-  std::string BlacklistFile;
+  std::vector<std::string> BlacklistFiles;
   int SanitizeCoverage;
   int MsanTrackOrigins;
   int AsanFieldPadding;
   bool AsanZeroBaseShadow;
   bool UbsanTrapOnError;
   bool AsanSharedRuntime;
   bool LinkCXXRuntimes;
 

cfe/trunk/lib/AST/ASTContext.cpp

: FunctionProtoTypes(this_()), TemplateSpecializationTypes(this_()),
       GlobalNestedNameSpecifier(nullptr), Int128Decl(nullptr),
       UInt128Decl(nullptr), Float128StubDecl(nullptr),
       BuiltinVaListDecl(nullptr), ObjCIdDecl(nullptr), ObjCSelDecl(nullptr),
       ObjCClassDecl(nullptr), ObjCProtocolClassDecl(nullptr), BOOLDecl(nullptr),
       CFConstantStringTypeDecl(nullptr), ObjCInstanceTypeDecl(nullptr),
       FILEDecl(nullptr), jmp_bufDecl(nullptr), sigjmp_bufDecl(nullptr),
       ucontext_tDecl(nullptr), BlockDescriptorType(nullptr),
       BlockDescriptorExtendedType(nullptr), cudaConfigureCallDecl(nullptr),
-      FirstLocalImport(), LastLocalImport(),
-      SourceMgr(SM), LangOpts(LOpts),
-      SanitizerBL(new SanitizerBlacklist(LangOpts.SanitizerBlacklistFile, SM)),
+      FirstLocalImport(), LastLocalImport(), SourceMgr(SM), LangOpts(LOpts),
+      SanitizerBL(new SanitizerBlacklist(LangOpts.SanitizerBlacklistFiles, SM)),
       AddrSpaceMap(nullptr), Target(nullptr), PrintingPolicy(LOpts),
       Idents(idents), Selectors(sels), BuiltinInfo(builtins),
       DeclarationNames(*this), ExternalSource(nullptr), Listener(nullptr),
       Comments(SM), CommentsLoaded(false),
       CommentCommandTraits(BumpAlloc, LOpts.CommentOpts), LastSDM(nullptr, 0) {
   TUDecl = TranslationUnitDecl::Create(*this);
 }
 

cfe/trunk/lib/Basic/LangOptions.cpp

 #define BENIGN_LANGOPT(Name, Bits, Default, Description) Name = Default;
 #define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
   Name = Default;
 #include "clang/Basic/LangOptions.def"
 
   // FIXME: This should not be reset; modules can be different with different
   // sanitizer options (this affects __has_feature(address_sanitizer) etc).
   Sanitize.clear();
-  SanitizerBlacklistFile.clear();
+  SanitizerBlacklistFiles.clear();
 
   CurrentModule.clear();
   ImplementationOfModule.clear();
 }
 

cfe/trunk/lib/Basic/SanitizerBlacklist.cpp

 // User-provided blacklist used to disable/alter instrumentation done in
 // sanitizers.
 //
 //===----------------------------------------------------------------------===//
 #include "clang/Basic/SanitizerBlacklist.h"
 
 using namespace clang;
 
-SanitizerBlacklist::SanitizerBlacklist(StringRef BlacklistPath,
-                                       SourceManager &SM)
-    : SCL(llvm::SpecialCaseList::createOrDie(BlacklistPath)), SM(SM) {}
+SanitizerBlacklist::SanitizerBlacklist(
+    const std::vector<std::string> &BlacklistPaths, SourceManager &SM)
+    : SCL(llvm::SpecialCaseList::createOrDie(BlacklistPaths)), SM(SM) {}
 
 bool SanitizerBlacklist::isBlacklistedGlobal(StringRef GlobalName,
                                              StringRef Category) const {
   return SCL->inSection("global", GlobalName, Category);
 }
 
 bool SanitizerBlacklist::isBlacklistedType(StringRef MangledTypeName,
                                            StringRef Category) const {

cfe/trunk/lib/CodeGen/BackendUtil.cpp

static void addThreadSanitizerPass(const PassManagerBuilder &Builder,
   PM.add(createThreadSanitizerPass());
 }
 
 static void addDataFlowSanitizerPass(const PassManagerBuilder &Builder,
                                      PassManagerBase &PM) {
   const PassManagerBuilderWrapper &BuilderWrapper =
       static_cast<const PassManagerBuilderWrapper&>(Builder);
   const LangOptions &LangOpts = BuilderWrapper.getLangOpts();
-  PM.add(createDataFlowSanitizerPass(LangOpts.SanitizerBlacklistFile));
+  PM.add(createDataFlowSanitizerPass(LangOpts.SanitizerBlacklistFiles));
 }
 
 static TargetLibraryInfoImpl *createTLII(llvm::Triple &TargetTriple,
                                          const CodeGenOptions &CodeGenOpts) {
   TargetLibraryInfoImpl *TLII = new TargetLibraryInfoImpl(TargetTriple);
   if (!CodeGenOpts.SimplifyLibCalls)
     TLII->disableAllFunctions();
   return TLII;

cfe/trunk/lib/Driver/SanitizerArgs.cpp

 
 bool SanitizerArgs::needsUnwindTables() const {
   return hasOneOf(Sanitizers, NeedsUnwindTables);
 }
 
 void SanitizerArgs::clear() {
   Sanitizers.clear();
   RecoverableSanitizers.clear();
-  BlacklistFile = "";
+  BlacklistFiles.clear();
   SanitizeCoverage = 0;
   MsanTrackOrigins = 0;
   AsanFieldPadding = 0;
   AsanZeroBaseShadow = false;
   UbsanTrapOnError = false;
   AsanSharedRuntime = false;
   LinkCXXRuntimes = false;
 }
if (NeedsLsan && NeedsMsan)
     D.Diag(clang::diag::err_drv_argument_not_allowed_with)
       << lastArgumentForKind(D, Args, SanitizerKind::Leak)
       << lastArgumentForKind(D, Args, SanitizerKind::Memory);
   // FIXME: Currently -fsanitize=leak is silently ignored in the presence of
   // -fsanitize=address. Perhaps it should print an error, or perhaps
   // -f(-no)sanitize=leak should change whether leak detection is enabled by
   // default in ASan?
 
+  // Setup blacklist files.
+  // Add default blacklist from resource directory.
+  {
+    std::string BLPath;
+    if (getDefaultBlacklist(D, BLPath) && llvm::sys::fs::exists(BLPath))
+      BlacklistFiles.push_back(BLPath);
+  }
   // Parse -f(no-)sanitize-blacklist options.
-  if (Arg *BLArg = Args.getLastArg(options::OPT_fsanitize_blacklist,
-                                   options::OPT_fno_sanitize_blacklist)) {
-    if (BLArg->getOption().matches(options::OPT_fsanitize_blacklist)) {
-      std::string BLPath = BLArg->getValue();
-      if (llvm::sys::fs::exists(BLPath)) {
-        // Validate the blacklist format.
-        std::string BLError;
-        std::unique_ptr<llvm::SpecialCaseList> SCL(
-            llvm::SpecialCaseList::create(BLPath, BLError));
-        if (!SCL.get())
-          D.Diag(clang::diag::err_drv_malformed_sanitizer_blacklist) << BLError;
-        else
-          BlacklistFile = BLPath;
-      } else {
+  for (const auto *Arg : Args) {
+    if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) {
+      Arg->claim();
+      std::string BLPath = Arg->getValue();
+      if (llvm::sys::fs::exists(BLPath))
+        BlacklistFiles.push_back(BLPath);
         D.Diag(clang::diag::err_drv_no_such_file) << BLPath;
-      }
-    }
-  } else {
-    // If no -fsanitize-blacklist option is specified, try to look up for
-    // blacklist in the resource directory.
-    std::string BLPath;
-    if (getDefaultBlacklist(D, BLPath) && llvm::sys::fs::exists(BLPath))
-      BlacklistFile = BLPath;
+    } else if (Arg->getOption().matches(options::OPT_fno_sanitize_blacklist)) {
+      Arg->claim();
+      BlacklistFiles.clear();
+  // Validate blacklists format.
+  {
+    std::string BLError;
+    std::unique_ptr<llvm::SpecialCaseList> SCL(
+        llvm::SpecialCaseList::create(BlacklistFiles, BLError));
+    if (!SCL.get())
+      D.Diag(clang::diag::err_drv_malformed_sanitizer_blacklist) << BLError;
   }
 
   // Parse -f[no-]sanitize-memory-track-origins[=level] options.
   if (NeedsMsan) {
     if (Arg *A =
             Args.getLastArg(options::OPT_fsanitize_memory_track_origins_EQ,
                             options::OPT_fsanitize_memory_track_origins,
                             options::OPT_fno_sanitize_memory_track_origins)) {
void SanitizerArgs::addArgs(const llvm::opt::ArgList &Args,
 
   if (!RecoverableSanitizers.empty())
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-recover=" +
                                          toString(RecoverableSanitizers)));
 
   if (UbsanTrapOnError)
     CmdArgs.push_back("-fsanitize-undefined-trap-on-error");
 
-  if (!BlacklistFile.empty()) {
+  for (const auto &BLPath : BlacklistFiles) {
     SmallString<64> BlacklistOpt("-fsanitize-blacklist=");
-    BlacklistOpt += BlacklistFile;
+    BlacklistOpt += BLPath;
     CmdArgs.push_back(Args.MakeArgString(BlacklistOpt));
   }
 
   if (MsanTrackOrigins)
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-memory-track-origins=" +
                                          llvm::utostr(MsanTrackOrigins)));
   if (AsanFieldPadding)
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-address-field-padding=" +

cfe/trunk/lib/Frontend/CompilerInvocation.cpp

#include "clang/Frontend/LangStandards.def"
   }
 
   // Parse -fsanitize= arguments.
   parseSanitizerKinds("-fsanitize=", Args.getAllArgValues(OPT_fsanitize_EQ),
                       Diags, Opts.Sanitize);
   // -fsanitize-address-field-padding=N has to be a LangOpt, parse it here.
   Opts.SanitizeAddressFieldPadding =
       getLastArgIntValue(Args, OPT_fsanitize_address_field_padding, 0, Diags);
-  Opts.SanitizerBlacklistFile = Args.getLastArgValue(OPT_fsanitize_blacklist);
+  Opts.SanitizerBlacklistFiles = Args.getAllArgValues(OPT_fsanitize_blacklist);
 }
 
 static void ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
                                   FileManager &FileMgr,
                                   DiagnosticsEngine &Diags) {
   using namespace options;
   Opts.ImplicitPCHInclude = Args.getLastArgValue(OPT_include_pch);
   Opts.ImplicitPTHInclude = Args.getLastArgValue(OPT_include_pth);

cfe/trunk/test/Driver/fsanitize-blacklist.c

 // General blacklist usage.
 
 // PR12920
 // REQUIRES: clang-driver, shell
 
 // RUN: echo "fun:foo" > %t.good
+// RUN: echo "fun:bar" > %t.second
 // RUN: echo "badline" > %t.bad
 
-// RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST
-// CHECK-BLACKLIST: -fsanitize-blacklist
+// RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST
+// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good
+// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.second
 
 // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag.
 // RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE
 // CHECK-NO-SANITIZE-NOT: -fsanitize-blacklist
 
 // Flag -fno-sanitize-blacklist wins if it is specified later.
 // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fno-sanitize-blacklist %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-BLACKLIST
 // CHECK-NO-BLACKLIST-NOT: -fsanitize-blacklist
 
 // Driver barks on unexisting blacklist files.
 // RUN: %clang -fno-sanitize-blacklist -fsanitize-blacklist=unexisting.txt %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SUCH-FILE
 // CHECK-NO-SUCH-FILE: error: no such file or directory: 'unexisting.txt'
 
 // Driver properly reports malformed blacklist files.
-// RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.bad %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BAD-BLACKLIST
-// CHECK-BAD-BLACKLIST: error: malformed sanitizer blacklist
+// RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.second -fsanitize-blacklist=%t.bad -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BAD-BLACKLIST
+// CHECK-BAD-BLACKLIST: error: malformed sanitizer blacklist: 'error parsing file '{{.*}}.bad': malformed line 1: 'badline''
+
+// -fno-sanitize-blacklist disables all blacklists specified earlier.
+// RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fno-sanitize-blacklist -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ONLY-FIRST-DISABLED
+// CHECK-ONLY_FIRST-DISABLED-NOT: good
+// CHECK-ONLY-FIRST-DISABLED: -fsanitize-blacklist={{.*}}.second
+// CHECK-ONLY_FIRST-DISABLED-NOT: good