This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Do not crash if common symbol has alignment 0.
ClosedPublic

Authored by grimar on Sep 30 2016, 2:22 AM.

Details

Summary

id_000024,sig_08,src_000002,op_arith16,pos_94,val_-16

contains a common symbol with zero alignment. I did not found
in specification that it is correct (like it says that for sections) and
since we were fine living without it, I made it an error instead
of assigning 1 like we do for sections.

Diff Detail

Event Timeline

grimar updated this revision to Diff 73016.Sep 30 2016, 2:22 AM
grimar retitled this revision from to [ELF] - Do not crash if common symbol has alignment 0..
grimar updated this object.
grimar added reviewers: ruiu, rafael, davide.
grimar added subscribers: llvm-commits, grimar.
ruiu accepted this revision.Sep 30 2016, 8:23 AM
ruiu edited edge metadata.

LGTM. We could handle alignment 0 as alignment 1 here, but we shouldn't do that. This is a crash bug, but we've never received a bug report about it, so it is very likely that there's no such object file out there that assumes alignment 0 = alignment 1.

This revision is now accepted and ready to land.Sep 30 2016, 8:23 AM
emaste added a subscriber: emaste.Sep 30 2016, 8:33 AM

it is very likely that there's no such object file out there

Agreed, and it will be easy for a user to track down if they ever do receive this error message.

ruiu requested changes to this revision.Sep 30 2016, 10:53 AM
ruiu edited edge metadata.

I'd move this to InputFiles.cpp.

This revision now requires changes to proceed.Sep 30 2016, 10:53 AM
grimar updated this revision to Diff 73259.Oct 3 2016, 4:26 AM
grimar edited edge metadata.
  • Addressed review comments.
rafael accepted this revision.Oct 3 2016, 9:27 AM
rafael edited edge metadata.

LGTM

ruiu accepted this revision.Oct 3 2016, 9:33 AM
ruiu edited edge metadata.

LGTM

This revision is now accepted and ready to land.Oct 3 2016, 9:33 AM
This revision was automatically updated to reflect the committed changes.