[ubsan] Add -fubsan-strip-path-components=N

[filcab]

Summary

This option allows the user to control how much of the file name is
emitted by UBSan. Tuning this option allows one to save space in the
resulting binary, which is helpful for restricted execution
environments.

With a positive N, UBSan skips the first N path components.
With a negative N, UBSan only keeps the last N path components.

[rsmith]

include/clang/Driver/Options.td

1102 ↗

(On Diff #55427)

This should be spelled -fsanitize-undefined-strip-path-components= or similar to match the other -fsanitize-... options.

lib/CodeGen/CGExpr.cpp

2400–2402 ↗

(On Diff #55427)

This is a fragile way to compute the resulting string: you're relying on PLoc.getFilename() returning a nul-terminated string; this code would fail if PLoc.getFilename() started (very reasonably) returning a StringRef. Also, *I on a path iterator doesn't necessarily produce a StringRef whose contents are taken from the path being iterated (particularly for paths ending in a /, which can't happen here, but there seem to be no guarantees that this doesn't happen in other cases too).

In the forward iteration case, you can use FilenameString.substr(I - path::begin(FilenameString)) to get a correct StringRef denoting the relevant piece of the path. The path reverse iterators don't have an operator- but one could presumably be added.

[filcab]

Addressed Richard's comments.
Waiting on D19724 (operator- on Path's reverse_iterator) before landing this.

[filcab]

Remove unneeded comments.
Simplify code.

[filcab]

Remove .data() call

[ygribov]

Can we have generic option for other sanitizers?

[filcab]

In D19666#422150, @ygribov wrote:

Can we have generic option for other sanitizers?

Do other sanitizers emit paths this way?
For ASan, for example, we end up emitting them only when there are global constructors involved, where we'd emit an __asan_global.
I don't know much about msan and tsan, but I'd think it wouldn't make as much a difference as this does in ubsan, since ubsan basically emits checks in every (non-trivial) file you compile.

I'm not opposed to doing it for other sanitizers, but the better option would be to measure first to see if it's useful. If it is, then deprecate this flag and do a more generic one.
BTW, how would we pass this information along to llvm-land so the other sanitizers could use it? Is there a mechanism already in place?

[filcab]

Improve Windows support

[rsmith]

docs/UndefinedBehaviorSanitizer.rst

235 ↗

(On Diff #56289)

"option" -> "Option" or "The option"

lib/CodeGen/CGExpr.cpp

2385–2386 ↗

(On Diff #56289)

This doesn't look right: if I == E, we were asked to keep at least as many components as there were, so we should keep the entire string. I think the substr(I - E) codepath is appropriate in either case here.

test/CodeGen/ubsan-strip-path-components.cpp

6 ↗

(On Diff #56289)

I think this test is incorrect too.

[filcab]

lib/CodeGen/CGExpr.cpp

2385–2386 ↗

(On Diff #56289)

Ah, right.
My rationale was "if the number of stuff to strip (positive or negative) is greater than the number of path components, keep only the filename", which doesn't really fit with the negative case.
Will change today.

test/CodeGen/ubsan-strip-path-components.cpp

6 ↗

(On Diff #56289)

Yep, test update coming.

[filcab]

Address Richard's comments.
Final Windows fixes (force a Linux target so our mangled label matches).

[rsmith]

LGTM

cfe/trunk/docs/UndefinedBehaviorSanitizer.rst

 
 Current Status
 ==============
 
 UndefinedBehaviorSanitizer is available on selected platforms starting from LLVM
 3.3. The test suite is integrated into the CMake build and can be run with
 ``check-ubsan`` command.
 
+Additional Configuration
+========================
+
+UndefinedBehaviorSanitizer adds static check data for each check unless it is
+in trap mode. This check data includes the full file name. The option
+``-fsanitize-undefined-strip-path-components=N`` can be used to trim this
+information. If ``N`` is positive, file information emitted by
+UndefinedBehaviorSanitizer will drop the first ``N`` components from the file
+path. If ``N`` is negative, the last ``N`` components will be kept.
+
+Example
+-------
+
+For a file called ``/code/library/file.cpp``, here is what would be emitted:
+* Default (No flag, or ``-fsanitize-undefined-strip-path-components=0``): ``/code/library/file.cpp``
+* ``-fsanitize-undefined-strip-path-components=1``: ``code/library/file.cpp``
+* ``-fsanitize-undefined-strip-path-components=2``: ``library/file.cpp``
+* ``-fsanitize-undefined-strip-path-components=-1``: ``file.cpp``
+* ``-fsanitize-undefined-strip-path-components=-2``: ``library/file.cpp``
+
 More Information
 ================
 
 * From LLVM project blog:
   `What Every C Programmer Should Know About Undefined Behavior
   <http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html>`_
 * From John Regehr's *Embedded in Academia* blog:
   `A Guide to Undefined Behavior in C and C++
   <http://blog.regehr.org/archives/213>`_

cfe/trunk/include/clang/Driver/Options.td

def fno_sanitize_cfi_cross_dso : Flag<["-"], "fno-sanitize-cfi-cross-dso">,
                                  Group<f_clang_Group>, Flags<[CC1Option]>,
                                  HelpText<"Disable control flow integrity (CFI) checks for cross-DSO calls.">;
 def fsanitize_stats : Flag<["-"], "fsanitize-stats">,
                               Group<f_clang_Group>, Flags<[CC1Option]>,
                               HelpText<"Enable sanitizer statistics gathering.">;
 def fno_sanitize_stats : Flag<["-"], "fno-sanitize-stats">,
                                  Group<f_clang_Group>, Flags<[CC1Option]>,
                                  HelpText<"Disable sanitizer statistics gathering.">;
+def fsanitize_undefined_strip_path_components_EQ : Joined<["-"], "fsanitize-undefined-strip-path-components=">,
+  Group<f_clang_Group>, Flags<[CC1Option]>, MetaVarName<"<number>">,
+  HelpText<"Strip (or keep only, if negative) a given number of path components "
+           "when emitting check metadata.">;
 def funsafe_math_optimizations : Flag<["-"], "funsafe-math-optimizations">,
   Group<f_Group>;
 def fno_unsafe_math_optimizations : Flag<["-"], "fno-unsafe-math-optimizations">,
   Group<f_Group>;
 def fassociative_math : Flag<["-"], "fassociative-math">, Group<f_Group>;
 def fno_associative_math : Flag<["-"], "fno-associative-math">, Group<f_Group>;
 def freciprocal_math :
   Flag<["-"], "freciprocal-math">, Group<f_Group>, Flags<[CC1Option]>,

cfe/trunk/include/clang/Frontend/CodeGenOptions.def

 ENUM_CODEGENOPT(Inlining, InliningMethod, 2, NoInlining)
 
 // Vector functions library to use.
 ENUM_CODEGENOPT(VecLib, VectorLibrary, 1, NoLibrary)
 
 /// The default TLS model to use.
 ENUM_CODEGENOPT(DefaultTLSModel, TLSModel, 2, GeneralDynamicTLSModel)
 
+/// Number of path components to strip when emitting checks. (0 == full
+/// filename)
+VALUE_CODEGENOPT(EmitCheckPathComponentsToStrip, 32, 0)
+
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
 #undef VALUE_CODEGENOPT
 

cfe/trunk/lib/CodeGen/CGExpr.cpp

 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/MathExtras.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Transforms/Utils/SanitizerStats.h"
 
 using namespace clang;
 using namespace CodeGen;
 
 //===--------------------------------------------------------------------===//
 //                        Miscellaneous Helper Methods
 //===--------------------------------------------------------------------===//

 /// \endcode
 /// For an invalid SourceLocation, the Filename pointer is null.
 llvm::Constant *CodeGenFunction::EmitCheckSourceLocation(SourceLocation Loc) {
   llvm::Constant *Filename;
   int Line, Column;
 
   PresumedLoc PLoc = getContext().getSourceManager().getPresumedLoc(Loc);
   if (PLoc.isValid()) {
-    auto FilenameGV = CGM.GetAddrOfConstantCString(PLoc.getFilename(), ".src");
+    StringRef FilenameString = PLoc.getFilename();
+
+    int PathComponentsToStrip =
+        CGM.getCodeGenOpts().EmitCheckPathComponentsToStrip;
+    if (PathComponentsToStrip < 0) {
+      assert(PathComponentsToStrip != INT_MIN);
+      int PathComponentsToKeep = -PathComponentsToStrip;
+      auto I = llvm::sys::path::rbegin(FilenameString);
+      auto E = llvm::sys::path::rend(FilenameString);
+      while (I != E && --PathComponentsToKeep)
+        ++I;
+
+      FilenameString = FilenameString.substr(I - E);
+    } else if (PathComponentsToStrip > 0) {
+      auto I = llvm::sys::path::begin(FilenameString);
+      auto E = llvm::sys::path::end(FilenameString);
+      while (I != E && PathComponentsToStrip--)
+        ++I;
+
+      if (I != E)
+        FilenameString =
+            FilenameString.substr(I - llvm::sys::path::begin(FilenameString));
+      else
+        FilenameString = llvm::sys::path::filename(FilenameString);
+    }
+
+    auto FilenameGV = CGM.GetAddrOfConstantCString(FilenameString, ".src");
     CGM.getSanitizerMetadata()->disableSanitizerForGlobal(
                           cast<llvm::GlobalVariable>(FilenameGV.getPointer()));
     Filename = FilenameGV.getPointer();
     Line = PLoc.getLine();
     Column = PLoc.getColumn();
   } else {
     Filename = llvm::Constant::getNullValue(Int8PtrTy);
     Line = Column = 0;

cfe/trunk/lib/Driver/Tools.cpp

   // -fno-slp-vectorize-aggressive is default.
   if (Args.hasFlag(options::OPT_fslp_vectorize_aggressive,
                    options::OPT_fno_slp_vectorize_aggressive, false))
     CmdArgs.push_back("-vectorize-slp-aggressive");
 
   if (Arg *A = Args.getLastArg(options::OPT_fshow_overloads_EQ))
     A->render(Args, CmdArgs);
 
+  if (Arg *A = Args.getLastArg(
+          options::OPT_fsanitize_undefined_strip_path_components_EQ))
+    A->render(Args, CmdArgs);
+
   // -fdollars-in-identifiers default varies depending on platform and
   // language; only pass if specified.
   if (Arg *A = Args.getLastArg(options::OPT_fdollars_in_identifiers,
                                options::OPT_fno_dollars_in_identifiers)) {
     if (A->getOption().matches(options::OPT_fdollars_in_identifiers))
       CmdArgs.push_back("-fdollars-in-identifiers");
     else
       CmdArgs.push_back("-fno-dollars-in-identifiers");

cfe/trunk/lib/Frontend/CompilerInvocation.cpp

parseSanitizerKinds("-fsanitize-trap=",
                       Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
                       Opts.SanitizeTrap);
 
   Opts.CudaGpuBinaryFileNames =
       Args.getAllArgValues(OPT_fcuda_include_gpubinary);
 
   Opts.Backchain = Args.hasArg(OPT_mbackchain);
 
+  Opts.EmitCheckPathComponentsToStrip = getLastArgIntValue(
+      Args, OPT_fsanitize_undefined_strip_path_components_EQ, 0, Diags);
+
   return Success;
 }
 
 static void ParseDependencyOutputArgs(DependencyOutputOptions &Opts,
                                       ArgList &Args) {
   using namespace options;
   Opts.OutputFile = Args.getLastArgValue(OPT_dependency_file);
   Opts.Targets = Args.getAllArgValues(OPT_MT);

cfe/trunk/test/CodeGen/ubsan-strip-path-components.cpp

+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -emit-llvm -fsanitize=unreachable -o - | FileCheck %s -check-prefix=REGULAR -check-prefix=CHECK
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -emit-llvm -fsanitize=unreachable -o - -fsanitize-undefined-strip-path-components=0 | FileCheck %s -check-prefix=REGULAR -check-prefix=CHECK
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -emit-llvm -fsanitize=unreachable -o - -fsanitize-undefined-strip-path-components=2 | FileCheck %s -check-prefix=REMOVE-FIRST-TWO -check-prefix=CHECK
+
+// Try to strip too much:
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -emit-llvm -fsanitize=unreachable -o - -fsanitize-undefined-strip-path-components=-99999 | FileCheck %s -check-prefix=REGULAR
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -emit-llvm -fsanitize=unreachable -o - -fsanitize-undefined-strip-path-components=99999 | FileCheck %s -check-prefix=LAST-ONLY
+
+// Check stripping from the file name
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -emit-llvm -fsanitize=unreachable -o - -fsanitize-undefined-strip-path-components=-2 | FileCheck %s -check-prefix=LAST-TWO
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -emit-llvm -fsanitize=unreachable -o - -fsanitize-undefined-strip-path-components=-1 | FileCheck %s -check-prefix=LAST-ONLY
+
+// REGULAR: @[[SRC:[0-9.a-zA-Z_]+]] =      private unnamed_addr constant [{{.*}} x i8] c"{{.*test(.|\\5C)CodeGen(.|\\5C)ubsan-strip-path-components\.cpp}}\00", align 1
+
+// First path component: "/" or "$drive_letter:", then a name, or '\5C' on Windows
+// REMOVE-FIRST-TWO: @[[STR:[0-9.a-zA-Z_]+]] = private unnamed_addr constant [{{.*}} x i8] c"{{(.:|/)([^\\/]*(/|\\5C))}}[[REST:.*ubsan-strip-path-components\.cpp]]\00", align 1
+// REMOVE-FIRST-TWO: @[[SRC:[0-9.a-zA-Z_]+]] = private unnamed_addr constant [{{.*}} x i8] c"[[REST]]\00", align 1
+
+// LAST-TWO: @[[SRC:[0-9.a-zA-Z_]+]] =     private unnamed_addr constant [{{.*}} x i8] c"CodeGen{{/|\\5C}}ubsan-strip-path-components.cpp\00", align 1
+// LAST-ONLY: @[[SRC:[0-9.a-zA-Z_]+]] =    private unnamed_addr constant [{{.*}} x i8] c"ubsan-strip-path-components.cpp\00", align 1
+
+// CHECK: @[[STATIC_DATA:[0-9.a-zA-Z_]+]] = private unnamed_addr global { { [{{.*}} x i8]*, i32, i32 } } { { [{{.*}} x i8]*, i32, i32 } { [{{.*}} x i8]* @[[SRC]], i32 [[@LINE+6]], i32 3 } }
+void g(const char *);
+void f() {
+  // CHECK-LABEL: @_Z1fv(
+  g(__FILE__);
+  // CHECK: call void @__ubsan_handle_builtin_unreachable(i8* bitcast ({ { [{{.*}} x i8]*, i32, i32 } }* @[[STATIC_DATA]] to i8*)) {{.*}}, !nosanitize
+  __builtin_unreachable();
+}

cfe/trunk/test/Driver/fubsan-strip-path-components.cpp

+// RUN: %clang %s -### -o %t.o -fsanitize-undefined-strip-path-components=42 2>&1 | FileCheck %s
+// CHECK: "-fsanitize-undefined-strip-path-components=42"