This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by inglorion on Apr 18 2018, 4:32 PM.

Details

Summary

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

Event Timeline

inglorion created this revision.Apr 18 2018, 4:32 PM
ruiu added inline comments.Apr 18 2018, 4:57 PM
lld/COFF/Chunks.cpp
274–276

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 updated this revision to Diff 143157.Apr 19 2018, 1:27 PM

@ruiu, how about this?

ruiu added a comment.Apr 19 2018, 2:55 PM

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 updated this revision to Diff 143397.Apr 20 2018, 2:53 PM
inglorion marked an inline comment as done.

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

ruiu accepted this revision.Apr 20 2018, 3:08 PM

LGTM

This revision is now accepted and ready to land.Apr 20 2018, 3:08 PM
This revision was automatically updated to reflect the committed changes.