<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 22, 2016 at 4:16 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><div><div class="h5"><blockquote type="cite"><div>On Mar 22, 2016, at 4:14 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 3:59 PM, Pete Cooper via llvm-commits<span> </span><span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</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">Author: pete<br>Date: Tue Mar 22 17:59:35 2016<br>New Revision: 264115<br><br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=264115&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=264115&view=rev</a><br>Log:<br>Avoid memcpy from nullptr.  NFC.<br><br>This was caught by the UBSan bot.  When the atom has no size, we would<br>issue a memcpy with size0 and a nullptr for the source.<br><br>Also, this code should never have references inside an empty atom so<br>add an assert for that while we're here.<br><br>Modified:<br>   <span> </span>lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp<br><br>Modified: lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp?rev=264115&r1=264114&r2=264115&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp?rev=264115&r1=264114&r2=264115&view=diff</a><br>==============================================================================<br>--- lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp (original)<br>+++ lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp Tue Mar 22 17:59:35 2016<br>@@ -420,6 +420,11 @@ void ArchHandler_x86::generateAtomConten<br>                                           FindAddressForAtom findSectionAddress,<br>                                           uint64_t imageBaseAddress,<br>                                           uint8_t *atomContentBuffer) {<br>+  if (!atom.size()) {<br>+    assert(atom.begin() == atom.end() &&<br>+           "Cannot have references without content");<br></blockquote><div><br></div><div>Um, that seems like a problematic API, if size can be distinct from end() - begin() ? Perhaps some of these should be renamed to avoid confusion?</div></div></div></blockquote></div></div>Yeah…</div><div><br></div><div>Its not very standard.  begin and end walk the Reference’s, but size() is about the number of bytes of content in the atom.  The two are completely unrelated.</div><div><br></div><div>We should probably make size count the references, or remove size entirely, and have a contentSize() method where we have size() now.</div></div></blockquote><div><br></div><div>Yeah, if the object is logically its content, then size() there might make sense. But the fact that two different sequences were both chosen to "be" the atom, that seems like neither's unambiguously obvious.<br><br>The begin/end could be moved into LLVM's pretty common prefix approach: "references_begin(), references_end()" style (and a range accessor of "references()" (if you can get away without begin/end and just having the range accessor, all the better))</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 for pointing this out. I became somewhat used to it, but its actually quite non-intuitive so i should really fix it.</div><div><br></div><div>Cheers,</div><div>Pete</div><span class=""><div><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">+    return;<br>+  }<br>   // Copy raw bytes.<br>   memcpy(atomContentBuffer, atom.rawContent().data(), atom.size());<br>   // Apply fix-ups.<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></blockquote></div></div></blockquote></div><br></span></div></blockquote></div><br></div></div>