Skip to content
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

Remove dynamic_cast from hardness collection code paths #8423

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
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
10 changes: 7 additions & 3 deletions src/goto-checker/bmc_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,16 @@
}
}

void convert_symex_target_equation(
static void convert_symex_target_equation(
symex_target_equationt &equation,
decision_proceduret &decision_procedure,
solver_hardnesst *hardness_collector,
message_handlert &message_handler)
{
messaget msg(message_handler);
msg.status() << "converting SSA" << messaget::eom;

equation.convert(decision_procedure);
equation.convert(decision_procedure, hardness_collector);
}

std::unique_ptr<memory_model_baset>
Expand Down Expand Up @@ -370,7 +371,10 @@
<< messaget::eom;

convert_symex_target_equation(
equation, property_decider.get_decision_procedure(), ui_message_handler);
equation,

Check warning on line 374 in src/goto-checker/bmc_util.cpp

View check run for this annotation

Codecov / codecov/patch

src/goto-checker/bmc_util.cpp#L374

Added line #L374 was not covered by tests
property_decider.get_decision_procedure(),
property_decider.get_hardness_collector(),

Check warning on line 376 in src/goto-checker/bmc_util.cpp

View check run for this annotation

Codecov / codecov/patch

src/goto-checker/bmc_util.cpp#L376

Added line #L376 was not covered by tests
ui_message_handler);
property_decider.update_properties_goals_from_symex_target_equation(
properties);
property_decider.convert_goals();
Expand Down
6 changes: 0 additions & 6 deletions src/goto-checker/bmc_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,13 @@ class decision_proceduret;
class goto_symex_property_decidert;
class goto_tracet;
class memory_model_baset;
class message_handlert;
class namespacet;
class optionst;
class symex_bmct;
class symex_target_equationt;
struct trace_optionst;
class ui_message_handlert;

void convert_symex_target_equation(
symex_target_equationt &equation,
decision_proceduret &decision_procedure,
message_handlert &message_handler);

/// Returns a function that checks whether an SSA step is an assertion
/// with \p property_id. Usually used for `build_goto_trace`.
ssa_step_predicatet
Expand Down
12 changes: 9 additions & 3 deletions src/goto-checker/goto_symex_property_decider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,14 @@ void goto_symex_property_decidert::add_constraint_from_goals(
exprt goal_disjunction = disjunction(disjuncts);
decision_procedure.set_to_true(goal_disjunction);

with_solver_hardness(decision_procedure, [](solver_hardnesst &hardness) {
if(auto hardness = solver->hardness_collector())
{
// SSA expr and involved steps have already been collected
// in symex_target_equationt::convert_assertions
exprt ssa_expr_unused;
std::vector<goto_programt::const_targett> involved_steps_unused;
hardness.register_assertion_ssas(ssa_expr_unused, involved_steps_unused);
});
hardness->register_assertion_ssas(ssa_expr_unused, involved_steps_unused);
};
}

decision_proceduret::resultt goto_symex_property_decidert::solve()
Expand All @@ -125,6 +126,11 @@ boolbvt &goto_symex_property_decidert::get_boolbv_decision_procedure() const
return solver->boolbv_decision_procedure();
}

solver_hardnesst *goto_symex_property_decidert::get_hardness_collector() const
{
return solver->hardness_collector();
}

symex_target_equationt &goto_symex_property_decidert::get_equation() const
{
return equation;
Expand Down
4 changes: 4 additions & 0 deletions src/goto-checker/goto_symex_property_decider.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ class goto_symex_property_decidert
/// Returns the solver instance
boolbvt &get_boolbv_decision_procedure() const;

/// Returns a pointer to the hardness collector or `nullptr` when hardness
/// collection is not set up.
solver_hardnesst *get_hardness_collector() const;

/// Return the equation associated with this instance
symex_target_equationt &get_equation() const;

Expand Down
7 changes: 4 additions & 3 deletions src/goto-checker/multi_path_symex_checker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,9 @@ void multi_path_symex_checkert::report()
{
if(options.is_set("write-solver-stats-to"))
{
with_solver_hardness(
property_decider.get_decision_procedure(),
[](solver_hardnesst &hardness) { hardness.produce_report(); });
if(auto hardness = property_decider.get_hardness_collector())
{
hardness->produce_report();
}
}
}
7 changes: 5 additions & 2 deletions src/goto-checker/single_loop_incremental_symex_checker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,18 @@ operator()(propertiest &properties)

log.status() << "converting SSA" << messaget::eom;
equation.convert_without_assertions(
property_decider.get_decision_procedure());
property_decider.get_decision_procedure(),
property_decider.get_hardness_collector());

property_decider.update_properties_goals_from_symex_target_equation(
properties);

// We convert the assertions in a new context.
property_decider.get_decision_procedure().push();
equation.convert_assertions(
property_decider.get_decision_procedure(), false);
property_decider.get_decision_procedure(),
property_decider.get_hardness_collector(),
false);
property_decider.convert_goals();

current_equation_converted = true;
Expand Down
75 changes: 51 additions & 24 deletions src/goto-checker/solver_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ Author: Daniel Kroening, Peter Schrammel
#include <util/unicode.h>
#include <util/version.h>

#include <goto-symex/solver_hardness.h>
#include <solvers/flattening/bv_dimacs.h>
#include <solvers/prop/prop.h>
#include <solvers/prop/solver_resource_limits.h>
Expand Down Expand Up @@ -71,6 +70,16 @@ solver_factoryt::solvert::solvert(
{
}

solver_factoryt::solvert::solvert(
std::unique_ptr<boolbvt> p1,
std::unique_ptr<propt> p2,
std::shared_ptr<solver_hardnesst> p3)
: hardness_ptr(std::move(p3)),
prop_ptr(std::move(p2)),
decision_procedure_is_boolbvt_ptr(std::move(p1))
{
}

stack_decision_proceduret &solver_factoryt::solvert::decision_procedure() const
{
PRECONDITION(
Expand All @@ -88,6 +97,11 @@ boolbvt &solver_factoryt::solvert::boolbv_decision_procedure() const
return *decision_procedure_is_boolbvt_ptr;
}

solver_hardnesst *solver_factoryt::solvert::hardness_collector() const
{
return hardness_ptr.get();
}

void solver_factoryt::set_decision_procedure_time_limit(
solver_resource_limitst &decision_procedure)
{
Expand Down Expand Up @@ -168,8 +182,9 @@ static void emit_solver_warning(
template <typename SatcheckT>
static typename std::enable_if<
!std::is_base_of<hardness_collectort, SatcheckT>::value,
std::unique_ptr<SatcheckT>>::type
make_satcheck_prop(message_handlert &message_handler, const optionst &options)
std::pair<std::unique_ptr<SatcheckT>, std::shared_ptr<solver_hardnesst>>>::
type
make_satcheck_prop(message_handlert &message_handler, const optionst &options)
{
auto satcheck = std::make_unique<SatcheckT>(message_handler);
if(options.is_set("write-solver-stats-to"))
Expand All @@ -179,27 +194,29 @@ make_satcheck_prop(message_handlert &message_handler, const optionst &options)
<< "Configured solver does not support --write-solver-stats-to. "
<< "Solver stats will not be written." << messaget::eom;
}
return satcheck;
return {std::move(satcheck), nullptr};
}

template <typename SatcheckT>
static typename std::enable_if<
std::is_base_of<hardness_collectort, SatcheckT>::value,
std::unique_ptr<SatcheckT>>::type
make_satcheck_prop(message_handlert &message_handler, const optionst &options)
std::pair<std::unique_ptr<SatcheckT>, std::shared_ptr<solver_hardnesst>>>::
type
make_satcheck_prop(message_handlert &message_handler, const optionst &options)
{
auto satcheck = std::make_unique<SatcheckT>(message_handler);
if(options.is_set("write-solver-stats-to"))
{
std::unique_ptr<solver_hardnesst> solver_hardness =
std::make_unique<solver_hardnesst>();
solver_hardness->set_outfile(options.get_option("write-solver-stats-to"));
satcheck->solver_hardness = std::move(solver_hardness);
}
return satcheck;
if(!options.is_set("write-solver-stats-to"))
return {std::move(satcheck), nullptr};

std::shared_ptr<solver_hardnesst> solver_hardness =
std::make_shared<solver_hardnesst>();
solver_hardness->set_outfile(options.get_option("write-solver-stats-to"));
satcheck->solver_hardness = solver_hardness;

return {std::move(satcheck), std::move(solver_hardness)};
}

static std::unique_ptr<propt>
static std::pair<std::unique_ptr<propt>, std::shared_ptr<solver_hardnesst>>
get_sat_solver(message_handlert &message_handler, const optionst &options)
{
const bool no_simplifier = options.get_bool_option("beautify") ||
Expand Down Expand Up @@ -326,12 +343,15 @@ get_sat_solver(message_handlert &message_handler, const optionst &options)

std::unique_ptr<solver_factoryt::solvert> solver_factoryt::get_default()
{
auto sat_solver = get_sat_solver(message_handler, options);
auto sat_solver_and_hardness_opt = get_sat_solver(message_handler, options);

bool get_array_constraints =
options.get_bool_option("show-array-constraints");
auto bv_pointers = std::make_unique<bv_pointerst>(
ns, *sat_solver, message_handler, get_array_constraints);
ns,
*sat_solver_and_hardness_opt.first,
message_handler,
get_array_constraints);

if(options.get_option("arrays-uf") == "never")
bv_pointers->unbounded_array = bv_pointerst::unbounded_arrayt::U_NONE;
Expand All @@ -341,7 +361,10 @@ std::unique_ptr<solver_factoryt::solvert> solver_factoryt::get_default()
set_decision_procedure_time_limit(*bv_pointers);

std::unique_ptr<boolbvt> boolbv = std::move(bv_pointers);
return std::make_unique<solvert>(std::move(boolbv), std::move(sat_solver));
return std::make_unique<solvert>(
std::move(boolbv),
std::move(sat_solver_and_hardness_opt.first),
std::move(sat_solver_and_hardness_opt.second));
}

std::unique_ptr<solver_factoryt::solvert> solver_factoryt::get_dimacs()
Expand Down Expand Up @@ -376,11 +399,11 @@ std::unique_ptr<solver_factoryt::solvert> solver_factoryt::get_external_sat()

std::unique_ptr<solver_factoryt::solvert> solver_factoryt::get_bv_refinement()
{
std::unique_ptr<propt> prop = get_sat_solver(message_handler, options);
auto prop_and_hardness_opt = get_sat_solver(message_handler, options);

bv_refinementt::infot info;
info.ns = &ns;
info.prop = prop.get();
info.prop = prop_and_hardness_opt.first.get();
info.output_xml = output_xml_in_refinement;

// we allow setting some parameters
Expand All @@ -396,7 +419,9 @@ std::unique_ptr<solver_factoryt::solvert> solver_factoryt::get_bv_refinement()
std::make_unique<bv_refinementt>(info);
set_decision_procedure_time_limit(*decision_procedure);
return std::make_unique<solvert>(
std::move(decision_procedure), std::move(prop));
std::move(decision_procedure),
std::move(prop_and_hardness_opt.first),
std::move(prop_and_hardness_opt.second));
}

/// the string refinement adds to the bit vector refinement specifications for
Expand All @@ -407,8 +432,8 @@ solver_factoryt::get_string_refinement()
{
string_refinementt::infot info;
info.ns = &ns;
auto prop = get_sat_solver(message_handler, options);
info.prop = prop.get();
auto prop_and_hardness_opt = get_sat_solver(message_handler, options);
info.prop = prop_and_hardness_opt.first.get();
info.refinement_bound = DEFAULT_MAX_NB_REFINEMENT;
info.output_xml = output_xml_in_refinement;
if(options.get_bool_option("max-node-refinement"))
Expand All @@ -422,7 +447,9 @@ solver_factoryt::get_string_refinement()
std::make_unique<string_refinementt>(info);
set_decision_procedure_time_limit(*decision_procedure);
return std::make_unique<solvert>(
std::move(decision_procedure), std::move(prop));
std::move(decision_procedure),
std::move(prop_and_hardness_opt.first),
std::move(prop_and_hardness_opt.second));
}

std::unique_ptr<std::ofstream> open_outfile_and_check(
Expand Down
7 changes: 7 additions & 0 deletions src/goto-checker/solver_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Author: Daniel Kroening, Peter Schrammel
#ifndef CPROVER_GOTO_CHECKER_SOLVER_FACTORY_H
#define CPROVER_GOTO_CHECKER_SOLVER_FACTORY_H

#include <goto-symex/solver_hardness.h> // IWYU pragma: keep
#include <solvers/flattening/boolbv.h>
#include <solvers/smt2/smt2_dec.h>

Expand Down Expand Up @@ -46,11 +47,17 @@ class solver_factoryt final
std::unique_ptr<stack_decision_proceduret> p1,
std::unique_ptr<std::ofstream> p2);
solvert(std::unique_ptr<boolbvt> p1, std::unique_ptr<propt> p2);
solvert(
std::unique_ptr<boolbvt> p1,
std::unique_ptr<propt> p2,
std::shared_ptr<solver_hardnesst> p3);

stack_decision_proceduret &decision_procedure() const;
boolbvt &boolbv_decision_procedure() const;
solver_hardnesst *hardness_collector() const;

private:
std::shared_ptr<solver_hardnesst> hardness_ptr;
// the objects are deleted in the opposite order they appear below
std::unique_ptr<std::ofstream> ofstream_ptr;
std::unique_ptr<propt> prop_ptr;
Expand Down
2 changes: 1 addition & 1 deletion src/goto-instrument/accelerate/scratch_program.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
return false;
}

equation.convert(*checker);
equation.convert(*checker, nullptr);

Check warning on line 66 in src/goto-instrument/accelerate/scratch_program.cpp

View check run for this annotation

Codecov / codecov/patch

src/goto-instrument/accelerate/scratch_program.cpp#L66

Added line #L66 was not covered by tests

#ifdef DEBUG
std::cout << "Finished symex, invoking decision procedure.\n";
Expand Down
27 changes: 2 additions & 25 deletions src/goto-symex/solver_hardness.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,15 @@ Author: Diffblue Ltd.
#ifndef CPROVER_SOLVERS_SOLVER_HARDNESS_H
#define CPROVER_SOLVERS_SOLVER_HARDNESS_H

#include <goto-programs/goto_program.h>

#include <solvers/hardness_collector.h>
#include <solvers/prop/prop_conv_solver.h>

#include <string>
#include <unordered_map>
#include <unordered_set>
#include <vector>

#include <goto-programs/goto_program.h>

/// A structure that facilitates collecting the complexity statistics from a
/// decision procedure. The idea is to associate some solver complexity metric
/// with each line-of-code, GOTO instruction, and SSA step. The motivation is to
Expand Down Expand Up @@ -158,26 +157,4 @@ struct hash<solver_hardnesst::hardness_ssa_keyt>
};
} // namespace std

static inline void with_solver_hardness(
decision_proceduret &maybe_hardness_collector,
std::function<void(solver_hardnesst &hardness)> handler)
{
// FIXME I am wondering if there is a way to do this that is a bit less
// dynamically typed.
if(
auto prop_conv_solver =
dynamic_cast<prop_conv_solvert *>(&maybe_hardness_collector))
{
if(auto hardness_collector = prop_conv_solver->get_hardness_collector())
{
if(hardness_collector->solver_hardness)
{
auto &solver_hardness = static_cast<solver_hardnesst &>(
*(hardness_collector->solver_hardness));
handler(solver_hardness);
}
}
}
}

#endif // CPROVER_SOLVERS_SOLVER_HARDNESS_H
Loading
Loading