public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: Dodji Seketeli <dodji@redhat.com>
To: libabigail@sourceware.org
Subject: [PATCH, applied] ir: Bug 29934 - Fix propagated canonical type confirmation
Date: Thu, 29 Dec 2022 12:21:01 +0100	[thread overview]
Message-ID: <878riqbgma.fsf@redhat.com> (raw)

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


                 reply	other threads:[~2022-12-29 11:21 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878riqbgma.fsf@redhat.com \
    --to=dodji@redhat.com \
    --cc=libabigail@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).