Page MenuHomePhabricator

[ELF] --gc-sections: collect unused .gcc_except_table in section groups and associated text sections
ClosedPublic

Authored by MaskRay on Mon, Nov 16, 4:19 PM.

Details

Summary

try ... catch in an inline function produces .gcc_except_table.* in a COMDAT group with GCC or
newer Clang (since D83655). For --gc-sections, currently we scan .eh_frame
pieces and mark liveness of such a .gcc_except_table.* and then the associated
.text.* (if a member in a section group is retained, the others should be
retained as well).

Essentially all .text.* and .gcc_except_table.* compiled from inline
functions with try ... catch cannot be discarded by the imprecise --gc-sections.
Compared with the state before D83655, the output .gcc_except_table is smaller
(non-prevailing copies in COMDAT groups can now be discarded) but .text may be larger,
i.e. size regression.

This patch teaches the .eh_frame piece scanning code to not mark
.gcc_except_table in a section group, thus allow unused .text.* and
.gcc_except_table.* in a section group to be discarded.

Note, non-group .gcc_except_table can still not be discarded. That is the status quo.

Diff Detail

Event Timeline

MaskRay created this revision.Mon, Nov 16, 4:19 PM
MaskRay requested review of this revision.Mon, Nov 16, 4:19 PM
MaskRay edited the summary of this revision. (Show Details)Mon, Nov 16, 4:23 PM
MaskRay edited the summary of this revision. (Show Details)Mon, Nov 16, 4:27 PM
MaskRay edited the summary of this revision. (Show Details)
echristo accepted this revision.Mon, Nov 16, 6:25 PM

I think this is reasonable, if neither George or Peter say anything I think this looks ok to me.

lld/ELF/MarkLive.cpp
115–116

That's an awful lot of ! in that if condition - perhaps that could be rewritten?

This revision is now accepted and ready to land.Mon, Nov 16, 6:25 PM
MaskRay updated this revision to Diff 305639.Mon, Nov 16, 6:46 PM
MaskRay marked an inline comment as done.

Reduce the number of !

grimar accepted this revision.Tue, Nov 17, 1:05 AM

Looks reasonable to me too.

No objections from me. Although unrelated to the patch, isLSDA might be better called fromFDE, or couldBeLSDA? Maybe one for a later follow up.

MaskRay updated this revision to Diff 305821.Tue, Nov 17, 9:09 AM

Rename isLSDA to fromFDE

This revision was landed with ongoing or failed builds.Tue, Nov 17, 9:11 AM
This revision was automatically updated to reflect the committed changes.