Require a balanced heap state at join points. #219
Merged
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
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.
We can require a balanced HEA state in addition to STK. Heap allocations can be dynamic, which makes this a bit more complicated. We need to correctly interleave static and dynamic allocations, which requires a stack.
The really annoying problem, though, is that the compiler had a bug for 12 years where dynamic arrays were not freed on "break" statements inside loops. This messes up our analysis because the join point will see an empty heap state on one predecessor, and lingering dynamic variables in the other.
Two plugins in my corpus were affected by this bug. One of them still had the source code available, and I was able to work around it by detecting break statements and ignoring the balancing error. (I think it is okay, in this case, to pick a random predecessor's state to inherit, because the only entries should be extra dynamic entries.)
I could not find the source to the other plugin, and I can't tell what's going on from its control flow. It's found in attachments 138975, 146934, 150386, and 150800. It appears to be a discontinued deathmatch plugin by H3bus, and the thread for it has been scrubbed. Given that these binaries were made between 10/2014 and 12/2015, for SourceMod 1.6.2, I think I'm okay making it fail validation. But I'll keep an eye out for cases where the bug detection does not work.