[cfe-dev] [llvm-dev] RFC: Implementing -fno-delete-null-pointer-checks in clang

[David Chisnall via cfe-dev]

On 21 Apr 2018, at 22:19, Chris Lattner via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
>> 
>> On Apr 20, 2018, at 2:06 AM, Csaba Raduly via cfe-dev <cfe-dev at lists.llvm.org> wrote:
>> 
>> On 4/20/18, James Y Knight wrote:
>>> 
>>> 
>>> Yep. "-fnull-pointer-is-valid" has been suggested before.
>>> 
>> 
>> -fplacate-linux-kernel-developers ?
> 
> Please, lets keep this discussion on topic and productive.  The semantics described upstream of (the inverse of) "Assume that programs cannot safely dereference null pointers, and that no code or data element resides at address zero.” is entirely reasonable.

I disagree, because it depends on what you mean by dereference.  If it is safe to dereference NULL, then that means that it is also safe to hoist the null dereference above the check.  For example:

	if (x == NULL)
		return;
	y = *x;

Is safe to transform into:

	y = *x;
	if (x == NULL)
		return;

The load is guaranteed not to trap by the fact that NULL a dereferencable address.

As I understand it, the issue in GCC was that they treated the address calculation as a dereference.  The word ‘dereference’ does not appear in the C standard, which talks only about use, and therefore treats *x and &x->y both as uses.  This is intentional, because on segmented architectures it is possible that NULL + X will give you an out-of-bounds and unrepresentable pointer, possibly even causing a trap.

A Linux kernel compiled with clang was not vulnerable, because LLVM was not eliding this check, which makes this flag somewhat pointless.  LLVM assumed that a reachable load or a store of an pointer guaranteed that it was not NULL, but that a GEP did not.

I believe that the correct fix is to explicitly permit GEPs with a NULL base in the LLVM IR spec.  As I understand it, this is already implicitly permitted because code that predates __builtin_offsetof uses a cast of 0 to a struct type and takes the address of a field to get the offset.

Any code that is loading or storing from null prior to a null check probably doesn’t mind if the null check is then elided, because it’s going to trap anyway (supporting C code that allows loads and stores from an address with a bit pattern of 0 when interpreted as an integer is incredibly hard, as our friends at IBM can attest).  

David