[UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

Summary:
Quote from http://eel.is/c++draft/expr.add#4:
```
4     When an expression J that has integral type is added to or subtracted
      from an expression P of pointer type, the result has the type of P.
(4.1) If P evaluates to a null pointer value and J evaluates to 0,
      the result is a null pointer value.
(4.2) Otherwise, if P points to an array element i of an array object x with n
      elements ([dcl.array]), the expressions P + J and J + P
      (where J has the value j) point to the (possibly-hypothetical) array
      element i+j of x if 0≤i+j≤n and the expression P - J points to the
      (possibly-hypothetical) array element i−j of x if 0≤i−j≤n.
(4.3) Otherwise, the behavior is undefined.
```

Therefore, as per the standard, applying non-zero offset to `nullptr`
(or making non-`nullptr` a `nullptr`, by subtracting pointer's integral value
from the pointer itself) is undefined behavior. (*if* `nullptr` is not defined,
i.e. e.g. `-fno-delete-null-pointer-checks` was *not* specified.)

To make things more fun, in C (6.5.6p8), applying *any* offset to null pointer
is undefined, although Clang front-end pessimizes the code by not lowering
that info, so this UB is "harmless".

Since rL369789 (D66608 `[InstCombine] icmp eq/ne (gep inbounds P, Idx..), null -> icmp eq/ne P, null`)
LLVM middle-end uses those guarantees for transformations.
If the source contains such UB's, said code may now be miscompiled.
Such miscompilations were already observed:
* https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20190826/687838.html
* https://github.com/google/filament/pull/1566

Surprisingly, UBSan does not catch those issues
... until now. This diff teaches UBSan about these UB's.

`getelementpointer inbounds` is a pretty frequent instruction,
so this does have a measurable impact on performance;
I've addressed most of the obvious missing folds (and thus decreased the performance impact by ~5%),
and then re-performed some performance measurements using my [[ https://github.com/darktable-org/rawspeed | RawSpeed ]] benchmark:
(all measurements done with LLVM ToT, the sanitizer never fired.)
* no sanitization vs. existing check: average `+21.62%` slowdown
* existing check vs. check after this patch: average `22.04%` slowdown
* no sanitization vs. this patch: average `48.42%` slowdown

Reviewers: vsk, filcab, rsmith, aaron.ballman, vitalybuka, rjmccall, #sanitizers

Reviewed By: rsmith

Subscribers: kristof.beyls, nickdesaulniers, nikic, ychen, dtzWill, xbolva00, dberris, arphaman, rupprecht, reames, regehr, llvm-commits, cfe-commits

Tags: #clang, #sanitizers, #llvm

Differential Revision: https://reviews.llvm.org/D67122

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@374293 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 25 insertions, 3 deletions

diff --git a/lib/sanitizer_common/sanitizer_suppressions.h b/lib/sanitizer_common/sanitizer_suppressions.h
index f9da7af7e..2d88b1f72 100644
--- a/lib/sanitizer_common/sanitizer_suppressions.h
+++ b/lib/sanitizer_common/sanitizer_suppressions.h
@@ -42,7 +42,7 @@ class SuppressionContext {
   void GetMatched(InternalMmapVector<Suppression *> *matched);
 
  private:
-  static const int kMaxSuppressionTypes = 32;
+  static const int kMaxSuppressionTypes = 64;
   const char **const suppression_types_;
   const int suppression_types_num_;
 
diff --git a/lib/ubsan/ubsan_checks.inc b/lib/ubsan/ubsan_checks.inc
index 7e7216c5b..7de14ec2d 100644
--- a/lib/ubsan/ubsan_checks.inc
+++ b/lib/ubsan/ubsan_checks.inc
@@ -18,6 +18,9 @@
 
 UBSAN_CHECK(GenericUB, "undefined-behavior", "undefined")
 UBSAN_CHECK(NullPointerUse, "null-pointer-use", "null")
+UBSAN_CHECK(NullptrWithOffset, "nullptr-with-offset", "pointer-overflow")
+UBSAN_CHECK(NullptrWithNonZeroOffset, "nullptr-with-nonzero-offset", "pointer-overflow")
+UBSAN_CHECK(NullptrAfterNonZeroOffset, "nullptr-after-nonzero-offset", "pointer-overflow")
 UBSAN_CHECK(PointerOverflow, "pointer-overflow", "pointer-overflow")
 UBSAN_CHECK(MisalignedPointerUse, "misaligned-pointer-use", "alignment")
 UBSAN_CHECK(AlignmentAssumption, "alignment-assumption", "alignment")
diff --git a/lib/ubsan/ubsan_handlers.cpp b/lib/ubsan/ubsan_handlers.cpp
index e832581f9..3f9da75a1 100644
--- a/lib/ubsan/ubsan_handlers.cpp
+++ b/lib/ubsan/ubsan_handlers.cpp
@@ -691,14 +691,33 @@ static void handlePointerOverflowImpl(PointerOverflowData *Data,
                                       ValueHandle Result,
                                       ReportOptions Opts) {
   SourceLocation Loc = Data->Loc.acquire();
-  ErrorType ET = ErrorType::PointerOverflow;
+  ErrorType ET;
+
+  if (Base == 0 && Result == 0)
+    ET = ErrorType::NullptrWithOffset;
+  else if (Base == 0 && Result != 0)
+    ET = ErrorType::NullptrWithNonZeroOffset;
+  else if (Base != 0 && Result == 0)
+    ET = ErrorType::NullptrAfterNonZeroOffset;
+  else
+    ET = ErrorType::PointerOverflow;
 
   if (ignoreReport(Loc, Opts, ET))
     return;
 
   ScopedReport R(Opts, Loc, ET);
 
-  if ((sptr(Base) >= 0) == (sptr(Result) >= 0)) {
+  if (ET == ErrorType::NullptrWithOffset) {
+    Diag(Loc, DL_Error, ET, "applying zero offset to null pointer");
+  } else if (ET == ErrorType::NullptrWithNonZeroOffset) {
+    Diag(Loc, DL_Error, ET, "applying non-zero offset %0 to null pointer")
+        << Result;
+  } else if (ET == ErrorType::NullptrAfterNonZeroOffset) {
+    Diag(
+        Loc, DL_Error, ET,
+        "applying non-zero offset to non-null pointer %0 produced null pointer")
+        << (void *)Base;
+  } else if ((sptr(Base) >= 0) == (sptr(Result) >= 0)) {
     if (Base > Result)
       Diag(Loc, DL_Error, ET,
            "addition of unsigned offset to %0 overflowed to %1")