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

[Pete Cooper via llvm-commits]

> On Mar 22, 2016, at 4:14 PM, David Blaikie <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.

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/32d09e15/attachment.html>