Don't walk diff trees indefinitely when applying suppressions

When applying suppressions to diff graphs, if the same diff node
appears twice in the graph in a way that creates a cycle, we can get
trapped in the cycle when walking the graph.  That results in a an
infinite loop.  Note that the infinite loop appears in the
diff::traverse member function, in the for-loop that walks the
children nodes of a given node.

This patch avoids walking the same node twice when applying
suppressions.

The test binaries that exhibit the issue come from the two following
packages:

rh-mariadb101-mariadb-10.1.14-2.el6 and	rh-mariadb101-mariadb-10.1.16-1.el6.

The sub-packages compared are
rh-mariadb101-mariadb-server-10.1.14-2.el6.x86_64.rpm and
rh-mariadb101-mariadb-server-10.1.16-1.el6.x86_64.rpm.

The debug info packages are
rh-mariadb101-mariadb-debuginfo-10.1.16-1.el6.x86_64.rpm and
rh-mariadb101-mariadb-debuginfo-10.1.14-2.el6.x86_64.rpm.

Once the packages are properly extracted the command line that
exhibits the infinite loop is:

    abidiff --d1 rh-mariadb101-mariadb-10.1.14-2.el6.x86_64/usr/lib/debug --d2 rh-mariadb101-mariadb-10.1.16-1.el6.x86_64/usr/lib/debug rh-mariadb101-mariadb-10.1.14-2.el6.x86_64/opt/rh/rh-mariadb101/root/usr/lib64/mysql/plugin/ha_connect.so rh-mariadb101-mariadb-10.1.16-1.el6.x86_64/opt/rh/rh-mariadb101/root/usr/lib64/mysql/plugin/ha_connect.so

This patch has no test because of the cheer size of these packages;
the debug info packages alone take more than 100MB :-( I really hope
we set up a separate repository with big test packages to overcome
this.  Maybe something using fedabipkgdiff ...

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
This commit is contained in:
Dodji Seketeli 2016-08-23 12:38:36 +02:00
parent 103a6eb94f
commit 09154361a4

View File

@ -12513,8 +12513,9 @@ apply_suppressions(diff* diff_tree)
// Apply suppressions to functions and variables that have
// changed sub-types.
suppression_categorization_visitor v;
diff_tree->context()->forget_visited_diffs();
bool s = diff_tree->context()->visiting_a_node_twice_is_forbidden();
diff_tree->context()->forbid_visiting_a_node_twice(false);
diff_tree->context()->forbid_visiting_a_node_twice(true);
diff_tree->traverse(v);
diff_tree->context()->forbid_visiting_a_node_twice(s);
}
@ -12545,8 +12546,9 @@ apply_suppressions(const corpus_diff* diff_tree)
// changed functions, variables, as well as sub-types of these,
// and apply suppression specifications to these ...
suppression_categorization_visitor v;
diff_tree->context()->forget_visited_diffs();
bool s = diff_tree->context()->visiting_a_node_twice_is_forbidden();
diff_tree->context()->forbid_visiting_a_node_twice(false);
diff_tree->context()->forbid_visiting_a_node_twice(true);
const_cast<corpus_diff*>(diff_tree)->traverse(v);
diff_tree->context()->forbid_visiting_a_node_twice(s);