Page MenuHomePhabricator

[ELF] Error on archive with missing index

Authored by sbc100 on Jun 25 2019, 11:41 AM.



This matches the wasm lld and GNU ld behavior.

The ELF linker has special handling for bitcode archives but if that
doesn't kick in we probably want to error out rather than silently
ignore the library.

Diff Detail


Event Timeline

sbc100 created this revision.Jun 25 2019, 11:41 AM
ruiu added a comment.Jun 26 2019, 4:32 AM

I think this is a backward-incompatible change. Previously, you could create an archive containing both bitcode files and ELF files, and this patch disallow that combination, which seems a bit too risky change to me.

This change only effects archives without an index. If you previously created an archive with both bitcode *and* ELF files *and* you didn't create and index, the previous behaviour was to completely ignore the archive. This seems bad to me. The new behaviour is to give an error. We could downgrade it to a warning but both GNU ld and wasm-ld give a hard error in this case.

Are you saying that ignoring these hybrid archives is a behavior that you want to keep?

The previous behaviour was also to ignore archives that contain only ELF files but no index.

ruiu added a comment.Jun 27 2019, 6:42 AM

The previous behaviour was also to ignore archives that contain only ELF files but no index.

Is that true? If an archive has no index, each individual member is handled as if they were surrounded by --start-lib/--end-lib, so they wouldn't be ignored, I think.

I'm pretty certain yes. The old code only loads archives one object at a time if all objects are bitcode. I didn't change that part.

The comment also says exactly that does't it?

Can you take a closer look? I'm pretty sure this change is correct. At the very least an improvement, and not a regression.

MaskRay added inline comments.Jul 1 2019, 8:27 PM
8 ↗(On Diff #206492)

/dev/null if you don't need the output file?

ruiu added a comment.Jul 2 2019, 2:14 AM


Yeah, looks like I misunderstood the existing code. Thank you for pointing that out.

sbc100 marked 2 inline comments as done.Jul 2 2019, 7:27 PM
sbc100 added inline comments.
8 ↗(On Diff #206492)

Done. I assume that magically works on windows too somehow.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 2 2019, 7:30 PM
This revision was automatically updated to reflect the committed changes.
aykevl added a subscriber: aykevl.Nov 12 2019, 8:52 AM

Just a note after the fact: this change broke linking in TinyGo. It used to manually build an archive (without index) with object files from compiler-rt and that worked just fine in LLVM 8 - it did read symbols from those archives even without a symbol table. In LLVM 9, ld.lld refuses to link those libraries. We'll work around it by manually building a symbol table for lld.

For details:

sbc100 marked an inline comment as done.Nov 12 2019, 9:01 AM

I'm pretty sure this change went from silently ignoring the archive to erroring out. So I think your use case would have been broken even before this change. See the return statement on line 227. Perhaps there are no needed symbols in your archive? In that case maybe this would be a regression but that seems unlikely.