Commit Graph

6 Commits

Author SHA1 Message Date
Fangrui Song 6b3351792c [test] Add {{.*}} to make tests immune to dso_local/dso_preemptable/(none) differences
For a definition (of most linkage types), dso_local is set for ELF -fno-pic/-fpie
and COFF, but not for Mach-O.  This nuance causes unneeded binary format differences.

This patch replaces (function) `define ` with `define{{.*}} `,
(variable/constant/alias) `= ` with `={{.*}} `, or inserts appropriate `{{.*}} `
if there is an explicit linkage.

* Clang will set dso_local for Mach-O, which is currently implied by TargetMachine.cpp. This will make COFF/Mach-O and executable ELF similar.
* Eventually I hope we can make dso_local the textual LLVM IR default (write explicit "dso_preemptable" when applicable) and -fpic ELF will be similar to everything else. This patch helps move toward that goal.
2020-12-30 20:52:01 -08:00
Roman Lebedev 536b0ee40a [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

llvm-svn: 374293
2019-10-10 09:25:02 +00:00
Vedant Kumar 175b6d1f28 [ubsan] Teach the pointer overflow check that "p - <unsigned> <= p" (PR33430)
The pointer overflow check gives false negatives when dealing with
expressions in which an unsigned value is subtracted from a pointer.
This is summarized in PR33430 [1]: ubsan permits the result of the
subtraction to be greater than "p", but it should not.

To fix the issue, we should track whether or not the pointer expression
is a subtraction. If it is, and the indices are unsigned, we know to
expect "p - <unsigned> <= p".

I've tested this by running check-{llvm,clang} with a stage2
ubsan-enabled build. I've also added some tests to compiler-rt, which
are in D34122.

[1] https://bugs.llvm.org/show_bug.cgi?id=33430

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

llvm-svn: 307955
2017-07-13 20:55:26 +00:00
Vedant Kumar 6dbf4274a5 [ubsan] Detect invalid unsigned pointer index expression (clang)
Adding an unsigned offset to a base pointer has undefined behavior if
the result of the expression would precede the base. An example from
@regehr:

  int foo(char *p, unsigned offset) {
    return p + offset >= p; // This may be optimized to '1'.
  }

  foo(p, -1); // UB.

This patch extends the pointer overflow check in ubsan to detect invalid
unsigned pointer index expressions. It changes the instrumentation to
only permit non-negative offsets in pointer index expressions when all
of the GEP indices are unsigned.

Testing: check-llvm, check-clang run on a stage2, ubsan-instrumented
build.

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

llvm-svn: 305216
2017-06-12 18:42:51 +00:00
Vedant Kumar 7f2e3d1eba Relax test to try and appease builders. NFC.
I'm not sure why, but on some bots, the order of two instructions are
swapped (as compared to the output on my machine). Loosen up the
CHECK-NEXT directives to deal with this.

Failing bot: http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/3097

llvm-svn: 304486
2017-06-01 22:27:39 +00:00
Vedant Kumar a125eb55cb [ubsan] Add a check for pointer overflow UB
Check pointer arithmetic for overflow.

For some more background on this check, see:

  https://wdtz.org/catching-pointer-overflow-bugs.html
  https://reviews.llvm.org/D20322

Patch by Will Dietz and John Regehr!

This version of the patch is different from the original in a few ways:

  - It introduces the EmitCheckedInBoundsGEP utility which inserts
    checks when the pointer overflow check is enabled.

  - It does some constant-folding to reduce instrumentation overhead.

  - It does not check some GEPs in CGExprCXX. I'm not sure that
    inserting checks here, or in CGClass, would catch many bugs.

Possible future directions for this check:

  - Introduce CGF.EmitCheckedStructGEP, to detect overflows when
    accessing structures.

Testing: Apart from the added lit test, I ran check-llvm and check-clang
with a stage2, ubsan-instrumented clang. Will and John have also done
extensive testing on numerous open source projects.

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

llvm-svn: 304459
2017-06-01 19:22:18 +00:00