[Driver] Load all necessary default sanitizer blacklists

[vsk]

Summary

This patch removes a limitation which causes us to load at most one
default sanitizer blacklist when multiple sanitizers are enabled. E.g if
asan + cfi are enabled, and default blacklists for both sanitizers are
present, we would only load one of the blacklists.

The new behavior would be to load all available default blacklists for
enabled sanitizers.

[pcc]

This seems reasonable to me, although it's unfortunate that the design of the sanitizer blacklist feature does not (at present) allow different blacklists for different sanitizers.

@eugenis what do you think?

[eugenis]

Looks fine.

[vsk]

In D32043#728427, @pcc wrote:

This seems reasonable to me, although it's unfortunate that the design of the sanitizer blacklist feature does not (at present) allow different blacklists for different sanitizers.

IMO this might be a real problem. If we load multiple default blacklists, diagnostics which appear when compiling with one sanitizer could disappear when another sanitizer is enabled.

So long as we don't allow different blacklists for different sanitizers, maybe there should just be one default sanitizer blacklist file.

Specifically, we'd look for "sanitizer_blacklist.txt" in the resource dir. If we find it, load it (regardless of which sanitizers are enabled) and we're done. If we don't find it, fall back to the behavior in this patch (check if a sanitizer is enabled, then try to load its blacklist). Wdyt?

[eugenis]

We definitely want different blacklists for ASan and MSan.

[vsk]

Ok. I will take a step back and see what it will take to implement per-sanitizer blacklists.

[vsk]

Obsoleted by: https://reviews.llvm.org/D32842

lib/Driver/SanitizerArgs.cpp

 /// "-fsanitize=alignment".
 static std::string describeSanitizeArg(const llvm::opt::Arg *A,
                                        SanitizerMask Mask);
 
 /// Produce a string containing comma-separated names of sanitizers in \p
 /// Sanitizers set.
 static std::string toString(const clang::SanitizerSet &Sanitizers);
 
-static bool getDefaultBlacklist(const Driver &D, SanitizerMask Kinds,
-                                std::string &BLPath) {
-  const char *BlacklistFile = nullptr;
-  if (Kinds & Address)
-    BlacklistFile = "asan_blacklist.txt";
-  else if (Kinds & Memory)
-    BlacklistFile = "msan_blacklist.txt";
-  else if (Kinds & Thread)
-    BlacklistFile = "tsan_blacklist.txt";
-  else if (Kinds & DataFlow)
-    BlacklistFile = "dfsan_abilist.txt";
-  else if (Kinds & CFI)
-    BlacklistFile = "cfi_blacklist.txt";
+/// Add default blacklists from the resource directory.
+static void findDefaultBlacklists(const Driver &D, SanitizerMask Kinds,
+                                  std::vector<std::string> &BlacklistFiles) {
+  const std::pair<SanitizerMask, const char *> DefaultBlacklists[] = {
+      {Address, "asan_blacklist.txt"},
+      {Memory, "msan_blacklist.txt"},
+      {Thread, "tsan_blacklist.txt"},
+      {DataFlow, "dfsan_abilist.txt"},
+      {CFI, "cfi_blacklist.txt"}};
+  for (const auto &BL : DefaultBlacklists) {
+    if (!(Kinds & BL.first))
+      continue;
 
-  if (BlacklistFile) {
     clang::SmallString<64> Path(D.ResourceDir);
-    llvm::sys::path::append(Path, BlacklistFile);
-    BLPath = Path.str();
-    return true;
+    llvm::sys::path::append(Path, BL.second);
+    if (llvm::sys::fs::exists(Path))
+      BlacklistFiles.push_back(Path.str());
   }
-  return false;
 }
 
 /// Sets group bits for every group that has at least one representative already
 /// enabled in \p Kinds.
 static SanitizerMask setGroupBits(SanitizerMask Kinds) {
 #define SANITIZER(NAME, ID)
 #define SANITIZER_GROUP(NAME, ID, ALIAS)                                       \
   if (Kinds & SanitizerKind::ID)                                               \
for (const auto *Arg : Args) {
     }
   }
   RecoverableKinds &= Kinds;
   RecoverableKinds &= ~Unrecoverable;
 
   TrappingKinds &= Kinds;
 
   // Setup blacklist files.
-  // Add default blacklist from resource directory.
-  {
-    std::string BLPath;
-    if (getDefaultBlacklist(D, Kinds, BLPath) && llvm::sys::fs::exists(BLPath))
-      BlacklistFiles.push_back(BLPath);
-  }
+  findDefaultBlacklists(D, Kinds, BlacklistFiles);
+
   // Parse -f(no-)sanitize-blacklist options.
   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);
         ExtraDeps.push_back(BLPath);

test/Driver/fsanitize-blacklist.c

 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST
 // CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good" "-fsanitize-blacklist={{.*}}.second
 
 // Now, check for -fdepfile-entry flags.
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST2
 // CHECK-BLACKLIST2: -fdepfile-entry={{.*}}.good" "-fdepfile-entry={{.*}}.second
 
 // Check that the default blacklist is not added as an extra dependency.
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-BLACKLIST --implicit-check-not=fdepfile-entry
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address -resource-dir=%S/Inputs/resource_dir %s -### &> %t.cc1_asan
+// RUN: FileCheck %s --check-prefix=CHECK-DEFAULT-BLACKLIST --implicit-check-not=fdepfile-entry -input-file %t.cc1_asan
 // CHECK-DEFAULT-BLACKLIST: -fsanitize-blacklist={{.*}}asan_blacklist.txt
 
+// Check that default blacklists are not added unless the matching sanitizer is
+// enabled, even if the blacklist exists.
+// RUN: FileCheck %s --implicit-check-not=cfi_blacklist.txt -input-file %t.cc1_asan
+
+// Check that we can add multiple default blacklists if the matching sanitizers
+// are enabled.
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address,cfi -flto -fvisibility=hidden -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=MULTIPLE-DEFAULT-BLACKLISTS
+// MULTIPLE-DEFAULT-BLACKLISTS-DAG: -fsanitize-blacklist={{.*}}asan_blacklist.txt
+// MULTIPLE-DEFAULT-BLACKLISTS-DAG: -fsanitize-blacklist={{.*}}cfi_blacklist.txt
+
 // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag.
 // RUN: %clang -target x86_64-linux-gnu -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE --check-prefix=DELIMITERS
 // CHECK-NO-SANITIZE-NOT: -fsanitize-blacklist
 
 // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag.
 // Now, check for the absense of -fdepfile-entry flags.
 // RUN: %clang -target x86_64-linux-gnu -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE2 --check-prefix=DELIMITERS
 // CHECK-NO-SANITIZE2-NOT: -fdepfile-entry