From: Dodji Seketeli Date: Wed, 19 Apr 2023 15:45:27 +0000 (+0200) Subject: ir: fix canonical type propagation canceling error X-Git-Tag: upstream/2.3~8 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4bbf997eb7fcf55a04302b26ac18ae5252336164;p=platform%2Fupstream%2Flibabigail.git 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 --- diff --git a/src/abg-ir-priv.h b/src/abg-ir-priv.h index bea7e2ec..67124805 100644 --- 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(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(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 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(i); + cancel_ct_propagation(t); + } + } + #ifdef WITH_DEBUG_SELF_COMPARISON const unordered_map& diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 67f1a6bc..c899f92e 100644 --- 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(); } }