Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BPF programs use single threaded LLD #6088

Merged
merged 1 commit into from Sep 25, 2019

Conversation

@jackcmay
Copy link
Contributor

commented Sep 25, 2019

Problem

Multi-threaded LLD is causing only partial relocation to take place. This results in what should be a double relocation (relocation of a symbol which contains an address that is also relocated) but only the first relocation is created correctly. The second relocation requires that an offset be written to the memory location being relocated, but that offset is zero in the final ELF. It's unclear why the LLD multi-threaded configuration is causing this issue and more investigation is warranted but not necessary at this time and will be time-consuming.

Summary of Changes

Use LLD in single-thread mode. There is very little performance impact since BPF programs include a small number of small object files.

The PR also pulls in rbpf changes that force a load failure if any invalid relocations are found.

Fixes #3108

@garious

This comment has been minimized.

Copy link
Member

commented Sep 25, 2019

Also, consider dumping LLD. Though it's been around for awhile now, it's still the least proven component in the toolchain.

@jackcmay

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2019

Thanks @garious , Will consider if this pops up again

@jackcmay jackcmay added the automerge label Sep 25, 2019
@solana-grimes

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2019

💔 Unable to automerge due to CI failure

@codecov

This comment has been minimized.

Copy link

commented Sep 25, 2019

Codecov Report

Merging #6088 into master will decrease coverage by 0.4%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           master   #6088     +/-   ##
========================================
- Coverage    74.7%   74.3%   -0.5%     
========================================
  Files         226     238     +12     
  Lines       40660   42301   +1641     
========================================
+ Hits        30413   31431   +1018     
- Misses      10247   10870    +623
@jackcmay jackcmay merged commit 03dc4a2 into solana-labs:master Sep 25, 2019
11 checks passed
11 checks passed
Summary 1 rule matches and 7 potential rules
Details
buildkite/solana Build #12192 passed (1 hour, 43 minutes, 50 seconds)
Details
buildkite/solana/bench Passed (19 minutes, 12 seconds)
Details
buildkite/solana/checks Passed (4 minutes, 17 seconds)
Details
buildkite/solana/coverage Passed (23 minutes, 12 seconds)
Details
buildkite/solana/local-cluster Passed (21 minutes, 31 seconds)
Details
buildkite/solana/pipeline-upload Passed (5 seconds)
Details
buildkite/solana/shellcheck Passed (28 seconds)
Details
buildkite/solana/stable Passed (18 minutes, 24 seconds)
Details
buildkite/solana/stable-perf Passed (34 minutes, 50 seconds)
Details
ci-gate Pull Request accepted for CI pipeline
@jackcmay jackcmay deleted the jackcmay:fix-bad-relocations branch Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.