Merge pull request #5288 from danpoe/tests/restrict-function-pointers

Fix function pointer restriction bug where jump targets were not preserved

Author: danpoe

regression/goto-instrument/restrict-function-pointer-goto-target/test.c

@@ -0,0 +1,16 @@
+#include <assert.h>
+
+typedef void (*fp_t)();
+
+void f()
+{
+}
+
+int main(void)
+{
+  fp_t fp = f;
+  goto L;
+
+L:
+  fp();
+}

regression/goto-instrument/restrict-function-pointer-goto-target/test.desc

@@ -0,0 +1,13 @@
+CORE
+test.c
+--restrict-function-pointer main.function_pointer_call.1/f
+^EXIT=0$
+^SIGNAL=0$
+\[main.assertion.1\] line \d+ dereferenced function pointer at main.function_pointer_call.1 must be f: SUCCESS
+--
+--
+This test checks that a function pointer call that is the target of a goto (in
+the goto program) is correctly handled. Previously there was a bug, as the
+function pointer labelling step which is part of the restrict function pointer
+feature inserted a new instruction before the function pointer call, but did
+not adjust the goto targets.

src/goto-programs/label_function_pointer_call_sites.cpp

@@ -15,13 +15,14 @@ void label_function_pointer_call_sites(goto_modelt &goto_model)
   for(auto &goto_function : goto_model.goto_functions.function_map)
   {
     std::size_t function_pointer_call_counter = 0;
+
     for_each_instruction_if(
       goto_function.second,
       [](const goto_programt::targett it) {
         return it->is_function_call() && can_cast_expr<dereference_exprt>(
                                            it->get_function_call().function());
       },
-      [&](goto_programt::targett it) {
+      [&](goto_programt::targett &it) {
         auto const &function_call = it->get_function_call();
         auto const &function_pointer_dereference =
           to_dereference_expr(function_call.function());
@@ -52,13 +53,22 @@ void label_function_pointer_call_sites(goto_modelt &goto_model)
 
         // add assignment to the new function pointer variable, followed by a
         // call of the new variable
-        auto const assign_instruction = goto_programt::make_assignment(
+        auto assign_instruction = goto_programt::make_assignment(
           code_assignt{new_function_pointer,
                        function_pointer_dereference.pointer()},
           source_location);
-        goto_function.second.body.insert_before(it, assign_instruction);
-        to_code_function_call(it->code).function() =
+
+        goto_function.second.body.insert_before_swap(it, assign_instruction);
+        const auto next = std::next(it);
+        to_code_function_call(next->code).function() =
           dereference_exprt{new_function_pointer};
+        // we need to increment the iterator once more (in addition to the
+        // increment already done by for_each_goto_function_if()). This is
+        // because insert_before_swap() inserts a new instruction after the
+        // instruction pointed to by it (and then swaps the contents with the
+        // previous instruction). We need to increment the iterator as we also
+        // need to skip over this newly inserted instruction.
+        it++;
       });
   }
 }