On Fri, Feb 04 2022, Greg Steuck <[email protected]> wrote:
> How do people feel about shipping the minimal UBSan runtime library[1]
> in the base system? It takes very little to build (Makefile + a few
> ifdefs that both jca@ and I hacked together). The library is tiny and
> useful enough for finding UB in base.

As already discussed I agree with that this should be shipped in base.
To make it easier for users to find it, I would suggest documenting that
only ubsan_minimal is supported for now (as far as I'm concerned I did
not look at non-minimal-ubsan and I don't plan to do so).

> % ls -l /usr/lib/clang/13.0.0/lib/libclang_rt.ubsan_minimal*
> -r--r--r--  1 root  bin  51516 Feb  4 20:02 
> /usr/lib/clang/13.0.0/lib/libclang_rt.ubsan_minimal.a
>
> % cd /usr/src/games/battlestar
> % make obj && make clean; make LDFLAGS='-fsanitize=undefined 
> -fsanitize-minimal-runtime' COPTS='-g -fsanitize=undefined 
> -fsanitize-minimal-runtime -fno-wrapv'
> % ./obj/battlestar
> Version 4.2, fall 1984.
> First Adventure game written by His Lordship, the honorable
> Admiral D.W. Riggle
>
> ubsan: shift-out-of-bounds

FWIW I can't get ubsan_minimal to kick in with the wip Makefile/diff
I shared earlier.  I'd love to have your latest diffs to do some
testing.

> % egdb ./obj/battlestar 
> (gdb) b __ubsan_handle_shift_out_of_bounds_minimal
> Breakpoint 1 at 0x23630: file 
> /usr/src/gnu/lib/libclang_rt/ubsan_minimal/../../../llvm/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp,
>  line 104.
> (gdb) bt
> No stack.
> (gdb) r
> Starting program: /usr/obj/games/battlestar/battlestar 
> Version 4.2, fall 1984.
> First Adventure game written by His Lordship, the honorable
> Admiral D.W. Riggle
>
>
> Breakpoint 1, __ubsan_handle_shift_out_of_bounds_minimal ()
>     at 
> /usr/src/gnu/lib/libclang_rt/ubsan_minimal/../../../llvm/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp:104
> 104     HANDLER(shift_out_of_bounds, "shift-out-of-bounds")
> (gdb) bt
> #0  __ubsan_handle_shift_out_of_bounds_minimal ()
>     at 
> /usr/src/gnu/lib/libclang_rt/ubsan_minimal/../../../llvm/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp:104
> #1  0x000007434ee9f654 in initialize (filename=<optimized out>) at 
> /usr/src/games/battlestar/init.c:67
> #2  0x000007434ee8af06 in main (argc=1, argv=<optimized out>) at 
> /usr/src/games/battlestar/battlestar.c:58
>
> (gdb) up
> #1  0x00000b1eddd2a654 in initialize (filename=<optimized out>) at 
> /usr/src/games/battlestar/init.c:67
> 67                              SetBit(location[p->room].objects, p->obj);
>
> (gdb) !grep SetBit extern.h
> #define SetBit(array, index)    (array[index/BITS] |= (1 << (index % BITS)))
>
> Yup, the usual, shifting too far without 1U. Hence the patch:

The diff below does not apply (tabs vs spaces) but makes sense.  ok jca@

> --- a/games/battlestar/extern.h
> +++ b/games/battlestar/extern.h
> @@ -39,9 +39,9 @@
>  #define OUTSIDE                (position > 68 && position < 246 && position 
> != 218)
>  #define rnd(x)         arc4random_uniform(x)
>  #define max(a,b)       ((a) < (b) ? (b) : (a))
> -#define TestBit(array, index)  (array[index/BITS] & (1 << (index % BITS)))
> -#define SetBit(array, index)   (array[index/BITS] |= (1 << (index % BITS)))
> -#define ClearBit(array, index) (array[index/BITS] &= ~(1 << (index % BITS)))
> +#define TestBit(array, index)  (array[index/BITS] & (1U << (index % BITS)))
> +#define SetBit(array, index)   (array[index/BITS] |= (1U << (index % BITS)))
> +#define ClearBit(array, index) (array[index/BITS] &= ~(1U << (index % BITS)))
>  /*
>   * These macros yield words to use with objects (followed but not preceded
>   * by spaces, or with no spaces if the expansion is the empty string).
>
> OK to fix battlestar while we are at it?
>
> [1] 
> https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#minimal-runtime
>

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to