From bde2536bc12837acc347e2f58895b41b6f9063d7 Mon Sep 17 00:00:00 2001 From: Polgreen Date: Tue, 24 Jul 2018 16:01:10 +0200 Subject: [PATCH] Remove function pointers slightly more precisely Scans the goto program for assigns to function pointers and builds a map of assigns to those function pointers. Then uses only these functions to replace the function pointer with. If a function pointer is ever written to with anything more complicated than a straight forward address_of a function, defaults to normal behaviour. Only works for: - standalone function pointers - function pointers that are 1 level deep in a struct Current unsound-ness to be fixed: - if a function pointer is part of a struct, and the struct is assigned to in total, this is not detected. --- .../remove_function_pointers.cpp | 127 ++++++++++++++++-- 1 file changed, 116 insertions(+), 11 deletions(-) diff --git a/src/goto-programs/remove_function_pointers.cpp b/src/goto-programs/remove_function_pointers.cpp index 6f23b39384a..041c189e4b2 100644 --- a/src/goto-programs/remove_function_pointers.cpp +++ b/src/goto-programs/remove_function_pointers.cpp @@ -30,6 +30,8 @@ Author: Daniel Kroening, kroening@kroening.com #include "compute_called_functions.h" #include "remove_const_function_pointers.h" +#define UNKNOWN_ASSIGN "##unknown##" + class remove_function_pointerst { public: @@ -67,6 +69,11 @@ class remove_function_pointerst const namespacet ns; symbol_tablet &symbol_table; bool add_safety_assertion; + std::map> function_pointer_map; + // counters to provide detailed statistics + std::size_t number_of_function_pointers_replaced = 0; + std::size_t max_size_of_case_split = 0; + std::size_t total_number_of_replacement_candidates = 0; // We can optionally halt the FP removal if we aren't able to use // remove_const_function_pointerst to successfully narrow to a small @@ -106,8 +113,22 @@ class remove_function_pointerst const irep_idt &in_function_id, code_function_callt &function_call, goto_programt &dest); + + /// Scans through goto functions and builds a map of function + /// pointers to a set of functions that have been written to each pointer, + /// stored as irep_idts. + /// The map is stored as variable in remove_function_pointerst + /// \param goto_functions: goto functions to scan through + void gather_all_writes_to_function_pointers(goto_functionst &goto_functions); }; +static irep_idt get_member_identifier(const member_exprt &member) +{ + // we only expect member expressions that have a symbol as compound operand + return id2string(to_symbol_expr(member.compound()).get_identifier()) + "." + + id2string(member.get_component_name()); +} + remove_function_pointerst::remove_function_pointerst( message_handlert &_message_handler, symbol_tablet &_symbol_table, @@ -361,19 +382,43 @@ void remove_function_pointerst::remove_function_pointer( } } - remove_function_pointer(goto_program, function_id, target, functions); -} + // we only handle cases where the LHS is a symbol or a member of a struct + if(pointer.id() == ID_symbol || pointer.id() == ID_member) + { + irep_idt identifier; + if(pointer.id() == ID_symbol) + { + identifier = + to_symbol_expr(to_dereference_expr(code.function()).pointer()) + .get_identifier(); + } + else + { + identifier = get_member_identifier(to_member_expr(pointer)); + } -void remove_function_pointerst::remove_function_pointer( - goto_programt &goto_program, - const irep_idt &function_id, - goto_programt::targett target, - const functionst &functions) -{ - const code_function_callt &code = target->get_function_call(); + auto entry = function_pointer_map.find(identifier); + if(entry != function_pointer_map.end()) + { + log.status() << "Replacing function pointer from map of possible assigns" + << messaget::eom; - const exprt &function = code.function(); - const exprt &pointer = to_dereference_expr(function).pointer(); + if(entry->second.find(UNKNOWN_ASSIGN) == entry->second.end()) + { + for(auto func_itr = functions.begin(); func_itr != functions.end();) + { + if( + entry->second.find(func_itr->get_identifier()) == + entry->second.end()) + { + func_itr = functions.erase(func_itr); + } + else + func_itr++; + } + } + } + } // the final target is a skip goto_programt final_skip; @@ -457,6 +502,10 @@ void remove_function_pointerst::remove_function_pointer( log.statistics().source_location = target->source_location; log.statistics() << "replacing function pointer by " << functions.size() << " possible targets" << messaget::eom; + number_of_function_pointers_replaced++; + total_number_of_replacement_candidates += functions.size(); + if(functions.size() > max_size_of_case_split) + max_size_of_case_split = functions.size(); // list the names of functions when verbosity is at debug level log.conditional_output( @@ -503,6 +552,7 @@ bool remove_function_pointerst::remove_function_pointers( void remove_function_pointerst::operator()(goto_functionst &functions) { + gather_all_writes_to_function_pointers(functions); bool did_something=false; for(goto_functionst::function_mapt::iterator f_it= @@ -517,7 +567,21 @@ void remove_function_pointerst::operator()(goto_functionst &functions) } if(did_something) + { functions.compute_location_numbers(); + + log.statistics() << "Statistics:\n" + << "Replaced " << number_of_function_pointers_replaced + << " function pointers " + << " with " << total_number_of_replacement_candidates + << " possible candidate functions. \n" + << "Max number of candidates in a single replacement: " + << max_size_of_case_split << "\n" + << "Mean number of candidates in a single replacement: " + << total_number_of_replacement_candidates / + number_of_function_pointers_replaced + << messaget::eom; + } } bool remove_function_pointers( @@ -570,3 +634,44 @@ void remove_function_pointers(message_handlert &_message_handler, add_safety_assertion, only_remove_const_fps); } + +void remove_function_pointerst::gather_all_writes_to_function_pointers( + goto_functionst &goto_functions) +{ + for(auto &gf_entry : goto_functions.function_map) + { + goto_programt &goto_program = gf_entry.second.body; + Forall_goto_program_instructions(target, goto_program) + { + if(!target->is_assign()) + continue; + + const code_assignt &code_assign = to_code_assign(target->code); + const auto &lhs = code_assign.lhs(); + const auto &rhs = code_assign.rhs(); + + if(lhs.type().id() == ID_pointer && lhs.type().subtype().id() == ID_code) + { + irep_idt lhs_identifier; + if(lhs.id() == ID_symbol) + lhs_identifier = to_symbol_expr(lhs).get_identifier(); + else if(lhs.id() == ID_member) + { + lhs_identifier = get_member_identifier(to_member_expr(lhs)); + } + else + continue; + + if(rhs.id() == ID_address_of && rhs.type().subtype().id() == ID_code) + { + irep_idt rhs_identifier = + to_symbol_expr(to_address_of_expr(rhs).object()).get_identifier(); + + function_pointer_map[lhs_identifier].insert(rhs_identifier); + } + else + function_pointer_map[lhs_identifier] = {{UNKNOWN_ASSIGN}}; + } + } + } +}