]> sourceware.org Git - libabigail.git/commitdiff
Don't walk diff trees indefinitely when applying suppressions
authorDodji Seketeli <dodji@redhat.com>
Tue, 23 Aug 2016 10:38:36 +0000 (12:38 +0200)
committerDodji Seketeli <dodji@redhat.com>
Tue, 23 Aug 2016 10:38:36 +0000 (12:38 +0200)
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>
src/abg-comparison.cc

index 9f1dec72d88801171fdb189a0d031a7cc616ef81..94d830c1ddc6a6ae7b225aa260cda899f88aa154 100644 (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);
 
This page took 0.044261 seconds and 5 git commands to generate.