Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.

[krasin]

Summary

Clang sanitizers, such as AddressSanitizer, ThreadSanitizer, MemorySanitizer,
Control Flow Integrity and others, use blacklists to specify which types / functions
should not be instrumented to avoid false positives or suppress known failures.

This change adds the blacklist filenames to the list of dependencies of the rules,
generated with -M/-MM/-MD/-MMD. This lets CMake/Ninja recognize that certain
C/C++/ObjC files need to be recompiled (if a blacklist is updated).

[pcc]

We should also make blacklists appear in the --show-includes output.

lib/Frontend/DependencyFile.cpp

418–422

If you move this code to the constructor you won't need to add an extra data member to this class.

test/Frontend/dependency-gen.c

26

Can we also test that the default blacklist appears in the -MD output if appropriate?

[krasin]

Add more test cases.

[krasin]

In D11968#222338, @pcc wrote:

We should also make blacklists appear in the --show-includes output.

Is it about Windows compatibility? Do you mean that the output of

bin/clang-cl /Zs /showIncludes ~/lala.cc -fsanitize=address

should include the default blacklist (and any explicit blacklists, if they were specified)?

lib/Frontend/DependencyFile.cpp

418–422

I considered that. In this case, extra deps will appear even before the source file itself.

Right now, for my local test code, it generates:

lala.o: /usr/local/google/home/krasin/lala.cc \
  /usr/local/google/home/krasin/foo.h \
  /usr/local/google/home/krasin/bar.h \
  /usr/local/google/home/krasin/blacklist.txt

If moved to the constructor, it will become:

lala.o: /usr/local/google/home/krasin/blacklist.txt \
  /usr/local/google/home/krasin/lala.cc \
  /usr/local/google/home/krasin/foo.h \
  /usr/local/google/home/krasin/bar.h

This decreases the readability of the generated rules, as the main source becomes harder to discover.

Please, let me know, if you think it's not a problem; I will move the code to the constructor and eliminate the extra member.

[pcc]

should include the default blacklist (and any explicit blacklists, if they were specified)?

Yes, exactly.

lib/Frontend/DependencyFile.cpp

418–422

These files are for machine consumption. If make and ninja accept this output, I don't think this should be a problem.

[krasin]

Windows compat

[krasin]

Please, take another look. The test for --show-includes is on the way.

[krasin]

More tests / fix tests.

[rsmith]

LGTM

[pcc]

LGTM

[krasin]

Thank you, Peter.

I will commit once I have restored my password (the email to Chris is sent)

include/clang/Frontend/DependencyOutputOptions.h

public:
   /// predefines buffer. If the output file is "-", output will be sent to
   /// stderr.
   std::string HeaderIncludeOutputFile;
 
   /// A list of names to use as the targets in the dependency file; this list
   /// must contain at least one entry.
   std::vector<std::string> Targets;
 
+  /// A list of filenames to be used as extra dependencies for every target.
+  std::vector<std::string> ExtraDeps;
+
   /// \brief The file to write GraphViz-formatted header dependencies to.
   std::string DOTOutputFile;
 
   /// \brief The directory to copy module dependencies to when collecting them.
   std::string ModuleDependencyOutputDir;
 
 public:
   DependencyOutputOptions() {

include/clang/Frontend/Utils.h

 /// AttachDependencyGraphGen - Create a dependency graph generator, and attach
 /// it to the given preprocessor.
   void AttachDependencyGraphGen(Preprocessor &PP, StringRef OutputFile,
                                 StringRef SysRoot);
 
 /// AttachHeaderIncludeGen - Create a header include list generator, and attach
 /// it to the given preprocessor.
 ///
+/// \param ExtraHeaders - If not empty, will write the header filenames, just
+/// like they were included during a regular preprocessing. Useful for
+/// implicit include dependencies, like sanitizer blacklists.
 /// \param ShowAllHeaders - If true, show all header information instead of just
 /// headers following the predefines buffer. This is useful for making sure
 /// includes mentioned on the command line are also reported, but differs from
 /// the default behavior used by -H.
 /// \param OutputPath - If non-empty, a path to write the header include
 /// information to, instead of writing to stderr.
 /// \param ShowDepth - Whether to indent to show the nesting of the includes.
 /// \param MSStyle - Whether to print in cl.exe /showIncludes style.
-void AttachHeaderIncludeGen(Preprocessor &PP, bool ShowAllHeaders = false,
+void AttachHeaderIncludeGen(Preprocessor &PP,
+                            const std::vector<std::string> &ExtraHeaders,
+                            bool ShowAllHeaders = false,
                             StringRef OutputPath = "",
                             bool ShowDepth = true, bool MSStyle = false);
 
 /// Cache tokens for use with PCH. Note that this requires a seekable stream.
 void CacheTokens(Preprocessor &PP, raw_pwrite_stream *OS);
 
 /// The ChainedIncludesSource class converts headers to chained PCHs in
 /// memory, mainly for testing.

lib/Frontend/CompilerInstance.cpp

void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
   // If we don't have a collector, but we are collecting module dependencies,
   // then we're the top level compiler instance and need to create one.
   if (!ModuleDepCollector && !DepOpts.ModuleDependencyOutputDir.empty())
     ModuleDepCollector = std::make_shared<ModuleDependencyCollector>(
         DepOpts.ModuleDependencyOutputDir);
 
   // Handle generating header include information, if requested.
   if (DepOpts.ShowHeaderIncludes)
-    AttachHeaderIncludeGen(*PP);
+    AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps);
   if (!DepOpts.HeaderIncludeOutputFile.empty()) {
     StringRef OutputPath = DepOpts.HeaderIncludeOutputFile;
     if (OutputPath == "-")
       OutputPath = "";
-    AttachHeaderIncludeGen(*PP, /*ShowAllHeaders=*/true, OutputPath,
+    AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps,
+                           /*ShowAllHeaders=*/true, OutputPath,
                            /*ShowDepth=*/false);
   }
 
   if (DepOpts.PrintShowIncludes) {
-    AttachHeaderIncludeGen(*PP, /*ShowAllHeaders=*/false, /*OutputPath=*/"",
+    AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps,
+                           /*ShowAllHeaders=*/false, /*OutputPath=*/"",
                            /*ShowDepth=*/true, /*MSStyle=*/true);
   }
 }
 
 std::string CompilerInstance::getSpecificModuleCachePath() {
   // Set up the module path, including the hash for the
   // module-creation options.
   SmallString<256> SpecificModuleCache(getHeaderSearchOpts().ModuleCachePath);

lib/Frontend/CompilerInvocation.cpp

static void ParseDependencyOutputArgs(DependencyOutputOptions &Opts,
   Opts.HeaderIncludeOutputFile = Args.getLastArgValue(OPT_header_include_file);
   Opts.AddMissingHeaderDeps = Args.hasArg(OPT_MG);
   Opts.PrintShowIncludes = Args.hasArg(OPT_show_includes);
   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_fsanitize_blacklist);
 }
 
 bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
                                 DiagnosticsEngine *Diags) {
   using namespace options;
   bool Success = true;
 
   Opts.DiagnosticLogFile = Args.getLastArgValue(OPT_diagnostic_log_file);

lib/Frontend/DependencyFile.cpp

 public:
   DFGImpl(const Preprocessor *_PP, const DependencyOutputOptions &Opts)
     : PP(_PP), OutputFile(Opts.OutputFile), Targets(Opts.Targets),
       IncludeSystemHeaders(Opts.IncludeSystemHeaders),
       PhonyTarget(Opts.UsePhonyTargets),
       AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
       SeenMissingHeader(false),
       IncludeModuleFiles(Opts.IncludeModuleFiles),
-      OutputFormat(Opts.OutputFormat) {}
+      OutputFormat(Opts.OutputFormat) {
+    for (auto ExtraDep : Opts.ExtraDeps) {
+      AddFilename(ExtraDep);
+    }
+  }
 
   void FileChanged(SourceLocation Loc, FileChangeReason Reason,
                    SrcMgr::CharacteristicKind FileType,
                    FileID PrevFID) override;
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, const FileEntry *File,
                           StringRef SearchPath, StringRef RelativePath,
void DFGImpl::OutputDependencyFile() {
   std::error_code EC;
   llvm::raw_fd_ostream OS(OutputFile, EC, llvm::sys::fs::F_Text);
   if (EC) {
     PP->getDiagnostics().Report(diag::err_fe_error_opening) << OutputFile
                                                             << EC.message();
     return;
   }
 
   // Write out the dependency targets, trying to avoid overly long
   // lines when possible. We try our best to emit exactly the same
   // dependency file as GCC (4.2), assuming the included files are the
   // same.
   const unsigned MaxColumns = 75;
   unsigned Columns = 0;
 
   for (std::vector<std::string>::iterator
          I = Targets.begin(), E = Targets.end(); I != E; ++I) {
     unsigned N = I->length();
     if (Columns == 0) {
       Columns += N;
     } else if (Columns + N + 2 > MaxColumns) {

lib/Frontend/HeaderIncludeGen.cpp

public:
   }
 
   void FileChanged(SourceLocation Loc, FileChangeReason Reason,
                    SrcMgr::CharacteristicKind FileType,
                    FileID PrevFID) override;
 };
 }
 
-void clang::AttachHeaderIncludeGen(Preprocessor &PP, bool ShowAllHeaders,
+static void PrintHeaderInfo(raw_ostream *OutputFile, const char* Filename,
+                            bool ShowDepth, unsigned CurrentIncludeDepth,
+                            bool MSStyle) {
+    // Write to a temporary string to avoid unnecessary flushing on errs().
+    SmallString<512> Pathname(Filename);
+    if (!MSStyle)
+      Lexer::Stringify(Pathname);
+
+    SmallString<256> Msg;
+    if (MSStyle)
+      Msg += "Note: including file:";
+
+    if (ShowDepth) {
+      // The main source file is at depth 1, so skip one dot.
+      for (unsigned i = 1; i != CurrentIncludeDepth; ++i)
+        Msg += MSStyle ? ' ' : '.';
+
+      if (!MSStyle)
+        Msg += ' ';
+    }
+    Msg += Pathname;
+    Msg += '\n';
+
+    OutputFile->write(Msg.data(), Msg.size());
+    OutputFile->flush();
+}
+
+void clang::AttachHeaderIncludeGen(Preprocessor &PP,
+                                   const std::vector<std::string> &ExtraHeaders,
+                                   bool ShowAllHeaders,
                                    StringRef OutputPath, bool ShowDepth,
                                    bool MSStyle) {
   raw_ostream *OutputFile = MSStyle ? &llvm::outs() : &llvm::errs();
   bool OwnsOutputFile = false;
 
   // Open the output file, if used.
   if (!OutputPath.empty()) {
     std::error_code EC;
     llvm::raw_fd_ostream *OS = new llvm::raw_fd_ostream(
         OutputPath.str(), EC, llvm::sys::fs::F_Append | llvm::sys::fs::F_Text);
     if (EC) {
       PP.getDiagnostics().Report(clang::diag::warn_fe_cc_print_header_failure)
           << EC.message();
       delete OS;
     } else {
       OS->SetUnbuffered();
       OS->SetUseAtomicWrites(true);
       OutputFile = OS;
       OwnsOutputFile = true;
     }
   }
 
+  // Print header info for extra headers, pretending they were discovered
+  // by the regular preprocessor. The primary use case is to support
+  // proper generation of Make / Ninja file dependencies for implicit includes,
+  // such as sanitizer blacklists. It's only important for cl.exe
+  // compatibility, the GNU way to generate rules is -M / -MM / -MD / -MMD.
+  for (auto Header : ExtraHeaders) {
+    PrintHeaderInfo(OutputFile, Header.c_str(), ShowDepth, 2, MSStyle);
+  }
   PP.addPPCallbacks(llvm::make_unique<HeaderIncludesCallback>(&PP,
                                                               ShowAllHeaders,
                                                               OutputFile,
                                                               OwnsOutputFile,
                                                               ShowDepth,
                                                               MSStyle));
 }
 
void HeaderIncludesCallback::FileChanged(SourceLocation Loc,
   // headers and in the predefines at a depth past the initial file and command
   // line buffers.
   bool ShowHeader = (HasProcessedPredefines ||
                      (ShowAllHeaders && CurrentIncludeDepth > 2));
 
   // Dump the header include information we are past the predefines buffer or
   // are showing all headers.
   if (ShowHeader && Reason == PPCallbacks::EnterFile) {
-    // Write to a temporary string to avoid unnecessary flushing on errs().
-    SmallString<512> Filename(UserLoc.getFilename());
-    if (!MSStyle)
-      Lexer::Stringify(Filename);
-
-    SmallString<256> Msg;
-    if (MSStyle)
-      Msg += "Note: including file:";
-
-    if (ShowDepth) {
-      // The main source file is at depth 1, so skip one dot.
-      for (unsigned i = 1; i != CurrentIncludeDepth; ++i)
-        Msg += MSStyle ? ' ' : '.';
-
-      if (!MSStyle)
-        Msg += ' ';
-    }
-    Msg += Filename;
-    Msg += '\n';
-
-    OutputFile->write(Msg.data(), Msg.size());
-    OutputFile->flush();
+    PrintHeaderInfo(OutputFile, UserLoc.getFilename(),
+                    ShowDepth, CurrentIncludeDepth, MSStyle);
   }
 }

test/Frontend/dependency-gen.c

 // CHECK-THREE: {{ }}a/b{{[/\\]}}x.h
 // RUN: %clang -MD -MF - %s -fsyntax-only -I .//./a/b/ | FileCheck -check-prefix=CHECK-FOUR %s
 // CHECK-FOUR: {{ }}a/b{{[/\\]}}x.h
 // RUN: %clang -MD -MF - %s -fsyntax-only -I a/b/. | FileCheck -check-prefix=CHECK-FIVE %s
 // CHECK-FIVE: {{ }}a/b/.{{[/\\]}}x.h
 // RUN: cd a/b
 // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s
 // CHECK-SIX: {{ }}x.h
-
+// RUN: echo "fun:foo" > %t.blacklist
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s
+// CHECK-SEVEN: {{ }}x.h
+// CHECK-SEVEN: .blacklist
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s
+// CHECK-EIGHT: {{ }}x.h
+// CHECK-EIGHT: asan_blacklist.txt
+// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s
+// CHECK-NINE: {{ }}x.h
+// CHECK-NINE-NOT: asan_blacklist.txt
 #ifndef INCLUDE_FLAG_TEST
 #include <x.h>
 #endif