Merge pull request diffblue#117 from trtikm/cleanup/general-tidy-and-fix

Cleanup/general tidy and fix

Author: NathanJPhillips

src/goto-analyzer/taint_rules.cpp

@@ -115,7 +115,7 @@ boost::optional<taint_rulet::tainted_locationt> taint_rulet::tainted_locationt::
     return boost::none;
   }
   taint_tokent token = taint_tokent::fresh();
-  tokens.insert(named_tokenst::value_type(taint_string, token));
+  tokens.left.insert({ taint_string, token });
   return tainted_locationt(location, token, json["dereference"].is_true());
 }
 
@@ -140,7 +140,7 @@ std::unique_ptr<taint_rulet> taint_rulet::load(
       tainted_locationt::load(input_json, tokens, message_handler);
     return std::unique_ptr<taint_rulet>(new taint_rulet(input, *input /* TODO */));
   }
-  else
+  else if(is_sink_json.is_false() || is_sink_json.is_null())
   {
     boost::optional<tainted_locationt> input =
         tainted_locationt::load(input_json, tokens, message_handler),
@@ -155,6 +155,13 @@ std::unique_ptr<taint_rulet> taint_rulet::load(
     }
     return std::unique_ptr<taint_rulet>(new taint_rulet(input, *result));
   }
+  else
+  {
+    msg.error()
+      << "Rule has isSink that is not true or false, ignoring it."
+      << messaget::eom;
+    return nullptr;
+  }
 }
 
 std::unique_ptr<taint_rulest> taint_rulest::load(
@@ -172,7 +179,6 @@ std::unique_ptr<taint_rulest> taint_rulest::load(
   }
 
   std::unique_ptr<taint_rulest> rules(new taint_rulest());
-  named_tokenst tokens;
   for(const jsont &rule_json : root.array)
   {
     if(!rule_json.is_object())
@@ -204,10 +210,11 @@ std::unique_ptr<taint_rulest> taint_rulest::load(
     //   names (without packages).
     const std::string method_name = method_json.value;
 
-    std::unique_ptr<taint_rulet> rule = taint_rulet::load(rule_json, tokens, message_handler);
+    std::unique_ptr<taint_rulet> rule =
+      taint_rulet::load(rule_json, rules->named_tokens, message_handler);
     if(rule != nullptr)
       // TODO: Convert base_class_name to symbol_exprt
-      rules->add(symbol_exprt(), move(rule));
+      rules->add(irep_idt(), move(rule));
   }
 
   return rules->empty() ? nullptr : move(rules);

src/goto-analyzer/taint_rules.h

@@ -104,8 +104,10 @@ class taint_rulet
     files.
 
 \*******************************************************************/
-class taint_rulest : private std::map<symbol_exprt, std::shared_ptr<taint_rulet>>
+class taint_rulest : private std::map<irep_idt, std::shared_ptr<taint_rulet>>
 {
+private:
+  named_tokenst named_tokens;
 public:
 
   /*******************************************************************\
@@ -122,9 +124,9 @@ class taint_rulest : private std::map<symbol_exprt, std::shared_ptr<taint_rulet>
       Adds a rule to the set.
 
   \*******************************************************************/
-  void add(symbol_exprt function, std::shared_ptr<taint_rulet> rule)
+  void add(const irep_idt &fn_name, std::shared_ptr<taint_rulet> rule)
   {
-    insert({ function, rule });
+    insert({ fn_name, rule });
   }
 
 
@@ -148,10 +150,14 @@ class taint_rulest : private std::map<symbol_exprt, std::shared_ptr<taint_rulet>
     Purpose:
 
   \*******************************************************************/
-  std::shared_ptr<taint_rulet> find(const symbol_exprt &function) const
+  std::shared_ptr<taint_rulet> find(const irep_idt &fn_name) const
   {
+    // We currently match directly on the name
+    // TODO: Search for any methods derived on base classes of the class name
+    //  used in the call
+    // class_hierarchyt::idst parents = class_hierarchy.get_parents_trans(class_id);
     const_iterator it =
-      std::map<symbol_exprt, std::shared_ptr<taint_rulet>>::find(function);
+      std::map<irep_idt, std::shared_ptr<taint_rulet>>::find(fn_name);
     return it == end() ? nullptr : it->second;
   }
 

src/goto-analyzer/taint_summary.cpp

@@ -126,12 +126,12 @@ static void  initialise_domain(
       else if (it->type == FUNCTION_CALL)
       {
         code_function_callt const&  fn_call = to_code_function_call(it->code);
-        if (fn_call.function().id() == ID_symbol)
+        const exprt &callee_expr = fn_call.function();
+        if (callee_expr.id() == ID_symbol)
         {
-          std::string const  callee_ident =
-              as_string(to_symbol_expr(fn_call.function()).get_identifier());
-
-          auto const&  fn_type = functions_map.at(callee_ident).type;
+          irep_idt callee_id = to_symbol_expr(callee_expr).get_identifier();
+          const std::string callee_ident = as_string(callee_id);
+          const code_typet &fn_type = functions_map.at(callee_id).type;
 
           taint_summary_ptrt const  summary =
               database.find<taint_summaryt>(callee_ident);
@@ -390,9 +390,6 @@ static void  build_symbols_substitution(
   namespace_utilst nsu(ns);
   auto parameter_indices=get_parameter_indices(fn_type);
 
-  std::string const  callee_ident =
-      as_string(to_symbol_expr(fn_call.function()).get_identifier());
-
   for (const std::pair<taint_lvaluet, taint_sett>& lvalue_taint : summary->input())
   {
     assert(!lvalue_taint.second.is_bottom());
@@ -947,20 +944,19 @@ numbered_lvalue_to_taint_mapt  transform(
   case FUNCTION_CALL:
     {
       const code_function_callt &fn_call = to_code_function_call(I.code);
-      if (fn_call.function().id() == ID_symbol)
+      const exprt &callee_expr = fn_call.function();
+      if (callee_expr.id() == ID_symbol)
       {
         msg.debug() << "Recognised FUNCTION_CALL instruction." << messaget::eom;
 
-        const std::string callee_ident =
-            as_string(to_symbol_expr(fn_call.function()).get_identifier());
-
-        const code_typet &fn_type = functions_map.at(callee_ident).type;
+        irep_idt callee_id = to_symbol_expr(callee_expr).get_identifier();
+        const std::string callee_ident = as_string(callee_id);
+        const code_typet &fn_type = functions_map.at(callee_id).type;
 
         assert(lvsa != nullptr);
         // Find a matching rule
         taint_rulest taint_rules;
-        std::shared_ptr<taint_rulet> taint_rule =
-          taint_rules.find(to_symbol_expr(fn_call.function()));
+        std::shared_ptr<taint_rulet> taint_rule = taint_rules.find(callee_id);
         // Find the summary
         taint_summary_ptrt const summary =
           database.find<taint_summaryt>(callee_ident);
@@ -1057,7 +1053,7 @@ numbered_lvalue_to_taint_mapt  transform(
             summary->output(),
             symbols_substitution,
             caller_ident,
-            callee_ident,
+            callee_id,
             fn_call,
             fn_type,
             ns,
@@ -1069,19 +1065,19 @@ numbered_lvalue_to_taint_mapt  transform(
         else
         {
           msg.warning()
-            << "!!! WARNING !!! : No summary and no transition rule was "
-                "found for the called function "
-            << as_string(callee_ident)
-            << "So, we use identity as a transformation function."
+            << "!!! WARNING !!! : No summary was found for the called function "
+            << callee_ident
+            << "Identity will be used as a transformation function."
             << messaget::eom;
         }
       }
       else
         msg.warning()
-          << "!!! WARNING !!! : Recognised FUNCTION_CALL instruction "
-            "using non-identifier call expression. Such call is not "
-            "supported. So, we use identity as a transformation "
-            "function." << messaget::eom;
+          << "!!! WARNING !!! : Recognised FUNCTION_CALL instruction using a "
+            "non-identifier call expression. All such calls should have been "
+            "removed by the remove-function-pointers pass."
+            "Identity will be used as a transformation function."
+          << messaget::eom;
     }
     break;
   case OTHER: