public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: Dodji Seketeli <dodji@seketeli.org>
To: Dodji Seketeli via Libabigail <libabigail@sourceware.org>
Cc: Dodji Seketeli <dodji@redhat.com>
Subject: [PATCH 2/3] dwarf-reader: Remove redundant qualifiers from qualified types
Date: Sat, 23 Jul 2022 01:31:00 +0200	[thread overview]
Message-ID: <87h738913f.fsf@seketeli.org> (raw)
In-Reply-To: <87sfms91n0.fsf@redhat.com> (Dodji Seketeli via Libabigail's message of "Sat, 23 Jul 2022 01:19:15 +0200")

Hello,

While looking at something else, I noticed that there are some
qualified types built from the DWARF debug info that read:

        const volatile const int m[5]

That's a tree of (chained) qualified types that end up having some
redundant qualifiers.  That IR tree might look like:

        [C] --> [C|V] --> [int]

We want to edit that IR tree to make it look like:

        [C] --> [V]  --> [int]

And that would serialize as

        const volatile int m[5]

This patch introduces the editing of the qualified type IR tree to
remove the redundant qualifiers, right after the IR is built from
DWARF.

	* include/abg-fwd.h (strip_redundant_quals_from_underyling_types):
	Declare new function.
	* include/abg-ir.h (operator&=): Declare new binary operator for
	qualified_type_def::CV and ...
	* src/abg-ir.cc (+operator&=): ... define it here.
	(strip_redundant_quals_from_underyling_types): Define new function
	and ...
	* src/abg-dwarf-reader.cc (maybe_strip_qualification): ... Use it
	here.
	* tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt:
	Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to master.
---
 include/abg-fwd.h                             |   3 +
 include/abg-ir.h                              |   3 +
 src/abg-dwarf-reader.cc                       |   5 +-
 src/abg-ir.cc                                 | 115 ++++++++++++++++++
 .../qualifier-typedef-array-report-1.txt      |  40 +++---
 5 files changed, 145 insertions(+), 21 deletions(-)

diff --git a/include/abg-fwd.h b/include/abg-fwd.h
index d32e226a..e84e8a85 100644
--- a/include/abg-fwd.h
+++ b/include/abg-fwd.h
@@ -827,6 +827,9 @@ strip_typedef(const type_base_sptr);
 decl_base_sptr
 strip_useless_const_qualification(const qualified_type_def_sptr t);
 
+void
+strip_redundant_quals_from_underyling_types(const qualified_type_def_sptr&);
+
 type_base_sptr
 peel_typedef_type(const type_base_sptr&);
 
diff --git a/include/abg-ir.h b/include/abg-ir.h
index 86a6ce27..2d4b1006 100644
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -2243,6 +2243,9 @@ operator|(qualified_type_def::CV, qualified_type_def::CV);
 qualified_type_def::CV&
 operator|=(qualified_type_def::CV&, qualified_type_def::CV);
 
+qualified_type_def::CV&
+operator&=(qualified_type_def::CV&, qualified_type_def::CV);
+
 qualified_type_def::CV
 operator&(qualified_type_def::CV, qualified_type_def::CV);
 
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 0c34f617..e5159c89 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -13544,6 +13544,7 @@ maybe_strip_qualification(const qualified_type_def_sptr t,
   decl_base_sptr result = t;
   type_base_sptr u = t->get_underlying_type();
 
+  strip_redundant_quals_from_underyling_types(t);
   result = strip_useless_const_qualification(t);
   if (result.get() != t.get())
     return result;
@@ -13590,6 +13591,7 @@ maybe_strip_qualification(const qualified_type_def_sptr t,
 	  qualified_type_def::CV quals = qualified->get_cv_quals();
 	  quals |= t->get_cv_quals();
 	  qualified->set_cv_quals(quals);
+	  strip_redundant_quals_from_underyling_types(qualified);
 	  result = is_decl(u);
 	}
       else
@@ -13598,6 +13600,7 @@ maybe_strip_qualification(const qualified_type_def_sptr t,
 	    (new qualified_type_def(element_type,
 				    t->get_cv_quals(),
 				    t->get_location()));
+	  strip_redundant_quals_from_underyling_types(qual_type);
 	  add_decl_to_scope(qual_type, is_decl(element_type)->get_scope());
 	  array->set_element_type(qual_type);
 	  ctxt.schedule_type_for_late_canonicalization(is_type(qual_type));
@@ -15584,7 +15587,7 @@ build_ir_node_from_die(read_context&	ctxt,
 	    ctxt.associate_die_to_type(die, ty, where_offset);
 	    result =
 	      add_decl_to_scope(d, ctxt.cur_transl_unit()->get_global_scope());
-	    maybe_canonicalize_type(die, ctxt);
+	    maybe_canonicalize_type(is_type(result), ctxt);
 	  }
       }
       break;
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 5c1ac35b..988b8005 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -6715,6 +6715,113 @@ strip_useless_const_qualification(const qualified_type_def_sptr t)
   return result;
 }
 
+/// Merge redundant qualifiers from a tree of qualified types.
+///
+/// Suppose a tree of qualified types leads to:
+///
+///     const virtual const restrict const int;
+///
+/// Suppose the IR tree of qualified types ressembles (with C meaning
+/// const, V meaning virtual and R meaning restrict):
+///
+///     [C|V]-->[C|R] -->[C] --> [int].
+///
+/// This function walks the IR and remove the redundant CV qualifiers
+/// so the IR becomes:
+///
+///     [C|V] --> [R] --> []  -->[int].
+///
+/// Note that the empty qualified type (noted []) represents a
+/// qualified type with no qualifier.  It's rare, but it can exist.
+/// I've put it here just for the sake of example.
+///
+/// The resulting IR thus represents the (merged) type:
+///
+///    const virtual restrict int.
+///
+/// This function is a sub-routine of the overload @ref
+/// strip_useless_const_qualification which doesn't return any value.
+///
+/// @param t the qualified type to consider.
+///
+/// @param redundant_quals the (redundant) qualifiers to be removed
+/// from the qualifiers of the underlying types of @p t.
+///
+/// @return the underlying type of @p t which might have had its
+/// redundant qualifiers removed.
+static qualified_type_def_sptr
+strip_redundant_quals_from_underyling_types(const qualified_type_def_sptr& t,
+					    qualified_type_def::CV redundant_quals)
+{
+  if (!t)
+    return t;
+
+  // We must NOT edit canonicalized types.
+  ABG_ASSERT(!t->get_canonical_type());
+
+  qualified_type_def_sptr underlying_qualified_type =
+    is_qualified_type(t->get_underlying_type());
+
+  // Let's build 'currated qualifiers' that are the qualifiers of the
+  // current type from which redundant qualifiers are removed.
+  qualified_type_def::CV currated_quals = t->get_cv_quals();
+
+  // Remove the redundant qualifiers from these currated qualifiers
+  currated_quals &= ~redundant_quals;
+  t->set_cv_quals(currated_quals);
+
+  // The redundant qualifiers, moving forward, is now the union of the
+  // previous set of redundant qualifiers and the currated qualifiers.
+  redundant_quals |= currated_quals;
+
+  qualified_type_def_sptr result = t;
+  if (underlying_qualified_type)
+    // Now remove the redundant qualifiers from the qualified types
+    // potentially carried by the underlying type.
+    result =
+      strip_redundant_quals_from_underyling_types(underlying_qualified_type,
+						  redundant_quals);
+
+  return result;
+}
+
+/// Merge redundant qualifiers from a tree of qualified types.
+///
+/// Suppose a tree of qualified types leads to:
+///
+///     const virtual const restrict const int;
+///
+/// Suppose the IR tree of qualified types ressembles (with C meaning
+/// const, V meaning virtual and R meaning restrict):
+///
+///     [C|V]-->[C|R] -->[C] --> [int].
+///
+/// This function walks the IR and remove the redundant CV qualifiers
+/// so the IR becomes:
+///
+///     [C|V] --> [R] --> []  -->[int].
+///
+/// Note that the empty qualified type (noted []) represents a
+/// qualified type with no qualifier.  It's rare, but it can exist.
+/// I've put it here just for the sake of example.
+///
+/// The resulting IR thus represents the (merged) type:
+///
+///    const virtual restrict int.
+///
+/// @param t the qualified type to consider.  The IR below the
+/// argument to this parameter will be edited to remove redundant
+/// qualifiers where applicable.
+void
+strip_redundant_quals_from_underyling_types(const qualified_type_def_sptr& t)
+{
+  if (!t)
+    return;
+
+  qualified_type_def::CV redundant_quals = qualified_type_def::CV_NONE;
+  strip_redundant_quals_from_underyling_types(t, redundant_quals);
+}
+
 /// Return the leaf underlying type node of a @ref typedef_decl node.
 ///
 /// If the underlying type of a @ref typedef_decl node is itself a
@@ -15653,6 +15760,14 @@ operator|=(qualified_type_def::CV& l, qualified_type_def::CV r)
   return l;
 }
 
+/// Overloaded bitwise &= operator for cv qualifiers.
+qualified_type_def::CV&
+operator&=(qualified_type_def::CV& l, qualified_type_def::CV r)
+{
+  l = l & r;
+  return l;
+}
+
 /// Overloaded bitwise AND operator for CV qualifiers.
 qualified_type_def::CV
 operator&(qualified_type_def::CV lhs, qualified_type_def::CV rhs)
diff --git a/tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt b/tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt
index 13949efc..d45c9c6c 100644
--- a/tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt
+++ b/tests/data/test-abidiff-exit/qualifier-typedef-array-report-1.txt
@@ -47,39 +47,39 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
             underlying type 'typedef A' at qualifier-typedef-array-v0.c:1:1 changed:
               entity changed from 'typedef A' to compatible type 'void*[7]'
           type of 'C v_c' changed:
-            entity changed from 'typedef C' to compatible type 'const volatile void* const[7]'
+            entity changed from 'typedef C' to compatible type 'const volatile void*[7]'
               array element type 'void* const' changed:
-                'void* const' changed to 'const volatile void* const'
-              type name changed from 'void* const[7]' to 'const volatile void* const[7]'
+                'void* const' changed to 'const volatile void*'
+              type name changed from 'void* const[7]' to 'const volatile void*[7]'
               type size hasn't changed
           type of 'C r_c' changed:
-            entity changed from 'typedef C' to compatible type 'restrict const void* const[7]'
+            entity changed from 'typedef C' to compatible type 'restrict const void*[7]'
               array element type 'void* const' changed:
-                'void* const' changed to 'restrict const void* const'
-              type name changed from 'void* const[7]' to 'restrict const void* const[7]'
+                'void* const' changed to 'restrict const void*'
+              type name changed from 'void* const[7]' to 'restrict const void*[7]'
               type size hasn't changed
           type of 'D v_d' changed:
-            entity changed from 'typedef D' to compatible type 'const volatile void* const[7]'
+            entity changed from 'typedef D' to compatible type 'const volatile void*[7]'
               array element type 'void* const' changed:
-                'void* const' changed to 'const volatile void* const'
-              type name changed from 'void* const[7]' to 'const volatile void* const[7]'
+                'void* const' changed to 'const volatile void*'
+              type name changed from 'void* const[7]' to 'const volatile void*[7]'
               type size hasn't changed
           type of 'D r_d' changed:
-            entity changed from 'typedef D' to compatible type 'restrict const void* const[7]'
+            entity changed from 'typedef D' to compatible type 'restrict const void*[7]'
               array element type 'void* const' changed:
-                'void* const' changed to 'restrict const void* const'
-              type name changed from 'void* const[7]' to 'restrict const void* const[7]'
+                'void* const' changed to 'restrict const void*'
+              type name changed from 'void* const[7]' to 'restrict const void*[7]'
               type size hasn't changed
           type of 'E r_e' changed:
-            entity changed from 'typedef E' to compatible type 'restrict const volatile volatile void* const[7]'
-              array element type 'const volatile void* const' changed:
-                'const volatile void* const' changed to 'restrict const volatile volatile void* const'
-              type name changed from 'const volatile void* const[7]' to 'restrict const volatile volatile void* const[7]'
+            entity changed from 'typedef E' to compatible type 'restrict const volatile void*[7]'
+              array element type 'const volatile void*' changed:
+                'const volatile void*' changed to 'restrict const volatile void*'
+              type name changed from 'const volatile void*[7]' to 'restrict const volatile void*[7]'
               type size hasn't changed
           type of 'F r_f' changed:
-            entity changed from 'typedef F' to compatible type 'restrict const volatile volatile void* const[7]'
-              array element type 'const volatile void* const' changed:
-                'const volatile void* const' changed to 'restrict const volatile volatile void* const'
-              type name changed from 'const volatile void* const[7]' to 'restrict const volatile volatile void* const[7]'
+            entity changed from 'typedef F' to compatible type 'restrict const volatile void*[7]'
+              array element type 'const volatile void*' changed:
+                'const volatile void*' changed to 'restrict const volatile void*'
+              type name changed from 'const volatile void*[7]' to 'restrict const volatile void*[7]'
               type size hasn't changed
 
-- 
2.36.1


  parent reply	other threads:[~2022-07-22 23:31 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-22 23:19 [PATCH 0/3] Make integral types of same base and size compatible Dodji Seketeli
2022-07-22 23:28 ` [PATCH 1/3] ir: Disambiguate sorting of array element types Dodji Seketeli
2022-07-22 23:31 ` Dodji Seketeli [this message]
2022-07-22 23:32 ` [PATCH 3/3] ir: Consider integral types of same kind and size as equivalent Dodji Seketeli
2022-08-10 15:23   ` Giuliano Procida
2022-08-11  2:22     ` Ben Woodard
2022-08-12 15:26       ` Giuliano Procida
2022-08-16 19:56         ` Ben Woodard
2022-08-16 16:54     ` Dodji Seketeli
2022-08-16 17:06       ` Ben Woodard
2022-08-16 18:10       ` Giuliano Procida
2022-08-18 16:29         ` Dodji Seketeli
2022-08-18 17:52           ` Ben Woodard
2022-08-19 15:30             ` Dodji Seketeli

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=87h738913f.fsf@seketeli.org \
    --to=dodji@seketeli.org \
    --cc=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).