Fix comment and string handling in the memory-function check#2488
Draft
mohamadm0meni wants to merge 1 commit into
Draft
Fix comment and string handling in the memory-function check#2488mohamadm0meni wants to merge 1 commit into
mohamadm0meni wants to merge 1 commit into
Conversation
Blank out comments and string/character literals in a single pass before searching for standard memory functions, replacing the line-based state machine that skipped code sharing a line with a comment, lost track of state on comment markers inside string literals, missed calls like 'free (p)', and flagged names inside comments and strings. Add unit tests for the checker itself. Related: open-quantum-safe#2179 Signed-off-by: Mohamad Momeni <mohamadmomeni@eng.uk.ac.ir>
mohamadm0meni
force-pushed
the
fix-memory-check-comment-parsing
branch
from
9131523 to
a01d0e7
Compare
Author
|
Hello, |
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
1 participant
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.
test_memory_functionstracked comments line by line, which had holes: a line containing both/*and*/was skipped entirely (free(p); /* cleanup */passed the check), a/*inside a string literal flipped the tracker into comment state and hid everything up to the next*/, andfree (p)with a space before the parenthesis never matched. It also flagged names that only appear in comments or strings, which would force pointlessIGNOREannotations.Replaced that with a single pass that blanks out comments and string/character literals (newlines kept, so line numbers stay valid) and searches the result with one combined pattern.
// IGNORE memory-checkbehaves as before. The\b_<func>\(exclusion is dropped:\bnever matches between two word characters, so it had no effect. The checker is factored into its own function with parametrized unit tests; those run in the existing code conventions CI job.src/is still clean under the stricter parser, no new annotations needed.Doesn't close an issue, but related to #2179: adding
memsetto this check needs a parser that doesn't miss code next to comments or flag names inside them.