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

Verify the View dimensions before allocating? #6691

Closed
Shihab-Shahriar opened this issue Jan 3, 2024 · 2 comments
Closed

Verify the View dimensions before allocating? #6691

Shihab-Shahriar opened this issue Jan 3, 2024 · 2 comments

Comments

@Shihab-Shahriar
Copy link
Contributor

Shihab-Shahriar commented Jan 3, 2024

I had a bug which looked like Kokkos failed to allocate an otherwise reasonable amount of space. But turned out I had actually overflowed an int variable that I was using as an extent to instantiate a kokkos view. Like the n variable below:

Kokkos::realloc(Kokkos::view_alloc(space, Kokkos::WithoutInitializing), v, n)

I wonder if you would consider adding a check for this sort of degenrate cases? Currently the size_t argument type for the extents masks this type of error. So maybe something along the line of:

// original function
void realloc(size_t n){
}

// an overload for signed types
void realloc(int n){
  if(n>0) realloc(n)
}
@dalg24
Copy link
Member

dalg24 commented Jan 3, 2024

turned out I had actually overflowed an int variable

Overflow of signed integers yields undefined behavior (UB) so you would need to handle it on your side regardless of whether Kokkos::realloc can detect that a negative integer was provided as argument.

That said, I do agree that it is somewhat unfortunate that realloc takes the extents in each dimension as std::size_t and that it silently wraps around when a negative integer is passed.
The issue is well beyond realloc or resize, you get the same behavior when you construct a View with an extent passed a negative integer.

@crtrott
Copy link
Member

crtrott commented Jan 3, 2024

We will address this with the mdspan refactor.

@dalg24 dalg24 closed this as completed Jan 4, 2024
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

No branches or pull requests

3 participants