Move goto_programt::(const_)targett comparison to a struct

This makes sure no one can inadvertently use less-than comparison on
those iterators, which may lead to non-reproducible behaviour
(see diffblue#6782).

Author: tautschnig

jbmc/src/java_bytecode/java_bytecode_convert_method_class.h

@@ -240,7 +240,23 @@ class java_bytecode_convert_methodt
 
 public:
   typedef std::map<method_offsett, converted_instructiont> address_mapt;
-  typedef std::pair<const methodt &, const address_mapt &> method_with_amapt;
+  struct method_with_amapt
+  {
+    method_with_amapt(const methodt &m, const address_mapt &a)
+      : method_with_amap(m, a)
+    {
+    }
+
+    std::pair<const methodt &, const address_mapt &> method_with_amap;
+
+    struct target_less_than // NOLINT(readability/identifiers)
+    {
+      bool operator()(const method_offsett &a, const method_offsett &b) const
+      {
+        return a < b;
+      }
+    };
+  };
   typedef cfg_dominators_templatet<method_with_amapt, method_offsett, false>
     java_cfg_dominatorst;
 

jbmc/src/java_bytecode/java_local_variable_table.cpp

@@ -46,8 +46,8 @@ struct procedure_local_cfg_baset<
 
   void operator()(const method_with_amapt &args)
   {
-    const auto &method=args.first;
-    const auto &amap=args.second;
+    const auto &method = args.method_with_amap.first;
+    const auto &amap = args.method_with_amap.second;
     for(const auto &inst : amap)
     {
       // Map instruction PCs onto node indices:
@@ -109,18 +109,18 @@ struct procedure_local_cfg_baset<
   static java_bytecode_convert_methodt::method_offsett
   get_first_node(const method_with_amapt &args)
   {
-    return args.second.begin()->first;
+    return args.method_with_amap.second.begin()->first;
   }
 
   static java_bytecode_convert_methodt::method_offsett
   get_last_node(const method_with_amapt &args)
   {
-    return (--args.second.end())->first;
+    return (--args.method_with_amap.second.end())->first;
   }
 
   static bool nodes_empty(const method_with_amapt &args)
   {
-    return args.second.empty();
+    return args.method_with_amap.second.empty();
   }
 };
 

src/analyses/ai_storage.h

@@ -100,7 +100,8 @@ class ai_storage_baset
 class trace_map_storaget : public ai_storage_baset
 {
 protected:
-  typedef std::map<locationt, trace_set_ptrt> trace_mapt;
+  typedef std::map<locationt, trace_set_ptrt, goto_programt::target_less_than>
+    trace_mapt;
   trace_mapt trace_map;
 
   // This retains one part of a shared_ptr to the history object

src/analyses/call_graph.h

@@ -98,7 +98,7 @@ class call_grapht
   typedef goto_programt::const_targett locationt;
 
   /// Type of a set of callsites
-  typedef std::set<locationt> locationst;
+  typedef std::set<locationt, goto_programt::target_less_than> locationst;
 
   /// Type mapping from call-graph edges onto the set of call instructions
   /// that make that call.

src/analyses/cfg_dominators.h

@@ -36,7 +36,7 @@ template <class P, class T, bool post_dom>
 class cfg_dominators_templatet
 {
 public:
-  typedef std::set<T> target_sett;
+  typedef std::set<T, typename P::target_less_than> target_sett;
 
   struct nodet
   {
@@ -218,12 +218,12 @@ void cfg_dominators_templatet<P, T, post_dom>::fixedpoint(P &program)
       {
         if(*n_it==current)
           ++n_it;
-        else if(*n_it<*o_it)
+        else if(typename P::target_less_than()(*n_it, *o_it))
         {
           changed=true;
           node.dominators.erase(n_it++);
         }
-        else if(*o_it<*n_it)
+        else if(typename P::target_less_than()(*o_it, *n_it))
           ++o_it;
         else
         {

src/analyses/dependence_graph.h

@@ -172,7 +172,9 @@ class dep_graph_domaint:public ai_domain_baset
   node_indext node_id;
   bool has_changed;
 
-  typedef std::set<goto_programt::const_targett> depst;
+  typedef std::
+    set<goto_programt::const_targett, goto_programt::target_less_than>
+      depst;
 
   // Set of locations with control instructions on which the instruction at this
   // location has a control dependency on

src/analyses/flow_insensitive_analysis.h

@@ -93,9 +93,9 @@ class flow_insensitive_analysis_baset
   typedef flow_insensitive_abstract_domain_baset statet;
   typedef goto_programt::const_targett locationt;
 
-  std::set<locationt> seen_locations;
+  std::set<locationt, goto_programt::target_less_than> seen_locations;
 
-  std::map<locationt, unsigned> statistics;
+  std::map<locationt, unsigned, goto_programt::target_less_than> statistics;
 
   bool seen(const locationt &l)
   {
@@ -155,7 +155,11 @@ class flow_insensitive_analysis_baset
 protected:
   const namespacet &ns;
 
-  typedef std::priority_queue<locationt> working_sett;
+  typedef std::priority_queue<
+    locationt,
+    std::vector<locationt>,
+    goto_programt::target_less_than>
+    working_sett;
 
   locationt get_next(working_sett &working_set);
 

src/analyses/is_threaded.h

@@ -44,7 +44,9 @@ class is_threadedt
   }
 
 protected:
-  typedef std::set<goto_programt::const_targett> is_threaded_sett;
+  typedef std::
+    set<goto_programt::const_targett, goto_programt::target_less_than>
+      is_threaded_sett;
   is_threaded_sett is_threaded_set;
 
   void compute(

src/analyses/lexical_loops.h

@@ -152,7 +152,7 @@ bool lexical_loops_templatet<P, T>::compute_lexical_loop(
     "loop header should come lexically before the tail");
 
   std::stack<T> stack;
-  std::set<T> loop_instructions;
+  std::set<T, typename P::target_less_than> loop_instructions;
 
   loop_instructions.insert(loop_head);
   loop_instructions.insert(loop_tail);

src/analyses/local_cfg.h

@@ -29,7 +29,9 @@ class local_cfgt
     successorst successors;
   };
 
-  typedef std::map<goto_programt::const_targett, node_nrt> loc_mapt;
+  typedef std::
+    map<goto_programt::const_targett, node_nrt, goto_programt::target_less_than>
+      loc_mapt;
   loc_mapt loc_map;
 
   typedef std::vector<nodet> nodest;

src/analyses/local_may_alias.h

@@ -140,7 +140,9 @@ class local_may_alias_factoryt
   typedef std::map<irep_idt, std::unique_ptr<local_may_aliast> > fkt_mapt;
   fkt_mapt fkt_map;
 
-  typedef std::map<goto_programt::const_targett, irep_idt > target_mapt;
+  typedef std::
+    map<goto_programt::const_targett, irep_idt, goto_programt::target_less_than>
+      target_mapt;
   target_mapt target_map;
 };
 

src/analyses/loop_analysis.h

@@ -22,7 +22,7 @@ class loop_analysist;
 template <class T>
 class loop_templatet
 {
-  typedef std::set<T> loop_instructionst;
+  typedef std::set<T, goto_programt::target_less_than> loop_instructionst;
   loop_instructionst loop_instructions;
 
   friend loop_analysist<T>;
@@ -83,7 +83,7 @@ class loop_analysist
 public:
   typedef loop_templatet<T> loopt;
   // map loop headers to loops
-  typedef std::map<T, loopt> loop_mapt;
+  typedef std::map<T, loopt, goto_programt::target_less_than> loop_mapt;
 
   loop_mapt loop_map;
 

src/analyses/reaching_definitions.h

@@ -114,9 +114,9 @@ inline bool operator<(
   const reaching_definitiont &a,
   const reaching_definitiont &b)
 {
-  if(a.definition_at<b.definition_at)
+  if(goto_programt::target_less_than()(a.definition_at, b.definition_at))
     return true;
-  if(b.definition_at<a.definition_at)
+  if(goto_programt::target_less_than()(b.definition_at, a.definition_at))
     return false;
 
   if(a.bit_begin.is_unknown() != b.bit_begin.is_unknown())
@@ -250,7 +250,8 @@ class rd_range_domaint:public ai_domain_baset
 
   // each element x represents a range of bits [x.first, x.second)
   typedef std::multimap<range_spect, range_spect> rangest;
-  typedef std::map<locationt, rangest> ranges_at_loct;
+  typedef std::map<locationt, rangest, goto_programt::target_less_than>
+    ranges_at_loct;
 
   const ranges_at_loct &get(const irep_idt &identifier) const;
   void clear_cache(const irep_idt &identifier) const

src/analyses/static_analysis.h

@@ -317,7 +317,7 @@ class static_analysist:public static_analysis_baset
   }
 
 protected:
-  typedef std::map<locationt, T> state_mapt;
+  typedef std::map<locationt, T, goto_programt::target_less_than> state_mapt;
   state_mapt state_map;
 
   virtual statet &get_state(locationt l)

src/analyses/variable-sensitivity/context_abstract_object.h

@@ -132,7 +132,7 @@ class context_abstract_objectt : public abstract_objectt
 
   exprt to_predicate_internal(const exprt &name) const override;
 
-  typedef std::set<locationt> locationst;
+  typedef std::set<locationt, goto_programt::target_less_than> locationst;
 
   virtual context_abstract_object_ptrt
   update_location_context_internal(const locationst &locations) const = 0;

src/analyses/variable-sensitivity/data_dependency_context.cpp

@@ -327,10 +327,11 @@ data_dependency_contextt::abstract_object_merge_internal(
  *
  * \return set of data dependencies
  */
-std::set<goto_programt::const_targett>
+std::set<goto_programt::const_targett, goto_programt::target_less_than>
 data_dependency_contextt::get_data_dependencies() const
 {
-  std::set<goto_programt::const_targett> result;
+  std::set<goto_programt::const_targett, goto_programt::target_less_than>
+    result;
   for(const auto &d : data_deps)
     result.insert(d);
   return result;
@@ -341,10 +342,11 @@ data_dependency_contextt::get_data_dependencies() const
  *
  * \return set of data dominators
  */
-std::set<goto_programt::const_targett>
+std::set<goto_programt::const_targett, goto_programt::target_less_than>
 data_dependency_contextt::get_data_dominators() const
 {
-  std::set<goto_programt::const_targett> result;
+  std::set<goto_programt::const_targett, goto_programt::target_less_than>
+    result;
   for(const auto &d : data_dominators)
     result.insert(d);
   return result;

src/analyses/variable-sensitivity/data_dependency_context.h

@@ -56,8 +56,10 @@ class data_dependency_contextt : public write_location_contextt
 
   bool has_been_modified(const abstract_object_pointert &before) const override;
 
-  std::set<goto_programt::const_targett> get_data_dependencies() const;
-  std::set<goto_programt::const_targett> get_data_dominators() const;
+  std::set<goto_programt::const_targett, goto_programt::target_less_than>
+  get_data_dependencies() const;
+  std::set<goto_programt::const_targett, goto_programt::target_less_than>
+  get_data_dominators() const;
 
   void output(std::ostream &out, const class ai_baset &ai, const namespacet &ns)
     const override;

src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.cpp

@@ -326,10 +326,13 @@ bool variable_sensitivity_dependence_domaint::merge_control_dependencies(
   for(const auto &c_dep : other_control_deps)
   {
     // find position to insert
-    while(it != control_deps.end() && it->first < c_dep.first)
+    while(it != control_deps.end() &&
+          goto_programt::target_less_than()(it->first, c_dep.first))
       ++it;
 
-    if(it == control_deps.end() || c_dep.first < it->first)
+    if(
+      it == control_deps.end() ||
+      goto_programt::target_less_than()(c_dep.first, it->first))
     {
       // hint points at position that will follow the new element
       control_deps.insert(it, c_dep);
@@ -340,9 +343,11 @@ bool variable_sensitivity_dependence_domaint::merge_control_dependencies(
       INVARIANT(
         it != control_deps.end(), "Property of branch needed for safety");
       INVARIANT(
-        !(it->first < c_dep.first), "Property of loop needed for safety");
+        !(goto_programt::target_less_than()(it->first, c_dep.first)),
+        "Property of loop needed for safety");
       INVARIANT(
-        !(c_dep.first < it->first), "Property of loop needed for safety");
+        !(goto_programt::target_less_than()(c_dep.first, it->first)),
+        "Property of loop needed for safety");
 
       tvt &branch1 = it->second;
       const tvt &branch2 = c_dep.second;

src/analyses/variable-sensitivity/variable_sensitivity_dependence_graph.h

@@ -179,13 +179,19 @@ class variable_sensitivity_dependence_domaint
       data_depst;
   data_depst domain_data_deps;
 
-  typedef std::map<goto_programt::const_targett, tvt> control_depst;
+  typedef std::
+    map<goto_programt::const_targett, tvt, goto_programt::target_less_than>
+      control_depst;
   control_depst control_deps;
 
-  typedef std::set<goto_programt::const_targett> control_dep_candidatest;
+  typedef std::
+    set<goto_programt::const_targett, goto_programt::target_less_than>
+      control_dep_candidatest;
   control_dep_candidatest control_dep_candidates;
 
-  typedef std::set<goto_programt::const_targett> control_dep_callst;
+  typedef std::
+    set<goto_programt::const_targett, goto_programt::target_less_than>
+      control_dep_callst;
   control_dep_callst control_dep_calls;
   control_dep_callst control_dep_call_candidates;
 

src/cprover/state_encoding.cpp

@@ -92,7 +92,8 @@ class state_encodingt
   loct first_loc;
   symbol_exprt entry_state = symbol_exprt(irep_idt(), typet());
   exprt return_lhs = nil_exprt();
-  using incomingt = std::map<loct, std::vector<loct>>;
+  using incomingt =
+    std::map<loct, std::vector<loct>, goto_programt::target_less_than>;
   incomingt incoming;
 
   static symbol_exprt va_args(irep_idt function);

src/goto-checker/fault_localization_provider.h

@@ -21,7 +21,11 @@ class namespacet;
 
 struct fault_location_infot
 {
-  typedef std::map<goto_programt::const_targett, std::size_t> score_mapt;
+  typedef std::map<
+    goto_programt::const_targett,
+    std::size_t,
+    goto_programt::target_less_than>
+    score_mapt;
   score_mapt scores;
 };
 

src/goto-checker/symex_coverage.cpp

@@ -80,7 +80,11 @@ class goto_program_coverage_recordt : public coverage_recordt
     }
 
     unsigned hits;
-    std::map<goto_programt::const_targett, coverage_conditiont> conditions;
+    std::map<
+      goto_programt::const_targett,
+      coverage_conditiont,
+      goto_programt::target_less_than>
+      conditions;
   };
 
   typedef std::map<unsigned, coverage_linet> coverage_lines_mapt;

src/goto-checker/symex_coverage.h

@@ -64,9 +64,16 @@ class symex_coveraget
     goto_programt::const_targett succ;
   };
 
-  typedef std::map<goto_programt::const_targett, coverage_infot>
+  typedef std::map<
+    goto_programt::const_targett,
+    coverage_infot,
+    goto_programt::target_less_than>
     coverage_innert;
-  typedef std::map<goto_programt::const_targett, coverage_innert> coveraget;
+  typedef std::map<
+    goto_programt::const_targett,
+    coverage_innert,
+    goto_programt::target_less_than>
+    coveraget;
   coveraget coverage;
 
   bool

src/goto-diff/change_impact.cpp

@@ -236,8 +236,9 @@ class change_impactt
     DEL_CTRL_DEP=1<<5
   };
 
-  typedef std::map<goto_programt::const_targett, unsigned>
-    goto_program_change_impactt;
+  typedef std::
+    map<goto_programt::const_targett, unsigned, goto_programt::target_less_than>
+      goto_program_change_impactt;
   typedef std::map<irep_idt, goto_program_change_impactt>
     goto_functions_change_impactt;