FailedChanges

Summary

  1. [UBSan] Split nullptr-and-nonzero-offset-variable.c in another direction
  2. Revert "[ASan] Do not misrepresent high value address dereferences as null dereferences" As it was breaking bots running sanitizer lint check This reverts r374265 (git b577efe4567f1f6a711ad36e1d17280dd1c4f009)
  3. [UBSan] Split nullptr-and-nonzero-offset-variable.cpp into C and C++ variants I do not understand the BB failire, it fully passes locally.
  4. [IfCvt][ARM] Optimise diamond if-conversion for code size Currently, the heuristics the if-conversion pass uses for diamond if-conversion are based on execution time, with no consideration for code size. This adds a new set of heuristics to be used when optimising for code size. This is mostly target-independent, because the if-conversion pass can see the code size of the instructions which it is removing. For thumb, there are a few passes (insertion of IT instructions, selection of narrow branches, and selection of CBZ instructions) which are run after if conversion and affect these heuristics, so I've added target hooks to better predict the code-size effect of a proposed if-conversion. Differential revision: https://reviews.llvm.org/D67350
  5. [UBSan] Revisit nullptr-and-nonzero-offset-variable.cpp test to hopefully make it pass on sanitizer-windows BB
  6. Remove rest of time-trace message as it is inconsistent style Other options which create output files don't produce output messages. Improve documentation to help find trace file. Differential Revision: https://reviews.llvm.org/D68710
  7. [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
  8. Recommit "[Clang] Pragma vectorize_width() implies vectorize(enable)" This was further discussed at the llvm dev list: http://lists.llvm.org/pipermail/llvm-dev/2019-October/135602.html I think the brief summary of that is that this change is an improvement, this is the behaviour that we expect and promise in ours docs, and also as a result there are cases where we now emit diagnostics whereas before pragmas were silently ignored. Two areas where we can improve: 1) the diagnostic message itself, and 2) and in some cases (e.g. -Os and -Oz) the vectoriser is (quite understandably) not triggering. Original commit message: Specifying the vectorization width was supposed to implicitly enable vectorization, except that it wasn't really doing this. It was only setting the vectorize.width metadata, but not vectorize.enable. This should fix PR27643.
  9. [update_cc_test_checks] Support 'clang | opt | FileCheck' Some clang lit tests use a pipeline of the form // RUN: %clang [args] -O0 %s | opt [specific optimizations] | FileCheck %s to make the expected test output depend on as few optimization phases as possible, for stability. But when you write a RUN line of this form, you lose the ability to use update_cc_test_checks.py to automatically generate the expected output, because it only supports two-stage pipelines consisting of '%clang | FileCheck' (or %clang_cc1). This change extends the set of supported RUN lines so that pipelines with an invocation of `opt` in the middle can still be automatically handled. To implement it, I've adjusted `get_function_body()` so that it can cope with an arbitrary sequence of intermediate pipeline commands. But the code that decides which RUN lines to consider is more conservative: it only adds clang | opt | FileCheck to the set of supported lines, because I didn't want to accidentally include some other kind of line that doesn't output IR at all. (Also in this commit is the minimal change to make this script work at all, after r373912 added an extra parameter to `add_ir_checks`.) Reviewers: MaskRay, xbolva00 Subscribers: llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D68406
Revision 374309 by lebedevri:
[UBSan] Split nullptr-and-nonzero-offset-variable.c in another direction
Change TypePath in RepositoryPath in Workspace
The file was removed/compiler-rt/trunk/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.ccompiler-rt.src/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.c
The file was modified/compiler-rt/trunk/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cppcompiler-rt.src/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
Revision 374308 by russell_gallop:
Revert "[ASan] Do not misrepresent high value address dereferences as null dereferences"

As it was breaking bots running sanitizer lint check

This reverts r374265 (git b577efe4567f1f6a711ad36e1d17280dd1c4f009)
Change TypePath in RepositoryPath in Workspace
The file was modified/compiler-rt/trunk/lib/asan/asan_errors.hcompiler-rt.src/lib/asan/asan_errors.h
The file was modified/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.hcompiler-rt.src/lib/sanitizer_common/sanitizer_common.h
The file was modified/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cppcompiler-rt.src/lib/sanitizer_common/sanitizer_linux.cpp
The file was modified/compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cppcompiler-rt.src/lib/sanitizer_common/sanitizer_mac.cpp
The file was modified/compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_report.cppcompiler-rt.src/lib/sanitizer_common/sanitizer_symbolizer_report.cpp
The file was modified/compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cppcompiler-rt.src/lib/sanitizer_common/sanitizer_win.cpp
The file was removed/compiler-rt/trunk/test/asan/TestCases/Posix/high-address-dereference.ccompiler-rt.src/test/asan/TestCases/Posix/high-address-dereference.c
Revision 374306 by lebedevri:
[UBSan] Split nullptr-and-nonzero-offset-variable.cpp into C and C++ variants

I do not understand the BB failire, it fully passes locally.
Change TypePath in RepositoryPath in Workspace
The file was added/compiler-rt/trunk/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.ccompiler-rt.src/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.c
The file was modified/compiler-rt/trunk/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cppcompiler-rt.src/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
Revision 374301 by ostannard:
[IfCvt][ARM] Optimise diamond if-conversion for code size

Currently, the heuristics the if-conversion pass uses for diamond if-conversion
are based on execution time, with no consideration for code size. This adds a
new set of heuristics to be used when optimising for code size.

This is mostly target-independent, because the if-conversion pass can
see the code size of the instructions which it is removing. For thumb,
there are a few passes (insertion of IT instructions, selection of
narrow branches, and selection of CBZ instructions) which are run after
if conversion and affect these heuristics, so I've added target hooks to
better predict the code-size effect of a proposed if-conversion.

Differential revision: https://reviews.llvm.org/D67350
Change TypePath in RepositoryPath in Workspace
The file was modified/llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.hllvm.src/include/llvm/CodeGen/TargetInstrInfo.h
The file was modified/llvm/trunk/lib/CodeGen/IfConversion.cppllvm.src/lib/CodeGen/IfConversion.cpp
The file was modified/llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.cppllvm.src/lib/Target/ARM/ARMBaseInstrInfo.cpp
The file was modified/llvm/trunk/lib/Target/ARM/ARMBaseInstrInfo.hllvm.src/lib/Target/ARM/ARMBaseInstrInfo.h
The file was added/llvm/trunk/test/CodeGen/ARM/ifcvt-size.mirllvm.src/test/CodeGen/ARM/ifcvt-size.mir
Revision 374298 by lebedevri:
[UBSan] Revisit nullptr-and-nonzero-offset-variable.cpp test to hopefully make it pass on sanitizer-windows BB
Change TypePath in RepositoryPath in Workspace
The file was modified/compiler-rt/trunk/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cppcompiler-rt.src/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
Revision 374294 by russell_gallop:
Remove rest of time-trace message as it is inconsistent style

Other options which create output files don't produce output messages.
Improve documentation to help find trace file.

Differential Revision: https://reviews.llvm.org/D68710
Change TypePath in RepositoryPath in Workspace
The file was modified/cfe/trunk/include/clang/Driver/Options.tdclang.src/include/clang/Driver/Options.td
The file was modified/cfe/trunk/tools/driver/cc1_main.cppclang.src/tools/driver/cc1_main.cpp
Revision 374293 by lebedevri:
[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
Change TypePath in RepositoryPath in Workspace
The file was modified/cfe/trunk/docs/ReleaseNotes.rstclang.src/docs/ReleaseNotes.rst
The file was modified/cfe/trunk/docs/UndefinedBehaviorSanitizer.rstclang.src/docs/UndefinedBehaviorSanitizer.rst
The file was modified/cfe/trunk/lib/CodeGen/CGExprScalar.cppclang.src/lib/CodeGen/CGExprScalar.cpp
The file was added/cfe/trunk/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.cclang.src/test/CodeGen/catch-nullptr-and-nonzero-offset-blacklist.c
The file was added/cfe/trunk/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cclang.src/test/CodeGen/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.c
The file was added/cfe/trunk/test/CodeGen/catch-nullptr-and-nonzero-offset-when-nullptr-is-defined.cclang.src/test/CodeGen/catch-nullptr-and-nonzero-offset-when-nullptr-is-defined.c
The file was added/cfe/trunk/test/CodeGen/catch-nullptr-and-nonzero-offset.cclang.src/test/CodeGen/catch-nullptr-and-nonzero-offset.c
The file was added/cfe/trunk/test/CodeGen/catch-pointer-overflow-volatile.cclang.src/test/CodeGen/catch-pointer-overflow-volatile.c
The file was added/cfe/trunk/test/CodeGen/catch-pointer-overflow.cclang.src/test/CodeGen/catch-pointer-overflow.c
The file was added/cfe/trunk/test/CodeGen/ubsan-pointer-overflow.cclang.src/test/CodeGen/ubsan-pointer-overflow.c
The file was modified/cfe/trunk/test/CodeGen/ubsan-pointer-overflow.mclang.src/test/CodeGen/ubsan-pointer-overflow.m
The file was added/cfe/trunk/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cppclang.src/test/CodeGenCXX/catch-nullptr-and-nonzero-offset-in-offsetof-idiom.cpp
The file was modified/compiler-rt/trunk/lib/sanitizer_common/sanitizer_suppressions.hcompiler-rt.src/lib/sanitizer_common/sanitizer_suppressions.h
The file was modified/compiler-rt/trunk/lib/ubsan/ubsan_checks.inccompiler-rt.src/lib/ubsan/ubsan_checks.inc
The file was modified/compiler-rt/trunk/lib/ubsan/ubsan_handlers.cppcompiler-rt.src/lib/ubsan/ubsan_handlers.cpp
The file was modified/compiler-rt/trunk/test/ubsan/TestCases/Pointer/index-overflow.cppcompiler-rt.src/test/ubsan/TestCases/Pointer/index-overflow.cpp
The file was added/compiler-rt/trunk/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-constants.cppcompiler-rt.src/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-constants.cpp
The file was added/compiler-rt/trunk/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-summary.cppcompiler-rt.src/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-summary.cpp
The file was added/compiler-rt/trunk/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cppcompiler-rt.src/test/ubsan/TestCases/Pointer/nullptr-and-nonzero-offset-variable.cpp
The file was modified/compiler-rt/trunk/test/ubsan/TestCases/Pointer/unsigned-index-expression.cppcompiler-rt.src/test/ubsan/TestCases/Pointer/unsigned-index-expression.cpp
The file was added/compiler-rt/trunk/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.ccompiler-rt.src/test/ubsan_minimal/TestCases/nullptr-and-nonzero-offset.c
The file was modified/llvm/trunk/docs/ReleaseNotes.rstllvm.src/docs/ReleaseNotes.rst
Revision 374288 by sjoerdmeijer:
Recommit "[Clang] Pragma vectorize_width() implies vectorize(enable)"

This was further discussed at the llvm dev list:

http://lists.llvm.org/pipermail/llvm-dev/2019-October/135602.html

I think the brief summary of that is that this change is an improvement,
this is the behaviour that we expect and promise in ours docs, and also
as a result there are cases where we now emit diagnostics whereas before
pragmas were silently ignored. Two areas where we can improve: 1) the
diagnostic message itself, and 2) and in some cases (e.g. -Os and -Oz)
the vectoriser is (quite understandably) not triggering.

Original commit message:

Specifying the vectorization width was supposed to implicitly enable
vectorization, except that it wasn't really doing this. It was only
setting the vectorize.width metadata, but not vectorize.enable.

This should fix PR27643.
Change TypePath in RepositoryPath in Workspace
The file was modified/cfe/trunk/lib/CodeGen/CGLoopInfo.cppclang.src/lib/CodeGen/CGLoopInfo.cpp
The file was modified/cfe/trunk/test/CodeGenCXX/pragma-loop-predicate.cppclang.src/test/CodeGenCXX/pragma-loop-predicate.cpp
The file was modified/cfe/trunk/test/CodeGenCXX/pragma-loop.cppclang.src/test/CodeGenCXX/pragma-loop.cpp
Revision 374287 by statham:
[update_cc_test_checks] Support 'clang | opt | FileCheck'

Some clang lit tests use a pipeline of the form

// RUN: %clang [args] -O0 %s | opt [specific optimizations] | FileCheck %s

to make the expected test output depend on as few optimization phases
as possible, for stability. But when you write a RUN line of this
form, you lose the ability to use update_cc_test_checks.py to
automatically generate the expected output, because it only supports
two-stage pipelines consisting of '%clang | FileCheck' (or %clang_cc1).

This change extends the set of supported RUN lines so that pipelines
with an invocation of `opt` in the middle can still be automatically
handled.

To implement it, I've adjusted `get_function_body()` so that it can
cope with an arbitrary sequence of intermediate pipeline commands. But
the code that decides which RUN lines to consider is more
conservative: it only adds clang | opt | FileCheck to the set of
supported lines, because I didn't want to accidentally include some
other kind of line that doesn't output IR at all.

(Also in this commit is the minimal change to make this script work at
all, after r373912 added an extra parameter to `add_ir_checks`.)

Reviewers: MaskRay, xbolva00

Subscribers: llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D68406
Change TypePath in RepositoryPath in Workspace
The file was modified/llvm/trunk/utils/update_cc_test_checks.pyllvm.src/utils/update_cc_test_checks.py