CONTRACTS: dfcc_is_cprover_symbol now uses allow lists to match symbols
#7708
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
|
If anyone can suggest a better solution (this one is sound but could be hard to maintain as we add new special symbols). |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #7708 +/- ##
===========================================
+ Coverage 78.55% 78.57% +0.01%
===========================================
Files 1686 1686
Lines 192927 193041 +114
===========================================
+ Hits 151560 151674 +114
Misses 41367 41367
☔ View full report in Codecov by Sentry. |
tautschnig
requested changes
May 12, 2023
src/goto-instrument/contracts/dynamic-frames/dfcc_is_cprover_symbol.cpp
Outdated
Show resolved
Hide resolved
src/goto-instrument/contracts/dynamic-frames/dfcc_is_cprover_symbol.cpp
Outdated
Show resolved
Hide resolved
src/goto-instrument/contracts/dynamic-frames/dfcc_is_cprover_symbol.cpp
Outdated
Show resolved
Hide resolved
feliperodri
added
aws
qinheping
force-pushed
the
contracts-use-allow-lists
branch
from
ce2c788 to
c3f978d
Compare
qinheping
requested review from
martin-cs,
peterschrammel,
kroening and
TGWDB
as code owners
qinheping
force-pushed
the
contracts-use-allow-lists
branch
2 times, most recently
from
3a8daf7 to
ae73ef9
Compare
feliperodri
approved these changes
May 19, 2023
| /// `__VERIFIER` or `nondet`. | ||
| bool dfcc_is_cprover_function_symbol(const irep_idt &id); | ||
|
|
||
| /// Returns `true` iff the symbol is one of the know CPROVER static |
There was a problem hiding this comment.
Suggested change
| /// Returns `true` iff the symbol is one of the know CPROVER static | |
| /// Returns `true` iff the symbol is one of the known CPROVER static |
The result of `dfcc_is_cprover_symbol` is used to skip frame conditon checks on symbols with special meaning. Before we were matching symbols based on their prefix, which would allow users to skip frame condition checks just by naming their symbols with these prefixes. We now use a closed allow list, and distinguish function symbolds form static var symbols. We add test that show that variables named with __CPROVER, __VERIFIER or nondet prefixes do not bypass frame condition checks.
qinheping
force-pushed
the
contracts-use-allow-lists
branch
from
ae73ef9 to
407914b
Compare
tautschnig
approved these changes
May 22, 2023
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.
Assignments to special
__CPROVER_*or*$objectsymbols that are essentially ghost state must not be checked under DFCC. In the same way built-in__CPROVER_or__VERIFIERornondet_functions must not be instrumented by DFCC.Before, these special symbols were detected by matching a prefix or a suffix, which could allow users to bypass DFCC checks by naming their symbols with these prefixes or suffixes.
Instead we propose to maintain finite sets of function symbols and static symbols which are known to be exempt from DFCC instrumentation, and to test membership in that list.
We add tests that demonstrate that checks are not bypassed even when symbols have the special prefixes.