Yes, i erroneously assumed that the "after" was meant,
but i was wrong:
> I really meant "performed before", for cases like 4u / -2,
> where -2 is implicitly converted to UINT_MAX - 2 before
> the computation. Conversions that are performed after
> a computation aren't part of the computation at all,
> so I think it's much clearer that they're not in scope
> for this sanitizer.
llvm-svn: 338306
Summary:
C and C++ are interesting languages. They are statically typed, but weakly.
The implicit conversions are allowed. This is nice, allows to write code
while balancing between getting drowned in everything being convertible,
and nothing being convertible. As usual, this comes with a price:
```
unsigned char store = 0;
bool consume(unsigned int val);
void test(unsigned long val) {
if (consume(val)) {
// the 'val' is `unsigned long`, but `consume()` takes `unsigned int`.
// If their bit widths are different on this platform, the implicit
// truncation happens. And if that `unsigned long` had a value bigger
// than UINT_MAX, then you may or may not have a bug.
// Similarly, integer addition happens on `int`s, so `store` will
// be promoted to an `int`, the sum calculated (0+768=768),
// and the result demoted to `unsigned char`, and stored to `store`.
// In this case, the `store` will still be 0. Again, not always intended.
store = store + 768; // before addition, 'store' was promoted to int.
}
// But yes, sometimes this is intentional.
// You can either make the conversion explicit
(void)consume((unsigned int)val);
// or mask the value so no bits will be *implicitly* lost.
(void)consume((~((unsigned int)0)) & val);
}
```
Yes, there is a `-Wconversion`` diagnostic group, but first, it is kinda
noisy, since it warns on everything (unlike sanitizers, warning on an
actual issues), and second, there are cases where it does **not** warn.
So a Sanitizer is needed. I don't have any motivational numbers, but i know
i had this kind of problem 10-20 times, and it was never easy to track down.
The logic to detect whether an truncation has happened is pretty simple
if you think about it - https://godbolt.org/g/NEzXbb - basically, just
extend (using the new, not original!, signedness) the 'truncated' value
back to it's original width, and equality-compare it with the original value.
The most non-trivial thing here is the logic to detect whether this
`ImplicitCastExpr` AST node is **actually** an implicit conversion, //or//
part of an explicit cast. Because the explicit casts are modeled as an outer
`ExplicitCastExpr` with some `ImplicitCastExpr`'s as **direct** children.
https://godbolt.org/g/eE1GkJ
Nowadays, we can just use the new `part_of_explicit_cast` flag, which is set
on all the implicitly-added `ImplicitCastExpr`'s of an `ExplicitCastExpr`.
So if that flag is **not** set, then it is an actual implicit conversion.
As you may have noted, this isn't just named `-fsanitize=implicit-integer-truncation`.
There are potentially some more implicit conversions to be warned about.
Namely, implicit conversions that result in sign change; implicit conversion
between different floating point types, or between fp and an integer,
when again, that conversion is lossy.
One thing i know isn't handled is bitfields.
This is a clang part.
The compiler-rt part is D48959.
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=21530 | PR21530 ]], [[ https://bugs.llvm.org/show_bug.cgi?id=37552 | PR37552 ]], [[ https://bugs.llvm.org/show_bug.cgi?id=35409 | PR35409 ]].
Partially fixes [[ https://bugs.llvm.org/show_bug.cgi?id=9821 | PR9821 ]].
Fixes https://github.com/google/sanitizers/issues/940. (other than sign-changing implicit conversions)
Reviewers: rjmccall, rsmith, samsonov, pcc, vsk, eugenis, efriedma, kcc, erichkeane
Reviewed By: rsmith, vsk, erichkeane
Subscribers: erichkeane, klimek, #sanitizers, aaron.ballman, RKSimon, dtzWill, filcab, danielaustin, ygribov, dvyukov, milianw, mclow.lists, cfe-commits, regehr
Tags: #sanitizers
Differential Revision: https://reviews.llvm.org/D48958
llvm-svn: 338288
Summary:
Setting UBSAN_OPTIONS=silence_unsigned_overflow=1 will silence all UIO
reports. This feature, combined with
-fsanitize-recover=unsigned-integer-overflow, is useful for providing
fuzzing signal without the excessive log output.
Helps with https://github.com/google/oss-fuzz/issues/910.
Reviewers: kcc, vsk
Reviewed By: vsk
Subscribers: vsk, kubamracek, Dor1s, llvm-commits
Differential Revision: https://reviews.llvm.org/D48660
llvm-svn: 335762
Diagnose 'unreachable' UB when a noreturn function returns.
1. Insert a check at the end of functions marked noreturn.
2. A decl may be marked noreturn in the caller TU, but not marked in
the TU where it's defined. To diagnose this scenario, strip away the
noreturn attribute on the callee and insert check after calls to it.
Testing: check-clang, check-ubsan, check-ubsan-minimal, D40700
rdar://33660464
Differential Revision: https://reviews.llvm.org/D40698
llvm-svn: 321231
In r309007, I made -fsanitize=null a hard prerequisite for -fsanitize=vptr. I
did not see the need for the two checks to have separate null checking logic
for the same pointer. I expected the two checks to either always be enabled
together, or to be mutually compatible.
In the mailing list discussion re: r309007 it became clear that that isn't the
case. If a codebase is -fsanitize=vptr clean but not -fsanitize=null clean,
it's useful to have -fsanitize=vptr emit its own null check. That's what this
patch does: with it, -fsanitize=vptr can be used without -fsanitize=null.
Differential Revision: https://reviews.llvm.org/D36112
llvm-svn: 309846
On some targets, passing zero to the clz() or ctz() builtins has undefined
behavior. I ran into this issue while debugging UB in __hash_table from libcxx:
the bug I was seeing manifested itself differently under -O0 vs -Os, due to a
UB call to clz() (see: libcxx/r304617).
This patch introduces a check which can detect UB calls to builtins.
llvm.org/PR26979
Differential Revision: https://reviews.llvm.org/D34590
llvm-svn: 309459
The instrumentation generated by -fsanitize=vptr does not null check a
user pointer before loading from it. This causes crashes in the face of
UB member calls (this=nullptr), i.e it's causing user programs to crash
only after UBSan is turned on.
The fix is to make run-time null checking a prerequisite for enabling
-fsanitize=vptr, and to then teach UBSan to reuse these run-time null
checks to make -fsanitize=vptr safe.
Testing: check-clang, check-ubsan, a stage2 ubsan-enabled build
Differential Revision: https://reviews.llvm.org/D35735https://bugs.llvm.org/show_bug.cgi?id=33881
llvm-svn: 309007
Check pointer arithmetic for overflow.
For some more background on this check, see:
https://wdtz.org/catching-pointer-overflow-bugs.htmlhttps://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
Printing out stack traces along with UBSan diagnostics is unsupported on
Darwin. That's because it isn't possible to use the fast unwinder or the
slow unwinder.
Apparently, it's inappropriate to use the fast unwinder for UBSan
issues. I'm not exactly sure why (see the comment in ubsan_diag.cc).
Forcing use of the fast unwinder produces decent results, AFAICT.
Darwin also does not appear to have a slow unwinder suitable for use
with the sanitizers. Apparently that's because of PR20800 [1][2]. But
that bug has been fixed. I'm not sure if there is anything preventing
use of the slow unwinder now.
Currently, passing UBSAN_OPTIONS=print_stacktrace=1 does nothing on
Darwin. This isn't good, but it might be a while before we can fix the
situation, so we should at least document it.
[1] https://github.com/google/sanitizers/issues/137
"We can't use the slow unwinder on OSX now, because Clang produces
incorrect unwind info for the ASan runtime functions on OSX
(http://llvm.org/PR20800)."
[2] https://bugs.llvm.org/show_bug.cgi?id=20800
Bug 20800 - Invalid compact unwind info generated for a function without
frame pointers on OSX
llvm-svn: 300295
PR32346 suggests that UBSan's docs about the -fsanitize,
-fno-sanitize-recover, and -fsanitize-trap options are not explicit
enough. Try to improve the wording.
llvm-svn: 298310
Teach UBSan to detect when a value with the _Nonnull type annotation
assumes a null value. Call expressions, initializers, assignments, and
return statements are all checked.
Because _Nonnull does not affect IRGen, the new checks are disabled by
default. The new driver flags are:
-fsanitize=nullability-arg (_Nonnull violation in call)
-fsanitize=nullability-assign (_Nonnull violation in assignment)
-fsanitize=nullability-return (_Nonnull violation in return stmt)
-fsanitize=nullability (all of the above)
This patch builds on top of UBSan's existing support for detecting
violations of the nonnull attributes ('nonnull' and 'returns_nonnull'),
and relies on the compiler-rt support for those checks. Eventually we
will need to update the diagnostic messages in compiler-rt (there are
FIXME's for this, which will be addressed in a follow-up).
One point of note is that the nullability-return check is only allowed
to kick in if all arguments to the function satisfy their nullability
preconditions. This makes it necessary to emit some null checks in the
function body itself.
Testing: check-clang and check-ubsan. I also built some Apple ObjC
frameworks with an asserts-enabled compiler, and verified that we get
valid reports.
Differential Revision: https://reviews.llvm.org/D30762
llvm-svn: 297700
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.
Reviewers: rsmith
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D19666
llvm-svn: 269309
Currently, the UBSan docs make it sound like the object-size sanitizer
will only detect out-of-bounds reads/writes. It also catches some
operations that don't necessarily access memory (invalid downcasts,
calls of methods on invalid pointers, ...). This patch adds a note
about this behavior in the docs.
llvm-svn: 267447
Summary:
Create a separate page describing UBSan tool, move the description of
fine-grained checks there, provide extra information about supported
platforms, symbolization etc. This text is compiled from four parts:
* Existing documentation copied from User's Manual
* Layout used in documentation for another sanitizers (ASan, MSan etc.)
* Text written from scratch
* Small parts taken from Michael Morrison's attempt at creating UBSan
page:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20141215/249503.html
Reviewers: kcc, rsmith, silvas
Subscribers: tberghammer, danalbert, srhines, kcc
Differential Revision: http://reviews.llvm.org/D15217
llvm-svn: 254733
Roman Lebedev