On Sat, Feb 05 2022, Jeremie Courreges-Anglas <[email protected]> wrote:
> 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

In case people wonder, the implementation is in
gnu/llvm/compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp

I suspect we should to ship a PIC/shared version of the library, along
with /usr/lib/clang/13.0.0/lib/libclang_rt.ubsan_minimal.a but ENOTIME
to look further right now.

>> 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).

Tentative proposal, maybe a bit premature

Index: clang-local.1
===================================================================
RCS file: /home/cvs/src/share/man/man1/clang-local.1,v
retrieving revision 1.22
diff -u -p -p -u -r1.22 clang-local.1
--- clang-local.1       7 Sep 2021 17:39:49 -0000       1.22
+++ clang-local.1       5 Feb 2022 17:11:48 -0000
@@ -93,6 +93,13 @@ option to treat signed integer overflows
 prevent dangerous optimizations which could remove security critical overflow
 checks.
 .It
+Only ubsan_minimal support is shipped by the base system.
+To make use of it, pass
+.Nm clang
+the following options:
+.Fl fsanitize=undefined
+.Fl fsanitize-minimal-runtime .
+.It
 The
 .Xr malloc 3 ,
 .Xr calloc 3 ,


>> % 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.

Stoopid me had already fixed extern.h, no wonder why I could not
reproduce this with my wip diff (seems to kick in properly now).

>> % 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
>>
<#secure method=pgpmime mode=sign>

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

Reply via email to