Merge pull request diffblue#416 from diffblue/bugfix/lvsa-casts

Bugfix/lvsa casts

Author: owen-jones-diffblue

regression/LVSA/TestCasts/Test.java

@@ -0,0 +1,40 @@
+package LVSA.TestCasts;
+
+public class Test {
+    public static TestInterface interfaceFromChild;
+    public static TestInterface interfaceFromParent;
+    public static TestParent parentFromChild;
+    public static TestParent parentFromInterface;
+    public static TestChild childFromInterface;
+    public static TestChild childFromParent;
+
+    public void cast_from_child() {
+        TestChild testChild = new TestChild();
+
+        interfaceFromChild = (TestInterface)testChild;
+        parentFromChild = (TestParent)testChild;
+    }
+
+    public void cast_from_parent() {
+        TestParent testParent = new TestChild();
+
+        interfaceFromParent = (TestInterface)testParent;
+        childFromParent = (TestChild)testParent;
+    }
+
+    public void cast_from_interface() {
+        TestInterface testInterface = (TestInterface)new TestChild();
+
+        parentFromInterface = (TestParent)testInterface;
+        childFromInterface = (TestChild)testInterface;
+    }
+}
+
+interface TestInterface {
+}
+
+class TestParent {
+}
+
+class TestChild extends TestParent implements TestInterface {
+}

regression/LVSA/TestCasts/test_casts.py

@@ -0,0 +1,64 @@
+import fasteners
+import regression.LVSA.lvsa_driver as driver
+import os
+import regression.utils as utils
+
+
+def _get_this_file_dir():
+    return os.path.dirname(os.path.abspath(__file__))
+
+
+def _get_folder_name():
+    return os.path.basename(_get_this_file_dir())
+
+
+@fasteners.interprocess_locked(os.path.join(os.path.dirname(__file__), ".build_lock"))
+def test_cast_from_child(tmpdir):
+    """ Test addition and removal of casts to DO's """
+    with utils.working_dir(_get_this_file_dir()):
+        os.system("javac Test.java")
+    lvsa_driver = driver.LvsaDriver(tmpdir, _get_folder_name()).with_test_function('cast_from_child')
+    lvsa_expectation = lvsa_driver.run()
+
+    value_set_expectation = lvsa_expectation.get_value_set_for_public_static('interfaceFromChild')
+    value_set_expectation.check_number_of_values(1)
+    value_set_expectation.check_contains_typecast_dynamic_object(1, True, 'LVSA.TestCasts.TestInterface')
+
+    value_set_expectation = lvsa_expectation.get_value_set_for_public_static('parentFromChild')
+    value_set_expectation.check_number_of_values(1)
+    value_set_expectation.check_contains_soft_cast_dynamic_object(1, True, 'LVSA.TestCasts.TestParent')
+
+
+@fasteners.interprocess_locked(os.path.join(os.path.dirname(__file__), ".build_lock"))
+def test_cast_from_parent(tmpdir):
+    """ Test addition and removal of casts to DO's """
+    with utils.working_dir(_get_this_file_dir()):
+        os.system("javac Test.java")
+    lvsa_driver = driver.LvsaDriver(tmpdir, _get_folder_name()).with_test_function('cast_from_parent')
+    lvsa_expectation = lvsa_driver.run()
+
+    value_set_expectation = lvsa_expectation.get_value_set_for_public_static('interfaceFromParent')
+    value_set_expectation.check_number_of_values(1)
+    value_set_expectation.check_contains_typecast_dynamic_object(1, True, 'LVSA.TestCasts.TestInterface')
+
+    value_set_expectation = lvsa_expectation.get_value_set_for_public_static('childFromParent')
+    value_set_expectation.check_number_of_values(1)
+    value_set_expectation.check_contains_dynamic_object(1, True)
+
+
+@fasteners.interprocess_locked(os.path.join(os.path.dirname(__file__), ".build_lock"))
+def test_cast_from_interface(tmpdir):
+    """ Test addition and removal of casts to DO's """
+    with utils.working_dir(_get_this_file_dir()):
+        os.system("javac Test.java")
+    lvsa_driver = driver.LvsaDriver(tmpdir, _get_folder_name()).with_test_function('cast_from_interface')
+    lvsa_expectation = lvsa_driver.run()
+
+    value_set_expectation = lvsa_expectation.get_value_set_for_public_static('parentFromInterface')
+    value_set_expectation.check_number_of_values(1)
+    value_set_expectation.check_contains_soft_cast_dynamic_object(1, True, 'LVSA.TestCasts.TestParent')
+
+    value_set_expectation = lvsa_expectation.get_value_set_for_public_static('childFromInterface')
+    value_set_expectation.check_number_of_values(1)
+    value_set_expectation.check_contains_dynamic_object(1, True)
+

regression/LVSA/lvsa_driver.py

@@ -10,7 +10,9 @@ class Expr:
 
     def __init__(self, expr):
         if expr['namedSub']['type']['id'] == 'symbol':
-            self.type_identifier = expr['namedSub']['type']['namedSub']['identifier']['id']
+            self.type_identifier = expr['namedSub']['type']['namedSub']['identifier']['id'].replace('java::', '')
+        elif expr['namedSub']['type']['id'] == 'struct':
+            self.type_identifier = expr['namedSub']['type']['namedSub']['base_name']['id']
         else:
             self.type_identifier = None
 
@@ -27,6 +29,28 @@ def __init__(self, dynamic_object_expr):
             dynamic_object_expr['namedSub']['is_most_recent_allocation']['id'] == 'most_recent_allocation'
 
 
+class TypecastDynamicObject(Expr):
+    """Turn the json for a typecast dynamic object expr into a python object"""
+
+    def __init__(self, typecast_dynamic_object_expr):
+        assert typecast_dynamic_object_expr['id'] == 'typecast'
+        assert typecast_dynamic_object_expr['sub'][0]['id'] == 'dynamic_object'
+        Expr.__init__(self, typecast_dynamic_object_expr)
+        self.dynamic_object = DynamicObject(typecast_dynamic_object_expr['sub'][0])
+
+
+class SoftCastDynamicObject(Expr):
+    """Turn the json for a soft-cast dynamic object expr into a python object"""
+
+    def __init__(self, soft_cast_dynamic_object_expr):
+        assert soft_cast_dynamic_object_expr['id'] == 'member'
+        assert soft_cast_dynamic_object_expr['namedSub']['component_name']['id'].startswith('@')
+        assert soft_cast_dynamic_object_expr['sub'][0]['id'] == 'dynamic_object'
+        Expr.__init__(self, soft_cast_dynamic_object_expr)
+        assert soft_cast_dynamic_object_expr['namedSub']['component_name']['id'] == '@' + self.type_identifier
+        self.dynamic_object = DynamicObject(soft_cast_dynamic_object_expr['sub'][0])
+
+
 class EvsType(Enum):
     root_object = '0'
     per_field = '1'
@@ -72,13 +96,19 @@ def __init__(self, evs_expr):
                 else:
                     assert False
 
+
 class ValueSetExpectation:
     """Encapsulate the states that a particular variable can take"""
 
     def __init__(self, var_state):
         self.var_state = var_state['values']
         self.null_objects = [x for x in self.var_state if x['id'] == 'NULL-object']
         self.dynamic_objects = [DynamicObject(x) for x in self.var_state if x['id'] == 'dynamic_object']
+        self.typecast_dynamic_objects = [TypecastDynamicObject(x) for x in self.var_state if x['id'] ==
+                                         'typecast' and x['sub'][0]['id'] == 'dynamic_object']
+        self.soft_cast_dynamic_objects = [SoftCastDynamicObject(x) for x in self.var_state if
+                                          x['id'] == 'member' and x['namedSub']['component_name']['id'].startswith(
+                                              '@') and x['sub'][0]['id'] == 'dynamic_object']
         self.external_value_sets = [ExternalValueSet(x) for x in self.var_state if x['id'] == 'external_value_set']
         self.unknown = [x for x in self.var_state if x['id'] == 'unknown']
 
@@ -100,6 +130,23 @@ def check_contains_dynamic_object(self, expected_number=1, is_most_recent_alloca
             matches = [x for x in matches if x.is_most_recent_allocation == is_most_recent_allocation]
         assert expected_number == len(matches)
 
+    def check_contains_typecast_dynamic_object(self, expected_number=1, is_most_recent_allocation=None, type=None):
+        matches = self.typecast_dynamic_objects
+        if is_most_recent_allocation is not None:
+            # Expect True or False
+            matches = [x for x in matches if x.dynamic_object.is_most_recent_allocation == is_most_recent_allocation]
+        if type is not None:
+            # Expect string
+            matches = [x for x in matches if x.type_identifier == type]
+        assert expected_number == len(matches)
+
+    def check_contains_soft_cast_dynamic_object(self, expected_number=1, is_most_recent_allocation=None, type=None):
+        matches = self.soft_cast_dynamic_objects
+        if is_most_recent_allocation is not None:
+            # Expect string
+            matches = [x for x in matches if x.type_identifier == type]
+        assert expected_number == len(matches)
+
     def check_contains_root_object_evs(self, expected_number=1, label_suffix=None):
         matches = [x for x in self.external_value_sets if x.evs_type == EvsType.root_object]
         if label_suffix is not None:
@@ -267,7 +314,8 @@ def run(self):
         class_path = os.path.join('LVSA', self.folder_name, self.class_filename)
         cmd = [executable_path, '--local-value-set-analysis', '--lvsa-summary-directory', self.temp_dir_path,
                '--function', fq_function_name, '--show-value-sets', '--json-ui']
-        if "SECURITY_REGRESSION_TESTS_USE_CSVSA" in os.environ and os.environ["SECURITY_REGRESSION_TESTS_USE_CSVSA"] != 0:
+        if "SECURITY_REGRESSION_TESTS_USE_CSVSA" in os.environ and os.environ[
+            "SECURITY_REGRESSION_TESTS_USE_CSVSA"] != 0:
             cmd.append("--lazy-methods-context-sensitive")
         if self.max_input_tree_depth is not None:
             cmd += ['--java-max-input-tree-depth', str(self.max_input_tree_depth)]

src/driver/sec_driver_parse_options.cpp

@@ -109,10 +109,13 @@ void sec_driver_parse_optionst::get_command_line_options(optionst &options)
 
 static irep_idt get_cprover_start_main_callee(const goto_modelt &goto_model)
 {
-  const goto_programt &start_body =
-    goto_model.get_goto_functions().function_map.at(
-      goto_functionst::entry_point()).body;
+  const goto_functionst::function_mapt &function_map =
+    goto_model.get_goto_functions().function_map;
+  auto entry_it = function_map.find(goto_functionst::entry_point());
+  if(entry_it == function_map.end())
+    return {};
 
+  const goto_programt &start_body = entry_it->second.body;
   auto last_call_it =
     std::find_if(
       start_body.instructions.rbegin(),
@@ -242,12 +245,15 @@ int sec_driver_parse_optionst::doit()
     local_value_set_analysist::dbt summarydb(serializer, 30);
     namespacet ns(goto_model.symbol_table);
 
-    const std::string function_name = cmdline.get_value("function");
     const bool print_function_summary = cmdline.isset("show-value-sets");
     const bool print_all_summaries = cmdline.isset("show-value-sets-all");
 
-    irep_idt fully_qualified_name =
-      get_cprover_start_main_callee(goto_model);
+    irep_idt fully_qualified_name = get_cprover_start_main_callee(goto_model);
+    if(fully_qualified_name.empty())
+    {
+      error() << "You must specify an entry point with --function" << eom;
+      return CPROVER_EXIT_USAGE_ERROR;
+    }
 
     call_grapht const call_graph(goto_model.goto_functions);
     std::vector<irep_idt> process_order =

src/pointer-analysis/local_value_set.cpp

@@ -67,13 +67,13 @@ upcast_to_type(exprt expr, const typet &intended_type, const namespacet &ns)
   while(type_of_expr != intended_type)
   {
     const optionalt<typet> base_type = get_base_type(type_of_expr, ns);
+    // Whether type_of_expr is object or an incomplete type class_hierarchy
+    // can not have found intended_type in the direct parents but did find it as
+    // an ancestor so intended_type must be an interface. Return nothing and
+    // do a hard cast
     if(!base_type)
       return {};
 
-    // If the base class is a stub class return unknown to remain sound
-    if(base_type->get_bool(ID_incomplete_class))
-      return exprt(ID_unknown, intended_type);
-
     update_expr_with_soft_cast_to_base_type(expr, ns);
 
     type_of_expr = ns.follow(expr.type());
@@ -86,18 +86,26 @@ upcast_to_type(exprt expr, const typet &intended_type, const namespacet &ns)
 }
 
 /// Remove all superclass accesses from `expr` and return the result
-static exprt remove_all_superclass_accesses(exprt expr)
+static exprt remove_all_typecasts_and_superclass_accesses(exprt expr)
 {
-  while(expr.id() == ID_member)
+  while(expr.id() == ID_member || expr.id() == ID_typecast)
   {
-    const member_exprt &member = to_member_expr(expr);
-    const std::string &component_name = id2string(member.get_component_name());
-    const bool is_superclass_access = has_prefix(component_name, "@") &&
-                                      component_name != "@lock" &&
-                                      component_name != "@class_identifier";
-    if(!is_superclass_access)
-      break;
-    expr = member.compound();
+    if(expr.id() == ID_typecast)
+    {
+      expr = expr.op0();
+    }
+    else
+    {
+      const member_exprt &member = to_member_expr(expr);
+      const std::string &component_name =
+        id2string(member.get_component_name());
+      const bool is_superclass_access = has_prefix(component_name, "@") &&
+                                        component_name != "@lock" &&
+                                        component_name != "@class_identifier";
+      if(!is_superclass_access)
+        break;
+      expr = member.compound();
+    }
   }
   return expr;
 }
@@ -110,15 +118,17 @@ void local_value_sett::make_union_adjusting_types(
 {
   for(const auto &num : src.read())
   {
+    // Remove soft-casts (and hard casts?) from the rhs value
     const exprt underlying_object =
-      remove_all_superclass_accesses(object_numbering[num.first]);
+      remove_all_typecasts_and_superclass_accesses(object_numbering[num.first]);
 
     if(underlying_object.id() == ID_external_value_set)
     {
       external_value_set_exprt evse_copy =
         to_external_value_set(underlying_object);
+      // Adjust the type of the EVS and return it
       evse_copy.type() = input_type;
-      insert(dest, evse_copy, num.second);
+      insert(dest, evse_copy);
     }
     else
     {
@@ -143,7 +153,7 @@ void local_value_sett::make_union_adjusting_types(
 
       if(no_cast_required || intended_type_eventually_void)
       {
-        insert(dest, num.first, num.second);
+        insert(dest, underlying_object);
       }
       else if(
         is_ancestor(
@@ -153,7 +163,8 @@ void local_value_sett::make_union_adjusting_types(
       {
         optionalt<exprt> cast_expr =
           upcast_to_type(underlying_object, intended_type, ns);
-        CHECK_RETURN(cast_expr);
+        if(!cast_expr)
+          cast_expr = typecast_exprt(underlying_object, intended_type);
 
         insert(dest, *cast_expr);
       }
@@ -203,10 +214,10 @@ static const typet &type_from_suffix(
   suffix_without_supertypes = suffix;
   const typet *ret = &original_type;
   size_t next_member = 1; // Skip initial '.'
-  if(suffix.size() != 0)
-    assert(suffix[0] == '.');
+  INVARIANT(suffix.size() != 1, "Suffix length is single character");
   while(next_member < suffix.size())
   {
+    INVARIANT(suffix[next_member - 1] == '.', "Suffix separator not found");
     ret = &ns.follow(*ret);
     assert(ret->id() == ID_struct || ret->id() == ID_union);
     // TODO: replace this silly string-chewing with a vector
@@ -266,12 +277,14 @@ static const typet &type_from_suffix(
 /// cast" means dereferencing a pointer to a class, accessing the field
 /// corresponding to its base class, and taking the address of that field -
 /// `&input->parent_class`
-static exprt remove_soft_casts(const exprt &input)
+static exprt remove_casts(exprt input)
 {
+  while(input.id() == ID_typecast)
+    input = input.op0();
   if(input.id() == ID_address_of)
   {
     const exprt &input_target = to_address_of_expr(input).object();
-    exprt expr = remove_all_superclass_accesses(input_target);
+    exprt expr = remove_all_typecasts_and_superclass_accesses(input_target);
     if(expr != input_target && expr.id() == ID_dereference)
       return to_dereference_expr(expr).pointer();
   }
@@ -293,11 +306,12 @@ void local_value_sett::get_value_set_rec(
     const typet &op_type = ns.follow(typecast_expr.op().type());
     if(op_type.id() == ID_pointer)
     {
-      // pointer-to-pointer -- we just ignore these
+      // Adjust the types of each rhs value to match the expected type
       object_mapt tmp;
-      exprt expr_to_cast = remove_soft_casts(typecast_expr.op());
+      // Hard casts have been removed by the simplifier but soft casts need
+      // to be removed too (maybe?)
+      exprt expr_to_cast = remove_casts(typecast_expr.op());
       get_value_set_rec(expr_to_cast, tmp, suffix, original_type, ns);
-      // ...except to fix up the types of external objects as they pass through:
       make_union_adjusting_types(dest, tmp, typecast_expr.type().subtype(), ns);
     }
   }