[RFC] Introduce support for multiple sanitizer blacklists (Clang side).

[pcc]

Summary

This adds support for multiple -fsanitize-blacklist flags, which cause
clang to load blacklists from each of the specified paths. This could be
useful for sanitizers such as DFSan where we may have multiple ABI lists,
each covering a specific library, which describes the ABI of that library.

This changes the semantics of the -fsanitize-blacklist flag, so I wanted to
make sure that we were comfortable with that. Now, the flag is additive, so a
-fsanitize-blacklist flag on the command line loads the specified blacklist
as well as the one in the resource directory. The -fno-sanitize-blacklist
flag works as before in that it prevents the blacklist in the resource
directory from being loaded, but it can also be used in combination with
-fsanitize-blacklist to load only user-specified blacklists.

Tests to come. I've removed the one test which fails as a result of the
changed semantics.

[samsonov]

include/clang/Driver/SanitizerArgs.h

12

You may include clang/Basic/LLVM.h and use plain StringRef instead of llvm::StringRef.

lib/CodeGen/BackendUtil.cpp

592

See my comment in LLVM part - this file doesn't look like the right place for this function.

lib/Driver/SanitizerArgs.cpp

128

nit: remove {} for if- and else- clauses for consistency.

[pcc]

Address comments.

include/clang/Driver/SanitizerArgs.h

12

Done.

lib/CodeGen/BackendUtil.cpp

592

I was imagining that we could add things like support for ad hoc entries here if we needed it.

lib/Driver/SanitizerArgs.cpp

128

Done.

[samsonov]

lib/CodeGen/BackendUtil.cpp

592

Can you provide an example for "ad hoc" entries which can't be explained by special case list format? It's somewhat unfortunate that you need to modify special case list twice - in Clang CodeGen and before passing it to the backend.

[pcc]

lib/CodeGen/BackendUtil.cpp

592

I was referring to the case where you might have an entry on the command line or dynamically loaded from an object file; see my comments in D4546.

I think we can avoid the double construction issue by modifying SanitizerBlacklist to only create a SpecialCaseList if the user has enabled a sanitizer that uses it.

[pcc]

Add tests and address reviewer comments.

[samsonov]

Support for adding special case list entries in commandline seems an overkill to me. Now, if you presumably need logic to dynamically construct special case list from object files, I don't see why Clang's codegen will be a good place for it. It won't even use this list, only pass it to the DFSan backend...

lib/Driver/SanitizerArgs.cpp

118

Hm. So now -fno-sanitize-blacklist only disables the default blacklist, but not blacklists specified in -fsanitize-blacklist= arguments? This looks weird. I'd suggest the following semantics:

• default blacklist in resource directory is implicitly enabled.
• -fsanitize-blacklist= specifes one more blacklist to use
• -fno-sanitize-blacklist - discards all blacklists previously defined in the command line, and the default blacklist.

That is,

BLSet.insert(DefaultBlacklist);
foreach (arg)
  if arg is -fsanitize-blacklist=, insert the blacklist into BLSet
  else if arg is -fno-sanitize-blacklist, clear BLSet.

[pcc]

Support for adding special case list entries in commandline seems an overkill to me. Now, if you presumably need logic to dynamically construct special case list from object files, I don't see why Clang's codegen will be a good place for it. It won't even use this list, only pass it to the DFSan backend...

I don't think I actually need support for entries on the command line. Forget I brought it up.

I can probably live with having dfsan take two lists of strings, one for the ABI list files and the other for the object files. The thing is that I wanted the ABI list to be collected into an object before being used to create the pass. That way, it would probably be easier to do error handling and such from Clang or whatever is building the ABI list. The existing SpecialCaseList object seemed like a reasonable place to do it, but maybe it would be better to use DFSanABIList for that purpose and make it a public class.

I was imagining that the code that extracts the list from the object files would live in DataFlowSanitizer.cpp and be called from clang.

lib/Driver/SanitizerArgs.cpp

118

Sounds good. I initially wanted to have a -fno-default-sanitize-blacklist flag but I imagined that would be even more confusing if we needed to keep -fno-sanitize-blacklist.

[samsonov]

I see. As I mentioned in offline discussion, you'd probably need smth. like LLVMObject library to dynamically extract the symbols you need from the shared objects, and it would we weird to have Clang CodeGen depend on it.

include/clang/CodeGen/BackendUtil.h

 //===--- BackendUtil.h - LLVM Backend Utilities -----------------*- 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_CODEGEN_BACKEND_UTIL_H
 #define LLVM_CLANG_CODEGEN_BACKEND_UTIL_H
 
 #include "clang/Basic/LLVM.h"
+#include <memory>
 
 namespace llvm {
   class Module;
+  class SpecialCaseList;
 }
 
 namespace clang {
   class DiagnosticsEngine;
   class CodeGenOptions;
   class TargetOptions;
   class LangOptions;
 
   enum BackendAction {
     Backend_EmitAssembly,  ///< Emit native assembly files
     Backend_EmitBC,        ///< Emit LLVM bitcode files
     Backend_EmitLL,        ///< Emit human-readable LLVM assembly
     Backend_EmitNothing,   ///< Don't emit anything (benchmarking mode)
     Backend_EmitMCNull,    ///< Run CodeGen, but don't emit anything
     Backend_EmitObj        ///< Emit native object files
   };
 
+  std::unique_ptr<llvm::SpecialCaseList>
+  CreateSpecialCaseList(const CodeGenOptions &CGOpts);
+
   void EmitBackendOutput(DiagnosticsEngine &Diags, const CodeGenOptions &CGOpts,
                          const TargetOptions &TOpts, const LangOptions &LOpts,
                          StringRef TDesc, llvm::Module *M, BackendAction Action,
                          raw_ostream *OS);
 }
 
 #endif

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 CLANG_LIB_DRIVER_SANITIZERARGS_H_
 #define CLANG_LIB_DRIVER_SANITIZERARGS_H_
 
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include <string>
 
 namespace clang {
 namespace driver {
 
 class Driver;
#include "clang/Basic/Sanitizers.def"
     NeedsLeakDetection = Leak,
     NeedsUbsanRt = Undefined | Integer,
     NotAllowedWithTrap = Vptr,
     HasZeroBaseShadow = Thread | Memory | DataFlow,
     NeedsUnwindTables = Address | Thread | Memory | DataFlow
   };
   unsigned Kind;
 
-  std::string BlacklistFile;
+  std::vector<std::string> BlacklistFiles;
   int MsanTrackOrigins;
   bool AsanZeroBaseShadow;
   bool UbsanTrapOnError;
   bool AsanSharedRuntime;
 
  public:
   SanitizerArgs();
   /// Parses the sanitizer arguments from an argument list.
public:
   }
   bool needsUnwindTables() const { return Kind & NeedsUnwindTables; }
   void addArgs(const llvm::opt::ArgList &Args,
                llvm::opt::ArgStringList &CmdArgs) const;
 
  private:
   void clear();
 
+  void addBlacklistFile(const Driver &D, llvm::StringRef Path);
+
   /// Parse a single value from a -fsanitize= or -fno-sanitize= value list.
   /// Returns OR of members of the \c SanitizeKind enumeration, or \c 0
   /// if \p Value is not known.
   static unsigned parse(const char *Value);
 
   /// Parse a -fsanitize= or -fno-sanitize= argument's values, diagnosing any
   /// invalid components.
   static unsigned parse(const Driver &D, const llvm::opt::Arg *A,

include/clang/Frontend/CodeGenOptions.h

public:
 
   /// The name for the split debug info file that we'll break out. This is used
   /// in the backend for setting the name in the skeleton cu.
   std::string SplitDwarfFile;
 
   /// The name of the relocation model to use.
   std::string RelocationModel;
 
-  /// Path to blacklist file for sanitizers.
-  std::string SanitizerBlacklistFile;
+  /// Paths to blacklist files for sanitizers.
+  std::vector<std::string> SanitizerBlacklistFiles;
 
   /// If not an empty string, trap intrinsics are lowered to calls to this
   /// function instead of to trap instructions.
   std::string TrapFuncName;
 
   /// A list of command-line options to forward to the LLVM backend.
   std::vector<std::string> BackendOptions;
 

lib/CodeGen/BackendUtil.cpp

 #include "clang/Basic/TargetOptions.h"
 #include "clang/Frontend/CodeGenOptions.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "clang/Frontend/Utils.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/SchedulerRegistry.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/PassManager.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/SpecialCaseList.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetLibraryInfo.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/IPO.h"
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
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 CodeGenOptions &CGOpts = BuilderWrapper.getCGOpts();
-  PM.add(createDataFlowSanitizerPass(CGOpts.SanitizerBlacklistFile));
+  PM.add(createDataFlowSanitizerPass(CreateSpecialCaseList(CGOpts)));
 }
 
 void EmitAssemblyHelper::CreatePasses() {
   unsigned OptLevel = CodeGenOpts.OptimizationLevel;
   CodeGenOptions::InliningMethod Inlining = CodeGenOpts.getInlining();
 
   // Handle disabling of LLVM optimization, where we want to preserve the
   // internal module before any optimization.
void EmitAssemblyHelper::EmitAssembly(BackendAction Action, raw_ostream *OS) {
   }
 
   if (CodeGenPasses) {
     PrettyStackTraceString CrashInfo("Code generation");
     CodeGenPasses->run(*TheModule);
   }
 }
 
+std::unique_ptr<SpecialCaseList> clang::CreateSpecialCaseList(const CodeGenOptions &CGOpts) {
+  auto SCL = llvm::make_unique<SpecialCaseList>();
+  for (const auto &Path : CGOpts.SanitizerBlacklistFiles) {
+    std::string Error;
+    if (!SCL->loadFromFile(Path, Error)) {
+      report_fatal_error(Error);
+    }
+  }
+
+  return std::move(SCL);
+}
+
 void clang::EmitBackendOutput(DiagnosticsEngine &Diags,
                               const CodeGenOptions &CGOpts,
                               const clang::TargetOptions &TOpts,
                               const LangOptions &LOpts, StringRef TDesc,
                               Module *M, BackendAction Action,
                               raw_ostream *OS) {
   EmitAssemblyHelper AsmHelper(Diags, CGOpts, TOpts, LOpts, M);
 

lib/CodeGen/CodeGenModule.cpp

 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
+#include "clang/CodeGen/BackendUtil.h"
 #include "clang/Frontend/CodeGenOptions.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Intrinsics.h"
: Context(C), LangOpts(C.getLangOpts()), CodeGenOpts(CGO), TheModule(M),
       ObjCRuntime(nullptr), OpenCLRuntime(nullptr), OpenMPRuntime(nullptr),
       CUDARuntime(nullptr), DebugInfo(nullptr), ARCData(nullptr),
       NoObjCARCExceptionsMetadata(nullptr), RRData(nullptr), PGOReader(nullptr),
       CFConstantStringClassRef(nullptr), ConstantStringClassRef(nullptr),
       NSConstantStringType(nullptr), NSConcreteGlobalBlock(nullptr),
       NSConcreteStackBlock(nullptr), BlockObjectAssign(nullptr),
       BlockObjectDispose(nullptr), BlockDescriptorType(nullptr),
       GenericBlockLiteralType(nullptr), LifetimeStartFn(nullptr),
-      LifetimeEndFn(nullptr), SanitizerBL(llvm::SpecialCaseList::createOrDie(
-                                  CGO.SanitizerBlacklistFile)) {
+      LifetimeEndFn(nullptr), SanitizerBL(CreateSpecialCaseList(CGO)) {
 
   // Initialize the type cache.
   llvm::LLVMContext &LLVMContext = M.getContext();
   VoidTy = llvm::Type::getVoidTy(LLVMContext);
   Int8Ty = llvm::Type::getInt8Ty(LLVMContext);
   Int16Ty = llvm::Type::getInt16Ty(LLVMContext);
   Int32Ty = llvm::Type::getInt32Ty(LLVMContext);
   Int64Ty = llvm::Type::getInt64Ty(LLVMContext);

lib/CodeGen/SanitizerBlacklist.h

 
 namespace clang {
 namespace CodeGen {
 
 class SanitizerBlacklist {
   std::unique_ptr<llvm::SpecialCaseList> SCL;
 
 public:
-  SanitizerBlacklist(llvm::SpecialCaseList *SCL) : SCL(SCL) {}
+  SanitizerBlacklist(std::unique_ptr<llvm::SpecialCaseList> SCL)
+      : SCL(std::move(SCL)) {}
   bool isIn(const llvm::Module &M,
             const StringRef Category = StringRef()) const;
   bool isIn(const llvm::Function &F) const;
   bool isIn(const llvm::GlobalVariable &G,
             const StringRef Category = StringRef()) const;
   bool isBlacklistedType(StringRef MangledTypeName) const;
 };
 }  // end namespace CodeGen
 }  // end namespace clang
 
 #endif

lib/Driver/SanitizerArgs.cpp

 #include "llvm/Support/SpecialCaseList.h"
 #include <memory>
 
 using namespace clang::driver;
 using namespace llvm::opt;
 
 void SanitizerArgs::clear() {
   Kind = 0;
-  BlacklistFile = "";
+  BlacklistFiles.clear();
   MsanTrackOrigins = 0;
   AsanZeroBaseShadow = false;
   UbsanTrapOnError = false;
   AsanSharedRuntime = false;
 }
 
 SanitizerArgs::SanitizerArgs() {
   clear();
if (NeedsLsan && NeedsMsan)
     D.Diag(diag::err_drv_argument_not_allowed_with)
       << lastArgumentForKind(D, Args, NeedsLeakDetection)
       << lastArgumentForKind(D, Args, NeedsMsanRt);
   // 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?
 
-  // 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(diag::err_drv_malformed_sanitizer_blacklist) << BLError;
-        else
-          BlacklistFile = BLPath;
-      } else {
-        D.Diag(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.
+  // If -fno-sanitize-blacklist option is not specified, try to look up the
+  if (!Args.hasArg(options::OPT_fno_sanitize_blacklist)) {
     std::string BLPath;
     if (getDefaultBlacklistForKind(D, Kind, BLPath) &&
         llvm::sys::fs::exists(BLPath))
-      BlacklistFile = BLPath;
+      addBlacklistFile(D, BLPath);
+  }
+
+  // Parse -fsanitize-blacklist options.
+  for (auto Arg : Args.filtered(options::OPT_fsanitize_blacklist)) {
+    Arg->claim();
+    if (llvm::sys::fs::exists(Arg->getValue())) {
+      addBlacklistFile(D, Arg->getValue());
+    } else {
+      D.Diag(diag::err_drv_no_such_file) << Arg->getValue();
+    }
   }
 
   // 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)) {
if (NeedsAsan) {
     AsanSharedRuntime =
         Args.hasArg(options::OPT_shared_libasan) ||
         (TC.getTriple().getEnvironment() == llvm::Triple::Android);
     AsanZeroBaseShadow =
         (TC.getTriple().getEnvironment() == llvm::Triple::Android);
   }
 }
 
+void SanitizerArgs::addBlacklistFile(const Driver &D, StringRef BLPath) {
+  // Validate the blacklist format.
+  std::string BLError;
+  llvm::SpecialCaseList SCL;
+  if (!SCL.loadFromFile(BLPath, BLError))
+    D.Diag(diag::err_drv_malformed_sanitizer_blacklist) << BLError;
+  else
+    BlacklistFiles.push_back(BLPath);
+}
+
 void SanitizerArgs::addArgs(const llvm::opt::ArgList &Args,
                             llvm::opt::ArgStringList &CmdArgs) const {
   if (!Kind)
     return;
   SmallString<256> SanitizeOpt("-fsanitize=");
 #define SANITIZER(NAME, ID) \
   if (Kind & ID) \
     SanitizeOpt += NAME ",";
 #include "clang/Basic/Sanitizers.def"
   SanitizeOpt.pop_back();
   CmdArgs.push_back(Args.MakeArgString(SanitizeOpt));
-  if (!BlacklistFile.empty()) {
+  for (const auto &File : BlacklistFiles) {
     SmallString<64> BlacklistOpt("-fsanitize-blacklist=");
-    BlacklistOpt += BlacklistFile;
+    BlacklistOpt += File;
     CmdArgs.push_back(Args.MakeArgString(BlacklistOpt));
   }
 
   if (MsanTrackOrigins)
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-memory-track-origins=" +
                                          llvm::utostr(MsanTrackOrigins)));
 
   // Workaround for PR16386.

lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   }
 
   Opts.InstrumentFunctions = Args.hasArg(OPT_finstrument_functions);
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
   Opts.CompressDebugSections = Args.hasArg(OPT_compress_debug_sections);
   Opts.DebugCompilationDir = Args.getLastArgValue(OPT_fdebug_compilation_dir);
   Opts.LinkBitcodeFile = Args.getLastArgValue(OPT_mlink_bitcode_file);
-  Opts.SanitizerBlacklistFile = Args.getLastArgValue(OPT_fsanitize_blacklist);
+  Opts.SanitizerBlacklistFiles = Args.getAllArgValues(OPT_fsanitize_blacklist);
   Opts.SanitizeMemoryTrackOrigins =
       getLastArgIntValue(Args, OPT_fsanitize_memory_track_origins_EQ, 0, Diags);
   Opts.SanitizeUndefinedTrapOnError =
       Args.hasArg(OPT_fsanitize_undefined_trap_on_error);
   Opts.SSPBufferSize =
       getLastArgIntValue(Args, OPT_stack_protector_buffer_size, 8, Diags);
   Opts.StackRealignment = Args.hasArg(OPT_mstackrealign);
   if (Arg *A = Args.getLastArg(OPT_mstack_alignment)) {

test/Driver/fsanitize-blacklist.c

 // General blacklist usage.
 
 // PR12920
 // REQUIRES: clang-driver, shell
 
 // RUN: echo "fun:foo" > %t.good
 // 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
 
 // 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