<div dir="ltr">FWIW, I agree with Mehdi that we should just assert that our types don't get called with size zero.<div><br></div><div>That said, I don't think we can be terribly cavalier with what we expect from standard allocator types, operator new, or malloc. And so I would expect LLVM_ATTRIBUTE_RETURNS_NOALIAS to not imply NONNULL, and while it seems reasonable to put NONNULL on *our* allocate function because of the assert and the fact that we assume the underlying allocation routine never produces a null, it doesn't seem reasonable for any old function with NOALIAS to have NONNULL inferred.</div><div><br></div><div>-Chandler</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Mar 22, 2016 at 9:18 PM Mehdi Amini via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></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><blockquote type="cite"><div>On Mar 22, 2016, at 5:39 PM, Pete Cooper via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:</div><br><div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><blockquote type="cite"><div><br>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.<span> </span></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></div></div></blockquote><div><br></div></div></div><div style="word-wrap:break-word"><div><div>Well except that if sizeof(struct{}) is 1, the allocator is never called with a 0.</div><div><br></div><div>I would consider forbidding zero sized allocation in the allocator (assert()) by design (hey we're controlling every possible uses!), unless there is a real use-case for that.</div><div><br></div><div>This would also be in line with the C++ standard requirement for allocator which specifies that the result of "a.allocate(0)" is unspecified (ref: C++14 Table 28 — Allocator requirements).</div></div></div><div style="word-wrap:break-word"><div><br><div>-- </div><div>Mehdi</div></div></div><div style="word-wrap:break-word"><div><div><br></div><br><blockquote type="cite"><div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><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>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 style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">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 style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Thanks,</div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">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 style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">_______________________________________________</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">LLVM Developers mailing list</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><a href="mailto:llvm-dev@lists.llvm.org" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" target="_blank">llvm-dev@lists.llvm.org</a><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a></div></blockquote></div></div>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>