libctf: check for problems with error returns

  We do this as a writable test because the only known-affected platforms
(with ssize_t longer than unsigned long) use PE, and we do not have support
for CTF linkage in the PE linker yet.

	PR libctf/30836
	* libctf/testsuite/libctf-writable/libctf-errors.*: New test.
---
 .../testsuite/libctf-writable/libctf-errors.c | 74 +++++++++++++++++++
 .../libctf-writable/libctf-errors.lk          |  1 +
 2 files changed, 75 insertions(+)
 create mode 100644 libctf/testsuite/libctf-writable/libctf-errors.c
 create mode 100644 libctf/testsuite/libctf-writable/libctf-errors.lk

Your patch looks good, and passes every test I can throw at it. I think
it can go in.  You cleaned up a bunch of outright errors in this area,
too, especially in ctf-dedup: thanks!

(You probably want to adjust the commit log so that the version history
is at the bottom rather than the top, or drop it entirely.)


Here's a testcase that fails on mingw64 in the absence of your patch,
without requiring a cross-build to an ELF arch.  (It also checks at
least one instance of the other classes of error return in libctf.)

I'll push this after your commit goes in.  (I can push it, with an
adjusted commit log, if you want.)
  

Hi Nick,

Thanks for validating this patch!

On 2023-10-13 16:01, Nick Alcock wrote:
> We do this as a writable test because the only known-affected platforms
> (with ssize_t longer than unsigned long) use PE, and we do not have support
> for CTF linkage in the PE linker yet.

Is it visible in PE too or only PE32+? Maybe not important, but the Arm 
built variant does not trigger the fault (same source tree as I found 
the issue in, but they build 32-bit and I build 64-bit) when I've 
executed the GCC testsuite.

> 	PR libctf/30836
> 	* libctf/testsuite/libctf-writable/libctf-errors.*: New test.
> ---
>   .../testsuite/libctf-writable/libctf-errors.c | 74 +++++++++++++++++++
>   .../libctf-writable/libctf-errors.lk          |  1 +
>   2 files changed, 75 insertions(+)
>   create mode 100644 libctf/testsuite/libctf-writable/libctf-errors.c
>   create mode 100644 libctf/testsuite/libctf-writable/libctf-errors.lk
> 
> Your patch looks good, and passes every test I can throw at it. I think
> it can go in.  You cleaned up a bunch of outright errors in this area,
> too, especially in ctf-dedup: thanks!

There is still some potential for cleanup as some functions are 
returning "unsigned long", but think it should perhaps be ctf_id_t instead.

> (You probably want to adjust the commit log so that the version history
> is at the bottom rather than the top, or drop it entirely.)

My plan was to drop that part.

Do you think I should have a changelog entry for the commit and if so, 
what should I write in it? Should I list every function that is touched 
(more or less half of the ctf_* functions defined...) or is there some 
better way to document this change?


> Here's a testcase that fails on mingw64 in the absence of your patch,
> without requiring a cross-build to an ELF arch.  (It also checks at
> least one instance of the other classes of error return in libctf.)
> 
> I'll push this after your commit goes in.  (I can push it, with an
> adjusted commit log, if you want.

Fine either way. Either reply to my question about changelog or just 
merge it with the correct answer :)

> 
> diff --git a/libctf/testsuite/libctf-writable/libctf-errors.c b/libctf/testsuite/libctf-writable/libctf-errors.c
> new file mode 100644
> index 00000000000..71f8268cfad
> --- /dev/null
> +++ b/libctf/testsuite/libctf-writable/libctf-errors.c
> @@ -0,0 +1,74 @@
> +/* Make sure that error returns are correct.  Usually this is trivially
> +   true, but on platforms with unusual type sizes all the casting might
> +   cause problems with unexpected sign-extension and truncation.  */
> +
> +#include <ctf-api.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +int
> +main (int argc, char *argv[])
> +{
> +  ctf_dict_t *fp;
> +  ctf_next_t *i = NULL;
> +  size_t boom = 0;
> +  ctf_id_t itype, stype;
> +  ctf_encoding_t encoding = {0};
> +  ctf_membinfo_t mi;
> +  ssize_t ret;
> +  int err;
> +
> +  if ((fp = ctf_create (&err)) == NULL)
> +    {
> +      fprintf (stderr, "%s: cannot create: %s\n", argv[0], ctf_errmsg (err));
> +      return 1;
> +    }
> +
> +  /* First error class: int return.  */
> +
> +  if (ctf_member_count (fp, 1024) >= 0)
> +    fprintf (stderr, "int return: non-error return: %i\n",
> +             ctf_member_count(fp, 1024));
> +
> +  /* Second error class: type ID return.  */
> +
> +  if (ctf_type_reference (fp, 1024) != CTF_ERR)
> +    fprintf (stderr, "ctf_id_t return: non-error return: %li\n",
> +             ctf_type_reference (fp, 1024));
> +
> +  /* Third error class: ssize_t return.  Create a type to iterate over first.  */
> +
> +  if ((itype = ctf_add_integer (fp, CTF_ADD_ROOT, "int", &encoding)) == CTF_ERR)
> +    fprintf (stderr, "cannot add int: %s\n", ctf_errmsg (ctf_errno (fp)));
> +  else if ((stype = ctf_add_struct (fp, CTF_ADD_ROOT, "foo")) == CTF_ERR)
> +    fprintf (stderr, "cannot add struct: %s\n", ctf_errmsg (ctf_errno (fp)));
> +  else if (ctf_add_member (fp, stype, "bar", itype) < 0)
> +    fprintf (stderr, "cannot add member: %s\n", ctf_errmsg (ctf_errno (fp)));

Should these be if-else-if-else-if-statments like above or just 3 
"free-standing" if-statements? I.e. should the 3 potential issue be 
visible in the same run or should they require 3 consecutive runs if all 
of them are failing?

> +
> +  if (ctf_member_info (fp, stype, "bar", &mi) < 0)
> +    fprintf (stderr, "cannot get member info: %s\n", ctf_errmsg (ctf_errno (fp)));
> +
> +  /* Iteration should never produce an offset bigger than the offset just returned,
> +     and should quickly terminate.  */
> +
> +  while ((ret = ctf_member_next (fp, stype, &i, NULL, NULL, 0)) >= 0) {
> +    if (ret > mi.ctm_offset)
> +      fprintf (stderr, "ssize_t return: unexpected offset: %zi\n", ret);
> +    if (boom++ > 1000)
> +      {
> +        fprintf (stderr, "member iteration went on way too long\n");
> +        exit (1);
> +      }
> +  }
> +
> +  /* Fourth error class (trivial): pointer return.  */
> +  if (ctf_type_aname (fp, 1024) != NULL)
> +    fprintf (stderr, "pointer return: non-error return: %p\n",
> +             ctf_type_aname (fp, 1024));
> +
> +  ctf_file_close (fp);
> +
> +  printf("All done.\n");
> +
> +  return 0;
> +}
> diff --git a/libctf/testsuite/libctf-writable/libctf-errors.lk b/libctf/testsuite/libctf-writable/libctf-errors.lk
> new file mode 100644
> index 00000000000..b944f73d013
> --- /dev/null
> +++ b/libctf/testsuite/libctf-writable/libctf-errors.lk
> @@ -0,0 +1 @@
> +All done.
>
  

On 13 Oct 2023, Torbjorn SVENSSON told this:

> Hi Nick,
>
> Thanks for validating this patch!
>
> On 2023-10-13 16:01, Nick Alcock wrote:
>> We do this as a writable test because the only known-affected platforms
>> (with ssize_t longer than unsigned long) use PE, and we do not have support
>> for CTF linkage in the PE linker yet.
>
> Is it visible in PE too or only PE32+? Maybe not important, but the
> Arm built variant does not trigger the fault (same source tree as I
> found the issue in, but they build 32-bit and I build 64-bit) when
> I've executed the GCC testsuite.

The underlying fault is client-side and is visible in anything that does
a ctf_member_next on a platform with sizeof(ssize_t) > sizeof(unsigned
long). The specific instance you saw (a hanging linker) requires CTF in
the input and a linker that deduplicates it, and right now that is ELF
only (because it was painful enough to add that I didn't have the
stamina to add anything else -- if anything else *is* added, it will
probably be PE next, but right now linking PE just blindly concatenates
CTF sections, which isn't very useful).

>> 	PR libctf/30836
>> 	* libctf/testsuite/libctf-writable/libctf-errors.*: New test.
>> ---
>>   .../testsuite/libctf-writable/libctf-errors.c | 74 +++++++++++++++++++
>>   .../libctf-writable/libctf-errors.lk          |  1 +
>>   2 files changed, 75 insertions(+)
>>   create mode 100644 libctf/testsuite/libctf-writable/libctf-errors.c
>>   create mode 100644 libctf/testsuite/libctf-writable/libctf-errors.lk
>> Your patch looks good, and passes every test I can throw at it. I think
>> it can go in.  You cleaned up a bunch of outright errors in this area,
>> too, especially in ctf-dedup: thanks!
>
> There is still some potential for cleanup as some functions are
> returning "unsigned long", but think it should perhaps be ctf_id_t
> instead.

The only one of those i can see is ctf_lookup_symbol_idx. That's
returning a symbol index, not a type ID, so should definitely not be
returning a ctf_id_t. Perhaps it should be an int, but I'm not sure how
big symbol tables in the set of all executable formats can actually
be...

Hm, actually, there is one bit you might want to adjust in ctf-lookup.c.
You fixed one call to ctf_lookup_symbol_idx () in
ctf_lookup_by_sym_or_name ():

    if ((symidx = ctf_lookup_symbol_idx (fp, symname)) == (unsigned long) -1)
      goto try_parent;

but the other one, in ctf_lookup_symbol_idx () itself (doing a recursive
call to check parent dicts), still says

      if ((psym = ctf_lookup_symbol_idx (fp->ctf_parent, symname))
          != CTF_ERR)

Both symidx and psym are unsigned long variables, so probably both
should be checking against (unsigned long) -1.

>> (You probably want to adjust the commit log so that the version history
>> is at the bottom rather than the top, or drop it entirely.)
>
> My plan was to drop that part.

Aha right.

> Do you think I should have a changelog entry for the commit and if so,
> what should I write in it? Should I list every function that is
> touched (more or less half of the ctf_* functions defined...) or is
> there some better way to document this change?

You can just say

	Affected functions adjusted.

or something. A giant list adds no value, I think.

>> Here's a testcase that fails on mingw64 in the absence of your patch,
>> without requiring a cross-build to an ELF arch.  (It also checks at
>> least one instance of the other classes of error return in libctf.)
>> I'll push this after your commit goes in.  (I can push it, with an
>> adjusted commit log, if you want.
>
> Fine either way. Either reply to my question about changelog or just merge it with the correct answer :)

... I think there's one more tiny change :/

>> +  /* Third error class: ssize_t return.  Create a type to iterate over first.  */
>> +
>> +  if ((itype = ctf_add_integer (fp, CTF_ADD_ROOT, "int", &encoding)) == CTF_ERR)
>> +    fprintf (stderr, "cannot add int: %s\n", ctf_errmsg (ctf_errno (fp)));
>> +  else if ((stype = ctf_add_struct (fp, CTF_ADD_ROOT, "foo")) == CTF_ERR)
>> +    fprintf (stderr, "cannot add struct: %s\n", ctf_errmsg (ctf_errno (fp)));
>> +  else if (ctf_add_member (fp, stype, "bar", itype) < 0)
>> +    fprintf (stderr, "cannot add member: %s\n", ctf_errmsg (ctf_errno (fp)));
>
> Should these be if-else-if-else-if-statments like above or just 3 "free-standing" if-statements? I.e. should the 3 potential issue
> be visible in the same run or should they require 3 consecutive runs if all of them are failing?

I was wondering if this was too clever (or unclear!) but the intent is
to only do the later things if the earlier ones didn't fail, i.e. what
would be foo || bar || baz in the shell. (Later operations will also
fail if any of them fail, but the fprintf's from the first failure will
suffice to make the whole test fail -- so maybe it's safe to just do
them in sequence even if the earlier ones failed. Excessive paranoia on
my part perhaps, given that we don't *expect* any of these to fail: it's
just setting things up for the *actual* tests, of ctf_member_info and
ctf_member_next.)

>> +  if (ctf_member_info (fp, stype, "bar", &mi) < 0)
>> +    fprintf (stderr, "cannot get member info: %s\n", ctf_errmsg (ctf_errno (fp)));
>> +
>> +  /* Iteration should never produce an offset bigger than the offset just returned,
>> +     and should quickly terminate.  */
>> +
>> +  while ((ret = ctf_member_next (fp, stype, &i, NULL, NULL, 0)) >= 0) {
>> +    if (ret > mi.ctm_offset)
>> +      fprintf (stderr, "ssize_t return: unexpected offset: %zi\n", ret);

(here.)
  

On 2023-10-15 21:18, Nick Alcock wrote:
> On 13 Oct 2023, Torbjorn SVENSSON told this:
> 
>> Hi Nick,
>>
>> Thanks for validating this patch!
>>
>> On 2023-10-13 16:01, Nick Alcock wrote:
>>> We do this as a writable test because the only known-affected platforms
>>> (with ssize_t longer than unsigned long) use PE, and we do not have support
>>> for CTF linkage in the PE linker yet.
>>
>> Is it visible in PE too or only PE32+? Maybe not important, but the
>> Arm built variant does not trigger the fault (same source tree as I
>> found the issue in, but they build 32-bit and I build 64-bit) when
>> I've executed the GCC testsuite.
> 
> The underlying fault is client-side and is visible in anything that does
> a ctf_member_next on a platform with sizeof(ssize_t) > sizeof(unsigned
> long). The specific instance you saw (a hanging linker) requires CTF in
> the input and a linker that deduplicates it, and right now that is ELF
> only (because it was painful enough to add that I didn't have the
> stamina to add anything else -- if anything else *is* added, it will
> probably be PE next, but right now linking PE just blindly concatenates
> CTF sections, which isn't very useful).

Right.

> 
>>> 	PR libctf/30836
>>> 	* libctf/testsuite/libctf-writable/libctf-errors.*: New test.
>>> ---
>>>    .../testsuite/libctf-writable/libctf-errors.c | 74 +++++++++++++++++++
>>>    .../libctf-writable/libctf-errors.lk          |  1 +
>>>    2 files changed, 75 insertions(+)
>>>    create mode 100644 libctf/testsuite/libctf-writable/libctf-errors.c
>>>    create mode 100644 libctf/testsuite/libctf-writable/libctf-errors.lk
>>> Your patch looks good, and passes every test I can throw at it. I think
>>> it can go in.  You cleaned up a bunch of outright errors in this area,
>>> too, especially in ctf-dedup: thanks!
>>
>> There is still some potential for cleanup as some functions are
>> returning "unsigned long", but think it should perhaps be ctf_id_t
>> instead.
> 
> The only one of those i can see is ctf_lookup_symbol_idx. That's
> returning a symbol index, not a type ID, so should definitely not be
> returning a ctf_id_t. Perhaps it should be an int, but I'm not sure how
> big symbol tables in the set of all executable formats can actually
> be...
> 
> Hm, actually, there is one bit you might want to adjust in ctf-lookup.c.
> You fixed one call to ctf_lookup_symbol_idx () in
> ctf_lookup_by_sym_or_name ():
> 
>      if ((symidx = ctf_lookup_symbol_idx (fp, symname)) == (unsigned long) -1)
>        goto try_parent;
> 
> but the other one, in ctf_lookup_symbol_idx () itself (doing a recursive
> call to check parent dicts), still says
> 
>        if ((psym = ctf_lookup_symbol_idx (fp->ctf_parent, symname))
>            != CTF_ERR)
> 
> Both symidx and psym are unsigned long variables, so probably both
> should be checking against (unsigned long) -1.

Thanks for the pointer. I found that I had replaced some (unsigned 
long)-1 with CTF_ERR that should be exactly that. V8 is reverting those 
chunks to be like they were before.

>>> (You probably want to adjust the commit log so that the version history
>>> is at the bottom rather than the top, or drop it entirely.)
>>
>> My plan was to drop that part.
> 
> Aha right.
> 
>> Do you think I should have a changelog entry for the commit and if so,
>> what should I write in it? Should I list every function that is
>> touched (more or less half of the ctf_* functions defined...) or is
>> there some better way to document this change?
> 
> You can just say
> 
> 	Affected functions adjusted.
> 
> or something. A giant list adds no value, I think.

I tried to compile an exhaustive list, but it was too big for my taste, 
so I went with your proposal (part of V8).

> 
>>> Here's a testcase that fails on mingw64 in the absence of your patch,
>>> without requiring a cross-build to an ELF arch.  (It also checks at
>>> least one instance of the other classes of error return in libctf.)
>>> I'll push this after your commit goes in.  (I can push it, with an
>>> adjusted commit log, if you want.
>>
>> Fine either way. Either reply to my question about changelog or just merge it with the correct answer :)
> 
> ... I think there's one more tiny change :/
> 
>>> +  /* Third error class: ssize_t return.  Create a type to iterate over first.  */
>>> +
>>> +  if ((itype = ctf_add_integer (fp, CTF_ADD_ROOT, "int", &encoding)) == CTF_ERR)
>>> +    fprintf (stderr, "cannot add int: %s\n", ctf_errmsg (ctf_errno (fp)));
>>> +  else if ((stype = ctf_add_struct (fp, CTF_ADD_ROOT, "foo")) == CTF_ERR)
>>> +    fprintf (stderr, "cannot add struct: %s\n", ctf_errmsg (ctf_errno (fp)));
>>> +  else if (ctf_add_member (fp, stype, "bar", itype) < 0)
>>> +    fprintf (stderr, "cannot add member: %s\n", ctf_errmsg (ctf_errno (fp)));
>>
>> Should these be if-else-if-else-if-statments like above or just 3 "free-standing" if-statements? I.e. should the 3 potential issue
>> be visible in the same run or should they require 3 consecutive runs if all of them are failing?
> 
> I was wondering if this was too clever (or unclear!) but the intent is
> to only do the later things if the earlier ones didn't fail, i.e. what
> would be foo || bar || baz in the shell. (Later operations will also
> fail if any of them fail, but the fprintf's from the first failure will
> suffice to make the whole test fail -- so maybe it's safe to just do
> them in sequence even if the earlier ones failed. Excessive paranoia on
> my part perhaps, given that we don't *expect* any of these to fail: it's
> just setting things up for the *actual* tests, of ctf_member_info and
> ctf_member_next.)
> 
>>> +  if (ctf_member_info (fp, stype, "bar", &mi) < 0)
>>> +    fprintf (stderr, "cannot get member info: %s\n", ctf_errmsg (ctf_errno (fp)));
>>> +
>>> +  /* Iteration should never produce an offset bigger than the offset just returned,
>>> +     and should quickly terminate.  */
>>> +
>>> +  while ((ret = ctf_member_next (fp, stype, &i, NULL, NULL, 0)) >= 0) {
>>> +    if (ret > mi.ctm_offset)
>>> +      fprintf (stderr, "ssize_t return: unexpected offset: %zi\n", ret);
> 
> (here.)

Ah, okay. In any case, I think it would be clearer if you get all the 
lines in one go that fails than just the first one (in case of multiple 
failures...). - But, that's only my 2 cents.
  

On 16 Oct 2023, Torbjorn SVENSSON told this:
> On 2023-10-15 21:18, Nick Alcock wrote:
>>>> +  if (ctf_member_info (fp, stype, "bar", &mi) < 0)
>>>> +    fprintf (stderr, "cannot get member info: %s\n", ctf_errmsg (ctf_errno (fp)));
>>>> +
>>>> +  /* Iteration should never produce an offset bigger than the offset just returned,
>>>> +     and should quickly terminate.  */
>>>> +
>>>> +  while ((ret = ctf_member_next (fp, stype, &i, NULL, NULL, 0)) >= 0) {
>>>> +    if (ret > mi.ctm_offset)
>>>> +      fprintf (stderr, "ssize_t return: unexpected offset: %zi\n", ret);
>> (here.)
>
> Ah, okay. In any case, I think it would be clearer if you get all the lines in one go that fails than just the first one (in case of
> multiple failures...). - But, that's only my 2 cents.

We do that for failures that actually relate to what's being tested, but
these lines are only setup, and each depends on the one before: if any
fail, all the ones after it are certain to.

I think I might change it to not do the actual tests if we can't do the
setup, though!

I'll give v8 a quick sanity check: more in about an hour.
  

./libtool --quiet --tag=CC --mode=link gcc -fmessage-length=0 -grecord-gcc-switches -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -g -Wno-error   -I../../libctf/../include -I../../libctf -I. -I./../bfd  /home/abuild/rpmbuild/BUILD/binutils-2.42/libctf/testsuite/libctf-writable/libctf-errors.c -o tmpdir/lookup libctf.la
/home/abuild/rpmbuild/BUILD/binutils-2.42/libctf/testsuite/libctf-writable/libctf-errors.c: In function 'main':
/home/abuild/rpmbuild/BUILD/binutils-2.42/libctf/testsuite/libctf-writable/libctf-errors.c:54:9: warning: 'stype' may be used uninitialized in this function [-Wmaybe-uninitialized]
   while ((ret = ctf_member_next (fp, stype, &i, NULL, NULL, 0)) >= 0) {
         ^
compilation of lookup program /home/abuild/rpmbuild/BUILD/binutils-2.42/libctf/testsuite/libctf-writable/libctf-errors.c failed with </home/abuild/rpmbuild/BUILD/binutils-2.42/libctf/testsuite/libctf-writable/libctf-errors.c: In function 'main':
/home/abuild/rpmbuild/BUILD/binutils-2.42/libctf/testsuite/libctf-writable/libctf-errors.c:54:9: warning: 'stype' may be used uninitialized in this function [-Wmaybe-uninitialized]
   while ((ret = ctf_member_next (fp, stype, &i, NULL, NULL, 0)) >= 0) {
         ^>ERROR: compilation of lookup program /home/abuild/rpmbuild/BUILD/binutils-2.42/libctf/testsuite/libctf-writable/libctf-errors.c failed
UNRESOLVED: /home/abuild/rpmbuild/BUILD/binutils-2.42/libctf/testsuite/libctf-writable/libctf-errors.c
  

On 30 Jan 2024, Andreas Schwab said:

> ./libtool --quiet --tag=CC --mode=link gcc -fmessage-length=0 -grecord-gcc-switches -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -g -Wno-error   -I../../libctf/../include -I../../libctf -I. -I./../bfd  /home/abuild/rpmbuild/BUILD/binutils-2.42/libctf/testsuite/libctf-writable/libctf-errors.c -o tmpdir/lookup libctf.la
> /home/abuild/rpmbuild/BUILD/binutils-2.42/libctf/testsuite/libctf-writable/libctf-errors.c: In function 'main':
> /home/abuild/rpmbuild/BUILD/binutils-2.42/libctf/testsuite/libctf-writable/libctf-errors.c:54:9: warning: 'stype' may be used uninitialized in this function [-Wmaybe-uninitialized]
>    while ((ret = ctf_member_next (fp, stype, &i, NULL, NULL, 0)) >= 0) {
>          ^

Thanks! I was being sloppy on error paths and the compiler has spotted
my sin... will push the attached fix shortly, if your compiler is happy
(none of my compilers spot this one).

------------- >8 -----------
From 78fd0d4e60e8aed4c7396837d64cf1d1114b8c24 Mon Sep 17 00:00:00 2001
From: Nick Alcock <nick.alcock@oracle.com>
Date: Tue, 30 Jan 2024 14:18:54 +0000
Subject: [PATCH] libctf: fix uninitialized variables in testsuite

Just because a path is an error path doesn't mean the program terminates
there if you don't ask it to. (And we don't want to -- but that means
we need to initialize the variables that are missed if an error happens to
*something*. Type ID 0 (unimplemented) will do: it'll induce further
ECTF_BADID errors, but that's no bad thing.)
---
 libctf/testsuite/libctf-writable/libctf-errors.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libctf/testsuite/libctf-writable/libctf-errors.c b/libctf/testsuite/libctf-writable/libctf-errors.c
index 71f8268cfad..2790b608396 100644
--- a/libctf/testsuite/libctf-writable/libctf-errors.c
+++ b/libctf/testsuite/libctf-writable/libctf-errors.c
@@ -12,7 +12,7 @@ main (int argc, char *argv[])
   ctf_dict_t *fp;
   ctf_next_t *i = NULL;
   size_t boom = 0;
-  ctf_id_t itype, stype;
+  ctf_id_t itype = 0, stype = 0;
   ctf_encoding_t encoding = {0};
   ctf_membinfo_t mi;
   ssize_t ret;

base-commit: b960445a45981873c5b1718824ea9d3b5749433a
  

On Jan 30 2024, Nick Alcock wrote:

> diff --git a/libctf/testsuite/libctf-writable/libctf-errors.c b/libctf/testsuite/libctf-writable/libctf-errors.c
> index 71f8268cfad..2790b608396 100644
> --- a/libctf/testsuite/libctf-writable/libctf-errors.c
> +++ b/libctf/testsuite/libctf-writable/libctf-errors.c
> @@ -12,7 +12,7 @@ main (int argc, char *argv[])
>    ctf_dict_t *fp;
>    ctf_next_t *i = NULL;
>    size_t boom = 0;
> -  ctf_id_t itype, stype;
> +  ctf_id_t itype = 0, stype = 0;
>    ctf_encoding_t encoding = {0};
>    ctf_membinfo_t mi;
>    ssize_t ret;

This silences the warnings and all dependent tests pass.
  

Andreas Schwab <schwab@suse.de> writes:

> On Jan 30 2024, Nick Alcock wrote:
>
>> diff --git a/libctf/testsuite/libctf-writable/libctf-errors.c b/libctf/testsuite/libctf-writable/libctf-errors.c
>> index 71f8268cfad..2790b608396 100644
>> --- a/libctf/testsuite/libctf-writable/libctf-errors.c
>> +++ b/libctf/testsuite/libctf-writable/libctf-errors.c
>> @@ -12,7 +12,7 @@ main (int argc, char *argv[])
>>    ctf_dict_t *fp;
>>    ctf_next_t *i = NULL;
>>    size_t boom = 0;
>> -  ctf_id_t itype, stype;
>> +  ctf_id_t itype = 0, stype = 0;
>>    ctf_encoding_t encoding = {0};
>>    ctf_membinfo_t mi;
>>    ssize_t ret;
>
> This silences the warnings and all dependent tests pass.

I just ran into this myself today, looks like it never got committed?

thanks,
sam
  

On 9 Mar 2024, Sam James told this:

> Andreas Schwab <schwab@suse.de> writes:
>
>> On Jan 30 2024, Nick Alcock wrote:
>>
>>> diff --git a/libctf/testsuite/libctf-writable/libctf-errors.c b/libctf/testsuite/libctf-writable/libctf-errors.c
>>> index 71f8268cfad..2790b608396 100644
>>> --- a/libctf/testsuite/libctf-writable/libctf-errors.c
>>> +++ b/libctf/testsuite/libctf-writable/libctf-errors.c
>>> @@ -12,7 +12,7 @@ main (int argc, char *argv[])
>>>    ctf_dict_t *fp;
>>>    ctf_next_t *i = NULL;
>>>    size_t boom = 0;
>>> -  ctf_id_t itype, stype;
>>> +  ctf_id_t itype = 0, stype = 0;
>>>    ctf_encoding_t encoding = {0};
>>>    ctf_membinfo_t mi;
>>>    ssize_t ret;
>>
>> This silences the warnings and all dependent tests pass.
>
> I just ran into this myself today, looks like it never got committed?

Ugh sorry you're quite right! I'll rebase, retest, and push today. Mea
culpa, it got lost toward the base of the huge pile of stuff I'm working
on right now.
  

Nick Alcock <nick.alcock@oracle.com> writes:

> On 9 Mar 2024, Sam James told this:
>
>> Andreas Schwab <schwab@suse.de> writes:
>>
>>> On Jan 30 2024, Nick Alcock wrote:
>>>
>>>> diff --git a/libctf/testsuite/libctf-writable/libctf-errors.c b/libctf/testsuite/libctf-writable/libctf-errors.c
>>>> index 71f8268cfad..2790b608396 100644
>>>> --- a/libctf/testsuite/libctf-writable/libctf-errors.c
>>>> +++ b/libctf/testsuite/libctf-writable/libctf-errors.c
>>>> @@ -12,7 +12,7 @@ main (int argc, char *argv[])
>>>>    ctf_dict_t *fp;
>>>>    ctf_next_t *i = NULL;
>>>>    size_t boom = 0;
>>>> -  ctf_id_t itype, stype;
>>>> +  ctf_id_t itype = 0, stype = 0;
>>>>    ctf_encoding_t encoding = {0};
>>>>    ctf_membinfo_t mi;
>>>>    ssize_t ret;
>>>
>>> This silences the warnings and all dependent tests pass.
>>
>> I just ran into this myself today, looks like it never got committed?
>
> Ugh sorry you're quite right! I'll rebase, retest, and push today. Mea
> culpa, it got lost toward the base of the huge pile of stuff I'm working
> on right now.

It happens ;)

Cheers Nick!