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

Bug when creating RangeInclusive of size 1 #42

Closed
bchalios opened this issue Aug 23, 2022 · 3 comments · Fixed by #44
Closed

Bug when creating RangeInclusive of size 1 #42

bchalios opened this issue Aug 23, 2022 · 3 comments · Fixed by #44
Assignees

Comments

@bchalios
Copy link
Contributor

I came across a bug which manifests when allocating with an alignment other than 0.

The problem manifests when we get a candidate range which can fit our allocation, but in order
to respect the alignment properties the allocated range starts one byte after the start of the candidate node.
In this case we try to add back an InnerNode of size 1.

This should work, but the constructor of RangeInclusive fails due to this check which is meant to check
for ranges bigger than u64::MAX + 1.

MRE:

use vm_allocator::{AddressAllocator, AllocPolicy};

fn main() {
    let mut allocator = AddressAllocator::new(1, 1000).unwrap();
    //This should return us range [2,11] 
    let addr = allocator.allocate(10, 2, AllocPolicy::FirstMatch)
    // but it will fail because internally the allocator which will try to
    // create a Node with Range [1,1] which will fail
   .unwrap();
    println!("Allocated address: {}", addr.start());
}

Above will fail with:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidRange(1, 1)', src/main.rs:9:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
@bchalios
Copy link
Contributor Author

bchalios commented Aug 23, 2022

Even-more-MRE:

use vm_allocator::{AddressAllocator, AllocPolicy};

fn main() {
    let _ = AddressAllocator::new(1, 1).unwrap();

@AlexandruCihodaru AlexandruCihodaru self-assigned this Aug 23, 2022
@AlexandruCihodaru AlexandruCihodaru mentioned this issue Aug 23, 2022
3 tasks
@andreeaflorescu
Copy link
Member

@bchalios do you want to submit a patch with the fix?

@bchalios
Copy link
Contributor Author

Hi Andreea, I spoke with @AlexandruCihodaru offline, he said he wanted to give it a go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants