[cfe-commits] Undefined Behavior when invoking StringRef constructor on a null pointer

[Benjamin Kramer]

On 30.04.2011, at 12:18, Matthieu Monrocq wrote:

> The  llvm::StringRef  class has a constructor taking a  const char*  parameter.
> 
> This constructor is extremely simple, and I am afraid too simple. It directly invokes  ::strlen  on the parameter, without checking whether or not the pointer is null or not.
> 
> Unfortunately as many C functions, strlen is not required by the standard to check its input, and indeed popular implementations assume that the input is not null, which results in undefined behavior.
> 
> As far as I see it, there are two ways to deal with this:
> - using an assert, to check that the input is non-null. It does not slow-down the program built with asserts disabled, but does not allow us to invoke StringRef on null pointers
> - using a simple inlined test (ternary operator ?:) to either invoke strlen or set the length to 0. Makes migrating from  const char*  to  llvm::StringRef  easier.
> 
> I've used the second approach in the patch enclosed (which gmail thoroughly refused to. I have not measured the performance impact though.

Hi Matthieu,

We had this discussion before <http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090928/088120.html>.

The result was that we don't want to have a NULL check in StringRef's ctor because it would slow down many users of StringRef and instead code that passes NULL to StringRef should be fixed.

An assert would be ok, but I don't think it's needed because strlen(NULL) is going to crash anyway.