solana-labs / rbpf

Rust virtual machine and JIT compiler for eBPF programs
Apache License 2.0
272 stars 163 forks source link

Refactor - Separates the text section from read-only sections #553

Closed Lichtso closed 2 months ago

Lichtso commented 6 months ago

This PR removes the indirection in which the text section is retrieved from the possibly concatenated read-only sections. There is only ever one text-section and no need to concatenate it. So instead the text section can be borrowed as a sub-slice of the relocated ELF bytes directly. Also removes the mem_size of text_section_info as that was double counted and the debug name, as it was unused.

codecov-commenter commented 6 months ago

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.00%. Comparing base (a996494) to head (5749444). Report is 1 commits behind head on main.

Files Patch % Lines
src/elf.rs 92.30% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #553 +/- ## ========================================== + Coverage 89.92% 90.00% +0.08% ========================================== Files 22 22 Lines 9735 9685 -50 ========================================== - Hits 8754 8717 -37 + Misses 981 968 -13 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alessandrod commented 6 months ago

There is only ever one text-section and no need to concatenate it

I haven't looked at the change yet but this is not true?

Lichtso commented 6 months ago

There is only ever one text-section and no need to concatenate it

We only consider the byte range of one section: https://github.com/solana-labs/rbpf/blob/179a0f94b68ae0bef892b214750a54448d61b1be/src/elf.rs#L415

which is exactly named ".text": https://github.com/solana-labs/rbpf/blob/179a0f94b68ae0bef892b214750a54448d61b1be/src/elf.rs#L396