public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Nick Alcock <nick.alcock@oracle.com>
To: binutils@sourceware.org
Subject: [PATCH 2/2] libctf: impose an ordering on conflicting types
Date: Wed, 27 Apr 2022 16:51:30 +0100	[thread overview]
Message-ID: <20220427155130.238504-2-nick.alcock@oracle.com> (raw)
In-Reply-To: <20220427155130.238504-1-nick.alcock@oracle.com>

When two types conflict and they are not types which can have forwards
(say, two arrays of different sizes with the same name in two different
TUs) the CTF deduplicator uses a popularity contest to decide what to
do: the type cited by the most other types ends up put into the shared
dict, while the others are relegated to per-CU child dicts.

This works well as long as one type *is* most popular -- but what if
there is a tie?  If several types have the same popularity count,
we end up picking the first we run across and promoting it, and
unfortunately since we are working over a dynhash in essentially
arbitrary order, this means we promote a random one.  So multiple
runs of ld with the same inputs can produce different outputs!
All the outputs are valid, but this is still undesirable.

Adjust things to use the same strategy used to sort types on the output:
when there is a tie, always put the type that appears in a CU that
appeared earlier on the link line (and if there is somehow still a tie,
which should be impossible, pick the type with the lowest type ID).

Add a testcase -- and since this emerged when trying out extern arrays,
check that those work as well (this requires a newer GCC, but since all
GCCs that can emit CTF at all are unreleased this is probably OK as
well).

Fix up one testcase that has slight type ordering changes as a result
of this change.

libctf/ChangeLog:

	* ctf-dedup.c (ctf_dedup_detect_name_ambiguity): Use
	cd_output_first_gid to break ties.

ld/ChangeLog:

	* testsuite/ld-ctf/array-conflicted-ordering.d: New test, using...
	* testsuite/ld-ctf/array-char-conflicting-1.c: ... this...
	* testsuite/ld-ctf/array-char-conflicting-2.c: ... and this.
	* testsuite/ld-ctf/array-extern.d: New test, using...
	* testsuite/ld-ctf/array-extern.c: ... this.
	* testsuite/ld-ctf/conflicting-typedefs.d: Adjust for ordering
	changes.
---
 .../ld-ctf/array-char-conflicting-1.c         |  9 ++++++
 .../ld-ctf/array-char-conflicting-2.c         |  9 ++++++
 .../ld-ctf/array-conflicted-ordering.d        | 26 +++++++++++++++
 ld/testsuite/ld-ctf/array-extern.c            |  1 +
 ld/testsuite/ld-ctf/array-extern.d            | 32 +++++++++++++++++++
 ld/testsuite/ld-ctf/conflicting-typedefs.d    |  2 +-
 libctf/ctf-dedup.c                            | 21 +++++++++++-
 7 files changed, 98 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-ctf/array-char-conflicting-1.c
 create mode 100644 ld/testsuite/ld-ctf/array-char-conflicting-2.c
 create mode 100644 ld/testsuite/ld-ctf/array-conflicted-ordering.d
 create mode 100644 ld/testsuite/ld-ctf/array-extern.c
 create mode 100644 ld/testsuite/ld-ctf/array-extern.d

diff --git a/ld/testsuite/ld-ctf/array-char-conflicting-1.c b/ld/testsuite/ld-ctf/array-char-conflicting-1.c
new file mode 100644
index 00000000000..a6736a8a114
--- /dev/null
+++ b/ld/testsuite/ld-ctf/array-char-conflicting-1.c
@@ -0,0 +1,9 @@
+typedef char *array[10];
+
+static array digits_names = {"zero", "one", "two", "three", "four",
+			     "five", "six", "seven", "eight", "nine"};
+
+void *foo (void)
+{
+  return digits_names;
+}
diff --git a/ld/testsuite/ld-ctf/array-char-conflicting-2.c b/ld/testsuite/ld-ctf/array-char-conflicting-2.c
new file mode 100644
index 00000000000..1cc46f0a31b
--- /dev/null
+++ b/ld/testsuite/ld-ctf/array-char-conflicting-2.c
@@ -0,0 +1,9 @@
+typedef char *array[9];
+
+static array digits_names = {"one", "two", "three", "four",
+			     "five", "six", "seven", "eight", "nine"};
+
+void *bar (void)
+{
+  return digits_names;
+}
diff --git a/ld/testsuite/ld-ctf/array-conflicted-ordering.d b/ld/testsuite/ld-ctf/array-conflicted-ordering.d
new file mode 100644
index 00000000000..a8bbc3dd65e
--- /dev/null
+++ b/ld/testsuite/ld-ctf/array-conflicted-ordering.d
@@ -0,0 +1,26 @@
+#as:
+#source: array-char-conflicting-1.c
+#source: array-char-conflicting-2.c
+#objdump: --ctf
+#cc: -fPIC
+#ld: -shared --ctf-variables --hash-style=sysv
+#name: Arrays (conflicted)
+
+.*: +file format .*
+
+Contents of CTF section .ctf:
+
+  Header:
+    Magic number: 0xdff2
+    Version: 4 \(CTF_VERSION_3\)
+#...
+  Variables:
+    digits_names -> .* \(kind 4\) char \*\[10\] .*
+#...
+  Header:
+#...
+    Parent name: .ctf
+#...
+  Variables:
+    digits_names -> .* \(kind 4\) char \*\[9\] .*
+#...
diff --git a/ld/testsuite/ld-ctf/array-extern.c b/ld/testsuite/ld-ctf/array-extern.c
new file mode 100644
index 00000000000..730ba5a7d81
--- /dev/null
+++ b/ld/testsuite/ld-ctf/array-extern.c
@@ -0,0 +1 @@
+extern char * digits_names[];
diff --git a/ld/testsuite/ld-ctf/array-extern.d b/ld/testsuite/ld-ctf/array-extern.d
new file mode 100644
index 00000000000..4c9ce784e6a
--- /dev/null
+++ b/ld/testsuite/ld-ctf/array-extern.d
@@ -0,0 +1,32 @@
+#as:
+#source: array-char.c
+#source: array-extern.c
+#objdump: --ctf
+#ld: -shared --ctf-variables --hash-style=sysv
+#name: Arrays (extern)
+
+.*: +file format .*
+
+Contents of CTF section .ctf:
+
+  Header:
+    Magic number: 0xdff2
+    Version: 4 \(CTF_VERSION_3\)
+#...
+    Data object section:	.* \(0x[1-9a-f][0-9a-f]* bytes\)
+    Type section:	.* \(0x44 bytes\)
+    String section:	.*
+
+  Labels:
+
+  Data objects:
+    digits_names -> 0x[0-9a-f]*: \(kind 4\) char \*\[10\] .*
+
+  Function objects:
+
+  Variables:
+
+  Types:
+#...
+    0x[0-9a-f]*: \(kind 4\) .*\[10\] \(size .*
+#...
diff --git a/ld/testsuite/ld-ctf/conflicting-typedefs.d b/ld/testsuite/ld-ctf/conflicting-typedefs.d
index 4ae8de41364..beb1f7776c6 100644
--- a/ld/testsuite/ld-ctf/conflicting-typedefs.d
+++ b/ld/testsuite/ld-ctf/conflicting-typedefs.d
@@ -15,8 +15,8 @@ Contents of CTF section .ctf:
 #...
   Types:
     0x1: .*int .*
-    0x[0-9]:.*int .*
     0x[0-9]: \(kind 10\) word .* -> 0x[0-9]: \(kind 1\) .*int \(format 0x1\) \(size 0x[0-9a-f]*\) \(aligned at 0x[0-9a-f]*\)
+    0x[0-9]:.*int .*
 
   Strings:
 #...
diff --git a/libctf/ctf-dedup.c b/libctf/ctf-dedup.c
index b2fb0a13441..cddf4376eae 100644
--- a/libctf/ctf-dedup.c
+++ b/libctf/ctf-dedup.c
@@ -1502,12 +1502,17 @@ ctf_dedup_detect_name_ambiguity (ctf_dict_t *fp, ctf_dict_t **inputs)
 	     the most-popular type on insertion, and we want conflicting structs
 	     et al to have all forwards left intact, so the user is notified
 	     that this type is conflicting.  TODO: improve this in future by
-	     setting such forwards non-root-visible.)  */
+	     setting such forwards non-root-visible.)
+
+	     If multiple distinct types are "most common", pick the one that
+	     appears first on the link line, and within that, the one with the
+	     lowest type ID.  (See sort_output_mapping.)  */
 
 	  const void *key;
 	  const void *count;
 	  const char *hval;
 	  long max_hcount = -1;
+	  void *max_gid = NULL;
 	  const char *max_hval = NULL;
 
 	  if (ctf_dynhash_elements (name_counts) <= 1)
@@ -1517,10 +1522,24 @@ ctf_dedup_detect_name_ambiguity (ctf_dict_t *fp, ctf_dict_t **inputs)
 	  while ((err = ctf_dynhash_cnext (name_counts, &j, &key, &count)) == 0)
 	    {
 	      hval = (const char *) key;
+
 	      if ((long int) (uintptr_t) count > max_hcount)
 		{
 		  max_hcount = (long int) (uintptr_t) count;
 		  max_hval = hval;
+		  max_gid = ctf_dynhash_lookup (d->cd_output_first_gid, hval);
+		}
+	      else if ((long int) (uintptr_t) count == max_hcount)
+		{
+		  void *gid = ctf_dynhash_lookup (d->cd_output_first_gid, hval);
+
+		  if (CTF_DEDUP_GID_TO_INPUT(gid) < CTF_DEDUP_GID_TO_INPUT(max_gid)
+		      || (CTF_DEDUP_GID_TO_INPUT(gid) == CTF_DEDUP_GID_TO_INPUT(max_gid)
+			  && CTF_DEDUP_GID_TO_TYPE(gid) < CTF_DEDUP_GID_TO_TYPE(max_gid)))
+		    {
+		      max_hval = hval;
+		      max_gid = ctf_dynhash_lookup (d->cd_output_first_gid, hval);
+		    }
 		}
 	    }
 	  if (err != ECTF_NEXT_END)
-- 
2.36.0.262.gb007c8117e.dirty


  reply	other threads:[~2022-04-27 15:51 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-27 15:51 [PATCH 1/2] libctf: add a comment explaining how to use ctf_*open Nick Alcock
2022-04-27 15:51 ` Nick Alcock [this message]
2022-04-28 11:08   ` [PATCH 2/2] libctf: impose an ordering on conflicting types Nick Alcock

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=20220427155130.238504-2-nick.alcock@oracle.com \
    --to=nick.alcock@oracle.com \
    --cc=binutils@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).