<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 4:23 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 4:16 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=""><br class=""><div class=""><div class=""><div class="h5"><blockquote type="cite" class=""><div class="">On Mar 22, 2016, at 4:14 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank" class="">dblaikie@gmail.com</a>> wrote:</div><br class=""><div class=""><br class=""><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;" class=""><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 class=""> </span><span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span><span class=""> </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;">Author: pete<br class="">Date: Tue Mar 22 17:59:35 2016<br class="">New Revision: 264115<br class=""><br class="">URL:<span class=""> </span><a href="http://llvm.org/viewvc/llvm-project?rev=264115&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=264115&view=rev</a><br class="">Log:<br class="">Avoid memcpy from nullptr.  NFC.<br class=""><br class="">This was caught by the UBSan bot.  When the atom has no size, we would<br class="">issue a memcpy with size0 and a nullptr for the source.<br class=""><br class="">Also, this code should never have references inside an empty atom so<br class="">add an assert for that while we're here.<br class=""><br class="">Modified:<br class="">   <span class=""> </span>lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp<br class=""><br class="">Modified: lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp<br class="">URL:<span class=""> </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" class="">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp?rev=264115&r1=264114&r2=264115&view=diff</a><br class="">==============================================================================<br class="">--- lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp (original)<br class="">+++ lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp Tue Mar 22 17:59:35 2016<br class="">@@ -420,6 +420,11 @@ void ArchHandler_x86::generateAtomConten<br class="">                                           FindAddressForAtom findSectionAddress,<br class="">                                           uint64_t imageBaseAddress,<br class="">                                           uint8_t *atomContentBuffer) {<br class="">+  if (!atom.size()) {<br class="">+    assert(atom.begin() == atom.end() &&<br class="">+           "Cannot have references without content");<br class=""></blockquote><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">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 class=""><br class="">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>Thanks for that suggestion.  I think thats going to be much easier to understand than changing size() to contentSize().</div><div><br class=""></div><div>I’ll try get to this soon.  Got a few more UBSan failures to get through first.</div><div><br class=""></div><div>Cheers,</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 for pointing this out. I became somewhat used to it, but its actually quite non-intuitive so i should really fix it.</div><div class=""><br class=""></div><div class="">Cheers,</div><div class="">Pete</div><span class=""><div class=""><blockquote type="cite" class=""><div class=""><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 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;">+    return;<br class="">+  }<br class="">   // Copy raw bytes.<br class="">   memcpy(atomContentBuffer, atom.rawContent().data(), atom.size());<br class="">   // Apply fix-ups.<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></blockquote></div></div></blockquote></div></span></div></blockquote></div></div></blockquote></div><br class=""></body></html>