Throw if unique_ptr or array allocation fails due to SafeInt overflow #18941
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
…y return a nullptr. This is inconsistent as an actual memory allocation failure throws. An overflow would typically be due to bad input so an exception makes more sense given that. Change to throw so code using MakeUniquePtr* and AllocArray* doesn't need to check for nullptr. Add some extra info to the log message to help debugging. Should help with #18905 by avoiding the invalid attempted usage of a nullptr from the allocation. Extra info _might_ help with figuring out where the overflow is coming from which is the real issue.
edgchen1
reviewed
Dec 27, 2023
Cleanup the 2 places that were checking for a nullptr return and update the documentation to say the methods throw if they cannot allocate memory.
edgchen1
reviewed
Dec 28, 2023
edgchen1
reviewed
Dec 28, 2023
edgchen1
previously approved these changes
Dec 28, 2023
…uePtrFromOrtAllocator are allocating `void` and the incorrect value didn't impact the allocation size's correctness.
edgchen1
reviewed
Dec 29, 2023
…antAllocateDueToSizeCalcFailure
edgchen1
approved these changes
Jan 2, 2024
skottmckay
deleted the
skottmckay/ThrowIfWeCantAllocateDueToSizeCalcFailure
branch
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.
Description
If we fail to calculate the buffer size (due to overflow) we currently return a nullptr. This is inconsistent as an actual memory allocation failure throws. An overflow would typically be due to bad input so an exception makes more sense given that.
Change to throw so code using MakeUniquePtr* and AllocArray* doesn't need to check for nullptr.
Add some extra info to the log message to help debugging.
Motivation and Context
Should help with #18905 by avoiding the invalid attempted usage of a nullptr from the allocation. Extra info might help with figuring out where the overflow is coming from which is the real issue.