[PATCH] D19666: [ubsan] Add -fubsan-strip-path-components=N

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: include/clang/Driver/Options.td:1102
@@ -1101,1 +1101,3 @@
 def Wlarge_by_value_copy_EQ : Joined<["-"], "Wlarge-by-value-copy=">, Flags<[CC1Option]>;
+def fubsan_strip_path_components_EQ : Joined<["-"], "fubsan-strip-path-components=">,
+  Group<f_clang_Group>, Flags<[CC1Option]>, MetaVarName<"<number>">,
----------------
This should be spelled `-fsanitize-undefined-strip-path-components=` or similar to match the other `-fsanitize-...` options.

================
Comment at: lib/CodeGen/CGExpr.cpp:2400-2402
@@ -2371,1 +2399,5 @@
+
+    // We're calling .data() to get the full string from this component onwards.
+    // Not just the current component.
+    auto FilenameGV = CGM.GetAddrOfConstantCString(FilenameString.data(), ".src");
     CGM.getSanitizerMetadata()->disableSanitizerForGlobal(
----------------
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.


http://reviews.llvm.org/D19666