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

Test memory allocation robustness #182

Open
lemire opened this issue Sep 4, 2018 · 4 comments
Open

Test memory allocation robustness #182

lemire opened this issue Sep 4, 2018 · 4 comments

Comments

@lemire
Copy link
Member

lemire commented Sep 4, 2018

http://www.nongnu.org/failmalloc/

@dtenny
Copy link

dtenny commented Jan 17, 2022

Just to add to this topic. There are obvious bugs in several routines if allocations fail that should be fixed, where the return values aren't checked and attempts are made to de-reference what would be null pointers.

The list of code in which I observed the problem includes:

  • roaring_bitmap_and
  • roaring_bitmap_or
  • roaring_bitmap_andnot
  • roaring_bitmap_xor

I'm not sure how the topic is viewed here, but it's a red flag for production use in the sorts of environments where I would traditionally use such things.

@Oppen
Copy link
Collaborator

Oppen commented Jan 17, 2022

Yes. The thing is there are also cases where I think a fix would require API breaks or the like. I could make a cleanup PR for that during the week, at least for the cases where it can be done in an obvious way.
Said that, there are platforms (i.e. at the very least Linux) where the default behavior is for no allocation to ever fail (except in the case where you exhaust the address space, which is really really really unlikely to happen with 64 bits, and so easy to happen in 32 bits for the amount of data bitmaps are expected to support that it may not be advisable to use those architectures for this problem domain anyway), but rather end up segfaulting on access if the system can't allocate any pages to back the address it gave the application. It's called overcommit with lazy allocation.
In those cases adding the check is just a pessimization, both in the literal sense and in the readability sense. I have no idea to which other OSes that applies to, if any.

@Oppen
Copy link
Collaborator

Oppen commented Jan 17, 2022

Anyway, since this issue exists, there's obviously some interest in giving roaring bitmaps the correct semantics.

@lemire
Copy link
Member Author

lemire commented Jan 18, 2022

I agree that it is worth pursuing.

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

No branches or pull requests

3 participants