[m-dev.] undefined behaviour
jfischer at opturion.com
Thu Oct 6 13:45:17 AEDT 2016
On Wed, 28 Sep 2016, Peter Wang wrote:
> I have been trying out Address Sanitizer and Undefined Behaviour
> Sanitizer. So far the main warnings have to do with left shifts.
> 1. the left operand is negative
> 2. the right operand is equal to bits_per_int-1
> A lot of warnings are thrown up for sparse_bitset.m, which uses ints as
> bit vectors. Other UB are from hashing functions and lookup switches,
> again treating ints as bit vectors.
> e.g. left operand negative
> !:HashVal = !.HashVal `xor` (!.HashVal `unchecked_left_shift` 5),
> e.g. right operand equal to bits_per_int-1
> if ((((mercury__char_scalar_common_5)[(((mercury__char__Char_2 - (MR_Integer) 48)) >> (MR_Integer) 6)])
> & (((MR_Integer) 1 << ((((mercury__char__Char_2 - (MR_Integer) 48)) & (MR_Integer) 63))))))
> All of these basically want unsigned shifts instead.
> How shall we do that?
Short of adding unsigned types to Mercury, we should probably have add
an unsigned right shift operation to Mercury (e.g. like >>> in Java).
> I think we should provide checked arithmetic predicates in int.m, e.g.
> :- pred checked_abs(int::in, int::out) is semidet.
> :- pred checked_add(int::in, int::in, int::out) is semidet.
> :- pred checked_mul(int::in, int::in, int::out) is semidet.
> They can be implemented with gcc/clang builtin functions like
> __builtin_add_overflow, falling back to slower algorithms where
As far, as abs/1 is concerned I think we should change the existing
version to have the checked behaviour and add unchecked_abs (and
possibly semidet_abs). It may also be worth adding nabs/1 (negated
absolute value), which is sometimes useful and avoids the undesirable
behaviour with min_int.
I have no objections to adding the checked version of the arithmetic
operations to int.m.
> P.S. I added the following to Mmake.params to enable the sanitizers,
> building with mmake --use-mmc-make. Plain mmake requires more
> variables. Only tested hlc.gc.
> EXTRA_MCFLAGS += --cflag -fsanitize=address,undefined --ld-flag -fsanitize=address,undefined --ld-flag -pthread
It would be worht adding the option '--enable-sanitizer' to configure
and getting that to set it up.
More information about the developers