On Wed, Jun 05, 2013 at 07:57:28PM +0200, Marek Polacek wrote:
> There is of course a lot of stuff that needs to be done, more
> specifically:
> 0) fix an ICE which I've noticed right now ;(
> long a = 1;
> int b = 3;
> a <<= b;
> (error: mismatching comparison operand types)
> temporarily solved by surrounding "doing_shift = true;"
> with if (comptypes (type0, type1))
> But that needs a better solution I'm afraid. Bah.
> + tree t, tt;
> + tree orig = build2 (code, TREE_TYPE (op0), op0, op1);
> + tree prec = build_int_cst (TREE_TYPE (op0),
> + TYPE_PRECISION (TREE_TYPE (op0)));
You compare prec with op1, thus they should have the same type, shifts
are one of the few binary ops that can have different types of the
operands (result type is the same as first argument, second argument
is something else). So, if you use TREE_TYPE (op1) as the type of prec,
you should be fine. More importantly, perhaps you can just use
precm1 in all the places and just use GT_EXPR for tt with precm1, and
use it in EQ.
That said, the C99 rules look somewhat different. 0 << 31 is perfectly
valid, int x = 0; x << 31 is as well. Undefined is in C99 (likely C11 too
and maybe C89 as well?) is the usual shift count out of bounds (negative or
>= prec), or if the first operand is signed and negative, or if the first
operand is signed positive, but for x << y the expression x * 2^y overflows
in the type of x.
> 1) import & build the ubsan library from LLVM
> I've already spent some time on this, but failed miserably. I've thought
> that importing ubsan/ from LLVM into libsanitizer/, adding
> libsanitizer/ubsan/Makefile.{am,in}, editing libsanitizer/Makefile.am
> and libsanitizer/configure.ac, then something like aclocal && automake
> could be sufficient, but no. I'd very much appreciate any help with
> this; is someone willing to help me with this one? And it seemed so
> easy...
I'll look at this tomorrow.
> 2) construct arguments for ubsan library
> I guess that if we want to call for instance
> void __ubsan::__ubsan_handle_shift_out_of_bounds(ShiftOutOfBoundsData
> *Data,
> ValueHandle LHS, ValueHandle
> RHS)
> from GCC, we need to construct arguments compatible with
> ShiftOutOfBoundsData/ValueHandle.
> So, perhaps we need some helper function that constructs the CALL_EXPR
> for the builtin; so far I haven't spent much time on this and don't know
> what exactly to do here. Time to look at what asan/tsan do.
> 3) add parsing of -fsanitize=<...>
> LLVM supports e.g. -fsanitize=shift,divbyzero combination, we should too.
> This doesn't sound like a big deal; just parse the arguments and set
> various flags, or error out on invalid combinations.
> 4) and of course, more instrumentation (C/C++ FE, gimple level)
> What comes to mind is:
> - float/double to integer conversions,
> - integer overflows (a long list of various cases here),
> - invalid conversions of int to bool,
> - reaching a __builtin_unreachable() call,
> - VLAs size (e.g. negative size),
> - store to/load of misaligned address,
> - store to/load of null pointer,
> - etc.
> For the time being, I plan to work on overflows instrumentation.
For at least signed addition, subtraction, multiplication overflow we
ideally want to handle it very efficiently on CPUs that can handle it
efficiently, so pretty much say on x86_64/i386 addl followed by jo
We need some builtin for that, either one with two return values
(this can be done right now say by returning a vector or complex int,
one integer will be the result of the addition/subtraction/multiplication,
another one a flag whether we've overflowed), or maybe we want new tree code
for that or something.
> 2013-06-05 Marek Polacek <pola...@redhat.com>
>
> * Makefile.in: Add ubsan.c
Missing dot at end of line.
> * common.opt: Add -fsanitize=undefined option.
> * doc/invoke.texi: Document the new flag.
> * ubsan.h: New file.
> * ubsan.c): New file.
Extra ). If prefer if the support routines for ubsan instrumentation
done in the C/C++ FEs only would live in c-family/c-ubsan.[ch] or so.
ubsan.[ch] can perhaps then be used for any instrumentation done at the
gimplification level (if anything is suitable for that), or as support code
for both of that and c-ubsan.c.
> * sanitizer.def (DEF_SANITIZER_BUILTIN):
Define. ?
> * builtins.def: Define BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW and
> BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS.
* builtins.def (BUILT_IN_UBSAN_HANDLE_DIVREM_OVERFLOW,
BUILT_IN_UBSAN_HANDLE_SHIFT_OUT_OF_BOUNDS): Define.
cp/ stuff goes into cp/ ChangeLog, without cp/ paths.
> * cp/typeck.c (cp_build_binary_op): Add division by zero and shift
> instrumentation.
Please make sure you only add it for !processing_template_decl.
Again, c/ ChangeLog.
> * c/c-typeck.c (build_binary_op): Likewise.
> * builtin-attrs.def: Define ATTR_COLD.
(ATTR_COLD): Define.
Also, the question is where exactly to place these calls to c-ubsan.c
functions. You generally want it before stuff like short_compare and
similar handling, but on the other side you want it after type promotion
(seems ok already) but e.g. for the division also after conversion to a
single result_type. Say the ubsan division libcall wants both arguments
to have the same type (unlike ubsan shift call, which has two types of
course), so if you have long long l; char c; l / c or c / l you want
both arguments converted to long long already.
Jakub
