[PATCH] IOC (Undefined behavior checks only)

Clang Frontend
1

Hi All, Richard:

Attached are initial patches for IOC (Integer Overflow Checker),
with the goal of getting its functionality into mainline clang. IOC
has been used to find many bugs in open source applications, with a
number of developers expressing interest in having it available in
Clang so they can make use of it.

The checks added in these patches are (as of today :)) entirely
achievable through existing options (-ftrapv and
-fcatch-undefined-behavior), but use of the new flags results in
useful details about the nature of the failed check.
For now, all functionality is added with *new* flags in an attempt to
preserve all existing use cases. Exactly what each of the existing
flags should do merits discussion, and it's easy to do the flag
plumbing later once that's sorted out.

The third patch (002-*) is a simple follow-up adding an option to
terminate after a check fails, making it a step away from being able
to replace -ftrapv.

New flags:

-fioc-signed
  Nicer version of -ftrapv.

-fioc-shifts
  Enables IOC checks for shift by more than bitwidth. Trapping
versions of these checks are currently implemented with
-fcatch-undefined-behavior.

-fioc-strict-shifts
  Enables IOC checks for 'strict' shift rules in C99/C++11. Trapping
versions of these checks are currently implemented with
-fcatch-undefined-behavior as of earlier today.

Corresponding lit tests included.

Example execution:
$ ./test_ioc:
test_ioc.c:5:7: runtime error: signed addition overflow [ expr = '+=',
lval = (sint32) 2147483646, rval = (sint32) 10 ]
test_ioc.c:11:7: runtime error: signed multiplication overflow [ expr
= '*=', lval = (sint32) 1073741823, rval = (sint32) 1073741823 ]
test_ioc.c:17:11: runtime error: left shift by more than bitwidth [
expr = '<<', lval = (sint32) 1, rval = (sint32) 32 ]
test_ioc.c:19:14: runtime error: left shift into or beyond sign bit [
expr = '<<', lval = (sint32) -1, rval = (sint32) 2 ]
test_ioc.c:24:10: runtime error: signed subtraction overflow [ expr =
'unary -', lval = (sint32) 0, rval = (sint32) -2147483648 ]
test_ioc.c:32:5: runtime error: signed multiplication overflow [ expr
= '*=', lval = (sint64) 1004006004001, rval = (sint64) 1004006004001 ]
test_ioc.c:33:5: runtime error: signed multiplication overflow [ expr
= '*=', lval = (sint64) 5726162197579951681, rval = (sint64)
5726162197579951681 ]
test_ioc.c:39:10: runtime error: signed addition overflow [ expr =
'unary ++', lval = (sint32) 2147483647, rval = (sint32) 1 ]
test_ioc.c:44:11: runtime error: signed addition overflow [ expr =
'unary ++', lval = (sint32) 2147483647, rval = (sint32) 1 ]
test_ioc.c:61:5: runtime error: remainder by zero is undefined [ expr
= '%=', lval = (sint32) -2147483648, rval = (sint32) 0 ]

Please review or reply with any general comments.

Thank you for your time,

~Will

0001-Add-fioc-signed-shifts-strict-shifts-to-check-for-un.patch (39.2 KB)

0001-Add-basic-IOC-runtime-logging-library.patch (12.3 KB)

0002-ioc-Add-option-to-abort-on-error-instead-of-resuming.patch (6.18 KB)

2

Hi all, Richard,

Ping! :slight_smile:

Also, updated patches (as well as follow-ups for things like
conversion checks[1]) are available in my forks[2] of clang and
compiler-rt:
https://github.com/dtzWill/ioc-clang
https://github.com/dtzWill/ioc-compiler-rt

One useful change included is the use of llvm.expect intrinsics to
indicate that undefined behavior is not the expected case, so that
optimizations (BB layout, etc) can be made accordingly. I've not
conducted performance experiments for this, but it seems like the
right thing to do regardless. I'll post back if I end up conducting
such experiem

Next big direction is to greatly improve the quality of the run-time
diagnostics by tackling the issue of how to best leverage clang's
Diagnostics to emit prettier and more useful messages. This is tricky
as we need to use Diagnostics to emit messages that are a function of
both static (source, type information, etc) and dynamic (operand
values) information. I'd love to see this happen, but I think that's
sufficiently complicated as to merit being tackled separately. Asan
and friends would probably benefit from such a solution and have
useful ideas regarding best way to make that happen, for example.

Anyway, regarding getting IOC functionality into Clang: it's my
understanding Richard has been out this past week, so mostly I wanted
to point to the updated patches and additional follow-up patches. If
you'd prefer I sent them as attachments directly (or any other changes
to better facilitate review) just let me know and I'd be happy to
accommodate. Little uncertain of best way to manage this (for example
perhaps the best answer isn't to try to apply these directly, but to
use the IOC forks above as reference points while implementing the
-fcatch-undefined-behavior proposal?), but I'm flexible and very
interested in seeing this advanced so just let me know.

Thank you for your time, and have a good one!

~Will

[1] Conversion checks are generally very useful for both debugging
purposes as well as ensuring your code does what you expect. I've
split out implicit vs explicit conversion checks; checking for
implicit conversions in particular can help catch otherwise very
difficult to notice bugs, even if it's not at all necessarily a bug.
A run-time compiler warning, if you will :). Running LLVM with these
checks led to PR13726 (minor hash function mistake), as well as a
number of places where the intent of the code could certainly be made
a great deal clearer (although I've yet to fully investigate these to
determine if they're bugs).
[2] I periodically update to latest by rebasing on top (so the commits
continue to make clean patches), just a heads-up.