From 4f4ab2b1d66770f6e3cc56ec844e47a5ceec1edb Mon Sep 17 00:00:00 2001 From: Dodji Seketeli Date: Tue, 27 Dec 2022 17:14:16 +0100 Subject: [PATCH] ir: Bug 29934 - Fix propagated canonical type confirmation 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 --- 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.34.1