On 08.03.2020 15:30, Kamil Rytarowski wrote:
> On 22.02.2020 17:25, Kamil Rytarowski wrote:
>> When running the ATF tests under MKLIBCSANITIZER [1], there are many
>> NULL pointer arithmetic issues .
>>
>> http://netbsd.org/~kamil/mksanitizer-reports/ubsan-2020-02-22-null-pointer.txt
>>
>> These issues are in macros like:
>> - IN_ADDRHASH_READER_FOREACH()
>> - IN_ADDRLIST_WRITER_INSERT_TAIL()
>> - IFADDR_READER_FOREACH()
>> - etc
>>
>> These macros wrap internally pserialize-safe linked lists.
>>
>> What's the proper approach to address this issue?
>>
>> These reports are responsible for around half of all kinds of the
>> remaining Undefined Behavior unique issues when executing ATF tests.
>>
>>
>> [1] ./build.sh -N0 -U -V MAKECONF=/dev/null -V HAVE_LLVM=yes -V MKGCC=no
>> -V MKLLVM=yes -V MKLIBCSANITIZER=yes -j8 -u -O /public/netbsd-llvm
>> distribution
>>
>>
>>
>
> NULL + 0 and NULL + x are both Undefined Behavior (as confirmed by a C
> committee member, it was discussed and confirmed to be intentional).
>
> NULL+x is now miscompiled by Clang/LLVM after this commit:
>
> https://reviews.llvm.org/rL369789
>
> This broke various programs like:
>
> "Performing base + offset pointer arithmetic is only allowed when base
> itself is not nullptr. In other words, the compiler is assumed to allow
> that base + offset is always non-null, which an upcoming compiler
> release will do in this case. The result is that CommandStream.cpp,
> which calls this in a loop until the result is nullptr, will never
> terminate (until it runs junk data and crashes)."
>
> https://github.com/google/filament/pull/1566
>
> LLVM middle-end uses those guarantees for transformations.
>
>
> This pushed the LLVM devs to implement this NULL+x and NULL+0 check in
> UBSan:
>
> https://reviews.llvm.org/D67122
>
> NULL+0 was added to UBSan proactively as it is as of today not
> miscompiled, but it is planned to so in not so distant time. It is a
> chicken-egg problem.
>
> There is however a fallback. If we want to use NULL+0, we must use
> -fno-delete-null-pointer-checks that avoids miscompilation and raising
> UBSan reports. If we want to allow NULL+x we must enable -fwrapv.
>
> This means that we can avoid pserialize reports by enabling these CFLAGS
> for clang as well.
>
> 22 # We are compiling the kernel code with
> no-delete-null-pointer-checks,
> 23 # and compiling without it, causes issues at least on sh3 by adding
> 24 # aborts after kern_assert on NULL pointer checks.
> 25 CFLAGS+= ${${ACTIVE_CC} == "gcc":?
> -fno-delete-null-pointer-checks :}
> 26
>
> https://nxr.netbsd.org/xref/src/sys/rump/Makefile.rump#25
>
> I'm going to test it and switch -fno-delete-null-pointer-check on
> Clang/LLVM.
>
>
> For the historical note, a reproducer miscompiling NULL + x:
>
> http://netbsd.org/~kamil/null_plus_x-ub.c
>
> 136 kamil@chieftec /tmp $ /usr/local/bin/clang -O0 null_plus_x-ub.c
> 137 kamil@chieftec /tmp $ ./a.out
> 1
> 138 kamil@chieftec /tmp $ /usr/local/bin/clang -O2 null_plus_x-ub.c
> 139 kamil@chieftec /tmp $ ./a.out
> 0
> 151 kamil@chieftec /tmp $ /usr/local/bin/clang -O3 -fwrapv null_plus_x-ub.c
> 152 kamil@chieftec /tmp $ ./a.out
>
> 1
>
> NULL+0 is planned to follow up.
>
There was also a request to make a proof that memcpy(NULL,NULL,0) is UB
and can be miscompiled.
Here is a reproducer:
http://netbsd.org/~kamil/memcpy-ub.c
131 kamil@rugged /tmp $ gcc -O0 memcpy.c
132 kamil@rugged /tmp $ ./a.out
1
133 kamil@rugged /tmp $ gcc -O2 memcpy.c
134 kamil@rugged /tmp $ ./a.out
0
A fallback for freestanding environment is to use
-fno-delete-null-pointer-check.
Attachment:
signature.asc
Description: OpenPGP digital signature