Page MenuHomePhabricator

Make lld::wasm::link() or lld::elf::link() more thread-safe
Needs ReviewPublic

Authored by seanyoung on Dec 13 2020, 12:37 PM.

Details

Summary

lld::wasm::link() or lld::elf::link() call InitializeAllTargets() which is not thread-safe if another thread is attempting to use the llvm libraries.

This is an issue for Solang:

https://github.com/hyperledger-labs/solang/blob/master/src/linker/linker.cpp

It uses ldd by calling lld::wasm::link() but another thread might be trying to build an llvm module, which fails if InitializeAllTargets() gets called.

Ideally I'd like to make lld entirely thread-safe and more amiable for using as a a library, that is a much larger change.

Diff Detail

Unit TestsFailed

TimeTest
50 msx64 windows > LLVM.CodeGen/XCore::threads.ll
Script: -- : 'RUN: at line 1'; c:\ws\w64\llvm-project\premerge-checks\build\bin\llc.exe -march=xcore < C:\ws\w64\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll | c:\ws\w64\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w64\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll

Event Timeline

seanyoung created this revision.Dec 13 2020, 12:37 PM
seanyoung requested review of this revision.Dec 13 2020, 12:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 13 2020, 12:37 PM
int3 added a comment.Dec 14 2020, 2:26 PM

Mach-O changes lgtm

wasm changes lgtm assuming other backends agree.

ELF looks good.

Ideally I'd like to make lld entirely thread-safe and more amiable for using as a a library, that is a much larger change.

I still want to give a caution:

  1. if a port calls fatal or error more than --error-limit times, the process will exit.
  2. some ports may reset cl::opt and cl::list options so you cannot rely on these option values after calling lld.

Sometimes it is better to spawn an external lld process to do the link.

Ideally I'd like to make lld entirely thread-safe and more amiable for using as a a library, that is a much larger change.

This is an explicit non-goal.

Ideally I'd like to make lld entirely thread-safe and more amiable for using as a a library, that is a much larger change.

This is an explicit non-goal.

Part of the original motivation for that decision was to unblock progress on making a fast production quality linker. Now that that seems to have been achieved - I think it'd be worth revisiting the discussion/accepting patches to enhance the library-compatibility of lld and assess them based on their performance impact, etc.

This patch improves the library-compatibility by make it more thread-safe, and it has no performance impact since it simply moves the initialization to an earlier stage.