ir: fix canonical type propagation canceling error

This patch (with the help of the 9 patches that precede it) fixes the
self-comparison error that is seen when running the command line:

    $ fedabipkgdiff --self-compare -a --from fc37 bpftrace

It turns out the root cause of the self-comparison error seen here is
that sometimes, when the speculatively computed canonical type (that
is called propagated canonical type) is known to be wrong, the code
forgets to cancel that speculation.  And that leads to wrong
comparisons during subsequent type canonicalizations and later during
type comparisons.

This patch fixes the logic the of the cancellation of propagated
canonical type that must happen when it is known that the propagated
canonical type is invalid.

	* src/abg-ir-priv.h
	(environment::priv::{confirm_ct_propagation_for_types_dependant_on,
	confirm_ct_propagation_for_types_dependant_on}): Asserting that
	the type should be recursive is wrong because the recursive-ness
	flag is set to false upon confirmation of the canonical type
	propagation.  So there can be some types that would make this
	assertion fail before we reach the end of the set of types to
	confirm the propagation for.
	(environment::priv::cancel_ct_propagation): Cancel the canonical
	type propagation for all types we are instructed to cancel, not
	just for types that are recursive or depends on recursive types.
	This is because the the recursive-ness flag is set to false upon
	cancellation of the canonical type propagation.  So there can be
	some types that would make this condition fail before we reach the
	end of the set of types to cancel the propagation for.
	(environment::priv::cancel_all_non_confirmed_propagated_canonical_types):
	Define new member function.
	* src/abg-ir.cc (return_comparison_result): Confirm the
	speculative canonical type propagation result when we are done
	comparing the current type and the result of the comparison is
	true.  Let's not try to be smart here.  Just be safe.  This
	optimization is fast enough as is.  Otherwise, if the result of
	the comparison is false, then all the speculatively propagated
	canonical types of all the non-confirmed types should be canceled.
	All of them.  Again, let's not try to be smart.  This is smart &
	fast enough as is.  And thing are going to be correct this way.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

diff --git a/src/abg-ir-priv.h b/src/abg-ir-priv.h
index bea7e2ec98cb333ba822c1e015d890ab43b3bedc..6712480547f49c68ef784247fdf0d8114ce68629 100644 (file)
--- a/src/abg-ir-priv.h
+++ b/src/abg-ir-priv.h
@@ -827,8 +827,6 @@ struct environment::priv
     for (auto i : types_with_non_confirmed_propagated_ct_)
       {
        type_base *t = reinterpret_cast<type_base*>(i);
-       ABG_ASSERT(t->get_environment().priv_->is_recursive_type(t)
-                  || t->priv_->depends_on_recursive_type());
        t->priv_->set_does_not_depend_on_recursive_type(dependant_type);
        if (!t->priv_->depends_on_recursive_type())
          {
@@ -887,8 +885,6 @@ struct environment::priv
     for (auto i : types_with_non_confirmed_propagated_ct_)
       {
        type_base *t = reinterpret_cast<type_base*>(i);
-       ABG_ASSERT(t->get_environment().priv_->is_recursive_type(t)
-                  || t->priv_->depends_on_recursive_type());
        t->priv_->set_does_not_depend_on_recursive_type();
        t->priv_->set_propagated_canonical_type_confirmed(true);
 #ifdef WITH_DEBUG_SELF_COMPARISON
@@ -1049,17 +1045,13 @@ struct environment::priv
 
     const environment& env = t->get_environment();
     env.priv_->cancel_ct_propagation_for_types_dependant_on(t);
-    if (t->priv_->depends_on_recursive_type()
-       || env.priv_->is_recursive_type(t))
-      {
-       // This cannot carry any tentative canonical type at this
-       // point.
-       clear_propagated_canonical_type(t);
-       // Reset the marking of the type as it no longer carries a
-       // tentative canonical type that might be later cancelled.
-       t->priv_->set_does_not_depend_on_recursive_type();
-       env.priv_->remove_from_types_with_non_confirmed_propagated_ct(t);
-      }
+    // This cannot carry any tentative canonical type at this
+    // point.
+    clear_propagated_canonical_type(t);
+    // Reset the marking of the type as it no longer carries a
+    // tentative canonical type that might be later canceled.
+    t->priv_->set_does_not_depend_on_recursive_type();
+    env.priv_->remove_from_types_with_non_confirmed_propagated_ct(t);
   }
 
   /// Clear the propagated canonical type of a given type.
@@ -1110,6 +1102,22 @@ struct environment::priv
     types_with_non_confirmed_propagated_ct_.erase(i);
   }
 
+  /// Cancel the propagated canonical types of all the types which
+  /// propagated canonical type have not yet been confirmed.
+  void
+  cancel_all_non_confirmed_propagated_canonical_types()
+  {
+    vector<uintptr_t> to_erase;
+    for (auto i : types_with_non_confirmed_propagated_ct_)
+      to_erase.push_back(i);
+
+    for (auto i : to_erase)
+      {
+       type_base *t = reinterpret_cast<type_base*>(i);
+       cancel_ct_propagation(t);
+      }
+  }
+
 #ifdef WITH_DEBUG_SELF_COMPARISON
 
   const unordered_map<string, uintptr_t>&

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 67f1a6bc2172c0dd63008bd956e9208f3760a506..c899f92e2435c441301009afd557b8bb1e2ec9de 100644 (file)
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -1041,40 +1041,29 @@ 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
-              && (// 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()))
+      else if (value == true && env.priv_->right_type_comp_operands_.empty())
        {
-         // Either:
-         // 
-         // A/ 'r' is neither recursive nor dependant on a
-         // recursive type
+         // 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.
          //
-         // 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.
+         // Let's confirm the canonical type resulting from the
+         // "canonical type propagation" optimization.
          env.priv_->confirm_ct_propagation(&r);
        }
+      else if (value == true)
+       // In any other case, we are not sure if propagated types
+       // should be confirmed yet.  So let's mark them as such.
+       env.priv_->add_to_types_with_non_confirmed_propagated_ct(is_type(&r));
       else if (value == false)
        {
          // The comparison of the current sub-type failed.  So all
-         // the types in
-         // env.prix_->types_with_non_confirmed_propagated_ct_
+         // the with non-confirmed propagated types (those in
+         // env.prix_->types_with_non_confirmed_propagated_ct_)
          // should see their tentatively propagated canonical type
          // cancelled.
-         env.priv_->cancel_ct_propagation(&r);
+         env.priv_->cancel_all_non_confirmed_propagated_canonical_types();
        }
     }