[binutils-gdb] libctf: dedup: enums with overlapping enumerators are conflicting

[binutils-gdb] libctf: dedup: enums with overlapping enumerators are conflicting

[Nick Alcock]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f8da1a05db64d8c5c700e07a008a1938858a7adf

commit f8da1a05db64d8c5c700e07a008a1938858a7adf
Author: Nick Alcock <nick.alcock@oracle.com>
Date:   Tue Jun 11 19:51:33 2024 +0100

    libctf: dedup: enums with overlapping enumerators are conflicting
    
    The CTF deduplicator was not considering enumerators inside enum types to be
    things that caused type conflicts, so if the following two TUs were linked
    together, you would end up with the following in the resulting dict:
    
    1.c:
    enum foo { A, B };
    
    2.c:
    enum bar { A, B };
    
    linked:
    
    enum foo { A, B };
    enum bar { A, B };
    
    This does work -- but it's not something that's valid C, and the general
    point of the shared dict is that it is something that you could potentially
    get from any valid C TU.
    
    So consider such types to be conflicting, but obviously don't consider
    actually identical enums to be conflicting, even though they too have (all)
    their identifiers in common.  This involves surprisingly little code. The
    deduplicator detects conflicting types by counting types in a hash table of
    hash tables:
    
    decorated identifier -> (type hash -> count)
    
    where the COUNT is the number of times a given hash has been observed: any
    name with more than one hash associated with it is considered conflicting
    (the count is used to identify the most common such name for promotion to
    the shared dict).
    
    Before now, those identifiers were all the identifiers of types (possibly
    decorated with their namespace on the front for enumerator identifiers), but
    we can equally well put *enumeration constant names* in there, undecorated
    like the identifiers of types in the global namespace, with the type hash
    being the hash of each enum containing that enumerator.  The existing
    conflicting-type-detection code will then accurately identify distinct enums
    with enumeration constants in common.  The enum that contains the most
    commonly-appearing enumerators will be promoted to the shared dict.
    
    libctf/
            * ctf-impl.h (ctf_dedup_t) <cd_name_counts>: Extend comment.
            * ctf-dedup.c (ctf_dedup_count_name): New, split out of...
            (ctf_dedup_populate_mappings): ... here.  Call it for all
            * enumeration constants in an enum as well as types.
    
    ld/
            * testsuite/ld-ctf/enum-3.c: New test CTF.
            * testsuite/ld-ctf/enum-4.c: Likewise.
            * testsuite/ld-ctf/overlapping-enums.d: New test.
            * testsuite/ld-ctf/overlapping-enums-2.d: Likewise.

Diff:
---
 ld/testsuite/ld-ctf/enum-3.c              |  3 +++
 ld/testsuite/ld-ctf/enum-4.c              |  3 +++
 ld/testsuite/ld-ctf/overlapping-enums-2.d | 36 ++++++++++++++++++++++++++
 ld/testsuite/ld-ctf/overlapping-enums.d   | 35 ++++++++++++++++++++++++++
 libctf/ctf-dedup.c                        | 42 ++++++++++++++++++++++++++-----
 libctf/ctf-impl.h                         |  3 ++-
 6 files changed, 115 insertions(+), 7 deletions(-)

diff --git a/ld/testsuite/ld-ctf/enum-3.c b/ld/testsuite/ld-ctf/enum-3.c
new file mode 100644
index 00000000000..c365aaff564
--- /dev/null
+++ b/ld/testsuite/ld-ctf/enum-3.c
@@ -0,0 +1,3 @@
+enum first_day_of_the_week {Sunday = 0};
+
+static enum first_day_of_the_week day __attribute__((used));
diff --git a/ld/testsuite/ld-ctf/enum-4.c b/ld/testsuite/ld-ctf/enum-4.c
new file mode 100644
index 00000000000..00634244107
--- /dev/null
+++ b/ld/testsuite/ld-ctf/enum-4.c
@@ -0,0 +1,3 @@
+enum intersecting_days_of_the_week {Montag = 1, Tuesday = 2};
+
+static enum intersecting_days_of_the_week day __attribute__((used));
diff --git a/ld/testsuite/ld-ctf/overlapping-enums-2.d b/ld/testsuite/ld-ctf/overlapping-enums-2.d
new file mode 100644
index 00000000000..1adfd86b89b
--- /dev/null
+++ b/ld/testsuite/ld-ctf/overlapping-enums-2.d
@@ -0,0 +1,36 @@
+#as:
+#source: enum.c
+#source: enum-4.c
+#objdump: --ctf
+#ld: -shared
+#name: Semioverlapping enumerators
+
+.*: +file format .*
+
+Contents of CTF section .ctf:
+
+  Header:
+    Magic number: 0xdff2
+    Version: 4 \(CTF_VERSION_3\)
+#...
+  Types:
+    0x1: \(kind 8\) enum day_of_the_week \(size 0x[0-9a-f]*\) \(aligned at 0x[0-9a-f]*\)
+         Monday: 0
+         Tuesday: 1
+         Wednesday: 2
+         Thursday: 3
+         Friday: 4
+         Saturday: 5
+         Sunday: 6
+#...
+  Strings:
+#...
+CTF archive member: .*enum.*\.c:
+#...
+  Types:
+    0x80000001: \(kind 8\) enum intersecting_days_of_the_week \(size 0x[0-9a-f]*\) \(aligned at 0x[0-9a-f]*\)
+                Montag: 1
+                Tuesday: 2
+
+  Strings:
+#...
diff --git a/ld/testsuite/ld-ctf/overlapping-enums.d b/ld/testsuite/ld-ctf/overlapping-enums.d
new file mode 100644
index 00000000000..7cf57d62452
--- /dev/null
+++ b/ld/testsuite/ld-ctf/overlapping-enums.d
@@ -0,0 +1,35 @@
+#as:
+#source: enum.c
+#source: enum-3.c
+#objdump: --ctf
+#ld: -shared
+#name: Overlapping enumerators
+
+.*: +file format .*
+
+Contents of CTF section .ctf:
+
+  Header:
+    Magic number: 0xdff2
+    Version: 4 \(CTF_VERSION_3\)
+#...
+  Types:
+    0x1: \(kind 8\) enum day_of_the_week \(size 0x[0-9a-f]*\) \(aligned at 0x[0-9a-f]*\)
+         Monday: 0
+         Tuesday: 1
+         Wednesday: 2
+         Thursday: 3
+         Friday: 4
+         Saturday: 5
+         Sunday: 6
+#...
+  Strings:
+#...
+CTF archive member: .*enum.*\.c:
+#...
+  Types:
+    0x80000001: \(kind 8\) enum first_day_of_the_week \(size 0x[0-9a-f]*\) \(aligned at 0x[0-9a-f]*\)
+                Sunday: 0
+
+  Strings:
+#...
diff --git a/libctf/ctf-dedup.c b/libctf/ctf-dedup.c
index c7db6ab4965..dd234945462 100644
--- a/libctf/ctf-dedup.c
+++ b/libctf/ctf-dedup.c
@@ -1149,6 +1149,9 @@ ctf_dedup_hash_type (ctf_dict_t *fp, ctf_dict_t *input,
   return NULL;
 }
 
+static int
+ctf_dedup_count_name (ctf_dict_t *fp, const char *name, void *id);
+
 /* Populate a number of useful mappings not directly used by the hashing
    machinery: the output mapping, the cd_name_counts mapping from name -> hash
    -> count of hashval deduplication state for a given hashed type, and the
@@ -1164,8 +1167,6 @@ ctf_dedup_populate_mappings (ctf_dict_t *fp, ctf_dict_t *input _libctf_unused_,
 {
   ctf_dedup_t *d = &fp->ctf_dedup;
   ctf_dynset_t *type_ids;
-  ctf_dynhash_t *name_counts;
-  long int count;
 
 #ifdef ENABLE_LIBCTF_HASH_DEBUGGING
   ctf_dprintf ("Hash %s, %s, into output mapping for %i/%lx @ %s\n",
@@ -1258,24 +1259,53 @@ ctf_dedup_populate_mappings (ctf_dict_t *fp, ctf_dict_t *input _libctf_unused_,
       && ctf_dynset_insert (type_ids, id) < 0)
     return ctf_set_errno (fp, errno);
 
+  if (ctf_type_kind_unsliced (input, type) == CTF_K_ENUM)
+    {
+      ctf_next_t *i = NULL;
+      const char *enumerator;
+
+      while ((enumerator = ctf_enum_next (input, type, &i, NULL)) != NULL)
+	{
+	  if (ctf_dedup_count_name (fp, enumerator, id) < 0)
+	    {
+	      ctf_next_destroy (i);
+	      return -1;
+	    }
+	}
+      if (ctf_errno (input) != ECTF_NEXT_END)
+	return ctf_set_errno (fp, ctf_errno (input));
+    }
+
   /* The rest only needs to happen for types with names.  */
   if (!decorated_name)
     return 0;
 
+  if (ctf_dedup_count_name (fp, decorated_name, id) < 0)
+    return -1;					/* errno is set for us. */
+
+  return 0;
+}
+
+static int
+ctf_dedup_count_name (ctf_dict_t *fp, const char *name, void *id)
+{
+  ctf_dedup_t *d = &fp->ctf_dedup;
+  ctf_dynhash_t *name_counts;
+  long int count;
+  const char *hval;
+
   /* Count the number of occurrences of the hash value for this GID.  */
 
   hval = ctf_dynhash_lookup (d->cd_type_hashes, id);
 
   /* Mapping from name -> hash(hashval, count) not already present?  */
-  if ((name_counts = ctf_dynhash_lookup (d->cd_name_counts,
-					 decorated_name)) == NULL)
+  if ((name_counts = ctf_dynhash_lookup (d->cd_name_counts, name)) == NULL)
     {
       if ((name_counts = ctf_dynhash_create (ctf_hash_string,
 					     ctf_hash_eq_string,
 					     NULL, NULL)) == NULL)
 	  return ctf_set_errno (fp, errno);
-      if (ctf_dynhash_cinsert (d->cd_name_counts, decorated_name,
-			       name_counts) < 0)
+      if (ctf_dynhash_cinsert (d->cd_name_counts, name, name_counts) < 0)
 	{
 	  ctf_dynhash_destroy (name_counts);
 	  return ctf_set_errno (fp, errno);
diff --git a/libctf/ctf-impl.h b/libctf/ctf-impl.h
index 03e1a66416a..eb89f8b4645 100644
--- a/libctf/ctf-impl.h
+++ b/libctf/ctf-impl.h
@@ -294,7 +294,8 @@ typedef struct ctf_dedup
   ctf_dynhash_t *cd_decorated_names[4];
 
   /* Map type names to a hash from type hash value -> number of times each value
-     has appeared.  */
+     has appeared.  Enumeration constants are tracked via the enum they appear
+     in.  */
   ctf_dynhash_t *cd_name_counts;
 
   /* Map global type IDs to type hash values.  Used to determine if types are