[lld] r264115 - Avoid memcpy from nullptr. NFC.

[Pete Cooper via llvm-commits]

> On Mar 22, 2016, at 4:23 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Tue, Mar 22, 2016 at 4:16 PM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
> 
>> On Mar 22, 2016, at 4:14 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>> 
>> 
>> 
>> On Tue, Mar 22, 2016 at 3:59 PM, Pete Cooper via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>> Author: pete
>> Date: Tue Mar 22 17:59:35 2016
>> New Revision: 264115
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=264115&view=rev <http://llvm.org/viewvc/llvm-project?rev=264115&view=rev>
>> Log:
>> Avoid memcpy from nullptr.  NFC.
>> 
>> This was caught by the UBSan bot.  When the atom has no size, we would
>> issue a memcpy with size0 and a nullptr for the source.
>> 
>> Also, this code should never have references inside an empty atom so
>> add an assert for that while we're here.
>> 
>> Modified:
>>     lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp
>> 
>> Modified: lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp?rev=264115&r1=264114&r2=264115&view=diff <http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp?rev=264115&r1=264114&r2=264115&view=diff>
>> ==============================================================================
>> --- lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp (original)
>> +++ lld/trunk/lib/ReaderWriter/MachO/ArchHandler_x86.cpp Tue Mar 22 17:59:35 2016
>> @@ -420,6 +420,11 @@ void ArchHandler_x86::generateAtomConten
>>                                            FindAddressForAtom findSectionAddress,
>>                                            uint64_t imageBaseAddress,
>>                                            uint8_t *atomContentBuffer) {
>> +  if (!atom.size()) {
>> +    assert(atom.begin() == atom.end() &&
>> +           "Cannot have references without content");
>> 
>> 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?
> Yeah…
> 
> 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.
> 
> We should probably make size count the references, or remove size entirely, and have a contentSize() method where we have size() now.
> 
> 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.
> 
> 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))
Thanks for that suggestion.  I think thats going to be much easier to understand than changing size() to contentSize().

I’ll try get to this soon.  Got a few more UBSan failures to get through first.

Cheers,
Pete
>  
> 
> Thanks for pointing this out. I became somewhat used to it, but its actually quite non-intuitive so i should really fix it.
> 
> Cheers,
> Pete
>>  
>> +    return;
>> +  }
>>    // Copy raw bytes.
>>    memcpy(atomContentBuffer, atom.rawContent().data(), atom.size());
>>    // Apply fix-ups.
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160322/7365bd84/attachment.html>