This is an archive of the discontinued LLVM Phabricator instance.

[LLD] [COFF] Check the aux section definition size for IMAGE_COMDAT_SELECT_SAME_SIZE
ClosedPublic

Authored by mstorsjo on Aug 26 2020, 1:32 PM.

Details

Summary

Binutils generated sections seem to be padded to a multiple of 16 bytes, but the aux section definition contains the original, unpadded section length.

The size check used for IMAGE_COMDAT_SELECT_SAME_SIZE previously only checked the size of the section itself. When checking the currently processed object file against the previously chosen comdat section, we easily have access to the aux section definition of the currently processed section, but the previously selected (from a different object file) isn't currently available, as far as I can see.

This patch adds the length as a member to SectionChunk, which is a frequently allocated class, and effort has been spent to minimize its size - so this is a step backwards in that regard.

This fixes statically linking clang-built C++ object files against libstdc++ built with GCC, if the object files contain e.g. typeinfo.

I'm tagging Reid here, who has spent effort on minimizing SectionChunk even though I know he's on leave and probably won't comment.

Are there other suggestions on how to achieve the same, without inflating SectionChunk?

We do have access to the SectionChunk for the leader, which gives the ObjFile that defined it, but we'd have to iterate over all symbols in that COFFObjectFile to find the coff_aux_section_definition that matches the SectionChunk. But maybe that's tolerable for the edge case of comdat collisions with IMAGE_COMDAT_SELECT_SAME_SIZE where the section sizes mismatch?

Diff Detail

Event Timeline

mstorsjo created this revision.Aug 26 2020, 1:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2020, 1:32 PM
mstorsjo requested review of this revision.Aug 26 2020, 1:32 PM
mstorsjo updated this revision to Diff 288116.Aug 26 2020, 2:22 PM

Updated to not change SectionChunk and instead iterate through the leader's object file, looking for the right coff_aux_section_definition, in the few cases where the size is needed.

Not my area of expertise, but this looks good to me. Before submitting, please make sure you update the commit description since you didn't end up adding any fields to SectionChunk.

amccarth accepted this revision.Aug 26 2020, 2:56 PM

Oops, I forgot to "Accept Revision."

This revision is now accepted and ready to land.Aug 26 2020, 2:56 PM