This is an archive of the discontinued LLVM Phabricator instance.

[LLD][COFF] Add index to disambiguate archive members when using -wholearchive
ClosedPublic

Authored by zero9178 on Aug 14 2019, 12:34 PM.

Details

Summary

PR42951:
When linking an archive with members that have the same name linking fails when using the -wholearchive option. This patch passes the index of the member in the archive to the offset parameter to disambiguate the member.

Diff Detail

Repository
rL LLVM

Event Timeline

zero9178 created this revision.Aug 14 2019, 12:34 PM
zero9178 edited projects, added lld; removed Restricted Project.Aug 14 2019, 12:38 PM
zero9178 removed subscribers: mehdi_amini, steven_wu, dexonsmith and 2 others.
Herald added a project: Restricted Project. · View Herald TranscriptAug 14 2019, 12:38 PM
zero9178 updated this revision to Diff 215217.Aug 14 2019, 1:15 PM

Fixed test to show regression and verify output

ruiu added inline comments.Aug 19 2019, 4:35 AM
COFF/Driver.cpp
191 ↗(On Diff #215217)

Just int would probably be better.

test/COFF/thinlto-whole-archives.ll
13–18 ↗(On Diff #215217)

This may break Windows buildbot on which path components are separated not by / but by \.

zero9178 updated this revision to Diff 218064.Aug 30 2019, 3:53 AM

Changed index from std::uint64_t to int. Made test accept both kinds of slashes

ruiu accepted this revision.Sep 2 2019, 1:11 AM

LGTM with this change.

COFF/Driver.cpp
191 ↗(On Diff #218064)

Another nit: memberIndex is better than memberOffset because offset implies that that variable contains a distance from the beginning of the file and a current member (which does not always increment by one).

This revision is now accepted and ready to land.Sep 2 2019, 1:11 AM
zero9178 updated this revision to Diff 218439.Sep 3 2019, 5:07 AM

Changed memberOffset to memberIndex as suggested by reviwer. Would need someone to commit this for me

zero9178 marked 3 inline comments as done.Sep 3 2019, 5:07 AM

Friendly after a week ping.

This revision was automatically updated to reflect the committed changes.