public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/2] libctf: add a comment explaining how to use ctf_*open
@ 2022-04-27 15:51 Nick Alcock
  2022-04-27 15:51 ` [PATCH 2/2] libctf: impose an ordering on conflicting types Nick Alcock
  0 siblings, 1 reply; 3+ messages in thread
From: Nick Alcock @ 2022-04-27 15:51 UTC (permalink / raw)
  To: binutils

Specifically, tell users what to pass to those functions that accept raw
section content, since it's fairly involved and easy to get wrong.
(.dynsym / .dynstr when CTF_F_DYNSTR is set, otherwise .symtab / .strtab).

include/ChangeLog:

	* ctf-api.h (ctf_*open): Improve comment.
---
 include/ctf-api.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/ctf-api.h b/include/ctf-api.h
index a3f45283b3c..153e012b5a0 100644
--- a/include/ctf-api.h
+++ b/include/ctf-api.h
@@ -314,7 +314,13 @@ extern ctf_next_t *ctf_next_copy (ctf_next_t *);
    archives: so they can be used to open both.  CTF files will appear to be an
    archive with one member named '.ctf'.  The low-level functions
    ctf_simple_open and ctf_bufopen return ctf_dict_t's directly, and cannot
-   be used on CTF archives.  */
+   be used on CTF archives.
+
+   Some of these functions take raw symtab and strtab section content in the
+   form of ctf_sect_t structures.  For CTF in ELF files, these should be
+   extracted from .dynsym and its associated string table (usually .dynsym)
+   whenever the CTF_F_DYNSTR flag is set in the CTF preamble (which it almost
+   always will be for linked objects, but not for .o files).  */
 
 extern ctf_archive_t *ctf_bfdopen (struct bfd *, int *);
 extern ctf_archive_t *ctf_bfdopen_ctfsect (struct bfd *, const ctf_sect_t *,

base-commit: 531c82a1c724079f98a4b069584681bc66da4dae
prerequisite-patch-id: 40f3e06181aa2cd54d4058e4b944a0d824f34f5f
prerequisite-patch-id: ef340a27f17470fb0544c063de87f3ae7de4dc2f
prerequisite-patch-id: a12de7cf51d1cfd6e9a1c61e851faa359cd3982b
prerequisite-patch-id: 2d39fffbdcc72a054184df623f9114f94b88cb64
prerequisite-patch-id: d55d8059be2d51962acc2e23a39a3921a3801199
prerequisite-patch-id: 3e0eea8043e3cd045779e30c375fe394c804c48f
prerequisite-patch-id: 4cb4bb30adb35d6ac591905c68c68c58fda191df
prerequisite-patch-id: 67d77176340cb47cc336c747a800d5748377136e
prerequisite-patch-id: c7fb48d926d5923680ecaa4e96c0d59891a751d3
prerequisite-patch-id: c097df29724255c0fcb3cbb4e33a1dd642684b5d
prerequisite-patch-id: fd1a706540f1780c1255b67784022e6268df3e9c
prerequisite-patch-id: 75b6b1d5cc24d11fb29cb0aa49a68e0fa9c402d6
prerequisite-patch-id: 644c13ece37062694e37224fb16d00947e3524c5
prerequisite-patch-id: 3900eda69a5ff8250d3372782e44de3053b3ba38
prerequisite-patch-id: 41947744e6c79fc3cc26e8dc3eb51812de7cd37c
prerequisite-patch-id: 4a566d02b8eb6d2fd01b305351e36b3b2fe300e0
prerequisite-patch-id: 47209dc4f1399290516ac8b96025f1984f887064
prerequisite-patch-id: 222cb080fcac03241c54f8ed254be5a441a1027d
prerequisite-patch-id: 8c2928f8cbff1d26e720b29a00ae824b26eb117d
prerequisite-patch-id: 026fe4a5b3a6e9e6824e68c3d6087bd560eaf404
prerequisite-patch-id: 0d678e933f28036e84cee8821c14db5d3a8c45f4
prerequisite-patch-id: d6c6a72b62a8b5e1a077f0578f592db9661e6271
prerequisite-patch-id: d318154951de43543c99211c5f518f478d68cebb
prerequisite-patch-id: 2c07a379863a0ea6d9b2a0a8158bb8920284241c
prerequisite-patch-id: 616d6d785dfaf8825cc55a3aeb8519e8e58569c3
prerequisite-patch-id: e8487d714c5a4b2a318b982913280255175b7698
prerequisite-patch-id: b3b209d80b57ab95d259834502f897f4636ee16a
prerequisite-patch-id: 1ce23787a13f7bdf114bc78b4cc220335af4fab2
prerequisite-patch-id: 3e20d1754c62887e0d4097a3f5dd094d15884206
prerequisite-patch-id: fbacee4a6adf081d67aa4b7c0106dc9b135b815f
prerequisite-patch-id: 4b5ce20b8eb1d5be0f9e4350099788ae8e86640d
prerequisite-patch-id: 9a1e4134e43425877a1d895d618496fff2f1cf3f
prerequisite-patch-id: 865cb1e5d11a00ab80aa4d8e4d1d33a887604982
prerequisite-patch-id: 4a80fbe279c2876b89cf795a032ad5f15d1a85cb
prerequisite-patch-id: cb2824b2c24303c0ba461c0dd9d82f7d618aeacc
prerequisite-patch-id: c490a4c4622b5229db11ebbc92d2bb4be6059290
prerequisite-patch-id: d49351257b621ee89ecb9253e375d87d452dd2ee
prerequisite-patch-id: d14e527f602406060dfe3c7bec7bc398b79d6b52
prerequisite-patch-id: d72cfacadcb022988856538304efbf233762a209
prerequisite-patch-id: 4b42cc9effa21721b820c472f3aa96a526d183d8
prerequisite-patch-id: 8e3a539b1c19cc66f0517ffdda2faf5483e0cadf
prerequisite-patch-id: d6b6641d5d50578518a5e66af864c28d6f5e135b
prerequisite-patch-id: bd49be55da65d3c0834589edcc968a2a3c69ac8a
prerequisite-patch-id: 9ed22b370da901499115758ec10e74d273bb944c
prerequisite-patch-id: e7c4f1809a3f293d479d2bb0721d5afcbbd2fc8f
prerequisite-patch-id: 07f52003a2ca2835cc1c48ab0523407027f737e4
prerequisite-patch-id: 5c6fcade90b1d7162810dd0b443363cb95ebe8ad
prerequisite-patch-id: f8c92965ab50137dce4da03f254791bd1e99ca7f
prerequisite-patch-id: 4952d7a2b2c9c999a29b7a0a977a8eaadbfd9c02
prerequisite-patch-id: 8ed93cc1f7e353cbf163f80ca6215dedf07524f8
prerequisite-patch-id: 0895d195998daf64e76003c8d8ca5f9657224829
prerequisite-patch-id: 49f2256590c40b9dd5b95053ff8e4b1ef63aab67
prerequisite-patch-id: 7638b29752342802329fe57895cb1960c5e63d8d
prerequisite-patch-id: a57258c269449415ff3373ce7e945d828ed2f70f
prerequisite-patch-id: 9dde5b4ca08a773e6d06aeffecde66ec7fc929b0
prerequisite-patch-id: 6616cb7d3be8b5d9777c0466f66671ab8809aa11
prerequisite-patch-id: 2098f5adc7e1a8e9700792796199f05864959594
prerequisite-patch-id: 058d42109d27390e1b71cc9561912a9ae3cc3ab6
prerequisite-patch-id: 84815b17d444e7e56dbff36e001392f9a79820a2
prerequisite-patch-id: 137d760d9dc554d7de70f1539a921dfe78843f61
prerequisite-patch-id: dfab5bd8525ebe14c776b582e5ea068ab133b249
prerequisite-patch-id: 205325a14cbc4fb3a87c48a486d263f02a80dd1f
prerequisite-patch-id: dcd841b94b3e691a4fb73a2d6ca8699efb704906
prerequisite-patch-id: 43231ba369f10d25b371699a174cbde625ce1dd8
prerequisite-patch-id: 02008197837e16c30f60ac8f29816e4f34402736
prerequisite-patch-id: 8b9ab351f553f84cdd31ea5cad0c2965943a4f56
-- 
2.36.0.262.gb007c8117e.dirty


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] libctf: impose an ordering on conflicting types
  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
  2022-04-28 11:08   ` Nick Alcock
  0 siblings, 1 reply; 3+ messages in thread
From: Nick Alcock @ 2022-04-27 15:51 UTC (permalink / raw)
  To: binutils

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/2] libctf: impose an ordering on conflicting types
  2022-04-27 15:51 ` [PATCH 2/2] libctf: impose an ordering on conflicting types Nick Alcock
@ 2022-04-28 11:08   ` Nick Alcock
  0 siblings, 0 replies; 3+ messages in thread
From: Nick Alcock @ 2022-04-28 11:08 UTC (permalink / raw)
  To: binutils

On 27 Apr 2022, Nick Alcock via Binutils verbalised:

> 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.

(Installed. Original reporter says it fixes his bug -- internal or
there'd be a sourceware bug. Maybe I should ask for one in future
from internal reporters anyway, since it's user-visible?)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-04-28 11:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH 2/2] libctf: impose an ordering on conflicting types Nick Alcock
2022-04-28 11:08   ` Nick Alcock

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).