Fixed coverity issue in Philox RNG engine #1905
Draft
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
…ease_counter_internal fuction in case w!=32 and w!=64
aelizaro
reviewed
Oct 17, 2024
| // 0 1 2 3 overflow position | ||
| // 1 2 3 0 set-up counter position in engine | ||
| // 2 1 0 3 raw_counter_position | ||
| int raw_counter_position = (Engine::word_count - overflown_position - 2) % Engine::word_count; |
There was a problem hiding this comment.
please, check this line if Engine::word_count - overflown_position - 2 would overflow or is it expected?
also is -2 here only for Engine::word_count = 4?
|
|
||
| // set the counter which value is 2-chunk bitsize | ||
| unsigned long long increment = ((unsigned long long)testedEngine.max() << Engine::word_size) | testedEngine.max(); | ||
| unsigned long long counter_increment = increment / 4; |
There was a problem hiding this comment.
Suggested change
| unsigned long long counter_increment = increment / 4; | |
| unsigned long long counter_increment = increment / Engine::word_count; |
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.
What introduces this PR:
wis not the recommended value (w!=32andw!=64)discard_overflow_testwas extended: previously only the second chunk of the counter was overflowing, now introduced the high-level loop, which iterates through the other positions of the counter(3rd and 4th) and also checks for the overflow.counter_management_test.set_counterand more complicatedincrease_counter_internal(unsigned long long __z)which is called bydiscard2*w) - this wayincrease_counter_internalwill take__zthat should be placed properly in the 1st chunk and propagated to the 2nd one. The right calculations are checked byreferenceEnginewhich is based on simplerset_counterfunction.