[LLVMdev] Dereferencing null pointers

[Raoux, Thomas F]

Hi,

 

I would like to understand better the meaning of constant null pointer in LLVM IR.

 

Can the optimizer assume that dereferencing a null pointer is always unreachable? Or is it only the case for address space 0? Is it ok to have null pointer be a valid pointer for an address space other than 0?

 

In InstCombine pass in InstCombiner::visitLoadInst(LoadInst &LI) I see that we replace load of null pointer by unreachable only for address space 0. But there is also code doing the following transformation for all the address spaces:

      // load (select (cond, null, P)) -> load P

      if(isa<ConstantPointerNull>(SI->getOperand(1)) &&

          LI.getPointerAddressSpace() == 0) {

        LI.setOperand(0, SI->getOperand(2));

        return &LI;

      }

 

Is this a bug? Would the correct behavior be to check that the pointers’ address space is 0?

 

Cheers,

Thomas

 

[Hal Finkel]

----- Original Message -----
> From: "Thomas F Raoux" <thomas....@intel.com>
> To: llv...@cs.uiuc.edu
> Sent: Thursday, December 11, 2014 1:03:39 PM
> Subject: [LLVMdev] Dereferencing null pointers
>
> Hi,
>
> I would like to understand better the meaning of constant null
> pointer in LLVM IR.
>
>
>
> Can the optimizer assume that dereferencing a null pointer is always
> unreachable? Or is it only the case for address space 0? Is it ok to
> have null pointer be a valid pointer for an address space other than
> 0?

Correct, it is valid to have a null pointer dereference in address spaces other than 0.

>
>
>
> In InstCombine pass in InstCombiner::visitLoadInst(LoadInst &LI) I
> see that we replace load of null pointer by unreachable only for
> address space 0. But there is also code doing the following
> transformation for all the address spaces:
>
> // load (select (cond, null, P)) -> load P
>
> if(isa<ConstantPointerNull>(SI->getOperand(1)) &&
>
> LI.getPointerAddressSpace() == 0) {
>
> LI.setOperand(0, SI->getOperand(2));
>
> return &LI;
>
> }
>
>
>
> Is this a bug? Would the correct behavior be to check that the
> pointers’ address space is 0?

But it does, no? it has && LI.getPointerAddressSpace() == 0

-Hal

>
>
>
> Cheers,
>
> Thomas
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLV...@cs.uiuc.edu http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory

_______________________________________________
LLVM Developers mailing list
LLV...@cs.uiuc.edu http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

[Raoux, Thomas F]

Sorry for the confusion, I pasted the code I fixed locally...

Here is the code at top of the trunk:

// load (select (cond, null, P)) -> load P

if (Constant *C = dyn_cast<Constant>(SI->getOperand(1)))
if (C->isNullValue()) {

LI.setOperand(0, SI->getOperand(2));
return &LI;
}

So it is a bug?

[Sanjoy Das]

Yup, that seems like a bug. I confirmed that opt -O3 transforms

define i32 @foo(i1 %c, i32 addrspace(2) *%a) {
entry:
%ptr = select i1 %c, i32 addrspace(2)* null, i32 addrspace(2)* %a
%v = load i32 addrspace(2)* %ptr
ret i32 %v
}

to

define i32 @foo(i1 %c, i32 addrspace(2)* nocapture readonly %a) #0 {
entry:
%v = load i32 addrspace(2)* %a
ret i32 %v
}

attributes #0 = { nounwind readonly }

Is there a principled way to prevent such bugs? Perhaps
ConstantPointerNull should imply addrspace(0), to get to the other
addrspaces, you need an addrspacecast?

-- Sanjoy

[Hal Finkel]

----- Original Message -----
> From: "Sanjoy Das" <san...@playingwithpointers.com>
> To: "Thomas F Raoux" <thomas....@intel.com>
> Cc: "Hal Finkel" <hfi...@anl.gov>, llv...@cs.uiuc.edu
> Sent: Thursday, December 11, 2014 1:59:10 PM
> Subject: Re: [LLVMdev] Dereferencing null pointers
>

> Yup, that seems like a bug. I confirmed that opt -O3 transforms

Agreed.

>
> define i32 @foo(i1 %c, i32 addrspace(2) *%a) {
> entry:
> %ptr = select i1 %c, i32 addrspace(2)* null, i32 addrspace(2)* %a
> %v = load i32 addrspace(2)* %ptr
> ret i32 %v
> }
>
> to
>
> define i32 @foo(i1 %c, i32 addrspace(2)* nocapture readonly %a) #0 {
> entry:
> %v = load i32 addrspace(2)* %a
> ret i32 %v
> }
>
> attributes #0 = { nounwind readonly }
>
>
> Is there a principled way to prevent such bugs? Perhaps
> ConstantPointerNull should imply addrspace(0), to get to the other
> addrspaces, you need an addrspacecast?

This is an interesting idea, but I'm not sure this is worth it. This is also used to optimize null pointer checks, and those can occur in any address space. There are several places where we need to check the address space on load/store optimizations (and we do), and we just need to do that here as well.

Can one of you submit a patch to llvm-commits?

-Hal

[Raoux, Thomas F]

I'll submit a patch by the end of the week.

-Thomas