-
Notifications
You must be signed in to change notification settings - Fork 277
Skeleton for well-formedness checking of goto models [blocks: #3118, #3147, #3188, #3191, #3187, #3193] #3123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
64d4f4e
907e09e
80cd47a
ba3649a
c73ff05
c0ebb38
552298f
8743abd
5f1f35e
6539283
90e2b53
0ec8986
d11cece
1d8fc32
f61e4e9
cc955f1
d638ea7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -530,6 +530,11 @@ int cbmc_parse_optionst::doit() | |
if(set_properties()) | ||
return CPROVER_EXIT_SET_PROPERTIES_FAILED; | ||
|
||
if(cmdline.isset("validate-goto-model")) | ||
{ | ||
goto_model.validate(validation_modet::INVARIANT); | ||
} | ||
|
||
return bmct::do_language_agnostic_bmc( | ||
path_strategy_chooser, options, goto_model, ui_message_handler); | ||
} | ||
|
@@ -977,6 +982,8 @@ void cbmc_parse_optionst::help() | |
" --xml-ui use XML-formatted output\n" | ||
" --xml-interface bi-directional XML interface\n" | ||
" --json-ui use JSON-formatted output\n" | ||
// NOLINTNEXTLINE(whitespace/line_length) | ||
HELP_VALIDATE | ||
HELP_GOTO_TRACE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd not add this flag to the help. It's for CBMC developers only, and is a distraction for users. |
||
HELP_FLUSH | ||
" --verbosity # verbosity level\n" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -406,6 +406,11 @@ int goto_analyzer_parse_optionst::doit() | |
if(process_goto_program(options)) | ||
return CPROVER_EXIT_INTERNAL_ERROR; | ||
|
||
if(cmdline.isset("validate-goto-model")) | ||
{ | ||
goto_model.validate(validation_modet::INVARIANT); | ||
} | ||
|
||
// show it? | ||
if(cmdline.isset("show-symbol-table")) | ||
{ | ||
|
@@ -875,6 +880,7 @@ void goto_analyzer_parse_optionst::help() | |
HELP_GOTO_CHECK | ||
"\n" | ||
"Other options:\n" | ||
HELP_VALIDATE | ||
" --version show version and exit\n" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (same here: this is noise to users) |
||
HELP_FLUSH | ||
HELP_TIMESTAMP | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -117,6 +117,19 @@ class goto_functionst | |
|
||
std::vector<function_mapt::const_iterator> sorted() const; | ||
std::vector<function_mapt::iterator> sorted(); | ||
|
||
/// Check that the goto functions are well-formed | ||
/// | ||
/// The validation mode indicates whether well-formedness check failures are | ||
/// reported via DATA_INVARIANT violations or exceptions. | ||
void validate(const namespacet &ns, const validation_modet vm) const | ||
{ | ||
for(const auto &entry : function_map) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have this loop on the |
||
{ | ||
const goto_functiont &goto_function = entry.second; | ||
goto_function.validate(ns, vm); | ||
} | ||
} | ||
}; | ||
|
||
#define Forall_goto_functions(it, functions) \ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we like to have this in the other driver programs too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have thought goto-instrument was the place for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think we should have the option in all of the tools to validate the goto model.
@martin-cs I think also in e.g.
cbmc
it would make sense to validate the goto program as a safety net sort of thing. Once right aftergoto_convert
and perhaps once more beforegoto_symex
to check that all the transformation passes likeremove_function_pointers
, etc. did not produce an ill-formed goto model.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danpoe : I'm very much in favour of validating the program between all transformation passes; that sounds great. To the point I might suggest making that the last step of each transformation. I think this will need to be able to check for various "normal forms" as well, i.e. returns and function pointers removed, etc. ( @tautschnig may have opinions on this subject ). As I first envisaged this, that should be enough to prove that all of the DATA_INVARIANTs hold.
i also like the "with exceptions when we load it and with invariants after" approach. My only question is about the specific, user accessible command-line flag to just check the validity of the goto-program and nothing else. That seems like something we definitely want but that it should probably be in goto-instrument as a "checking and modifying the goto-program" bit of functionality, rather than in every tool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, that makes sense. @sonodtt is currently working on some additional checks of the goto program that will allow to pass flags that indicate whether function pointers and returns are expected to have been removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool.
@sonodtt : having looked at this in the past ( #751 is the first bit I have written down ), may I suggest that you make the flags independent for each feature. For example, one of the last few differences between the
static_analyst
andait
frameworks is that the later requires the abscence of function pointers (naturally or removed) and return removal. function pointer removal is an (at the moment undocumented) precondition of return removal and so on. It would be amazing if we could actually map out and enforce these requirements on "normal forms".