This is an archive of the discontinued LLVM Phabricator instance.

Differential D18222

[lld] treat SHF_MERGE with sh_entsize == 0 as non-mergeable, not fatal error
ClosedPublic

Authored by emaste on Mar 16 2016, 1:19 PM.

Details

Reviewers
ruiu
grimar
Commits
rG2e97115a611b: [ELF] SHF_MERGE section with 0 entsize is not fatal
rLLD263660: [ELF] SHF_MERGE section with 0 entsize is not fatal
rL263660: [ELF] SHF_MERGE section with 0 entsize is not fatal
Summary

In a FreeBSD/i386 buildworld many objects have SHF_MERGE with sh_entsize == 0. For now just don't merge these.

Diff Detail

Event Timeline

emaste updated this revision to Diff 50850.Mar 16 2016, 1:19 PM
emaste retitled this revision from to [lld] treat SHF_MERGE with sh_entsize == 0 as non-mergeable, not fatal error.
emaste updated this object.
emaste added reviewers: ruiu, grimar.
emaste added a subscriber: davide.
Herald added a subscriber: emaste. · View Herald TranscriptMar 16 2016, 1:19 PM
ruiu added inline comments.Mar 16 2016, 1:24 PM
ELF/InputFiles.cpp
151–152

Can we just remove these two lines?

0 % EntSize == 0, so it should naturally fall through the next if statement.

ruiu edited edge metadata.Mar 16 2016, 1:26 PM

LGTM

ELF/InputFiles.cpp
151–152

Oh, sorry, I was confused. You can't divide by zero.

grimar accepted this revision.Mar 16 2016, 1:30 PM
grimar edited edge metadata.

LGTM

This revision is now accepted and ready to land.Mar 16 2016, 1:30 PM
Closed by commit rL263660: [ELF] SHF_MERGE section with 0 entsize is not fatal (authored by emaste). · Explain WhyMar 16 2016, 1:37 PM
This revision was automatically updated to reflect the committed changes.
emaste mentioned this in D18225: [ELF] SHF_MERGE section with 0 entsize is not fatal.Mar 16 2016, 2:02 PM

Revision Contents

PathSize
ELF/
4 lines

Diff 50850

ELF/InputFiles.cpp

Show First 20 Lines • Show All 142 Lines • ▼ Show 20 Lines
template <class ELFT> static bool shouldMerge(const typename ELFT::Shdr &Sec) { template <class ELFT> static bool shouldMerge(const typename ELFT::Shdr &Sec) {
typedef typename ELFT::uint uintX_t; typedef typename ELFT::uint uintX_t;
uintX_t Flags = Sec.sh_flags; uintX_t Flags = Sec.sh_flags;
if (!(Flags & SHF_MERGE)) if (!(Flags & SHF_MERGE))
return false; return false;
if (Flags & SHF_WRITE) if (Flags & SHF_WRITE)
fatal("writable SHF_MERGE sections are not supported"); fatal("writable SHF_MERGE sections are not supported");
uintX_t EntSize = Sec.sh_entsize; uintX_t EntSize = Sec.sh_entsize;
if (!EntSize || Sec.sh_size % EntSize) if (!EntSize)
return false;
ruiuUnsubmitted
Not Done
ReplyInline Actions

Can we just remove these two lines?

0 % EntSize == 0, so it should naturally fall through the next if statement.

ruiu: Can we just remove these two lines? 0 % EntSize == 0, so it should naturally fall through the…
ruiuUnsubmitted
Not Done
ReplyInline Actions

Oh, sorry, I was confused. You can't divide by zero.

ruiu: Oh, sorry, I was confused. You can't divide by zero.
if (Sec.sh_size % EntSize)
fatal("SHF_MERGE section size must be a multiple of sh_entsize"); fatal("SHF_MERGE section size must be a multiple of sh_entsize");
// Don't try to merge if the aligment is larger than the sh_entsize and this // Don't try to merge if the aligment is larger than the sh_entsize and this
// is not SHF_STRINGS. // is not SHF_STRINGS.
// //
// Since this is not a SHF_STRINGS, we would need to pad after every entity. // Since this is not a SHF_STRINGS, we would need to pad after every entity.
// It would be equivalent for the producer of the .o to just set a larger // It would be equivalent for the producer of the .o to just set a larger
// sh_entsize. // sh_entsize.
▲ Show 20 Lines • Show All 420 Lines • Show Last 20 Lines