Merge pull request #1009 from thk123/bugfix/arrays-structs-null-pointers

Fix for arrays containing structs will null pointers

Author: Daniel Kroening

regression/goto-analyzer/approx-array-variable-const-fp/test.desc

@@ -14,3 +14,4 @@ main.c
 ^\s*IF fp_tbl\[\(signed (long )*long int\)i\] == f8 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed (long )*long int\)i\] == f9 THEN GOTO [0-9]$
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/approx-const-fp-array-variable-cast-const-fp/test.desc

@@ -14,3 +14,4 @@ main.c
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f7 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f8 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f9 THEN GOTO [0-9]$
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/approx-const-fp-array-variable-const-fp-with-null/test.desc

@@ -5,6 +5,7 @@ main.c
 ^\s*IF fp == f2 THEN GOTO [0-9]$
 ^\s*IF fp == f3 THEN GOTO [0-9]$
 ^\s*IF fp == f4 THEN GOTO [0-9]$
+replacing function pointer by 3 possible targets
 ^SIGNAL=0$
 --
 ^warning: ignoring
@@ -14,3 +15,4 @@ main.c
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f7 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f8 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f9 THEN GOTO [0-9]$
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/approx-const-fp-array-variable-const-fp/test.desc

@@ -14,3 +14,4 @@ main.c
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f7 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f8 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f9 THEN GOTO [0-9]$
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/approx-const-fp-array-variable-const-pointer-const-struct-non-const-fp/test.desc

@@ -14,3 +14,4 @@ main.c
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f7 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f8 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f9 THEN GOTO [0-9]$
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/approx-const-fp-array-variable-const-struct-non-const-fp/test.desc

@@ -14,3 +14,4 @@ main.c
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f7 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f8 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f9 THEN GOTO [0-9]$
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/approx-const-fp-array-variable-invalid-cast-const-fp/test.desc

@@ -14,3 +14,4 @@ main.c
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f7 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f8 THEN GOTO [0-9]$
 ^\s*IF fp_tbl\[\(signed long int\)i\] == f9 THEN GOTO [0-9]$
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/approx-const-fp-array-variable-struct-const-fp-with-zero/main.c

@@ -0,0 +1,42 @@
+#include <stdio.h>
+
+void f1 (void) { printf("%i\n", 1); }
+void f2 (void) { printf("%i\n", 2); }
+void f3 (void) { printf("%i\n", 3); }
+void f4 (void) { printf("%i\n", 4); }
+void f5 (void) { printf("%i\n", 5); }
+void f6 (void) { printf("%i\n", 6); }
+void f7 (void) { printf("%i\n", 7); }
+void f8 (void) { printf("%i\n", 8); }
+void f9 (void) { printf("%i\n", 9); }
+
+typedef void(*void_fp)(void);
+
+struct action
+{
+  int x;
+  void_fp fun;
+};
+
+// Array with an empty final element
+const struct action fp_tbl[5] = {{1, f2}, {2, f3} ,{3, f4},};
+
+// There is a basic check that excludes all functions that aren't used anywhere
+// This ensures that check can't work in this example
+const void_fp fp_all[] = {f1, f2 ,f3, f4, f5 ,f6, f7, f8, f9};
+
+void func(int i)
+{
+  const void_fp fp = fp_tbl[i].fun;
+  fp();
+}
+
+int main()
+{
+  for(int i=0;i<3;i++)
+  {
+    func(i);
+  }
+
+  return 0;
+}

regression/goto-analyzer/approx-const-fp-array-variable-struct-const-fp-with-zero/test.desc

@@ -0,0 +1,18 @@
+CORE
+main.c
+--show-goto-functions --verbosity 10 --pointer-check
+^Removing function pointers and virtual functions$
+^\s*IF fp == f2 THEN GOTO [0-9]$
+^\s*IF fp == f3 THEN GOTO [0-9]$
+^\s*IF fp == f4 THEN GOTO [0-9]$
+^\s*ASSERT FALSE // invalid function pointer$
+^SIGNAL=0$
+--
+^warning: ignoring
+^\s*IF fp == f1 THEN GOTO [0-9]$
+^\s*IF fp == f5 THEN GOTO [0-9]$
+^\s*IF fp == f6 THEN GOTO [0-9]$
+^\s*IF fp == f7 THEN GOTO [0-9]$
+^\s*IF fp == f8 THEN GOTO [0-9]$
+^\s*IF fp == f9 THEN GOTO [0-9]$
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/no-match-array-literal-const-fp-null/test.desc

@@ -4,5 +4,6 @@ main.c
 ^Removing function pointers and virtual functions$
 ^\s*ASSERT FALSE // invalid function pointer$
 ^SIGNAL=0$
+function func: replacing function pointer by 0 possible targets
 --
 ^warning: ignoring

regression/goto-analyzer/no-match-const-fp-const-fp-null/test.desc

@@ -4,5 +4,6 @@ main.c
 ^Removing function pointers and virtual functions$
 ^\s*ASSERT FALSE // invalid function pointer$
 ^SIGNAL=0$
+replacing function pointer by 0 possible targets
 --
 ^warning: ignoring

regression/goto-analyzer/no-match-const-fp-const-pointer-const-struct-const-fp-null/test.desc

@@ -3,6 +3,7 @@ main.c
 --show-goto-functions --verbosity 10 --pointer-check
 ^Removing function pointers and virtual functions$
 ^\s*ASSERT FALSE // invalid function pointer$
+replacing function pointer by 9 possible targets
 ^SIGNAL=0$
 --
 ^warning: ignoring

regression/goto-analyzer/no-match-const-fp-dereference-const-pointer-null/test.desc

@@ -3,6 +3,7 @@ main.c
 --show-goto-functions --verbosity 10 --pointer-check
 ^Removing function pointers and virtual functions$
 ^\s*ASSERT FALSE // invalid function pointer$
+replacing function pointer by 9 possible targets
 ^SIGNAL=0$
 --
 ^warning: ignoring

regression/goto-analyzer/no-match-const-fp-null/test.desc

@@ -3,5 +3,6 @@ main.c
 --show-goto-functions --verbosity 10 --pointer-check
 ^Removing function pointers and virtual functions$
 ^\s*ASSERT FALSE // invalid function pointer$
+function func: replacing function pointer by 0 possible targets
 --
 ^warning: ignoring

regression/goto-analyzer/no-match-const-struct-non-const-fp-null/test.desc

@@ -4,5 +4,6 @@ main.c
 ^Removing function pointers and virtual functions$
 ^\s*ASSERT FALSE // invalid function pointer$
 ^SIGNAL=0$
+replacing function pointer by 0 possible targets
 --
 ^warning: ignoring

regression/goto-analyzer/precise-array-calculation-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-array-literal-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-fp-array-const-variable-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-fp-array-literal-const-fp-run-time/test.desc

@@ -5,3 +5,4 @@ main.c
 ^\s*f3\(\);$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-fp-array-literal-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-fp-array-literal-const-struct-non-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-fp-array-variable-const-pointer-const-struct-non-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-fp-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-fp-const-struct-const-array-literal-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-fp-const-struct-non-const-array-literal-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-fp-const-struct-non-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-fp-dereference-const-pointer-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-fp/test.desc

@@ -5,3 +5,4 @@ main.c
 ^\s*f2\(\);
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-pointer-const-struct-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-const-struct-non-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-derefence-const-pointer-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-derefence/test.desc

@@ -5,3 +5,4 @@ main.c
 ^\s*f2\(\);
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-dereference-address-pointer-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-dereference-const-struct-const-pointer-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-dereference-const-struct-const-pointer-const-struct-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

regression/goto-analyzer/precise-dereference-const-struct-pointer-const-fp/test.desc

@@ -6,3 +6,4 @@ main.c
 ^SIGNAL=0$
 --
 ^warning: ignoring
+function \w+: replacing function pointer by 9 possible targets

src/goto-programs/remove_const_function_pointers.cpp

@@ -9,29 +9,26 @@ Author: Thomas Kiley, thomas.kiley@diffblue.com
 /// \file
 /// Goto Programs
 
+#include "remove_const_function_pointers.h"
+
 #include <ansi-c/c_qualifiers.h>
 #include <util/simplify_expr.h>
 #include <util/arith_tools.h>
 
-#include "remove_const_function_pointers.h"
-
 #define LOG(message, irep) \
   debug() << "Case " << __LINE__ << " : " << message << "\n" \
           << irep.pretty() << eom;
 
 /// To take a function call on a function pointer, and if possible resolve it to
 /// a small collection of possible values.
 /// \param message_handler: The message handler for messaget
-/// \param base_expression: The function call through a function pointer
 /// \param ns: The namespace to use to resolve types
 /// \param symbol_table: The symbol table to look up symbols in
 remove_const_function_pointerst::remove_const_function_pointerst(
   message_handlert &message_handler,
-  const exprt &base_expression,
   const namespacet &ns,
   const symbol_tablet &symbol_table):
     messaget(message_handler),
-    original_expression(base_expression),
     ns(ns),
     symbol_table(symbol_table)
 {}
@@ -41,16 +38,18 @@ remove_const_function_pointerst::remove_const_function_pointerst(
 /// that are const and: - assigned directly to a function - assigned to a value
 /// in an array of functions - assigned to a const struct component Or
 /// variations within.
+/// \param base_expression: The function call through a function pointer
 /// \param out_functions: The functions that (symbols of type ID_code) the base
 /// expression could take.
 /// \return Returns true if it was able to resolve the call, false if not. If it
 ///   returns true, out_functions will be populated by all the possible values
 ///   the function pointer could be.
 bool remove_const_function_pointerst::operator()(
+  const exprt &base_expression,
   functionst &out_functions)
 {
   // Replace all const symbols with their values
-  exprt non_symbol_expression=replace_const_symbols(original_expression);
+  exprt non_symbol_expression=replace_const_symbols(base_expression);
   return try_resolve_function_call(non_symbol_expression, out_functions);
 }
 
@@ -165,6 +164,20 @@ bool remove_const_function_pointerst::try_resolve_function_call(
       resolved=false;
     }
   }
+  else if(simplified_expr.id()==ID_constant)
+  {
+    if(simplified_expr.is_zero())
+    {
+      // We have the null pointer - no need to throw everything away
+      // but we don't add any functions either
+      resolved=true;
+    }
+    else
+    {
+      LOG("Non-zero constant value found", simplified_expr);
+      resolved=false;
+    }
+  }
   else
   {
     LOG("Unrecognised expression", simplified_expr);
@@ -556,10 +569,7 @@ bool remove_const_function_pointerst::try_resolve_index_of(
 
             for(const exprt &resolved_array_entry : array_contents)
             {
-              if(!resolved_array_entry.is_zero())
-              {
-                out_expressions.push_back(resolved_array_entry);
-              }
+              out_expressions.push_back(resolved_array_entry);
             }
           }
         }

src/goto-programs/remove_const_function_pointers.h

@@ -27,11 +27,10 @@ class remove_const_function_pointerst:public messaget
   typedef std::list<exprt> expressionst;
   remove_const_function_pointerst(
     message_handlert &message_handler,
-    const exprt &base_expression,
     const namespacet &ns,
     const symbol_tablet &symbol_table);
 
-  bool operator()(functionst &out_functions);
+  bool operator()(const exprt &base_expression, functionst &out_functions);
 
 private:
   exprt replace_const_symbols(const exprt &expression) const;
@@ -93,7 +92,6 @@ class remove_const_function_pointerst:public messaget
   exprt get_component_value(
     const struct_exprt &struct_expr, const member_exprt &member_expr);
 
-  const exprt original_expression;
   const namespacet &ns;
   const symbol_tablet &symbol_table;
 };

src/goto-programs/remove_function_pointers.cpp

@@ -20,6 +20,7 @@ Author: Daniel Kroening, kroening@kroening.com
 #include <util/base_type.h>
 #include <ansi-c/c_qualifiers.h>
 #include <analyses/does_remove_const.h>
+#include <util/invariant.h>
 
 #include <util/c_types.h>
 
@@ -292,14 +293,15 @@ void remove_function_pointerst::remove_function_pointer(
   else
   {
     remove_const_function_pointerst fpr(
-    get_message_handler(), pointer, ns, symbol_table);
+    get_message_handler(), ns, symbol_table);
 
-    found_functions=fpr(functions);
+    found_functions=fpr(pointer, functions);
 
-    // Either found_functions is true therefore the functions should not
-    // be empty
-    // Or found_functions is false therefore the functions should be empty
-    assert(found_functions != functions.empty());
+    // if found_functions is false, functions should be empty
+    // however, it is possible for found_functions to be true and functions
+    // to be empty (this happens if the pointer can only resolve to the null
+    // pointer)
+    CHECK_RETURN(found_functions || functions.empty());
 
     if(functions.size()==1)
     {