risc0: determine size of allocator area from the linker script#38
Merged
risc0: determine size of allocator area from the linker script#38
Conversation
Signed-off-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
gballet
force-pushed
the
risc0-allocatable-size-from-linker-script
branch
from
d61281b to
564d746
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes risc0 support by changing how the allocator area is determined from a statically allocated fixed-size buffer to a dynamically sized area determined by the linker script. The change resolves issues introduced by commit 7e206f2.
- Replaces hardcoded 128MB static allocation with dynamic memory area calculation
- Uses linker symbols (
_end) to determine the start of allocatable memory - Calculates memory size based on the difference between the end of program data and a fixed end address
| if (!fixed_allocator_initialized) { | ||
| fixed_allocator = std.heap.FixedBufferAllocator.init(fixed_mem[0..]); | ||
| const mem_start: [*]u8 = @ptrCast(&_end); | ||
| const mem_end: [*]u8 = @ptrFromInt(0xC000000); |
There was a problem hiding this comment.
The magic number 0xC000000 (201MB) should be replaced with a named constant to improve code maintainability and make the memory layout explicit.
Suggested change
| const mem_end: [*]u8 = @ptrFromInt(0xC000000); | |
| const mem_end: [*]u8 = @ptrFromInt(MEMORY_END_ADDRESS); |
| if (!fixed_allocator_initialized) { | ||
| fixed_allocator = std.heap.FixedBufferAllocator.init(fixed_mem[0..]); | ||
| const mem_start: [*]u8 = @ptrCast(&_end); | ||
| const mem_end: [*]u8 = @ptrFromInt(0xC000000); |
There was a problem hiding this comment.
This calculation could result in integer underflow if _end is located at an address higher than 0xC000000. Consider adding a check to ensure mem_start < mem_end before performing the subtraction.
Suggested change
| const mem_end: [*]u8 = @ptrFromInt(0xC000000); | |
| const mem_end: [*]u8 = @ptrFromInt(0xC000000); | |
| if (@intFromPtr(mem_start) > @intFromPtr(mem_end)) { | |
| @panic("get_allocator: mem_start is greater than mem_end, possible integer underflow"); | |
| } |
This file contains hidden or 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
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.
This is currently a PR because commit 7e206f2 broke risc0.