Re: Clang UndefinedBehaviorSanitize (Postgres14) Detected undefined-behavior

On Sat, Aug 29, 2020 at 12:36:42PM -0400, Tom Lane wrote:
> Peter Geoghegan <pg(at)bowt(dot)ie> writes:
> > I wonder if we should start using -fno-delete-null-pointer-checks:
> > https://lkml.org/lkml/2018/4/4/601
> > This may not be strictly relevant to the discussion, but I was
> > reminded of it just now and thought I'd mention it.
>
> Hmm. gcc 8.3 defines this as:
>
> Assume that programs cannot safely dereference null pointers, and
> that no code or data element resides at address zero. This option
> enables simple constant folding optimizations at all optimization
> levels. In addition, other optimization passes in GCC use this
> flag to control global dataflow analyses that eliminate useless
> checks for null pointers; these assume that a memory access to
> address zero always results in a trap, so that if a pointer is
> checked after it has already been dereferenced, it cannot be null.
>
> AFAICS, that's a perfectly valid assumption for our usage. I can see why
> the kernel might not want it, but we set things up whenever possible to
> ensure that dereferencing NULL would crash.

We do assume dereferencing NULL would crash, but we also assume this
optimization doesn't happen:

=== opt-null.c
#include <string.h>
#include <unistd.h>

int my_memcpy(void *dest, const void *src, size_t n)
{
#ifndef REMOVE_MEMCPY
memcpy(dest, src, n);
#endif
if (src)
pause();
return 0;
}
===

$ gcc --version
gcc (Debian 8.3.0-6) 8.3.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ diff -sU1 <(gcc -O2 -fno-delete-null-pointer-checks -S -o- opt-null.c) <(gcc -O2 -S -o- opt-null.c)
--- /dev/fd/63 2020-09-03 19:23:53.206864378 -0700
+++ /dev/fd/62 2020-09-03 19:23:53.206864378 -0700
@@ -8,13 +8,8 @@
.cfi_startproc
- pushq %rbx
+ subq $8, %rsp
.cfi_def_cfa_offset 16
- .cfi_offset 3, -16
- movq %rsi, %rbx
call memcpy(at)PLT
- testq %rbx, %rbx
- je .L2
call pause(at)PLT
-.L2:
xorl %eax, %eax
- popq %rbx
+ addq $8, %rsp
.cfi_def_cfa_offset 8
$ diff -sU1 <(gcc -DREMOVE_MEMCPY -O2 -fno-delete-null-pointer-checks -S -o- opt-null.c) <(gcc -DREMOVE_MEMCPY -O2 -S -o- opt-null.c)
Files /dev/fd/63 and /dev/fd/62 are identical

So yes, it would be reasonable to adopt -fno-delete-null-pointer-checks and/or
remove -fno-sanitize=nonnull-attribute from buildfarm member thorntail.

> However, while grepping the manual for that I also found
>
> '-Wnull-dereference'
> Warn if the compiler detects paths that trigger erroneous or
> undefined behavior due to dereferencing a null pointer. This
> option is only active when '-fdelete-null-pointer-checks' is
> active, which is enabled by optimizations in most targets. The
> precision of the warnings depends on the optimization options used.
>
> I wonder whether turning that on would find anything interesting.

Promising. Sadly, it doesn't warn for the above test case.