[PATCH, applied] ir: Bug 29934 - Fix propagated canonical type confirmation

[PATCH, applied] ir: Bug 29934 - Fix propagated canonical type confirmation

[Dodji Seketeli]

Hello,

When canonicalization a type T, it can happen that one subtype S of T
compares equal to a type S' where S' is already canonicalized.  In
that case, we can deduce that the canonical type of S equals the
canonical type of S', even if we are currently in the process of
canonicalizing T.

In other words, the canonical type of S' is "propagated to S", in the
process of canonicalizing T.

This optimization is called "canonical type propagation" and is meant
to spead up the overall canonicalization process.

However, in some cases, the propagated canonical type can be
"cancelled" for the optimization to be correct.  In those cases, the
propagated canonical type is set to nil.

When analysing the binary libdovecot-sieve.so from the problem
reported at https://sourceware.org/bugzilla/show_bug.cgi?id=29934, we
encounter a case where a function type's propagated type is
erroneously cancelled.  That leaves the canonical type of that
function type not set and that later violates the assert

    ABG_ASSERT(is_non_canonicalized_type(t)) in
    abigail::ir::hash_as_canonical_type_or_constant.

I tracked this down to return_comparison_result which fails to confirm
a case of propagated canonical type and thus, some of them can end up
being erroneously cancelled.

Fixed thus.

	* src/abg-ir.cc (return_comparison_result): A type whose canonical
	type has been propagated must have its canonical type confirmed if
	that type is not recursive and is not dependant on any recursive
	type.  In that case, the canonical type will never be cancelled.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 src/abg-ir.cc | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 08fc3ad0..f32e8d1f 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -1040,15 +1040,30 @@ return_comparison_result(T& l, T& r, bool value,
 	  // eventually fails.
 	  env.priv_->add_to_types_with_non_confirmed_propagated_ct(is_type(&r));
 	}
-      else if (value == true && env.priv_->right_type_comp_operands_.empty())
+      else if (value == true
+	       && (// The type is neither recursive nor dependant on a
+		   // recursive type ...
+		   (!env.priv_->is_recursive_type(&r)
+		    && !is_type(&r)->priv_->depends_on_recursive_type()
+		    && is_type(&r)->priv_->canonical_type_propagated()
+		    && !is_type(&r)->priv_->propagated_canonical_type_confirmed())
+		   ||
+		   // ... or the comparison stack is empty, meaning,
+		   // comparing r & l is completely done.
+		   env.priv_->right_type_comp_operands_.empty()))
 	{
-	  // The type provided in the 'r' argument is the type that is
-	  // being canonicalized; 'r' is not a mere subtype being
-	  // compared, it's the whole type being canonicalized.  And
-	  // its canonicalization has just succeeded.  So let's
-	  // confirm the "canonical type propagation" of all the
-	  // sub-types that were compared during the comparison of
-	  // 'r'.
+	  // Either:
+	  // 
+	  // A/ 'r' is neither recursive nor dependant on a
+	  // recursive type
+	  //
+	  // B/ Or the type provided in the 'r' argument is the type
+	  // that is being canonicalized; 'r' is not a mere subtype
+	  // being compared, it's the whole type being canonicalized.
+	  // And its canonicalization has just succeeded.
+	  //
+	  // In both cases, let's confirm the canonical type resulting
+	  // from the "canonical type propagation" optimization.
 	  env.priv_->confirm_ct_propagation(&r);
 	}
       else if (value == false)
-- 
2.31.1


-- 
		Dodji