<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Mar 22, 2016, at 5:35 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br class="Apple-interchange-newline"><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Tue, Mar 22, 2016 at 5:30 PM, Pete Cooper<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:peter_cooper@apple.com" target="_blank" class="">peter_cooper@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><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;" class="">Hi all<div class=""><br class=""></div><div class="">(No idea if I have the correct audience. Please CC more people as needed).</div><div class=""><br class=""></div><div class="">I have an UBSan failure in BumpPtrAllocatorImpl.Allocate.</div><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">To put this in code, if I have</div><div class=""><br class=""></div><blockquote style="margin: 0px 0px 0px 40px; border: none; padding: 0px;" class=""><div class="">BumpPtrAllocator allocator;</div><div class="">StringRef s;</div><div class="">s.copy(allocator);</div></blockquote><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">So the question is, how do we want this to behave in our code?</div><div class=""><br class=""></div><div class="">Some options:</div><div class="">- Assert that Allocate never gets a size 0 allocation. So fix StringRef::copy to see this case</div><div class="">- Remove the attributes from Allocate but continue to return nullptr (or base of the allocator?) in this case</div><div class="">- Keep the attributes on Allocate and treat size 0 allocations as size 1</div></div></blockquote><div class=""><br class=""></div><div class="">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>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.<br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="">Can check for wording if that's helpful/desired.</div></div></div></blockquote>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 class=""></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><br class=""></div><div>Thanks,</div><div>Pete<br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_quote" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class=""> </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;" class=""><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Pete</div></div></blockquote></div></div></blockquote></div><br class=""></body></html>