Skip to content

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

Merged
merged 17 commits into from
Nov 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/cbmc/cbmc_parse_options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,11 @@ int cbmc_parse_optionst::doit()
if(set_properties())
return CPROVER_EXIT_SET_PROPERTIES_FAILED;

if(cmdline.isset("validate-goto-model"))
Copy link
Member

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?

Copy link
Collaborator

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.

Copy link
Contributor Author

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 after goto_convert and perhaps once more before goto_symex to check that all the transformation passes like remove_function_pointers, etc. did not produce an ill-formed goto model.

Copy link
Collaborator

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.

Copy link
Contributor Author

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.

Copy link
Collaborator

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 and ait 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".

{
goto_model.validate(validation_modet::INVARIANT);
}

return bmct::do_language_agnostic_bmc(
path_strategy_chooser, options, goto_model, ui_message_handler);
}
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The 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"
Expand Down
4 changes: 3 additions & 1 deletion src/cbmc/cbmc_parse_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ Author: Daniel Kroening, kroening@kroening.com
#ifndef CPROVER_CBMC_CBMC_PARSE_OPTIONS_H
#define CPROVER_CBMC_CBMC_PARSE_OPTIONS_H

#include <util/ui_message.h>
#include <util/parse_options.h>
#include <util/timestamper.h>
#include <util/ui_message.h>
#include <util/validation_interface.h>

#include <langapi/language.h>

Expand Down Expand Up @@ -73,6 +74,7 @@ class optionst;
OPT_FLUSH \
"(localize-faults)(localize-faults-method):" \
OPT_GOTO_TRACE \
OPT_VALIDATE \
"(claim):(show-claims)(floatbv)(all-claims)(all-properties)" // legacy, and will eventually disappear // NOLINT(whitespace/line_length)
// clang-format on

Expand Down
6 changes: 6 additions & 0 deletions src/goto-analyzer/goto_analyzer_parse_options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
{
Expand Down Expand Up @@ -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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(same here: this is noise to users)

HELP_FLUSH
HELP_TIMESTAMP
Expand Down
4 changes: 3 additions & 1 deletion src/goto-analyzer/goto_analyzer_parse_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,10 @@ Author: Daniel Kroening, kroening@kroening.com
#ifndef CPROVER_GOTO_ANALYZER_GOTO_ANALYZER_PARSE_OPTIONS_H
#define CPROVER_GOTO_ANALYZER_GOTO_ANALYZER_PARSE_OPTIONS_H

#include <util/ui_message.h>
#include <util/parse_options.h>
#include <util/timestamper.h>
#include <util/ui_message.h>
#include <util/validation_interface.h>

#include <langapi/language.h>

Expand Down Expand Up @@ -148,6 +149,7 @@ class optionst;
"(show)(verify)(simplify):" \
"(location-sensitive)(concurrent)" \
"(no-simplify-slicing)" \
OPT_VALIDATE \
// clang-format on

class goto_analyzer_parse_optionst:
Expand Down
23 changes: 23 additions & 0 deletions src/goto-instrument/goto_instrument_parse_options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,27 @@ int goto_instrument_parse_optionst::doit()

get_goto_program();

{
const bool validate_only = cmdline.isset("validate-goto-binary");

if(validate_only || cmdline.isset("validate-goto-model"))
{
goto_model.validate(validation_modet::EXCEPTION);

if(validate_only)
{
return CPROVER_EXIT_SUCCESS;
}
}
}

instrument_goto_program();

if(cmdline.isset("validate-goto-model"))
{
goto_model.validate(validation_modet::INVARIANT);
}

{
bool unwind_given=cmdline.isset("unwind");
bool unwindset_given=cmdline.isset("unwindset");
Expand Down Expand Up @@ -1572,6 +1591,10 @@ void goto_instrument_parse_optionst::help()
" --show-local-safe-pointers show pointer expressions that are trivially dominated by a not-null check\n" // NOLINT(*)
" --show-safe-dereferences show pointer expressions that are trivially dominated by a not-null check\n" // NOLINT(*)
" *and* used as a dereference operand\n" // NOLINT(*)
HELP_VALIDATE
// NOLINTNEXTLINE(whitespace/line_length)
" --validate-goto-binary check the well-formedness of the passed in goto\n"
" binary and then exit\n"
"\n"
"Safety checks:\n"
" --no-assertions ignore user assertions\n"
Expand Down
5 changes: 4 additions & 1 deletion src/goto-instrument/goto_instrument_parse_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ Author: Daniel Kroening, kroening@kroening.com
#ifndef CPROVER_GOTO_INSTRUMENT_GOTO_INSTRUMENT_PARSE_OPTIONS_H
#define CPROVER_GOTO_INSTRUMENT_GOTO_INSTRUMENT_PARSE_OPTIONS_H

#include <util/ui_message.h>
#include <util/parse_options.h>
#include <util/timestamper.h>
#include <util/ui_message.h>
#include <util/validation_interface.h>

#include <goto-programs/class_hierarchy.h>
#include <goto-programs/goto_functions.h>
Expand Down Expand Up @@ -101,6 +102,8 @@ Author: Daniel Kroening, kroening@kroening.com
OPT_GOTO_PROGRAM_STATS \
"(show-local-safe-pointers)(show-safe-dereferences)" \
OPT_REPLACE_CALLS \
"(validate-goto-binary)" \
OPT_VALIDATE \
// empty last line

// clang-format on
Expand Down
10 changes: 10 additions & 0 deletions src/goto-programs/goto_function.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,16 @@ class goto_functiont
parameter_identifiers = std::move(other.parameter_identifiers);
return *this;
}

/// Check that the goto function is 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
{
body.validate(ns, vm);
validate_full_type(type, ns, vm);
}
};

void get_local_identifiers(const goto_functiont &, std::set<irep_idt> &dest);
Expand Down
13 changes: 13 additions & 0 deletions src/goto-programs/goto_functions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have this loop on the goto_model level: one less file to modify. But this works just as good.

{
const goto_functiont &goto_function = entry.second;
goto_function.validate(ns, vm);
}
}
};

#define Forall_goto_functions(it, functions) \
Expand Down
12 changes: 12 additions & 0 deletions src/goto-programs/goto_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,18 @@ class goto_modelt : public abstract_goto_modelt
return goto_functions.function_map.find(id) !=
goto_functions.function_map.end();
}

/// Check that the goto model is well-formed
///
/// The validation mode indicates whether well-formedness check failures are
/// reported via DATA_INVARIANT violations or exceptions.
void validate(const validation_modet vm) const
{
symbol_table.validate();

const namespacet ns(symbol_table);
goto_functions.validate(ns, vm);
}
};

/// Class providing the abstract GOTO model interface onto an unrelated
Expand Down
57 changes: 57 additions & 0 deletions src/goto-programs/goto_program.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Author: Daniel Kroening, kroening@kroening.com
#include <iomanip>

#include <util/std_expr.h>
#include <util/validate.h>

#include <langapi/language_util.h>

Expand Down Expand Up @@ -668,6 +669,62 @@ bool goto_programt::instructiont::equals(const instructiont &other) const
// clang-format on
}

void goto_programt::instructiont::validate(
const namespacet &ns,
const validation_modet vm) const
{
validate_full_code(code, ns, vm);
validate_full_expr(guard, ns, vm);

switch(type)
{
case NO_INSTRUCTION_TYPE:
break;
case GOTO:
break;
case ASSUME:
break;
case ASSERT:
break;
case OTHER:
break;
case SKIP:
break;
case START_THREAD:
break;
case END_THREAD:
break;
case LOCATION:
break;
case END_FUNCTION:
break;
case ATOMIC_BEGIN:
break;
case ATOMIC_END:
break;
case RETURN:
break;
case ASSIGN:
DATA_CHECK(
code.get_statement() == ID_assign,
"assign instruction should contain an assign statement");
DATA_CHECK(targets.empty(), "assign instruction should not have a target");
break;
case DECL:
break;
case DEAD:
break;
case FUNCTION_CALL:
break;
case THROW:
break;
case CATCH:
break;
case INCOMPLETE_GOTO:
break;
}
}

bool goto_programt::equals(const goto_programt &other) const
{
if(instructions.size() != other.instructions.size())
Expand Down
24 changes: 22 additions & 2 deletions src/goto-programs/goto_program.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ Author: Daniel Kroening, kroening@kroening.com

#include <util/invariant.h>
#include <util/namespace.h>
#include <util/symbol_table.h>
#include <util/source_location.h>
#include <util/std_expr.h>
#include <util/std_code.h>
#include <util/std_expr.h>
#include <util/symbol_table.h>

enum class validation_modet;

/// The type of an instruction in a GOTO program.
enum goto_program_instruction_typet
Expand Down Expand Up @@ -398,6 +400,12 @@ class goto_programt
/// only be evaluated in the context of a goto_programt (see
/// goto_programt::equals).
bool equals(const instructiont &other) const;

/// Check that the instruction is 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;
};

// Never try to change this to vector-we mutate the list while iterating
Expand Down Expand Up @@ -677,6 +685,18 @@ class goto_programt
/// the same number of instructions, each pair of instructions compares equal,
/// and relative jumps have the same distance.
bool equals(const goto_programt &other) const;

/// Check that the goto program is 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 instructiont &ins : instructions)
{
ins.validate(ns, vm);
}
}
};

/// Get control-flow successors of a given instruction. The instruction is
Expand Down
5 changes: 5 additions & 0 deletions src/goto-programs/initialize_goto_model.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ goto_modelt initialize_goto_model(
goto_model.goto_functions,
message_handler);

if(cmdline.isset("validate-goto-model"))
{
goto_model.validate(validation_modet::EXCEPTION);
}

// stupid hack
config.set_object_bits_from_symbol_table(
goto_model.symbol_table);
Expand Down
3 changes: 3 additions & 0 deletions src/util/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ SRC = arith_tools.cpp \
union_find.cpp \
union_find_replace.cpp \
unwrap_nested_exception.cpp \
validate_code.cpp \
validate_expressions.cpp \
validate_types.cpp \
version.cpp \
xml.cpp \
xml_expr.cpp \
Expand Down
24 changes: 11 additions & 13 deletions src/util/exception_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,25 +55,23 @@ std::string deserialization_exceptiont::what() const
}

incorrect_goto_program_exceptiont::incorrect_goto_program_exceptiont(
std::string message,
source_locationt source_location)
: message(std::move(message)), source_location(std::move(source_location))
std::string message)
: message(std::move(message))
{
source_location.make_nil();
}

std::string incorrect_goto_program_exceptiont::what() const
{
if(source_location.is_nil())
return message;
else
return message + " (at: " + source_location.as_string() + ")";
}
std::string ret(message);

incorrect_goto_program_exceptiont::incorrect_goto_program_exceptiont(
std::string message)
: message(std::move(message))
{
source_location.make_nil();
if(!source_location.is_nil())
ret += " (at: " + source_location.as_string() + ")";

if(!diagnostics.empty())
ret += "\n" + diagnostics;

return ret;
}

unsupported_operation_exceptiont::unsupported_operation_exceptiont(
Expand Down
Loading