Skip to content

Rewriting call_grapht to inherit from grapht<T>, and introduce reachable_call_graph class #1509

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
wants to merge 7 commits into from
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
Binary file modified regression/ansi-c/arch_flags_mcpu_bad/object.intel
Binary file not shown.
Binary file modified regression/ansi-c/arch_flags_mcpu_good/object.arm
Binary file not shown.
9 changes: 7 additions & 2 deletions regression/ansi-c/arch_flags_mcpu_good/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,17 @@ The object file 'object.arm' was compiled from 'source.c' with goto-cc
along with an ARM cross-compiler on a 64-bit platform with the following
command line:

goto-cc --native-compiler=arm-none-eabi-gcc -mcpu=cortex-a15 -c source.c
goto-cc -o object.arm --native-compiler=arm-none-eabi-gcc -mcpu=cortex-a15 -c source.c

On Ubuntu, you can get a suitable compiler using:
To regenerate object.arm on an x86_64 machine, you will need to install
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think these changes belong in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely sure why this commit has appeared in the PR, since it is the head commit of the develop branch (or was when I created this branch). I created the branch by checking out develop and then cherry-picking my commits on top of it.

Nevertheless, I need to rebase against develop again anyway so I will make it disappear then

an ARM-32 cross compiler and pass the name of that compiler to the
--native-compiler flag. On Ubuntu, you can get a suitable compiler
using:

sudo apt install gcc-arm-none-eabi

which will install arm-none-eabi-gcc (amongst other things).

preproc.i is already pre-processed so that it can be linked in without
needing to invoke a pre-processor from a cross-compile toolchain on your
local machine. Linking it together with the ARM object file, while
Expand Down
Binary file modified regression/ansi-c/arch_flags_mthumb_bad/object.intel
Binary file not shown.
Binary file modified regression/ansi-c/arch_flags_mthumb_good/object.arm
Binary file not shown.
9 changes: 7 additions & 2 deletions regression/ansi-c/arch_flags_mthumb_good/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,17 @@ file 'object.arm' was compiled from 'source.c' with goto-cc along with
an ARM cross-compiler on a 64-bit platform with the following command
line:

goto-cc --native-compiler=arm-none-eabi-gcc -mthumb -c source.c
goto-cc -o object.arm --native-compiler=arm-none-eabi-gcc -mthumb -c source.c

On Ubuntu, you can get a suitable compiler using:
To regenerate object.arm on an x86_64 machine, you will need to install
an ARM-32 cross compiler and pass the name of that compiler to the
--native-compiler flag. On Ubuntu, you can get a suitable compiler
using:

sudo apt install gcc-arm-none-eabi

which will install arm-none-eabi-gcc (amongst other things).

preproc.i is already pre-processed so that it can be linked in without
needing to invoke a pre-processor from a cross-compile toolchain on your
local machine. Linking it together with the ARM object file, while
Expand Down
1 change: 1 addition & 0 deletions src/analyses/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ SRC = ai.cpp \
locals.cpp \
natural_loops.cpp \
reaching_definitions.cpp \
reachable_call_graph.cpp \
static_analysis.cpp \
uncaught_exceptions_analysis.cpp \
uninitialized_domain.cpp \
Expand Down
124 changes: 90 additions & 34 deletions src/analyses/call_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Author: Daniel Kroening, [email protected]
\*******************************************************************/

/// \file
/// Function Call Graphs
/// Function Call Graph

#include "call_graph.h"

Expand All @@ -18,14 +18,9 @@ call_grapht::call_grapht()
{
}

call_grapht::call_grapht(const goto_modelt &goto_model):
call_grapht(goto_model.goto_functions)
call_grapht::call_grapht(const goto_modelt &goto_model)
{
}

call_grapht::call_grapht(const goto_functionst &goto_functions)
{
forall_goto_functions(f_it, goto_functions)
forall_goto_functions(f_it, goto_model.goto_functions)
{
const goto_programt &body=f_it->second.body;
add(f_it->first, body);
Expand All @@ -51,50 +46,111 @@ void call_grapht::add(
const irep_idt &caller,
const irep_idt &callee)
{
graph.insert(std::pair<irep_idt, irep_idt>(caller, callee));
std::size_t caller_idx = node_numbering.number(caller);
if(caller_idx >= nodes.size())
{
node_indext node_index = add_node();
nodes[node_index].function_name = caller;
}

std::size_t callee_idx = node_numbering.number(callee);
if(callee_idx >= nodes.size())
{
node_indext node_index = add_node();
nodes[node_index].function_name = callee;
}

add_edge(caller_idx, callee_idx);
}

/// Returns an inverted copy of this call graph
/// \return Inverted (callee -> caller) call graph
call_grapht call_grapht::get_inverted() const

void call_grapht::output_dot_node(std::ostream &out, node_indext n) const
{
call_grapht result;
for(const auto &caller_callee : graph)
result.add(caller_callee.second, caller_callee.first);
return result;
const nodet &node = nodes.at(n);

for(const auto &edge : node.out)
{
out << " \"" << node.function_name << "\" -> " << "\""
<< nodes[edge.first].function_name << "\" " << " [arrowhead=\"vee\"];"
<< "\n";
}
}

void call_grapht::output_dot(std::ostream &out) const
void call_grapht::output_xml_node(std::ostream &out, node_indext n) const
{
out << "digraph call_graph {\n";
const nodet &node = nodes.at(n);

for(const auto &edge : graph)
for(const auto &edge : node.out)
{
out << " \"" << edge.first << "\" -> "
<< "\"" << edge.second << "\" "
<< " [arrowhead=\"vee\"];"
<< "\n";
out << "<call_graph_edge caller=\"";
xmlt::escape_attribute(id2string(node.function_name), out);
out << "\" callee=\"";
xmlt::escape_attribute(id2string(nodes[edge.first].function_name), out);
out << "\">\n";
}
}

out << "}\n";
void call_grapht::output_xml(std::ostream &out) const
{
for(node_indext n = 0; n < nodes.size(); n++)
Copy link
Contributor

Choose a reason for hiding this comment

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

Use C++11 for-loop (i.e. for(const auto &node : nodes) output_xml_node(out, node);)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason for this for loop is that the output_xml_node function requires a node indext to be passed to it.

The reason the output_xml_node function requires a node indext to be passed to it, rather than a reference to the node is to preserve the compatability with the output functions of grapht (see grapht::output_dot_node).

And the reason graphtoutput_dot_node needs to have the node index passed to it is because it outputs that node index (instead of outputting data stored in the node, because the template doesn't know what might be contained in the node).

In order to use a C++11 for loop I would have to reimplement the output functions of grapht, would you still prefer that?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK never mind then

output_xml_node(out, n);
}

void call_grapht::output(std::ostream &out) const
call_grapht call_grapht::get_inverted() const
{
for(const auto &edge : graph)
call_grapht result;
for(const auto &n : nodes)
{
out << edge.first << " -> " << edge.second << "\n";
for(const auto &i : n.in)
result.add(n.function_name, nodes[i.first].function_name);
}
return result;
}

void call_grapht::output_xml(std::ostream &out) const
std::list<irep_idt> call_grapht::shortest_function_path(
irep_idt src, irep_idt dest)
{
for(const auto &edge : graph)
std::list<irep_idt> result;
node_indext src_idx, dest_idx;
if(!get_node_index(src, src_idx))
throw "unable to find src function in call graph";
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're throwing, throw a structured error (probably just call_graph_errort : public std::invalid_argument {} that a caller can selectively catch

if(!get_node_index(dest, dest_idx))
throw "unable to find destination function in call graph";

patht path;
shortest_path(src_idx, dest_idx, path);
for(const auto &n : path)
{
out << "<call_graph_edge caller=\"";
xmlt::escape_attribute(id2string(edge.first), out);
out << "\" callee=\"";
xmlt::escape_attribute(id2string(edge.second), out);
out << "\">\n";
result.push_back(nodes[n].function_name);
}
return result;
}


std::unordered_set<irep_idt, irep_id_hash>
call_grapht::reachable_functions(irep_idt start_function)
{
std::unordered_set<irep_idt, irep_id_hash> result;
std::list<node_indext> worklist;
Copy link
Contributor

Choose a reason for hiding this comment

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

Use a depth-first search like https://github.com/diffblue/cbmc/blob/develop/src/util/graph.h#L448 (probably put get_reachable into grapht alongside visit_reachable in the process) -- that way you can avoid std::list which is allocates / deallocates every time a node is inserted / removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will reimplement the search.

If I put get_reachable into grapht, it will then return a list of node indexes, and I will need to add a function in call_grapht that interfaces to that function, converting the node indexes to function names. Still ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes that sounds right

node_indext start_index;

if(get_node_index(start_function, start_index))
worklist.push_back(start_index);
else
throw "no start function found in call graph";

while(!worklist.empty())
{
const node_indext id = worklist.front();
worklist.pop_front();

result.insert(nodes[id].function_name);
for(const auto &o : nodes[id].out)
{
if(result.find(nodes[o.first].function_name) == result.end())
worklist.push_back(o.first);
}
}

return result;
}
84 changes: 75 additions & 9 deletions src/analyses/call_graph.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,100 @@ Author: Daniel Kroening, [email protected]
\*******************************************************************/

/// \file
/// Function Call Graphs
/// Function Call Graph

#ifndef CPROVER_ANALYSES_CALL_GRAPH_H
#define CPROVER_ANALYSES_CALL_GRAPH_H

#include <iosfwd>
#include <map>
#include <unordered_set>

#include <goto-programs/goto_model.h>

class call_grapht
#include <util/irep.h>
#include <util/graph.h>
#include <util/numbering.h>


/// \brief Function call graph: each node represents a function
/// in the GOTO model, a directed edge from A to B indicates
/// that function A calls function B.
/// Inherits from grapht to allow forward and
/// backward traversal of the function call graph
struct call_graph_nodet: public graph_nodet<empty_edget>
{
typedef graph_nodet<empty_edget>::edget edget;
typedef graph_nodet<empty_edget>::edgest edgest;

irep_idt function_name;
bool visited = false;
};

class call_grapht: public grapht<call_graph_nodet>
{
public:
call_grapht();
explicit call_grapht(const goto_modelt &);
explicit call_grapht(const goto_functionst &);

void output_dot(std::ostream &out) const;
void output(std::ostream &out) const;
void add(const irep_idt &caller, const irep_idt &callee);
void output_xml(std::ostream &out) const;

typedef std::multimap<irep_idt, irep_idt> grapht;
grapht graph;

void add(const irep_idt &caller, const irep_idt &callee);
/// \return the inverted call graph
call_grapht get_inverted() const;

/// \brief get the names of all functions reachable from a start function
/// \param start name of initial function
/// \return set of all names of the reachable functions
std::unordered_set<irep_idt, irep_id_hash>
reachable_functions(irep_idt start);

/// \brief Function returns the shortest path on the function call graph
/// between a source and a destination function
/// \param src name of the starting function
/// \param dest name of the destination function
/// \return list of function names on the shortest path between src and dest
std::list<irep_idt>shortest_function_path(irep_idt src, irep_idt dest);

/// get the index of the node that corresponds to a function name
/// \param[in] function_name function_name passed by reference
/// \param[out] n variable for the node index to be written to
/// \return true if a node with the given function name exists,
/// false if it does not exist
bool get_node_index(const irep_idt& function_name, node_indext &n) const
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a health warning that this is a linear search

{
for(node_indext idx = 0; idx < nodes.size(); idx++)
{
if(nodes[idx].function_name == function_name)
{
n = idx;
return true;
}
}
return false;
}

/// \brief get a list of functions called by a function
/// \param function_name the irep_idt of the function
/// \return an unordered set of all functions called by function_name
std::unordered_set<irep_idt, irep_id_hash> get_successors(
Copy link
Contributor

Choose a reason for hiding this comment

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

Members of out are guaranteed to be unique, so we could return vector here and let the caller decide whether they need the more-costly hashtable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Actually the only place this is called is the unit test, so it could just be a function within the unit test

const irep_idt& function_name) const
{
std::unordered_set<irep_idt, irep_id_hash> result;
node_indext function_idx;
if(!get_node_index(function_name, function_idx))
return result;

for(const auto &o : nodes[function_idx].out)
result.insert(nodes[o.first].function_name);
return result;
}


protected:
void output_dot_node(std::ostream &out, node_indext n) const override;
void output_xml_node(std::ostream &out, node_indext n) const;
numbering<irep_idt> node_numbering;
void add(const irep_idt &function,
const goto_programt &body);
};
Expand Down
Loading