Should remove calling NULL pointer or not

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Should remove calling NULL pointer or not

Yin Ma

Hi,

 

For a small case, that calls NULL pointer function. LLVM explicitly converts

It to a store because it thinks it is not reachable like calling undefvalue.

In InstCombineCalls.cpp:930

 

I think it is not a right approach because calling null pointer function

Will segfault the program. Converting to a store will make program pass

Silently. This changes the behavior of a program.

 

So we need remove the case if (isa<ConstantPointerNull>(Callee) at

InstCombineCalls.cpp:918 and treat calling Null pointer reachable.

 

How do you think? Is there any reason that we should convert

a calling null pointer to a store?

 

Thanks,

 

Yin

 

 

 

 


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Should remove calling NULL pointer or not

John Criswell-4
On 11/6/13 6:36 PM, Yin Ma wrote:

Hi,

 

For a small case, that calls NULL pointer function. LLVM explicitly converts

It to a store because it thinks it is not reachable like calling undefvalue.

In InstCombineCalls.cpp:930

 

I think it is not a right approach because calling null pointer function

Will segfault the program. Converting to a store will make program pass

Silently. This changes the behavior of a program.

 

So we need remove the case if (isa<ConstantPointerNull>(Callee) at

InstCombineCalls.cpp:918 and treat calling Null pointer reachable.

 

How do you think? Is there any reason that we should convert

a calling null pointer to a store?


If calling a NULL function pointer yields undefined behavior (as defined by the C/C++ standards), then the optimization is correct: since the behavior is undefined, the compiler can change it as it sees fits.  In other words, the compiler is not required to maintain "incorrect" behavior.

The remaining question, then, is whether the C/C++ standards consider calling a NULL function pointer undefined behavior.  I suspect that it is undefined behavior, but to be honest, I do not know for certain.

-- John T.



 

Thanks,

 

Yin

 

 

 

 



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Should remove calling NULL pointer or not

Yin Ma

Hi John,

 

It seems the dereferencing a NULL pointer is undefined behavior but

Calling a function through a null pointer seems o.k.

 

If so , for this place, we need comment out the check.

 

http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#232

 

look at Notes from the October 2003 meeting.

 

Yin

 

From: John Criswell [mailto:[hidden email]]
Sent: Wednesday, November 06, 2013 6:28 PM
To: Yin Ma; 'llvmdev Dev'
Subject: Re: [LLVMdev] Should remove calling NULL pointer or not

 

On 11/6/13 6:36 PM, Yin Ma wrote:

Hi,

 

For a small case, that calls NULL pointer function. LLVM explicitly converts

It to a store because it thinks it is not reachable like calling undefvalue.

In InstCombineCalls.cpp:930

 

I think it is not a right approach because calling null pointer function

Will segfault the program. Converting to a store will make program pass

Silently. This changes the behavior of a program.

 

So we need remove the case if (isa<ConstantPointerNull>(Callee) at

InstCombineCalls.cpp:918 and treat calling Null pointer reachable.

 

How do you think? Is there any reason that we should convert

a calling null pointer to a store?


If calling a NULL function pointer yields undefined behavior (as defined by the C/C++ standards), then the optimization is correct: since the behavior is undefined, the compiler can change it as it sees fits.  In other words, the compiler is not required to maintain "incorrect" behavior.

The remaining question, then, is whether the C/C++ standards consider calling a NULL function pointer undefined behavior.  I suspect that it is undefined behavior, but to be honest, I do not know for certain.

-- John T.




 

Thanks,

 

Yin

 

 

 

 




_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

 


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Should remove calling NULL pointer or not

David Blaikie



On Thu, Nov 7, 2013 at 11:02 AM, Yin Ma <[hidden email]> wrote:

Hi John,

 

It seems the dereferencing a NULL pointer is undefined behavior but

Calling a function through a null pointer seems o.k.


What is the well defined behavior of calling a null function pointer?
 

 

If so , for this place, we need comment out the check.

 

http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#232

 

look at Notes from the October 2003 meeting.


This doesn't seem to describe well-defined behavior of calling a null function pointer. It talks about the narrow case of dereferencing a null pointer but not performing an lvalue to rvalue conversion with the result.

- David
 

 

Yin

 

From: John Criswell [mailto:[hidden email]]
Sent: Wednesday, November 06, 2013 6:28 PM
To: Yin Ma; 'llvmdev Dev'
Subject: Re: [LLVMdev] Should remove calling NULL pointer or not

 

On 11/6/13 6:36 PM, Yin Ma wrote:

Hi,

 

For a small case, that calls NULL pointer function. LLVM explicitly converts

It to a store because it thinks it is not reachable like calling undefvalue.

In InstCombineCalls.cpp:930

 

I think it is not a right approach because calling null pointer function

Will segfault the program. Converting to a store will make program pass

Silently. This changes the behavior of a program.

 

So we need remove the case if (isa<ConstantPointerNull>(Callee) at

InstCombineCalls.cpp:918 and treat calling Null pointer reachable.

 

How do you think? Is there any reason that we should convert

a calling null pointer to a store?


If calling a NULL function pointer yields undefined behavior (as defined by the C/C++ standards), then the optimization is correct: since the behavior is undefined, the compiler can change it as it sees fits.  In other words, the compiler is not required to maintain "incorrect" behavior.

The remaining question, then, is whether the C/C++ standards consider calling a NULL function pointer undefined behavior.  I suspect that it is undefined behavior, but to be honest, I do not know for certain.

-- John T.




 

Thanks,

 

Yin

 

 

 

 




_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

 


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Should remove calling NULL pointer or not

Daniel Sanders

It seems to me that the issue referenced by that meeting (http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#315) is saying that dereferencing the NULL pointer to an object is valid as part of evaluating the address of a member function (because the dereferenced NULL is not converted to an rvalue). It doesn't seem to be saying that calling a NULL function pointer is valid.

From: [hidden email] [mailto:[hidden email]] On Behalf Of David Blaikie
Sent: 07 November 2013 19:15
To: Yin Ma
Cc: llvmdev Dev
Subject: Re: [LLVMdev] Should remove calling NULL pointer or not

 

 

 

On Thu, Nov 7, 2013 at 11:02 AM, Yin Ma <[hidden email]> wrote:

Hi John,

 

It seems the dereferencing a NULL pointer is undefined behavior but

Calling a function through a null pointer seems o.k.

 

What is the well defined behavior of calling a null function pointer?

 

 

If so , for this place, we need comment out the check.

 

http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#232

 

look at Notes from the October 2003 meeting.

 

This doesn't seem to describe well-defined behavior of calling a null function pointer. It talks about the narrow case of dereferencing a null pointer but not performing an lvalue to rvalue conversion with the result.

 

- David

 

 

Yin

 

From: John Criswell [mailto:[hidden email]]
Sent: Wednesday, November 06, 2013 6:28 PM
To: Yin Ma; 'llvmdev Dev'
Subject: Re: [LLVMdev] Should remove calling NULL pointer or not

 

On 11/6/13 6:36 PM, Yin Ma wrote:

Hi,

 

For a small case, that calls NULL pointer function. LLVM explicitly converts

It to a store because it thinks it is not reachable like calling undefvalue.

In InstCombineCalls.cpp:930

 

I think it is not a right approach because calling null pointer function

Will segfault the program. Converting to a store will make program pass

Silently. This changes the behavior of a program.

 

So we need remove the case if (isa<ConstantPointerNull>(Callee) at

InstCombineCalls.cpp:918 and treat calling Null pointer reachable.

 

How do you think? Is there any reason that we should convert

a calling null pointer to a store?


If calling a NULL function pointer yields undefined behavior (as defined by the C/C++ standards), then the optimization is correct: since the behavior is undefined, the compiler can change it as it sees fits.  In other words, the compiler is not required to maintain "incorrect" behavior.

The remaining question, then, is whether the C/C++ standards consider calling a NULL function pointer undefined behavior.  I suspect that it is undefined behavior, but to be honest, I do not know for certain.

-- John T.



 

Thanks,

 

Yin

 

 

 

 



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

 


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

 


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Should remove calling NULL pointer or not

Philip Reames-4
In reply to this post by Yin Ma
I've definitely seen code like the following in various production code bases:

namespace {
  bool global = false;
}
class Foo { 
   void modify_global() { global = true; }
};

// usually, there's a layer of indirection (or two, or three) here
((Foo*)NULL)->modify_global();


Discussions on StackOverflow seem to clearly indicate that the above code is undefined.  Having said that, I have not tried to parse apart the actual spec on this one, so I'll leave that discussion to others. 

If we do decide this is undefined, do we have a general policy on how to warn about, or under which optimization levels to enable, such optimizations?  If not, it would probably worth some discussion on what a general policy might look like. 

Philip

On 11/6/13 4:36 PM, Yin Ma wrote:

Hi,

 

For a small case, that calls NULL pointer function. LLVM explicitly converts

It to a store because it thinks it is not reachable like calling undefvalue.

In InstCombineCalls.cpp:930

 

I think it is not a right approach because calling null pointer function

Will segfault the program. Converting to a store will make program pass

Silently. This changes the behavior of a program.

 

So we need remove the case if (isa<ConstantPointerNull>(Callee) at

InstCombineCalls.cpp:918 and treat calling Null pointer reachable.

 

How do you think? Is there any reason that we should convert

a calling null pointer to a store?

 

Thanks,

 

Yin

 

 

 

 



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Should remove calling NULL pointer or not

Sean Silva-2



On Mon, Nov 11, 2013 at 3:01 PM, Philip Reames <[hidden email]> wrote:
I've definitely seen code like the following in various production code bases:

namespace {
  bool global = false;
}
class Foo { 
   void modify_global() { global = true; }
};

// usually, there's a layer of indirection (or two, or three) here
((Foo*)NULL)->modify_global();


Discussions on StackOverflow seem to clearly indicate that the above code is undefined.  Having said that, I have not tried to parse apart the actual spec on this one, so I'll leave that discussion to others. 

IIRC the rule that makes this undefined is that a method must be called on an object of that type, and by definition null doesn't point at any object. (i.e. `this` can never be null).

-- Sean Silva
 

If we do decide this is undefined, do we have a general policy on how to warn about, or under which optimization levels to enable, such optimizations?  If not, it would probably worth some discussion on what a general policy might look like. 

Philip


On 11/6/13 4:36 PM, Yin Ma wrote:

Hi,

 

For a small case, that calls NULL pointer function. LLVM explicitly converts

It to a store because it thinks it is not reachable like calling undefvalue.

In InstCombineCalls.cpp:930

 

I think it is not a right approach because calling null pointer function

Will segfault the program. Converting to a store will make program pass

Silently. This changes the behavior of a program.

 

So we need remove the case if (isa<ConstantPointerNull>(Callee) at

InstCombineCalls.cpp:918 and treat calling Null pointer reachable.

 

How do you think? Is there any reason that we should convert

a calling null pointer to a store?

 

Thanks,

 

Yin

 

 

 

 



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev