Well-formedness checking for SSA

Follows the #3123 skeleton with additional checks in SSA_stept. Adds a condition
such that renaming checks are only run with the validate option.

Author: Petr Bauch

src/cbmc/bmc.cpp

@@ -639,6 +639,12 @@ void bmct::perform_symbolic_execution(
   goto_symext::get_goto_functiont get_goto_function)
 {
   symex.symex_from_entry_point_of(get_goto_function, symex_symbol_table);
+
+  if(options.get_bool_option("validate-ssa-equation"))
+  {
+    symex.validate(ns, validation_modet::INVARIANT);
+  }
+
   INVARIANT(
     options.get_bool_option("paths") || path_storage.empty(),
     "Branch points were saved even though we should have been "

src/cbmc/cbmc_parse_options.cpp

@@ -398,6 +398,11 @@ void cbmc_parse_optionst::get_command_line_options(optionst &options)
       "symex-coverage-report",
       cmdline.get_value("symex-coverage-report"));
 
+  if(cmdline.isset("validate-ssa-equation"))
+  {
+    options.set_option("validate-ssa-equation", true);
+  }
+
   PARSE_OPTIONS_GOTO_TRACE(cmdline, options);
 }
 

src/goto-symex/goto_symex.h

@@ -61,6 +61,12 @@ struct symex_configt final
   bool partial_loops;
   mp_integer debug_level;
 
+  /// \brief Should the additional validation checks be run?
+  ///
+  /// If this flag is set the checks for renaming (both level1 and level2) are
+  /// executed in the goto_symex_statet (in the assignment method).
+  bool run_validation_checks;
+
   explicit symex_configt(const optionst &options);
 };
 
@@ -455,6 +461,11 @@ class goto_symext
       "attempting to read remaining_vccs");
     return _remaining_vccs;
   }
+
+  void validate(const namespacet &ns, const validation_modet vm) const
+  {
+    target.validate(ns, vm);
+  }
 };
 
 void symex_transition(goto_symext::statet &state);

src/goto-symex/goto_symex_state.cpp

@@ -121,32 +121,6 @@ static bool check_renaming(const exprt &expr)
   return false;
 }
 
-static void assert_l1_renaming(const exprt &expr)
-{
-  #if 1
-  if(check_renaming_l1(expr))
-  {
-    std::cerr << expr.pretty() << '\n';
-    UNREACHABLE;
-  }
-  #else
-  (void)expr;
-  #endif
-}
-
-static void assert_l2_renaming(const exprt &expr)
-{
-  #if 1
-  if(check_renaming(expr))
-  {
-    std::cerr << expr.pretty() << '\n';
-    UNREACHABLE;
-  }
-  #else
-  (void)expr;
-  #endif
-}
-
 class goto_symex_is_constantt : public is_constantt
 {
 protected:
@@ -197,15 +171,18 @@ void goto_symex_statet::assignment(
   // the type might need renaming
   rename(lhs.type(), l1_identifier, ns);
   lhs.update_type();
-  assert_l1_renaming(lhs);
+  if(run_validation_checks)
+  {
+    DATA_INVARIANT(!check_renaming_l1(lhs), "lhs renaming failed on l1");
+  }
 
-  #if 0
+#if 0
   PRECONDITION(l1_identifier != get_original_name(l1_identifier)
       || l1_identifier=="goto_symex::\\guard"
       || ns.lookup(l1_identifier).is_shared()
       || has_prefix(id2string(l1_identifier), "symex::invalid_object")
       || has_prefix(id2string(l1_identifier), "symex_dynamic::dynamic_object"));
-  #endif
+#endif
 
   // do the l2 renaming
   if(level2.current_names.find(l1_identifier)==level2.current_names.end())
@@ -216,8 +193,11 @@ void goto_symex_statet::assignment(
   // in case we happen to be multi-threaded, record the memory access
   bool is_shared=l2_thread_write_encoding(lhs, ns);
 
-  assert_l2_renaming(lhs);
-  assert_l2_renaming(rhs);
+  if(run_validation_checks)
+  {
+    DATA_INVARIANT(!check_renaming(lhs), "lhs renaming failed on l2");
+    DATA_INVARIANT(!check_renaming(rhs), "rhs renaming failed on l2");
+  }
 
   // see #305 on GitHub for a simple example and possible discussion
   if(is_shared && lhs.type().id() == ID_pointer && !allow_pointer_unsoundness)
@@ -240,8 +220,11 @@ void goto_symex_statet::assignment(
     ssa_exprt l1_lhs(lhs);
     get_l1_name(l1_lhs);
 
-    assert_l1_renaming(l1_lhs);
-    assert_l1_renaming(l1_rhs);
+    if(run_validation_checks)
+    {
+      DATA_INVARIANT(!check_renaming_l1(l1_lhs), "lhs renaming failed on l1");
+      DATA_INVARIANT(!check_renaming_l1(l1_rhs), "rhs renaming failed on l1");
+    }
 
     value_set.assign(l1_lhs, l1_rhs, ns, rhs_is_simplified, is_shared);
   }

src/goto-symex/goto_symex_state.h

@@ -268,6 +268,9 @@ class goto_symex_statet final
   /// of a GOTO
   bool has_saved_next_instruction;
 
+  /// \brief Should the additional validation checks be run?
+  bool run_validation_checks;
+
 private:
   /// \brief Dangerous, do not use
   ///

src/goto-symex/symex_main.cpp

@@ -34,7 +34,8 @@ symex_configt::symex_configt(const optionst &options)
     simplify_opt(options.get_bool_option("simplify")),
     unwinding_assertions(options.get_bool_option("unwinding-assertions")),
     partial_loops(options.get_bool_option("partial-loops")),
-    debug_level(unsafe_string2int(options.get_option("debug-level")))
+    debug_level(unsafe_string2int(options.get_option("debug-level"))),
+    run_validation_checks(options.get_bool_option("validate-ssa-equation"))
 {
 }
 
@@ -307,6 +308,8 @@ void goto_symext::symex_from_entry_point_of(
 
   statet state;
 
+  state.run_validation_checks = symex_config.run_validation_checks;
+
   initialize_entry_point(
     state,
     get_goto_function,

src/goto-symex/symex_target_equation.cpp

@@ -978,3 +978,63 @@ void symex_target_equationt::SSA_stept::output(std::ostream &out) const
 
   out << "Guard: " << format(guard) << '\n';
 }
+
+/// Check that the SSA step is well-formed
+/// \param ns namespace to lookup identifiers
+/// \param vm validation mode to be used for reporting failures
+void symex_target_equationt::SSA_stept::validate(
+  const namespacet &ns,
+  const validation_modet vm) const
+{
+  validate_full_expr(guard, ns, vm);
+
+  switch(type)
+  {
+  case goto_trace_stept::typet::ASSERT:
+  case goto_trace_stept::typet::ASSUME:
+  case goto_trace_stept::typet::GOTO:
+  case goto_trace_stept::typet::CONSTRAINT:
+    validate_full_expr(cond_expr, ns, vm);
+    break;
+  case goto_trace_stept::typet::ASSIGNMENT:
+  case goto_trace_stept::typet::DECL:
+    validate_full_expr(ssa_lhs, ns, vm);
+    validate_full_expr(ssa_full_lhs, ns, vm);
+    validate_full_expr(original_full_lhs, ns, vm);
+    validate_full_expr(ssa_rhs, ns, vm);
+    DATA_CHECK(
+      vm,
+      base_type_eq(ssa_lhs.get_original_expr(), ssa_rhs, ns),
+      "Type inequality in SSA assignment\nlhs-type: " +
+        ssa_lhs.get_original_expr().type().id_string() +
+        "\nrhs-type: " + ssa_rhs.type().id_string());
+    break;
+  case goto_trace_stept::typet::INPUT:
+  case goto_trace_stept::typet::OUTPUT:
+    for(const auto &expr : io_args)
+      validate_full_expr(expr, ns, vm);
+    break;
+  case goto_trace_stept::typet::FUNCTION_CALL:
+    for(const auto &expr : ssa_function_arguments)
+      validate_full_expr(expr, ns, vm);
+  case goto_trace_stept::typet::FUNCTION_RETURN:
+  {
+    const symbolt *symbol;
+    DATA_CHECK(
+      vm,
+      called_function.empty() || !ns.lookup(called_function, symbol),
+      "unknown function identifier " + id2string(called_function));
+  }
+  break;
+  case goto_trace_stept::typet::NONE:
+  case goto_trace_stept::typet::LOCATION:
+  case goto_trace_stept::typet::DEAD:
+  case goto_trace_stept::typet::SHARED_READ:
+  case goto_trace_stept::typet::SHARED_WRITE:
+  case goto_trace_stept::typet::SPAWN:
+  case goto_trace_stept::typet::MEMORY_BARRIER:
+  case goto_trace_stept::typet::ATOMIC_BEGIN:
+  case goto_trace_stept::typet::ATOMIC_END:
+    break;
+  }
+}

src/goto-symex/symex_target_equation.h

@@ -267,6 +267,8 @@ class symex_target_equationt:public symex_targett
       std::ostream &out) const;
 
     void output(std::ostream &out) const;
+
+    void validate(const namespacet &ns, const validation_modet vm) const;
   };
 
   std::size_t count_assertions() const
@@ -323,6 +325,12 @@ class symex_target_equationt:public symex_targett
     return false;
   }
 
+  void validate(const namespacet &ns, const validation_modet vm) const
+  {
+    for(const SSA_stept &step : SSA_steps)
+      step.validate(ns, vm);
+  }
+
 protected:
   // for enforcing sharing in the expressions stored
   merge_irept merge_irep;

src/util/ssa_expr.h

@@ -139,6 +139,27 @@ class ssa_exprt:public symbol_exprt
   /* Used to determine whether or not an identifier can be built
    * before trying and getting an exception */
   static bool can_build_identifier(const exprt &src);
+
+  static void check(
+    const exprt &expr,
+    const validation_modet vm = validation_modet::INVARIANT)
+  {
+    DATA_CHECK(
+      vm, !expr.has_operands(), "SSA expression should not have operands");
+    DATA_CHECK(
+      vm, expr.id() == ID_symbol, "SSA expression symbols are symbols");
+    DATA_CHECK(vm, expr.get_bool(ID_C_SSA_symbol), "wrong SSA expression ID");
+  }
+
+  static void validate(
+    const exprt &expr,
+    const namespacet &ns,
+    const validation_modet vm = validation_modet::INVARIANT)
+  {
+    check(expr, vm);
+    validate_full_expr(
+      static_cast<const exprt &>(expr.find(ID_expression)), ns, vm);
+  }
 };
 
 /*! \brief Cast a generic exprt to an \ref ssa_exprt

src/util/validate_expressions.cpp

@@ -15,6 +15,7 @@ Author: Daniel Poetzl
 
 #include "expr.h"
 #include "namespace.h"
+#include "ssa_expr.h"
 #include "std_expr.h"
 #include "validate.h"
 
@@ -32,6 +33,10 @@ void call_on_expr(const exprt &expr, Args &&... args)
   {
     CALL_ON_EXPR(plus_exprt);
   }
+  else if(expr.get_bool(ID_C_SSA_symbol))
+  {
+    CALL_ON_EXPR(ssa_exprt);
+  }
   else
   {
 #ifdef REPORT_UNIMPLEMENTED_EXPRESSION_CHECKS