[clang] Report sanitizer blacklist as a dependency in cc1 instead of driver

[jkorous]

Summary

Report dependencies from cc1 instead of driver.

• This treats the default implicit blacklist added by driver just like any other blacklist dependency.
• This also makes clang-scan-deps report the blacklist dependencies.

Essentially reverts this commit:
Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.
4c3f237edb3e
https://github.com/llvm/llvm-project/commit/4c3f237edb3ed7f0fe1b21708ad02f0ce0b6dac2
https://reviews.llvm.org/D12544

[jkorous]

@pcc since you reviewed the above-mentioned patch - could you please take a look at this one too?

[pcc]

The current behaviour is deliberate, see crbug.com/527080 for more context. If you're trying to solve a problem with clang-scan-deps then it seems like it should be solved at a different layer.

[jkorous]

I am trying to solve two different things here.

1. The default sanitizer blacklist is a dependency about which clang-scan-deps needs to know. IIUC the driver just adds the option without reporting it dependency.
2. I need to get blacklist dependencies in clang-scan-deps which uses cc1 command as input.

How about ignoring the default blacklist in cc1 run with -MMD but reporting it with -MD? Would that work for you?

-MD                     Write a depfile containing user and system headers
-MMD                    Write a depfile containing user headers

[jkorous]

Ping.

[pcc]

In D69290#1717546, @jkorous wrote:

How about ignoring the default blacklist in cc1 run with -MMD but reporting it with -MD? Would that work for you?

Yes, that looks like it would work. It appears that goma has since been fixed so that it passes a relative resource directory path to clang, so it should no longer be emitting the absolute paths with -MD that were causing problems before. Moreover, Chromium passes -MMD, so this change would not affect it anyway.

+ @thakis as heads up and @eugenis since he was asking about this.

[eugenis]

FTR, I've had to workaround this with -fdepfile-entry in https://reviews.llvm.org/D69587

[jkorous]

Added a cc1 command line option -fsanitize-system-blacklist so driver can tell cc1 which blacklists are user-specified and which are system ones.

[jkorous]

@eugenis Does this make your workaround obsolete?

[eugenis]

In D69290#1729503, @jkorous wrote:

@eugenis Does this make your workaround obsolete?

Yes, looks like it should.

[jkorous]

@pcc Could you please take a look?

[pcc]

LGTM

clang/include/clang/Driver/SanitizerArgs.h

 class ToolChain;
 
 class SanitizerArgs {
   SanitizerSet Sanitizers;
   SanitizerSet RecoverableSanitizers;
   SanitizerSet TrapSanitizers;
 
   std::vector<std::string> BlacklistFiles;
-  std::vector<std::string> ExtraDeps;
   int CoverageFeatures = 0;
   int MsanTrackOrigins = 0;
   bool MsanUseAfterDtor = true;
   bool CfiCrossDso = false;
   bool CfiICallGeneralizePointers = false;
   bool CfiCanonicalJumpTables = false;
   int AsanFieldPadding = 0;
   bool SharedRuntime = false;

clang/lib/Driver/SanitizerArgs.cpp

SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   addDefaultBlacklists(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);
       } else {
         D.Diag(clang::diag::err_drv_no_such_file) << BLPath;
       }
     } else if (Arg->getOption().matches(options::OPT_fno_sanitize_blacklist)) {
       Arg->claim();
       BlacklistFiles.clear();
-      ExtraDeps.clear();
     }
   }
   // Validate blacklists format.
   {
     std::string BLError;
     std::unique_ptr<llvm::SpecialCaseList> SCL(
         llvm::SpecialCaseList::create(BlacklistFiles, BLError));
     if (!SCL.get())
if (!TrapSanitizers.empty())
     CmdArgs.push_back(
         Args.MakeArgString("-fsanitize-trap=" + toString(TrapSanitizers)));
 
   for (const auto &BLPath : BlacklistFiles) {
     SmallString<64> BlacklistOpt("-fsanitize-blacklist=");
     BlacklistOpt += BLPath;
     CmdArgs.push_back(Args.MakeArgString(BlacklistOpt));
   }
-  for (const auto &Dep : ExtraDeps) {
-    SmallString<64> ExtraDepOpt("-fdepfile-entry=");
-    ExtraDepOpt += Dep;
-    CmdArgs.push_back(Args.MakeArgString(ExtraDepOpt));
-  }
 
   if (MsanTrackOrigins)
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-memory-track-origins=" +
                                          Twine(MsanTrackOrigins)));
 
   if (MsanUseAfterDtor)
     CmdArgs.push_back("-fsanitize-memory-use-after-dtor");
 

clang/lib/Frontend/CompilerInvocation.cpp

static void ParseDependencyOutputArgs(DependencyOutputOptions &Opts,
   Opts.DOTOutputFile = Args.getLastArgValue(OPT_dependency_dot);
   Opts.ModuleDependencyOutputDir =
       Args.getLastArgValue(OPT_module_dependency_dir);
   if (Args.hasArg(OPT_MV))
     Opts.OutputFormat = DependencyOutputFormat::NMake;
   // Add sanitizer blacklists as extra dependencies.
   // They won't be discovered by the regular preprocessor, so
   // we let make / ninja to know about this implicit dependency.
-  Opts.ExtraDeps = Args.getAllArgValues(OPT_fdepfile_entry);
+  if (!Args.hasArg(OPT_fno_sanitize_blacklist)) {
+    for (const auto *A : Args.filtered(OPT_fsanitize_blacklist)) {
+      StringRef Val = A->getValue();
+      if (Val.find('=') == StringRef::npos)
+        Opts.ExtraDeps.push_back(Val);
+    }
+  }
+
+  // Propagate the extra dependencies.
+  for (const auto *A : Args.filtered(OPT_fdepfile_entry)) {
+    Opts.ExtraDeps.push_back(A->getValue());
+  }
+
   // Only the -fmodule-file=<file> form.
   for (const auto *A : Args.filtered(OPT_fmodule_file)) {
     StringRef Val = A->getValue();
     if (Val.find('=') == StringRef::npos)
       Opts.ExtraDeps.push_back(Val);
   }
 }
 

clang/test/ClangScanDeps/Inputs/sanitizer_blacklist_cdb.json

+[
+{
+  "directory": "DIR",
+  "command": "clang -fsanitize=address -fsanitize-blacklist=DIR/black.list DIR/sanitizer_blacklist_input.cpp",
+  "file": "DIR/sanitizer_blacklist_input.cpp"
+}
+]

clang/test/ClangScanDeps/sanitizer_blacklist.cpp

+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/sanitizer_blacklist_input.cpp
+// RUN: echo "fun:foo" > %t.dir/black.list
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/sanitizer_blacklist_cdb.json > %t.cdb
+// RUN: fail please
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources | FileCheck %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess | FileCheck %s
+//
+// CHECK: black.list

clang/test/Driver/fsanitize-blacklist.c

 // RUN: echo "fun:foo" > %t.good
 // RUN: echo "fun:bar" > %t.second
 // RUN: echo "badline" > %t.bad
 
 // 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
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=hwaddress -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-ASAN --implicit-check-not=fdepfile-entry --implicit-check-not=-fsanitize-blacklist=
 // CHECK-DEFAULT-BLACKLIST-ASAN: -fsanitize-blacklist={{.*[^w]}}asan_blacklist.txt
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-BLACKLIST-HWASAN --implicit-check-not=fdepfile-entry --implicit-check-not=-fsanitize-blacklist=
 // CHECK-DEFAULT-BLACKLIST-HWASAN: -fsanitize-blacklist={{.*}}hwasan_blacklist.txt
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-UBSAN-BLACKLIST --implicit-check-not=fdepfile-entry --implicit-check-not=-fsanitize-blacklist=
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=nullability -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-UBSAN-BLACKLIST --implicit-check-not=fdepfile-entry --implicit-check-not=-fsanitize-blacklist=

clang/test/Frontend/dependency-gen.c

 // CHECK-SIX: {{ }}x.h
 // RUN: echo "fun:foo" > %t.blacklist
 // RUN: %clang -MD -MF - %s -fsyntax-only -resource-dir=%S/Inputs/resource_dir_with_cfi_blacklist -fsanitize=cfi-vcall -flto -fvisibility=hidden -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s
 // CHECK-SEVEN: .blacklist
 // CHECK-SEVEN: {{ }}x.h
 #ifndef INCLUDE_FLAG_TEST
 #include <x.h>
 #endif
+
+// RUN: echo "fun:foo" > %t.blacklist1
+// RUN: echo "fun:foo" > %t.blacklist2
+// RUN: %clang -MD -MF - %s -fsyntax-only -resource-dir=%S/Inputs/resource_dir_with_cfi_blacklist -fsanitize=cfi-vcall -flto -fvisibility=hidden -fsanitize-blacklist=%t.blacklist1 -fsanitize-blacklist=%t.blacklist2 -I ./ | FileCheck -check-prefix=CHECK-DAG-TWO-BLACK-LISTS %s
+// CHECK-DAG-TWO-BLACK-LISTS: resource_dir_with_cfi_blacklist
+// CHECK-DAG-TWO-BLACK-LISTS: .blacklist1
+// CHECK-DAG-TWO-BLACK-LISTS: .blacklist2
+// CHECK-DAG-TWO-BLACK-LISTS: {{ }}x.h
 No newline at end of file