Fix #15005: FP uninitdata with compound assignment or increment - #8822
Open
aadanen wants to merge 2 commits into
Open
Fix #15005: FP uninitdata with compound assignment or increment#8822aadanen wants to merge 2 commits into
aadanen wants to merge 2 commits into
Conversation
This file contains hidden or 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
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.
isVariableUsage() currently returns nullptr for the lhs of assignment operators, because unless it's a pointer that gets dereferenced, it is just getting overwritten and its potentially uninitialized value is not read. I extend this idea to compound assignment and increment/decrement operators.
While working on this, I found some issues with CTU analysis.
this test only works because of the false positive "usage" from the increment operator. For example, this test breaks if we use regular assignment because of the current code properly handling "="
this code
misses the ctu error, and only throws
I think that the problem has to do with
the hardcoded pointer=true and alloc=ARRAY assignments tell isVariableUsage to return nullptr if it i isn't dereferenced, which it isn't. If we are going to hardcode these values to be this conservative then this test should fail because it currently only passes due to the FP bug I want to fix. I currently have a workaround where I use vartok->variable() to determine whether something is actually a pointer/array.
Maybe there can be another PR where we rework the ctu/uninitvar connection. Notably since I don't touch the "=" branch of the code the false negative I mentioned above remains unfixed. Let me know if I should apply my vartok workaround to that code in this PR, if it should be another PR, or if its just the wrong idea overall.