Improved allocation of .bss(-like) sections #505
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The existing strategy for allocating `.bss` in RAM has a few problems - only one contiguous `.bss` region per FEM is supported. - alignment was not properly calculated for `.bss`, leading to linker errors This commit generalizes our process for allocating `.text`, `.data`, `.rodata`, etc. to `.bss` and like as well. It also improves our support for different platforms by reducing the use of hardcoded section names for determining behavior in favor of attributes defined by the binary format like `sh_type` and `sh_flags` With the new system, instead of passing `unsafe_bss_segment` to `make_fem`, `.bss` sections are allocated during the `allocate_bom` step. This is supported by free space regions that don't map to any data on the parent resource. Changes (incomplete): - Use SHT_NOBITS type to detect .bss(-like) sections on ELF toolchains - Create `FreeSpaceWithoutData`` tag for allocating regions for .bss. These are created from `MemoryRegion` children of resources with `data_range=None` - Use SHF_ALLOC flag to determine whether section needs to be patched or not instead of hard coded list of names. - Pass flag to compiler to prevent `.eh_frame` generation - Backwards compatible with old bss allocation for now, emits deprecation warnings.
alchzh
commented
Oct 28, 2024
|
Marking as draft as I work on better test cases for this |
|
test cases done |
|
This is good to merge IMO, after merge conflicts are done, sorry for the delayed response. |
alchzh
added a commit
that referenced
this pull request
Jan 13, 2025
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The existing strategy for allocating
.bssin RAM has a few problems.bssregion per FEM is supported..bss, leading to linker errorsThis commit generalizes our process for allocating
.text,.data,.rodata, etc. to.bssand like as well. It also improves our support for different platforms by reducing the use of hardcoded section names for determining behavior in favor of attributes defined by the binary format likesh_typeandsh_flagsWith the new system, instead of passing
unsafe_bss_segmenttomake_fem,.bsssections are allocated during theallocate_bomstep. This is supported by free space regions that don't map to any data on the parent resource. The new code allows.bssregions to be allocated in existingRWregions that are mapped to data by zeroing out those areas.Changes (incomplete):
Use SHT_NOBITS type to detect .bss(-like) sections on ELF toolchains
Create
FreeSpaceWithoutDatatag for allocating regions for .bss. These are created fromMemoryRegionchildren of resources withdata_range=NoneUse SHF_ALLOC flag to determine whether section needs to be patched or not instead of hard coded list of names.
Pass flag to compiler to prevent
.eh_framegenerationBackwards compatible with old bss allocation for now, emits deprecation warnings.
I have reviewed the OFRAK contributor guide and attest that this pull request is in accordance with it.