[llvm-dev] UBSan, StringRef and Allocator.h

[Pete Cooper via llvm-dev]

> On Mar 22, 2016, at 5:57 PM, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:
> 
> On Tue, Mar 22, 2016 at 5:30 PM, Pete Cooper via llvm-dev
> <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>> Hi all
>> 
>> (No idea if I have the correct audience.  Please CC more people as needed).
>> 
>> I have an UBSan failure in BumpPtrAllocatorImpl.Allocate.
>> 
>> The problem is that lld requests that we StringRef::copy an empty string.
>> This passes a length of 0 to a BumpPtrAllocator.  The BumpPtrAllocator
>> happened to not have anything allocated yet so the CurPtr is nullptr, but
>> given that we need 0 space we think we have enough space and return an
>> allocation of size 0 at address nullptr.  This therefore returns nullptr
>> from Allocate, but that method is marked with LLVM_ATTRIBUTE_RETURNS_NONNULL
>> and LLVM_ATTRIBUTE_RETURNS_NOALIAS, both of which aren’t true in this case.
> 
> Why is noalias not valid here?
> 
> I thought LLVM's notion of noalias was based on data dependence --
> ptr_a does not alias ptr_b if a store to ptr_a does not have a data
> dependence with a load from ptr_b and vice versa (and for a zero-sized
> allocation there can be no data dependence since there cannot legally
> be such a load/store).  Is the C++ notion of noalias different?
That was an assumption on my part that “allocate(0) != allocate(0)’.  That is, I didn’t think you needed to dereference a pointer for no alias, just compare it.  I could be wrong as I’m just assuming thats the behavior without any formal knowledge on the subject.
> 
>> 
>> To put this in code, if I have
>> 
>> BumpPtrAllocator allocator;
>> StringRef s;
>> s.copy(allocator);
>> 
>> 
>> then i’m going to allocate 0 bytes in the allocator and get a
>> StringRef(nullptr, 0).  Its a valid StringRef, but an UBSan failures in the
>> allocator.
>> 
>> Lang and I looked up malloc behaviour online as this is fairly analogous.
>> The answer there is that you are allowed to return nullptr, or not, its
>> implementation defined.  So no help there.
>> 
>> So the question is, how do we want this to behave in our code?
>> 
>> Some options:
>> - Assert that Allocate never gets a size 0 allocation.  So fix
>> StringRef::copy to see this case
>> - Remove the attributes from Allocate but continue to return nullptr (or
>> base of the allocator?) in this case
>> - Keep the attributes on Allocate and treat size 0 allocations as size 1
> 
> A fourth option would be return a non-null pointer (maybe the address
> of some static char) for allocations of size 0; unless I'm mistaken
> about the noalias bit above.
Interesting.  I didn’t think of that, but there’s no reason that wouldn’t work, assuming the noalias is ok as you say.

Cheers,
Pete
> 
> -- Sanjoy

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160322/9b8df212/attachment.html>