[applied] ir: fix canonical type propagation canceling error
Commit Message
Hello,
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>
---
src/abg-ir-priv.h | 38 +++++++++++++++++++++++---------------
src/abg-ir.cc | 39 ++++++++++++++-------------------------
2 files changed, 37 insertions(+), 40 deletions(-)
@@ -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>&
@@ -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();
}
}