[4/4] libctf: get the offsets of fields of unnamed structs/unions right
Checks
Commit Message
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
Comments
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
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.
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. :)
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!
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.)
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.
@@ -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)
{
new file mode 100644
@@ -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;
new file mode 100644
@@ -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;
+}
new file mode 100644
@@ -0,0 +1,2 @@
+# source: unnamed-field-info-ctf.c
+Offset validation complete.