This is an archive of the discontinued LLVM Phabricator instance.

[lld/mac] Crash even less on undefined symbols with --icf=all
ClosedPublic

Authored by thakis on Nov 18 2021, 2:02 PM.

Details

Reviewers
int3
Group Reviewers
Restricted Project
Commits
rGbc20bcb39e02: [lld/mac] Crash even less on undefined symbols with --icf=all
Summary

Follow-up to https://reviews.llvm.org/D112643. Even after that change, we were
still asserting if two separate functions that are eligible for ICF (same size,
same data, same number of relocs, same reloc types, ...) referred to
Undefineds. This fixes that oversight.

Diff Detail

Event Timeline

thakis created this revision.Nov 18 2021, 2:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 18 2021, 2:02 PM
thakis requested review of this revision.Nov 18 2021, 2:02 PM
int3 accepted this revision.Nov 18 2021, 10:47 PM
int3 added a subscriber: int3.
int3 added inline comments.
lld/MachO/ICF.cpp
117–118

hmm having an Undefined here basically means that the link will error out, right? maybe add a comment to that effect

(tbh I'm not sure the assert is worth much any more, basically the only thing we are still excluding is LazySymbols...)

This revision is now accepted and ready to land.Nov 18 2021, 10:47 PM
thakis added inline comments.Nov 19 2021, 6:23 AM
lld/MachO/ICF.cpp
117–118

Correct, it'll error out later. Added a comment here and in the other place.

Thanks!

Herald added a project: Restricted Project. · View Herald TranscriptNov 19 2021, 6:23 AM