[PATCH 1/4] libctf: fix assertion failure with no system qsort_r

[PATCH 1/4] libctf: fix assertion failure with no system qsort_r

[Nick Alcock]

If no suitable qsort_r is found in libc, we fall back to an
implementation in ctf-qsort.c.  But this implementation routinely calls
the comparison function with two identical arguments. The comparison
function that ensures that the order of output types is stable is not
ready for this, misinterprets it as a type appearing more that once (a
can-never-happen condition) and fails with an assertion failure.

Fixed, audited for further instances of the same failure (none found)
and added a no-qsort test to my regular testsuite run.

libctf/:
	PR libctf/30013
	* ctf-dedup.c (sort_output_mapping): Inputs are always equal to
	themselves.
---
 libctf/ctf-dedup.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libctf/ctf-dedup.c b/libctf/ctf-dedup.c
index c46f69e7449..6297c45c84d 100644
--- a/libctf/ctf-dedup.c
+++ b/libctf/ctf-dedup.c
@@ -2306,6 +2306,10 @@ sort_output_mapping (const ctf_next_hkv_t *one, const ctf_next_hkv_t *two,
   ctf_id_t one_type;
   ctf_id_t two_type;
 
+  /* Inputs are always equal to themselves.  */
+  if (one == two)
+    return 0;
+
   one_gid = ctf_dynhash_lookup (d->cd_output_first_gid, one_hval);
   two_gid = ctf_dynhash_lookup (d->cd_output_first_gid, two_hval);
 
-- 
2.39.1.268.g9de2f9a303

[PATCH 2/4] libctf: work around an uninitialized variable warning

[Nick Alcock]

GCC 11+ complains that sym is uninitialized in ctf_symbol_next.  It
isn't, but it's not quite smart enough to figure that out (it requires
domain-specific knowledge of the state of the ctf_next_t iterator
over multiple calls).

libctf/
	* ctf-lookup.c (ctf_symbol_next): Initialize sym to a suitable
	value for returning if never reset during the function.
---
 libctf/ctf-lookup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libctf/ctf-lookup.c b/libctf/ctf-lookup.c
index 10ababf1489..950c0a809ac 100644
--- a/libctf/ctf-lookup.c
+++ b/libctf/ctf-lookup.c
@@ -651,7 +651,7 @@ ctf_id_t
 ctf_symbol_next (ctf_dict_t *fp, ctf_next_t **it, const char **name,
 		 int functions)
 {
-  ctf_id_t sym;
+  ctf_id_t sym = CTF_ERR;
   ctf_next_t *i = *it;
   int err;
 
-- 
2.39.1.268.g9de2f9a303

[PATCH 3/4] libctf: fix a comment typo

[Nick Alcock]

ctf_dedup's intern() function does not return a dynamically allocated
string, so I just spent ten minutes auditing for obvious memory leaks
that couldn't actually happen.  Update the comment to note what it
actually returns (a pointer into an atoms table: i.e. possibly not
a new string, and not so easily leakable).

libctf/
	* ctf-dedup.c (intern): Update comment.
---
 libctf/ctf-dedup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libctf/ctf-dedup.c b/libctf/ctf-dedup.c
index 6297c45c84d..5fdddfd0b54 100644
--- a/libctf/ctf-dedup.c
+++ b/libctf/ctf-dedup.c
@@ -412,7 +412,7 @@ intern (ctf_dict_t *fp, char *atom)
 /* Add an indication of the namespace to a type name in a way that is not valid
    for C identifiers.  Used to maintain hashes of type names to other things
    while allowing for the four C namespaces (normal, struct, union, enum).
-   Return a new dynamically-allocated string.  */
+   Return a pointer into the cd_decorated_names atoms table.  */
 static const char *
 ctf_decorate_type_name (ctf_dict_t *fp, const char *name, int kind)
 {
-- 
2.39.1.268.g9de2f9a303

[PATCH 4/4] libctf: get the offsets of fields of unnamed structs/unions right

[Nick Alcock]

We were failing to add the offsets of the containing struct/union
in this case, leading to all offsets being relative to the unnamed
struct/union itself.

libctf/
	PR libctf/30264
	* ctf-types.c (ctf_member_info): Add the offset of the unnamed
	member of the current struct as necessary.
	* testsuite/libctf-lookup/unnamed-field-info*: New test.
---
 libctf/ctf-types.c                            |  5 +-
 .../libctf-lookup/unnamed-field-info-ctf.c    | 36 +++++++++
 .../libctf-lookup/unnamed-field-info.c        | 79 +++++++++++++++++++
 .../libctf-lookup/unnamed-field-info.lk       |  2 +
 4 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 libctf/testsuite/libctf-lookup/unnamed-field-info-ctf.c
 create mode 100644 libctf/testsuite/libctf-lookup/unnamed-field-info.c
 create mode 100644 libctf/testsuite/libctf-lookup/unnamed-field-info.lk

diff --git a/libctf/ctf-types.c b/libctf/ctf-types.c
index d21f6d5ff99..dd82053e1d7 100644
--- a/libctf/ctf-types.c
+++ b/libctf/ctf-types.c
@@ -1417,7 +1417,10 @@ ctf_member_info (ctf_dict_t *fp, ctf_id_t type, const char *name,
 	  && (ctf_type_kind (fp, memb.ctlm_type) == CTF_K_STRUCT
 	      || ctf_type_kind (fp, memb.ctlm_type) == CTF_K_UNION)
 	  && (ctf_member_info (fp, memb.ctlm_type, name, mip) == 0))
-	return 0;
+	{
+	  mip->ctm_offset += (unsigned long) CTF_LMEM_OFFSET (&memb);
+	  return 0;
+	}
 
       if (strcmp (membname, name) == 0)
 	{
diff --git a/libctf/testsuite/libctf-lookup/unnamed-field-info-ctf.c b/libctf/testsuite/libctf-lookup/unnamed-field-info-ctf.c
new file mode 100644
index 00000000000..54d60f5b195
--- /dev/null
+++ b/libctf/testsuite/libctf-lookup/unnamed-field-info-ctf.c
@@ -0,0 +1,36 @@
+struct A
+{
+  int a;
+  char *b;
+  struct
+  {
+    struct
+    {
+      char *one;
+      int two;
+    };
+    union
+    {
+      char *three;
+    };
+  };
+  struct
+  {
+    int four;
+  };
+  union
+  {
+    struct
+    {
+      double x;
+      long y;
+    };
+    struct
+    {
+      struct { char *foo; } z;
+      float aleph;
+    };
+  };
+};
+
+struct A used;
diff --git a/libctf/testsuite/libctf-lookup/unnamed-field-info.c b/libctf/testsuite/libctf-lookup/unnamed-field-info.c
new file mode 100644
index 00000000000..9abe8b026bb
--- /dev/null
+++ b/libctf/testsuite/libctf-lookup/unnamed-field-info.c
@@ -0,0 +1,79 @@
+/* Make sure unnamed field offsets are relative to the containing struct.  */
+
+#include <ctf-api.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "unnamed-field-info-ctf.c"
+
+static void
+verify_offsetof_matching (ctf_dict_t *fp, ctf_id_t type, const char *name, size_t offset)
+{
+  ctf_membinfo_t mi;
+
+  if (ctf_member_info (fp, type, name, &mi) < 0)
+    goto err;
+
+  if (mi.ctm_offset != offset * 8)
+    fprintf (stderr, "field %s inconsistency: offsetof() says %zi bits, CTF says %zi\n",
+	     name, offset * 8, mi.ctm_offset);
+
+  return;
+
+ err:
+  fprintf (stderr, "Cannot look up field %s: %s\n", name,
+	   ctf_errmsg (ctf_errno (fp)));
+  return;
+}
+
+int
+main (int argc, char *argv[])
+{
+  ctf_dict_t *fp;
+  ctf_archive_t *ctf;
+  ctf_id_t type;
+  int err;
+
+  if (argc != 2)
+    {
+      fprintf (stderr, "Syntax: %s PROGRAM\n", argv[0]);
+      exit(1);
+    }
+
+  if ((ctf = ctf_open (argv[1], NULL, &err)) == NULL)
+    goto open_err;
+  if ((fp = ctf_dict_open (ctf, NULL, &err)) == NULL)
+    goto open_err;
+
+  /* Dig out some structure members by name.  */
+
+  if ((type = ctf_lookup_by_name (fp, "struct A") ) == CTF_ERR)
+    goto err;
+
+  verify_offsetof_matching (fp, type, "a", offsetof (struct A, a));
+  verify_offsetof_matching (fp, type, "b", offsetof (struct A, b));
+  verify_offsetof_matching (fp, type, "one", offsetof (struct A, one));
+  verify_offsetof_matching (fp, type, "two", offsetof (struct A, two));
+  verify_offsetof_matching (fp, type, "three", offsetof (struct A, three));
+  verify_offsetof_matching (fp, type, "four", offsetof (struct A, four));
+  verify_offsetof_matching (fp, type, "x", offsetof (struct A, x));
+  verify_offsetof_matching (fp, type, "y", offsetof (struct A, y));
+  verify_offsetof_matching (fp, type, "z", offsetof (struct A, z));
+  verify_offsetof_matching (fp, type, "aleph", offsetof (struct A, aleph));
+
+  ctf_dict_close (fp);
+  ctf_arc_close (ctf);
+
+  printf ("Offset validation complete.\n");
+
+  return 0;
+
+ open_err:
+  fprintf (stderr, "%s: cannot open: %s\n", argv[0], ctf_errmsg (err));
+  return 1;
+
+ err:
+  fprintf (stderr, "Cannot look up type: %s\n", ctf_errmsg (ctf_errno (fp)));
+  return 1;
+}
diff --git a/libctf/testsuite/libctf-lookup/unnamed-field-info.lk b/libctf/testsuite/libctf-lookup/unnamed-field-info.lk
new file mode 100644
index 00000000000..eae6a517d50
--- /dev/null
+++ b/libctf/testsuite/libctf-lookup/unnamed-field-info.lk
@@ -0,0 +1,2 @@
+# source: unnamed-field-info-ctf.c
+Offset validation complete.
-- 
2.39.1.268.g9de2f9a303

Re: [PATCH 4/4] libctf: get the offsets of fields of unnamed structs/unions right

[Alan Modra]

On Fri, Mar 24, 2023 at 01:36:25PM +0000, Nick Alcock via Binutils wrote:
> 	* testsuite/libctf-lookup/unnamed-field-info*: New test.

arm-linux-gnueabi  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
hppa-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
m68k-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
microblaze-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
mips-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
powerpc-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
s390-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
sh4-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c

The fails look like:
field b inconsistency: offsetof() says 64 bits, CTF says 32
field one inconsistency: offsetof() says 128 bits, CTF says 64
field two inconsistency: offsetof() says 192 bits, CTF says 96
field three inconsistency: offsetof() says 256 bits, CTF says 128
field four inconsistency: offsetof() says 320 bits, CTF says 160
field x inconsistency: offsetof() says 384 bits, CTF says 192
field y inconsistency: offsetof() says 448 bits, CTF says 256
field z inconsistency: offsetof() says 384 bits, CTF says 192
field aleph inconsistency: offsetof() says 448 bits, CTF says 224

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 4/4] libctf: get the offsets of fields of unnamed structs/unions right

[Nick Alcock]

On 25 Mar 2023, Alan Modra verbalised:

> On Fri, Mar 24, 2023 at 01:36:25PM +0000, Nick Alcock via Binutils wrote:
>> 	* testsuite/libctf-lookup/unnamed-field-info*: New test.
>
> arm-linux-gnueabi  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> hppa-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> m68k-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> microblaze-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> mips-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> powerpc-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> s390-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> sh4-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c

Weird. I tested on two of those! (arm and powerpc).

Are they all 32-bit platforms? It looks like it.

> The fails look like:
> field b inconsistency: offsetof() says 64 bits, CTF says 32
> field one inconsistency: offsetof() says 128 bits, CTF says 64
> field two inconsistency: offsetof() says 192 bits, CTF says 96
> field three inconsistency: offsetof() says 256 bits, CTF says 128
> field four inconsistency: offsetof() says 320 bits, CTF says 160
> field x inconsistency: offsetof() says 384 bits, CTF says 192
> field y inconsistency: offsetof() says 448 bits, CTF says 256
> field z inconsistency: offsetof() says 384 bits, CTF says 192
> field aleph inconsistency: offsetof() says 448 bits, CTF says 224

All failing platforms are 32-bit, and they're all out by a factor of
two. Just a coincidence, perhaps. (Definitely no more than a vague
hunch.)

I'll take a look. Maybe I can just make the test less picky -- all we're
actually interested in for this bug is "CTF says 0". But this is worthy
of investigation regardless. It's just as likely to be a compiler bug
as a bug in libctf, I'd guess.

-- 
NULL && (void)

Re: [PATCH 4/4] libctf: get the offsets of fields of unnamed structs/unions right

[Alan Modra]

On Mon, Mar 27, 2023 at 11:27:40AM +0100, Nick Alcock wrote:
> On 25 Mar 2023, Alan Modra verbalised:
> 
> > On Fri, Mar 24, 2023 at 01:36:25PM +0000, Nick Alcock via Binutils wrote:
> >> 	* testsuite/libctf-lookup/unnamed-field-info*: New test.
> >
> > arm-linux-gnueabi  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> > hppa-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> > m68k-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> > microblaze-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> > mips-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> > powerpc-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> > s390-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> > sh4-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
> 
> Weird. I tested on two of those! (arm and powerpc).
> 
> Are they all 32-bit platforms? It looks like it.

Yes, and built on x86_64-linux.  Which is why things go wrong.
"lookup" is compiled and running on x86_64, thus compiled in offsets
are for the 64-bit host.  The test objects are compiled by the
relevant target compiler, in this case all 32-bit.  Things would break
with a 32-bit host and 64-bit targets too of course.

> > The fails look like:
> > field b inconsistency: offsetof() says 64 bits, CTF says 32
> > field one inconsistency: offsetof() says 128 bits, CTF says 64
> > field two inconsistency: offsetof() says 192 bits, CTF says 96
> > field three inconsistency: offsetof() says 256 bits, CTF says 128
> > field four inconsistency: offsetof() says 320 bits, CTF says 160
> > field x inconsistency: offsetof() says 384 bits, CTF says 192
> > field y inconsistency: offsetof() says 448 bits, CTF says 256
> > field z inconsistency: offsetof() says 384 bits, CTF says 192
> > field aleph inconsistency: offsetof() says 448 bits, CTF says 224
> 
> All failing platforms are 32-bit, and they're all out by a factor of
> two. Just a coincidence, perhaps. (Definitely no more than a vague
> hunch.)
> 
> I'll take a look. Maybe I can just make the test less picky -- all we're
> actually interested in for this bug is "CTF says 0". But this is worthy
> of investigation regardless. It's just as likely to be a compiler bug
> as a bug in libctf, I'd guess.

It's funny how we tend to not suspect the test.  :)

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 4/4] libctf: get the offsets of fields of unnamed structs/unions right

[Nick Alcock]

On 27 Mar 2023, Alan Modra said:

> On Mon, Mar 27, 2023 at 11:27:40AM +0100, Nick Alcock wrote:
>> On 25 Mar 2023, Alan Modra verbalised:
>> > On Fri, Mar 24, 2023 at 01:36:25PM +0000, Nick Alcock via Binutils wrote:
>> >> 	* testsuite/libctf-lookup/unnamed-field-info*: New test.
>> >
>> > arm-linux-gnueabi  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
>> > hppa-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
>> > m68k-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
>> > microblaze-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
>> > mips-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
>> > powerpc-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
>> > s390-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
>> > sh4-linux-gnu  +FAIL: /home/alan/src/binutils-gdb/libctf/testsuite/libctf-lookup/unnamed-field-info.c
>> 
>> Weird. I tested on two of those! (arm and powerpc).
>> 
>> Are they all 32-bit platforms? It looks like it.
>
> Yes, and built on x86_64-linux.  Which is why things go wrong.
> "lookup" is compiled and running on x86_64, thus compiled in offsets
> are for the 64-bit host.  The test objects are compiled by the
> relevant target compiler, in this case all 32-bit.  Things would break
> with a 32-bit host and 64-bit targets too of course.

Oh ugh of course. I need to run this one only where host == target.
Will fix. (How did this not come up in my cross-compiler testing?
I guess they all had similar offsets -- I think most of my cross-targers
are 64-bit... I need to add a 32-bit one, of course.)

>> I'll take a look. Maybe I can just make the test less picky -- all we're
>> actually interested in for this bug is "CTF says 0". But this is worthy
>> of investigation regardless. It's just as likely to be a compiler bug
>> as a bug in libctf, I'd guess.
>
> It's funny how we tend to not suspect the test.  :)

Yeah. Stands to reason an evil trick like that won't work everywhere!

-- 
NULL && (void)

Re: [PATCH 4/4] libctf: get the offsets of fields of unnamed structs/unions right

[Nick Alcock]

On 27 Mar 2023, Alan Modra stated:
> On Mon, Mar 27, 2023 at 11:27:40AM +0100, Nick Alcock wrote:
>> Are they all 32-bit platforms? It looks like it.
>
> Yes, and built on x86_64-linux.  Which is why things go wrong.
> "lookup" is compiled and running on x86_64, thus compiled in offsets
> are for the 64-bit host.  The test objects are compiled by the
> relevant target compiler, in this case all 32-bit.  Things would break
> with a 32-bit host and 64-bit targets too of course.

I have a fix under test that just turns this test off when
cross-compiling. It just doesn't make sense to assume that datatypes
have similar sizes on distinct platforms, regardless of the state of
some woolly generality like "bitness". I suppose I could use the same
tricks Autoconf does to get sizeof()s without execution, but for this
test that feels like total overkill. This isn't a cross-compilation bug,
it bites everywhere, so native-only tests will suffice to stop it
returning.

>> I'll take a look. Maybe I can just make the test less picky -- all we're
>> actually interested in for this bug is "CTF says 0". But this is worthy
>> of investigation regardless. It's just as likely to be a compiler bug
>> as a bug in libctf, I'd guess.
>
> It's funny how we tend to not suspect the test.  :)

At least that's usually fixable with better ways to run the testsuite
that spot bugs in the tests :) This round of fixes is taking longer than
I'd hoped because valgrind and ASAN spotted uninitialized data usage and
memory leaks in a new test...

(I really should put the pile of scripts that controls my tests online
somewhere: even if some of them rely on things like homebrew
containerization hacks and so won't run without modification, the
combination might still be valuable. These days they're mostly run under
pueue, a rather nice queueing system for noninteractive programs.)

-- 
NULL && (void)

Re: [PATCH 4/4] libctf: get the offsets of fields of unnamed structs/unions right

[Nick Alcock]

On 6 Apr 2023, Nick Alcock via Binutils spake thusly:

> On 27 Mar 2023, Alan Modra stated:
>> On Mon, Mar 27, 2023 at 11:27:40AM +0100, Nick Alcock wrote:
>>> Are they all 32-bit platforms? It looks like it.
>>
>> Yes, and built on x86_64-linux.  Which is why things go wrong.
>> "lookup" is compiled and running on x86_64, thus compiled in offsets
>> are for the 64-bit host.  The test objects are compiled by the
>> relevant target compiler, in this case all 32-bit.  Things would break
>> with a 32-bit host and 64-bit targets too of course.
>
> I have a fix under test that just turns this test off when
> cross-compiling.

Now pushed as commit 30a794e9f1db2de9099ed4c494d917d4e86de0fd.

-- 
NULL && (void)