Page MenuHomePhabricator

[IR] Rename `comdat noduplicates` to `comdat nodeduplicate`
ClosedPublic

Authored by MaskRay on Jul 19 2021, 3:08 PM.

Details

Summary

In the textual format, noduplicates means no COMDAT/section group
deduplication is performed. Therefore, if both sets of sections are retained, and
they happen to define strong external symbols with the same names,
there will be a duplicate definition linker error.

In PE/COFF, the selection kind lowers to IMAGE_COMDAT_SELECT_NODUPLICATES.
The name describes the corollary instead of the immediate semantics. The name
can cause confusion to other binary formats (ELF, wasm) which have implemented/
want to implement the "no deduplication" selection kind. Rename it to be clearer.

Diff Detail

Event Timeline

MaskRay created this revision.Jul 19 2021, 3:08 PM
MaskRay requested review of this revision.Jul 19 2021, 3:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2021, 3:08 PM
rnk accepted this revision.Jul 19 2021, 3:40 PM

So, the reason the current name might feel inaccurate is that if you were writing a linker, you might imagine that two groups with the same name should be a linker error, but that is not in fact the case: it just means that both sets of sections are retained, and later symbol resolution may report duplicate symbols of the same name, if the group happens to contain strong external symbols with the same names.

I'm OK with the renaming.

This revision is now accepted and ready to land.Jul 19 2021, 3:40 PM
MaskRay edited the summary of this revision. (Show Details)Jul 19 2021, 3:53 PM
MaskRay updated this revision to Diff 360182.Jul 20 2021, 9:50 AM

update comments

Will comdat nodeduplication be a better name? deduplicate is a verb, not a noun.

nodeduplicate should be fine. We have --no-foobar style command line options For example, Mach-O ld64 has a -no_deduplicate

This revision was landed with ongoing or failed builds.Jul 20 2021, 12:47 PM
This revision was automatically updated to reflect the committed changes.