Skip to content

Function pointer removal: create fall-back function call #6987

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions regression/cbmc/Function_Pointer18/test.desc
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
CORE
main.c

^EXIT=0$
^SIGNAL=0$
\[f2.assertion.1\] line [0-9]+ assertion 0: SUCCESS
\[main.assertion.1\] line [0-9]+ assertion x == 1: SUCCESS
^\*\*\*\* WARNING: no body for function main::fptr_call
^\*\*\*\* WARNING: no body for function main::fptr_call\$0
\[f2.assertion.1\] line [0-9]+ assertion 0: FAILURE
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this now a failure? If I have read the right test case, there is no way f can be set to f2, so the assert in f2 cannot be reachable?

\[main.assertion.1\] line [0-9]+ assertion x == 1: FAILURE
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also doesn't make sense, why does one of these fail but not the other?

\[main.assertion.2\] line [0-9]+ assertion x == 2: SUCCESS
^VERIFICATION SUCCESSFUL$
^VERIFICATION FAILED$
^EXIT=10$
^SIGNAL=0$
--
^warning: ignoring
26 changes: 26 additions & 0 deletions regression/cbmc/Function_Pointer20/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#include <assert.h>

struct PtrWrapper
{
char *value_c;
};

void fn(struct PtrWrapper wrapper)
{
assert(wrapper.value_c == 'B');
}

void indirect(int (*fn_ptr)(char *), char *data)
{
fn_ptr(data);
assert(0);
}

int main()
{
struct PtrWrapper wrapper;
wrapper.value_c = 'A';

int (*alias)(char *) = (int (*)(char *))fn;
indirect(alias, &wrapper.value_c);
}
10 changes: 10 additions & 0 deletions regression/cbmc/Function_Pointer20/test.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
CORE
main.c

^\*\*\*\* WARNING: no body for function indirect::fptr_call
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why there is a warning in this context? There is only a single thing that fn_ptr can point to?

^\[indirect.assertion.1\] line 16 assertion 0: FAILURE$
^VERIFICATION FAILED$
^EXIT=10$
^SIGNAL=0$
--
^warning: ignoring
4 changes: 1 addition & 3 deletions regression/cbmc/Function_Pointer_Init_No_Candidate/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ typedef int (*other_function_type)(int n);

void foo(other_function_type other_function)
{
// returning from the function call is unreachable -> the following assertion
// should succeed
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there is a semantic change for cases where function pointers are invalid / NULL / otherwise not properly defined?

// requesting `pointer-check` will then catch the fact that there is no valid
// requesting `pointer-check` will catch the fact that there is no valid
// candidate function to call resulting in an invalid function pointer
// failure
assert(other_function(4) > 5);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
CORE
main.c
--function foo --pointer-check
^\[foo.assertion.\d+\] line \d+ assertion other_function\(4\) > 5: SUCCESS$
^\[foo.assertion.\d+\] line \d+ assertion other_function\(4\) > 5: FAILURE$
^\[foo.pointer_dereference.\d+\] line \d+ no candidates for dereferenced function pointer: FAILURE$
^EXIT=10$
^SIGNAL=0$
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ main.c
^main::1::fun1 \(\) -> value-set-begin: ptr ->\(f\) :value-set-end
^main::1::fun2_show \(\) -> value-set-begin: ptr ->\(f\), ptr ->\(g\) :value-set-end
^main::1::fun3_show \(\) -> value-set-begin: ptr ->\(f\), ptr ->\(g\) :value-set-end
^fun_global_show \(\) -> value-set-begin: ptr ->\(f\), ptr ->\(g\) :value-set-end
^fun_global_show \(\) -> value-set-begin: TOP :value-set-end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the test case this is slightly concerning as {f, g} is a legit and reasonable thing for it to be and it should only be set to TOP if something really wrong is occurring. This looks like we might need to be a bit careful about how this PR interacts with the abstract interpreter.

^EXIT=0$
^SIGNAL=0$
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ test.c
^SIGNAL=0$
^file test.c line 20 function main: replacing function pointer by 2 possible targets$
--
--
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

This test checks that the value-set-fi-based function pointer removal
precisely identifies the function to call for a particular function pointer
call.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ test.c
^SIGNAL=0$
^file test.c line 19 function main: replacing function pointer by 0 possible targets$
--
--
This test checks that the value-set-fi-based function pointer removal
precisely identifies the function to call for a particular function pointer
call.
15 changes: 12 additions & 3 deletions src/goto-instrument/value_set_fi_fp_removal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ void value_set_fi_fp_removal(
message.status() << "Instrumenting" << messaget::eom;

// now replace aliases by addresses
std::list<irep_idt> fall_back_fns;
for(auto &f : goto_model.goto_functions.function_map)
{
for(auto target = f.second.body.instructions.begin();
Expand Down Expand Up @@ -69,18 +70,26 @@ void value_set_fi_fp_removal(

if(functions.size() > 0)
{
remove_function_pointer(
fall_back_fns.push_back(remove_function_pointer(
message_handler,
goto_model.symbol_table,
f.second.body,
f.first,
target,
functions,
true);
true));
}
}
}
}
}
goto_model.goto_functions.update();

for(const auto &id : fall_back_fns)
{
goto_model.goto_functions.function_map[id].set_parameter_identifiers(
to_code_type(ns.lookup(id).type));
}

if(!fall_back_fns.empty())
goto_model.goto_functions.update();
}
Loading