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

[Matthieu Monrocq]

Hi Benjamin,

well, it might not crash actually, that's the issue with undefined behavior,
so I'll just add the assert.

Thanks for the link :)
Matthieu.

2011/4/30 Benjamin Kramer <benny.kra at googlemail.com>

>
> 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.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110430/2aba1a40/attachment.html>