<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 22, 2016 at 5:39 PM, Pete Cooper <span dir="ltr"><<a href="mailto:peter_cooper@apple.com" target="_blank">peter_cooper@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span class=""><blockquote type="cite"><div>On Mar 22, 2016, at 5:35 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><br><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">On Tue, Mar 22, 2016 at 5:30 PM, Pete Cooper<span> </span><span dir="ltr"><<a href="mailto:peter_cooper@apple.com" target="_blank">peter_cooper@apple.com</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word">Hi all<div><br></div><div>(No idea if I have the correct audience.  Please CC more people as needed).</div><div><br></div><div>I have an UBSan failure in BumpPtrAllocatorImpl.Allocate.</div><div><br></div><div>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.</div><div><br></div><div>To put this in code, if I have</div><div><br></div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div>BumpPtrAllocator allocator;</div><div>StringRef s;</div><div>s.copy(allocator);</div></blockquote><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>So the question is, how do we want this to behave in our code?</div><div><br></div><div>Some options:</div><div>- Assert that Allocate never gets a size 0 allocation.  So fix StringRef::copy to see this case</div><div>- Remove the attributes from Allocate but continue to return nullptr (or base of the allocator?) in this case</div><div>- Keep the attributes on Allocate and treat size 0 allocations as size 1</div></div></blockquote><div><br></div><div>I believe the last is closer to 'new's behavior - which I think returns a unique non-null address (well, unique amongst current allocations - can be recycled once deleted) if I recall correctly. </div></div></div></blockquote></span>That’s what I would have expected too.  Its like sizeof(struct {}) which can be a 1 depending on factors we don’t need to get in to here.<span class=""><br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div>Can check for wording if that's helpful/desired.</div></div></div></blockquote></span>No need for my benefit :)  I’m in agreement that this is a good behavior to go for, but will leave it to others to say if they’d like the extra detail.</div><div><br></div><div>One thing I did forget to say is that I’d like to fix StringRef::copy in all of the above cases.  I think that this method should always avoid the allocator and return StringRef(nullptr, 0) when length is 0.  I’ll get a patch up on llvm-commits if there’s no objections there.</div></div></blockquote><div><br></div><div>Sounds plausible to me</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br></div><div>Thanks,</div><div>Pete<br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br></div><div>Thanks,</div><div>Pete</div></div></blockquote></div></div></blockquote></div><br></div></blockquote></div><br></div></div>