Fix nullptr passed to memcpy in lld/COFF/Chunks.cpp

[inglorion]

Summary

ubsan found that we sometimes pass nullptr to memcpy in
SectionChunk::writeTo(). This change adds a check that avoids that.

[ruiu]

lld/COFF/Chunks.cpp

274–276 ↗

(On Diff #143027)

I feel like

if (A.size() > 0)
  memcpy(...)

is slightly better. Returning from the function is correct because we only process relocations after this point in this function, and if a section is size 0, there should be no relocations. But I had to think for a while to understand that doing it is correct. So, just skipping memcpy would be slightly better in my opinion.

[inglorion]

@ruiu, how about this?

[ruiu]

This is not actually that important, but I think I prefer my original suggestion. At least we shouldn't use assert() to report a broken file.

[inglorion]

Fair enough. Changed it to only do the memcpy if the ArrayRef is not
empty, and otherwise leave the function alone.

[ruiu]

LGTM

COFF/Chunks.cpp

void SectionChunk::applyRelARM64(uint8_t *Off, uint16_t Type, OutputSection *OS,
   }
 }
 
 void SectionChunk::writeTo(uint8_t *Buf) const {
   if (!hasData())
     return;
   // Copy section contents from source object file to output file.
   ArrayRef<uint8_t> A = getContents();
+  if (!A.empty())
     memcpy(Buf + OutputSectionOff, A.data(), A.size());
 
   // Apply relocations.
   size_t InputSize = getSize();
   for (const coff_relocation &Rel : Relocs) {
     // Check for an invalid relocation offset. This check isn't perfect, because
     // we don't have the relocation size, which is only known after checking the
     // machine and relocation type. As a result, a relocation may overwrite the
     // beginning of the following input section.