Add VFS support for sanitizers' blacklist

[jkorous]

Summary

I'd like to teach clang -cc1 to use VFS overlay when searching for blacklist file specified by -fsanitize-blacklist.

[arphaman]

Hi Jan, what's the is motivation for the sanitizer blacklists to support a VFS overlay in the driver?

[jkorous]

We discussed offline and agreed that we don't need to use overlay in driver at all. I simplified the patch.

[jkorous]

Added happy-path test.

[jkorous]

I completely forgot about error handling which would be tricky in current place where I devirtualize files. We discussed offline and seems like it will be better to use VFS in SanitizerBlacklist anyway.

[jkorous]

I took a look at SanitizerSpecialCaseList and I'm now thinking it might not be necessary to plumb VFS there. It's derived from llvm::SpecialCaseList whose sole interaction with filesystem is that it has couple named constructors that read a file and init an instance with its content.
It feels like dealing with paths and files is actually out of scope for SpecialCaseList and the natural interface is probably just a buffer.

[jkorous]

The test was incorrect.

[delcypher]

clang/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml

7 ↗

(On Diff #221397)

For template substitutions I prefer to have placeholders delimited by @ symbols e.g. @REAL_FILE@. This helps avoid situations where text like REAL_FILE might appear in the template where we don't want them substituted. Having a @ symbol at the end avoids issue where a placeholder is a prefix of another (e.g. REAL_FILE is a prefix of REAL_FILE_ABSOLUTE).

It doesn't matter for this patch but I think it's a good convention to follow.

clang/test/CodeGen/ubsan-blacklist.c

10 ↗

(On Diff #221397)

Are we allowed to use sed here? For Windows builds can we assume it's available?

[jkorous]

clang/test/CodeGen/ubsan-blacklist.c

10 ↗

(On Diff #221397)

Good question! Apparently I should just use %T etc. and echo to %t-vfsoverlay.yaml. However, since we use sed in other clang-scan-deps tests too - would you be fine with me addressing this in all tests at once as a separate patch?

[jkorous]

Addressed comments.

[delcypher]

clang/test/CodeGen/ubsan-blacklist.c

10 ↗

(On Diff #221397)

Sure separate patch sounds good.

[JDevlieghere]

LGTM

clang/lib/AST/ASTContext.cpp

833 ↗

(On Diff #223655)

You could reserve the number of elements in Paths.

[rnk]

Broke Windows:
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/11306

Looks related to this: https://bugs.llvm.org/show_bug.cgi?id=43272

The fix is probably to follow the pattern I started using in rC371663, use %/T and %/S instead of %T and %S.

cfe/trunk/lib/AST/ASTContext.cpp

 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Capacity.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
 #include <cstdint>
 #include <cstdlib>
 #include <map>
 #include <memory>
static bool isAddrSpaceMapManglingEnabled(const TargetInfo &TI,
   case LangOptions::ASMM_On:
     return true;
   case LangOptions::ASMM_Off:
     return false;
   }
   llvm_unreachable("getAddressSpaceMapMangling() doesn't cover anything.");
 }
 
+static std::vector<std::string>
+getRealPaths(llvm::vfs::FileSystem &VFS, llvm::ArrayRef<std::string> Paths) {
+  std::vector<std::string> Result;
+  llvm::SmallString<128> Buffer;
+  for (const auto &File : Paths) {
+    if (std::error_code EC = VFS.getRealPath(File, Buffer))
+      llvm::report_fatal_error("can't open file '" + File + "': " + EC.message());
+    Result.push_back(Buffer.str());
+  }
+  return Result;
+}
+
 ASTContext::ASTContext(LangOptions &LOpts, SourceManager &SM,
                        IdentifierTable &idents, SelectorTable &sels,
                        Builtin::Context &builtins)
     : ConstantArrayTypes(this_()), FunctionProtoTypes(this_()),
       TemplateSpecializationTypes(this_()),
       DependentTemplateSpecializationTypes(this_()),
       SubstTemplateTemplateParmPacks(this_()), SourceMgr(SM), LangOpts(LOpts),
-      SanitizerBL(new SanitizerBlacklist(LangOpts.SanitizerBlacklistFiles, SM)),
+      SanitizerBL(new SanitizerBlacklist(
+          getRealPaths(SM.getFileManager().getVirtualFileSystem(),
+                       LangOpts.SanitizerBlacklistFiles),
+          SM)),
       XRayFilter(new XRayFunctionFilter(LangOpts.XRayAlwaysInstrumentFiles,
                                         LangOpts.XRayNeverInstrumentFiles,
                                         LangOpts.XRayAttrListFiles, SM)),
       PrintingPolicy(LOpts), Idents(idents), Selectors(sels),
       BuiltinInfo(builtins), DeclarationNames(*this), Comments(SM),
       CommentCommandTraits(BumpAlloc, LOpts.CommentOpts),
       CompCategories(this_()), LastSDM(nullptr, 0) {
   TUDecl = TranslationUnitDecl::Create(*this);

cfe/trunk/test/CodeGen/Inputs/sanitizer-blacklist-vfsoverlay.yaml

+{
+  'version': 0,
+  'roots': [
+    { 'name': '@DIR@', 'type': 'directory',
+      'contents': [
+        { 'name': 'only-virtual-file.blacklist', 'type': 'file',
+          'external-contents': '@REAL_FILE@'
+        },
+        { 'name': 'invalid-virtual-file.blacklist', 'type': 'file',
+          'external-contents': '@NONEXISTENT_FILE@'
+        }
+      ]
+    }
+  ]
+}

cfe/trunk/test/CodeGen/ubsan-blacklist.c

 // Verify ubsan doesn't emit checks for blacklisted functions and files
 // RUN: echo "fun:hash" > %t-func.blacklist
 // RUN: echo "src:%s" | sed -e 's/\\/\\\\/g' > %t-file.blacklist
 // RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -emit-llvm %s -o - | FileCheck %s --check-prefix=DEFAULT
 // RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -fsanitize-blacklist=%t-func.blacklist -emit-llvm %s -o - | FileCheck %s --check-prefix=FUNC
 // RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -fsanitize-blacklist=%t-file.blacklist -emit-llvm %s -o - | FileCheck %s --check-prefix=FILE
 
+// RUN: rm -f %t-vfsoverlay.yaml
+// RUN: rm -f %t-nonexistent.blacklist
+// RUN: sed -e "s|@DIR@|%T|g" %S/Inputs/sanitizer-blacklist-vfsoverlay.yaml | sed -e "s|@REAL_FILE@|%t-func.blacklist|g" | sed -e "s|@NONEXISTENT_FILE@|%t-nonexistent.blacklist|g" > %t-vfsoverlay.yaml
+// RUN: %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay %t-vfsoverlay.yaml -fsanitize-blacklist=%T/only-virtual-file.blacklist -emit-llvm %s -o - | FileCheck %s --check-prefix=FUNC
+
+// RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay %t-vfsoverlay.yaml -fsanitize-blacklist=%T/invalid-virtual-file.blacklist -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID-MAPPED-FILE
+// INVALID-MAPPED-FILE: invalid-virtual-file.blacklist': No such file or directory
+
+// RUN: not %clang_cc1 -fsanitize=unsigned-integer-overflow -ivfsoverlay %t-vfsoverlay.yaml -fsanitize-blacklist=%t-nonexistent.blacklist -emit-llvm %s -o - 2>&1 | FileCheck %s --check-prefix=INVALID
+// INVALID: nonexistent.blacklist': No such file or directory
+
 unsigned i;
 
 // DEFAULT: @hash
 // FUNC: @hash
 // FILE: @hash
 unsigned hash() {
 // DEFAULT: call {{.*}}void @__ubsan
 // FUNC-NOT: call {{.*}}void @__ubsan