[v2] libctf: ctf_member_next needs to return (ssize_t)-1 on error

  v1 -> v2:
Changed all functions with signed interger return type to return -1 based on
comment from Alan.


Ok for trunk?

---

The function ctf_member_next should return (ssize_t)-1 on
error. As the function ctf_set_errno returns (ctf_id_t)-1L and that is
then casted to "unsigned long" as it's the return type of the function,
it's not compatible and causes the value 0xffffffff to be returned on
64-bit Windows builds. As a result, the check for a negative value in
ctf_dedup_rhash_type will never be true and a resulting infinit loop is
created.

This was found testing an arm-none-eabi toolchain built with
x86_64-w64-mingw32. If the same source tree is built with
i686-w64-mingw32, everything appears to be working correctly.

libctf/
	* ctf-create.c (ctf_add_enumerator): Ensure function returns -1 on error.
	(ctf_add_funcobjt_sym): Likewise.
	(ctf_add_member_encoded): Likewise.
	(ctf_add_member_offset): Likewise.
	(ctf_add_variable): Likewise.
	(ctf_grow_ptrtab): Likewise.
	(ctf_grow_vlen): Likewise.
	(ctf_rollback): Likewise.
	(ctf_set_array): Likewise.
	(ctf_update): Likewise.
	* ctf-dedup.c (ctf_dedup_atoms_init): Likewise.
	(ctf_dedup_conflictify_unshared): Likewise.
	(ctf_dedup_detect_name_ambiguity): Likewise.
	(ctf_dedup_emit_struct_members): Likewise.
	(ctf_dedup_emit_type): Likewise.
	(ctf_dedup_hash_kind): Likewise.
	(ctf_dedup_init): Likewise.
	(ctf_dedup_mark_conflicting_hash): Likewise.
	(ctf_dedup_multiple_input_dicts): Likewise.
	(ctf_dedup_populate_mappings): Likewise.
	(ctf_dedup_record_origin): Likewise.
	(ctf_dedup_rwalk_output_mapping): Likewise.
	(ctf_dedup_walk_output_mapping): Likewise.
	* ctf-dump.c (ctf_dump_append): Likewise.
	(ctf_dump_header): Likewise.
	(ctf_dump_header_sectfield): Likewise.
	(ctf_dump_header_strfield): Likewise.
	(ctf_dump_label): Likewise.
	(ctf_dump_member): Likewise.
	(ctf_dump_str): Likewise.
	(ctf_dump_type): Likewise.
	(ctf_dump_var): Likewise.
	* ctf-labels.c (ctf_label_info): Likewise.
	(ctf_label_iter): Likewise.
	* ctf-link.c (ctf_link_add_ctf_internal): Likewise.
	(ctf_link_add_cu_mapping): Likewise.
	(ctf_link_add): Likewise.
	(ctf_link_deduplicating_one_symtypetab): Likewise.
	(ctf_link_deduplicating_per_cu): Likewise.
	(ctf_link_deduplicating_variables): Likewise.
	(ctf_link): Likewise.
	(ctf_link_one_variable): Likewise.
	* ctf-lookup.c (ctf_func_args): Likewise.
	(ctf_func_info): Likewise.
	(grow_pptrtab): Likewise.
	* ctf-open.c (ctf_cuname_set): Likewise.
	(ctf_import): Likewise.
	(ctf_parent_name_set): Likewise.
	(ctf_setmodel): Likewise.
	* ctf-serialize.c (ctf_gzwrite): Likewise.
	(ctf_serialize): Likewise.
	(symtypetab_density): Likewise.
	* ctf-string.c (ctf_str_move_pending): Likewise.
	* ctf-types.c (ctf_array_info): Likewise.
	(ctf_func_type_info): Likewise.
	(ctf_member_count): Likewise.
	(ctf_member_info): Likewise.
	(ctf_member_next): Likewise.
	(ctf_type_align): Likewise.
	(ctf_type_encoding): Likewise.
	(ctf_type_rvisit): Likewise.
	(ctf_type_size): Likewise.

Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
Co-Authored-By: Yvan ROUX <yvan.roux@foss.st.com>
---
 libctf/ctf-create.c    | 137 ++++++++++++++++++++++++++++++++---------
 libctf/ctf-dedup.c     | 117 +++++++++++++++++++++++++----------
 libctf/ctf-dump.c      |  40 +++++++++---
 libctf/ctf-labels.c    |  15 +++--
 libctf/ctf-link.c      |  69 ++++++++++++++++-----
 libctf/ctf-lookup.c    |  15 ++++-
 libctf/ctf-open.c      |  33 +++++++---
 libctf/ctf-serialize.c |  43 +++++++++----
 libctf/ctf-string.c    |   5 +-
 libctf/ctf-types.c     |  78 +++++++++++++++++------
 10 files changed, 420 insertions(+), 132 deletions(-)
  

@Alan, any additional comments on this updated patch (except the missing 
semicolon that I mention below)?

Kind regards,
Torbjörn

On 2023-08-25 18:53, Torbjörn SVENSSON wrote:
> v1 -> v2:
> Changed all functions with signed interger return type to return -1 based on
> comment from Alan.
> 
> 
> Ok for trunk?
> 
> ---
> 
> The function ctf_member_next should return (ssize_t)-1 on
> error. As the function ctf_set_errno returns (ctf_id_t)-1L and that is
> then casted to "unsigned long" as it's the return type of the function,
> it's not compatible and causes the value 0xffffffff to be returned on
> 64-bit Windows builds. As a result, the check for a negative value in
> ctf_dedup_rhash_type will never be true and a resulting infinit loop is
> created.
> 
> This was found testing an arm-none-eabi toolchain built with
> x86_64-w64-mingw32. If the same source tree is built with
> i686-w64-mingw32, everything appears to be working correctly.
> 
> libctf/
> 	* ctf-create.c (ctf_add_enumerator): Ensure function returns -1 on error.
> 	(ctf_add_funcobjt_sym): Likewise.
> 	(ctf_add_member_encoded): Likewise.
> 	(ctf_add_member_offset): Likewise.
> 	(ctf_add_variable): Likewise.
> 	(ctf_grow_ptrtab): Likewise.
> 	(ctf_grow_vlen): Likewise.
> 	(ctf_rollback): Likewise.
> 	(ctf_set_array): Likewise.
> 	(ctf_update): Likewise.
> 	* ctf-dedup.c (ctf_dedup_atoms_init): Likewise.
> 	(ctf_dedup_conflictify_unshared): Likewise.
> 	(ctf_dedup_detect_name_ambiguity): Likewise.
> 	(ctf_dedup_emit_struct_members): Likewise.
> 	(ctf_dedup_emit_type): Likewise.
> 	(ctf_dedup_hash_kind): Likewise.
> 	(ctf_dedup_init): Likewise.
> 	(ctf_dedup_mark_conflicting_hash): Likewise.
> 	(ctf_dedup_multiple_input_dicts): Likewise.
> 	(ctf_dedup_populate_mappings): Likewise.
> 	(ctf_dedup_record_origin): Likewise.
> 	(ctf_dedup_rwalk_output_mapping): Likewise.
> 	(ctf_dedup_walk_output_mapping): Likewise.
> 	* ctf-dump.c (ctf_dump_append): Likewise.
> 	(ctf_dump_header): Likewise.
> 	(ctf_dump_header_sectfield): Likewise.
> 	(ctf_dump_header_strfield): Likewise.
> 	(ctf_dump_label): Likewise.
> 	(ctf_dump_member): Likewise.
> 	(ctf_dump_str): Likewise.
> 	(ctf_dump_type): Likewise.
> 	(ctf_dump_var): Likewise.
> 	* ctf-labels.c (ctf_label_info): Likewise.
> 	(ctf_label_iter): Likewise.
> 	* ctf-link.c (ctf_link_add_ctf_internal): Likewise.
> 	(ctf_link_add_cu_mapping): Likewise.
> 	(ctf_link_add): Likewise.
> 	(ctf_link_deduplicating_one_symtypetab): Likewise.
> 	(ctf_link_deduplicating_per_cu): Likewise.
> 	(ctf_link_deduplicating_variables): Likewise.
> 	(ctf_link): Likewise.
> 	(ctf_link_one_variable): Likewise.
> 	* ctf-lookup.c (ctf_func_args): Likewise.
> 	(ctf_func_info): Likewise.
> 	(grow_pptrtab): Likewise.
> 	* ctf-open.c (ctf_cuname_set): Likewise.
> 	(ctf_import): Likewise.
> 	(ctf_parent_name_set): Likewise.
> 	(ctf_setmodel): Likewise.
> 	* ctf-serialize.c (ctf_gzwrite): Likewise.
> 	(ctf_serialize): Likewise.
> 	(symtypetab_density): Likewise.
> 	* ctf-string.c (ctf_str_move_pending): Likewise.
> 	* ctf-types.c (ctf_array_info): Likewise.
> 	(ctf_func_type_info): Likewise.
> 	(ctf_member_count): Likewise.
> 	(ctf_member_info): Likewise.
> 	(ctf_member_next): Likewise.
> 	(ctf_type_align): Likewise.
> 	(ctf_type_encoding): Likewise.
> 	(ctf_type_rvisit): Likewise.
> 	(ctf_type_size): Likewise.
> 
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> Co-Authored-By: Yvan ROUX <yvan.roux@foss.st.com>
> ---
>   libctf/ctf-create.c    | 137 ++++++++++++++++++++++++++++++++---------
>   libctf/ctf-dedup.c     | 117 +++++++++++++++++++++++++----------
>   libctf/ctf-dump.c      |  40 +++++++++---
>   libctf/ctf-labels.c    |  15 +++--
>   libctf/ctf-link.c      |  69 ++++++++++++++++-----
>   libctf/ctf-lookup.c    |  15 ++++-
>   libctf/ctf-open.c      |  33 +++++++---
>   libctf/ctf-serialize.c |  43 +++++++++----
>   libctf/ctf-string.c    |   5 +-
>   libctf/ctf-types.c     |  78 +++++++++++++++++------
>   10 files changed, 420 insertions(+), 132 deletions(-)
> 
> diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c
> index 6b342dc64a2..91c466519f7 100644
> --- a/libctf/ctf-create.c
> +++ b/libctf/ctf-create.c
> @@ -60,7 +60,10 @@ ctf_grow_ptrtab (ctf_dict_t *fp)
>   
>         if ((new_ptrtab = realloc (fp->ctf_ptrtab,
>   				 new_ptrtab_len * sizeof (uint32_t))) == NULL)
> -	return (ctf_set_errno (fp, ENOMEM));
> +	{
> +	  ctf_set_errno (fp, ENOMEM);
> +	  return -1;
> +	}
>   
>         fp->ctf_ptrtab = new_ptrtab;
>         memset (fp->ctf_ptrtab + fp->ctf_ptrtab_len, 0,
> @@ -87,7 +90,8 @@ ctf_grow_vlen (ctf_dict_t *fp, ctf_dtdef_t *dtd, size_t vlen)
>   				dtd->dtd_vlen_alloc * 2)) == NULL)
>       {
>         dtd->dtd_vlen = old;
> -      return (ctf_set_errno (fp, ENOMEM));
> +      ctf_set_errno (fp, ENOMEM);
> +      return -1;
>       }
>     memset (dtd->dtd_vlen + dtd->dtd_vlen_alloc, 0, dtd->dtd_vlen_alloc);
>     dtd->dtd_vlen_alloc *= 2;
> @@ -197,7 +201,10 @@ int
>   ctf_update (ctf_dict_t *fp)
>   {
>     if (!(fp->ctf_flags & LCTF_RDWR))
> -    return (ctf_set_errno (fp, ECTF_RDONLY));
> +    {
> +      ctf_set_errno (fp, ECTF_RDONLY);
> +      return -1;
> +    }
>   
>     fp->ctf_dtoldid = fp->ctf_typemax;
>     return 0;
> @@ -391,10 +398,16 @@ ctf_rollback (ctf_dict_t *fp, ctf_snapshot_id_t id)
>     ctf_dvdef_t *dvd, *nvd;
>   
>     if (!(fp->ctf_flags & LCTF_RDWR))
> -    return (ctf_set_errno (fp, ECTF_RDONLY));
> +    {
> +      ctf_set_errno (fp, ECTF_RDONLY);
> +      return -1;
> +    }
>   
>     if (fp->ctf_snapshot_lu >= id.snapshot_id)
> -    return (ctf_set_errno (fp, ECTF_OVERROLLBACK));
> +    {
> +      ctf_set_errno (fp, ECTF_OVERROLLBACK);
> +      return -1;
> +    }
>   
>     for (dtd = ctf_list_next (&fp->ctf_dtdefs); dtd != NULL; dtd = ntd)
>       {
> @@ -723,11 +736,17 @@ ctf_set_array (ctf_dict_t *fp, ctf_id_t type, const ctf_arinfo_t *arp)
>     ctf_array_t *vlen;
>   
>     if (!(fp->ctf_flags & LCTF_RDWR))
> -    return (ctf_set_errno (fp, ECTF_RDONLY));
> +    {
> +      ctf_set_errno (fp, ECTF_RDONLY);
> +      return -1;
> +    }
>   
>     if (dtd == NULL
>         || LCTF_INFO_KIND (fp, dtd->dtd_data.ctt_info) != CTF_K_ARRAY)
> -    return (ctf_set_errno (fp, ECTF_BADID));
> +    {
> +      ctf_set_errno (fp, ECTF_BADID);
> +      return -1;
> +    }
>   
>     vlen = (ctf_array_t *) dtd->dtd_vlen;
>     fp->ctf_flags |= LCTF_DIRTY;
> @@ -1055,23 +1074,38 @@ ctf_add_enumerator (ctf_dict_t *fp, ctf_id_t enid, const char *name,
>     uint32_t kind, vlen, root;
>   
>     if (name == NULL)
> -    return (ctf_set_errno (fp, EINVAL));
> +    {
> +      ctf_set_errno (fp, EINVAL);
> +      return -1;
> +    }
>   
>     if (!(fp->ctf_flags & LCTF_RDWR))
> -    return (ctf_set_errno (fp, ECTF_RDONLY));
> +    {
> +      ctf_set_errno (fp, ECTF_RDONLY);
> +      return -1;
> +    }
>   
>     if (dtd == NULL)
> -    return (ctf_set_errno (fp, ECTF_BADID));
> +    {
> +      ctf_set_errno (fp, ECTF_BADID);
> +      return -1;
> +    }
>   
>     kind = LCTF_INFO_KIND (fp, dtd->dtd_data.ctt_info);
>     root = LCTF_INFO_ISROOT (fp, dtd->dtd_data.ctt_info);
>     vlen = LCTF_INFO_VLEN (fp, dtd->dtd_data.ctt_info);
>   
>     if (kind != CTF_K_ENUM)
> -    return (ctf_set_errno (fp, ECTF_NOTENUM));
> +    {
> +      ctf_set_errno (fp, ECTF_NOTENUM);
> +      return -1;
> +    }
>   
>     if (vlen == CTF_MAX_VLEN)
> -    return (ctf_set_errno (fp, ECTF_DTFULL));
> +    {
> +      ctf_set_errno (fp, ECTF_DTFULL);
> +      return -1;
> +    }
>   
>     old_vlen = dtd->dtd_vlen;
>     if (ctf_grow_vlen (fp, dtd, sizeof (ctf_enum_t) * (vlen + 1)) < 0)
> @@ -1090,7 +1124,10 @@ ctf_add_enumerator (ctf_dict_t *fp, ctf_id_t enid, const char *name,
>   
>     for (i = 0; i < vlen; i++)
>       if (strcmp (ctf_strptr (fp, en[i].cte_name), name) == 0)
> -      return (ctf_set_errno (fp, ECTF_DUPLICATE));
> +      {
> +	ctf_set_errno (fp, ECTF_DUPLICATE);
> +	return -1;
> +      }
>   
>     en[i].cte_name = ctf_str_add_pending (fp, name, &en[i].cte_name);
>     en[i].cte_value = value;
> @@ -1119,10 +1156,16 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
>     ctf_lmember_t *memb;
>   
>     if (!(fp->ctf_flags & LCTF_RDWR))
> -    return (ctf_set_errno (fp, ECTF_RDONLY));
> +    {
> +      ctf_set_errno (fp, ECTF_RDONLY);
> +      return -1;
> +    }
>   
>     if (dtd == NULL)
> -    return (ctf_set_errno (fp, ECTF_BADID));
> +    {
> +      ctf_set_errno (fp, ECTF_BADID);
> +      return -1;
> +    }
>   
>     if (name != NULL && name[0] == '\0')
>       name = NULL;
> @@ -1132,10 +1175,16 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
>     vlen = LCTF_INFO_VLEN (fp, dtd->dtd_data.ctt_info);
>   
>     if (kind != CTF_K_STRUCT && kind != CTF_K_UNION)
> -    return (ctf_set_errno (fp, ECTF_NOTSOU));
> +    {
> +      ctf_set_errno (fp, ECTF_NOTSOU);
> +      return -1;
> +    }
>   
>     if (vlen == CTF_MAX_VLEN)
> -    return (ctf_set_errno (fp, ECTF_DTFULL));
> +    {
> +      ctf_set_errno (fp, ECTF_DTFULL);
> +      return -1;
> +    }
>   
>     old_vlen = dtd->dtd_vlen;
>     if (ctf_grow_vlen (fp, dtd, sizeof (ctf_lmember_t) * (vlen + 1)) < 0)
> @@ -1156,7 +1205,10 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
>       {
>         for (i = 0; i < vlen; i++)
>   	if (strcmp (ctf_strptr (fp, memb[i].ctlm_name), name) == 0)
> -	  return (ctf_set_errno (fp, ECTF_DUPLICATE));
> +	  {
> +	    ctf_set_errno (fp, ECTF_DUPLICATE);
> +	    return -1;
> +	  }
>       }
>   
>     if ((msize = ctf_type_size (fp, type)) < 0 ||
> @@ -1212,7 +1264,8 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
>   			      "incomplete type %lx to struct %lx without "
>   			      "specifying explicit offset\n"),
>   			    name ? name : _("(unnamed member)"), type, souid);
> -	      return (ctf_set_errno (fp, ECTF_INCOMPLETE));
> +	      ctf_set_errno (fp, ECTF_INCOMPLETE);
> +	      return -1;
>   	    }
>   
>   	  if (ctf_type_encoding (fp, ltype, &linfo) == 0)
> @@ -1285,7 +1338,10 @@ ctf_add_member_encoded (ctf_dict_t *fp, ctf_id_t souid, const char *name,
>     int otype = type;
>   
>     if ((kind != CTF_K_INTEGER) && (kind != CTF_K_FLOAT) && (kind != CTF_K_ENUM))
> -    return (ctf_set_errno (fp, ECTF_NOTINTFP));
> +    {
> +      ctf_set_errno (fp, ECTF_NOTINTFP);
> +      return -1;
> +    }
>   
>     if ((type = ctf_add_slice (fp, CTF_ADD_NONROOT, otype, &encoding)) == CTF_ERR)
>       return -1;			/* errno is set for us.  */
> @@ -1307,10 +1363,16 @@ ctf_add_variable (ctf_dict_t *fp, const char *name, ctf_id_t ref)
>     ctf_dict_t *tmp = fp;
>   
>     if (!(fp->ctf_flags & LCTF_RDWR))
> -    return (ctf_set_errno (fp, ECTF_RDONLY));
> +    {
> +      ctf_set_errno (fp, ECTF_RDONLY);
> +      return -1;
> +    }
>   
>     if (ctf_dvd_lookup (fp, name) != NULL)
> -    return (ctf_set_errno (fp, ECTF_DUPLICATE));
> +    {
> +      ctf_set_errno (fp, ECTF_DUPLICATE);
> +      return -1;
> +    }
>   
>     if (ctf_lookup_by_id (&tmp, ref) == NULL)
>       return -1;			/* errno is set for us.  */
> @@ -1321,12 +1383,16 @@ ctf_add_variable (ctf_dict_t *fp, const char *name, ctf_id_t ref)
>       return -1;
>   
>     if ((dvd = malloc (sizeof (ctf_dvdef_t))) == NULL)
> -    return (ctf_set_errno (fp, EAGAIN));
> +    {
> +      ctf_set_errno (fp, EAGAIN);
> +      return -1;
> +    }
>   
>     if (name != NULL && (dvd->dvd_name = strdup (name)) == NULL)
>       {
>         free (dvd);
> -      return (ctf_set_errno (fp, EAGAIN));
> +      ctf_set_errno (fp, EAGAIN);
> +      return -1;
>       }
>     dvd->dvd_type = ref;
>     dvd->dvd_snapshots = fp->ctf_snapshots;
> @@ -1350,25 +1416,38 @@ ctf_add_funcobjt_sym (ctf_dict_t *fp, int is_function, const char *name, ctf_id_
>     ctf_dynhash_t *h = is_function ? fp->ctf_funchash : fp->ctf_objthash;
>   
>     if (!(fp->ctf_flags & LCTF_RDWR))
> -    return (ctf_set_errno (fp, ECTF_RDONLY));
> +    {
> +      ctf_set_errno (fp, ECTF_RDONLY);
> +      return -1;
> +    }
>   
>     if (ctf_dynhash_lookup (fp->ctf_objthash, name) != NULL ||
>         ctf_dynhash_lookup (fp->ctf_funchash, name) != NULL)
> -    return (ctf_set_errno (fp, ECTF_DUPLICATE));
> +    {
> +      ctf_set_errno (fp, ECTF_DUPLICATE);
> +      return -1;
> +    }
>   
>     if (ctf_lookup_by_id (&tmp, id) == NULL)
>       return -1;                                  /* errno is set for us.  */
>   
>     if (is_function && ctf_type_kind (fp, id) != CTF_K_FUNCTION)
> -    return (ctf_set_errno (fp, ECTF_NOTFUNC));
> +    {
> +      ctf_set_errno (fp, ECTF_NOTFUNC);
> +      return -1;
> +    }
>   
>     if ((dupname = strdup (name)) == NULL)
> -    return (ctf_set_errno (fp, ENOMEM));
> +    {
> +      ctf_set_errno (fp, ENOMEM);
> +      return -1;
> +    }
>   
>     if (ctf_dynhash_insert (h, dupname, (void *) (uintptr_t) id) < 0)
>       {
>         free (dupname);
> -      return (ctf_set_errno (fp, ENOMEM));
> +      ctf_set_errno (fp, ENOMEM);
> +      return -1;
>       }
>     return 0;
>   }
> diff --git a/libctf/ctf-dedup.c b/libctf/ctf-dedup.c
> index 5fdddfd0b54..f1c056caede 100644
> --- a/libctf/ctf-dedup.c
> +++ b/libctf/ctf-dedup.c
> @@ -378,7 +378,10 @@ ctf_dedup_atoms_init (ctf_dict_t *fp)
>         if ((fp->ctf_dedup_atoms_alloc
>   	   = ctf_dynset_create (htab_hash_string, htab_eq_string,
>   				free)) == NULL)
> -	return ctf_set_errno (fp, ENOMEM);
> +	{
> +	  ctf_set_errno (fp, ENOMEM);
> +	  return -1;
> +	}
>       }
>     fp->ctf_dedup_atoms = fp->ctf_dedup_atoms_alloc;
>     return 0;
> @@ -543,7 +546,10 @@ ctf_dedup_record_origin (ctf_dict_t *fp, int input_num, const char *decorated,
>   
>     if (populate_origin)
>       if (ctf_dynhash_cinsert (d->cd_struct_origin, decorated, origin) < 0)
> -      return ctf_set_errno (fp, errno);
> +      {
> +	ctf_set_errno (fp, errno);
> +	return -1;
> +      }
>     return 0;
>   }
>   
> @@ -1185,7 +1191,10 @@ ctf_dedup_populate_mappings (ctf_dict_t *fp, ctf_dict_t *input _libctf_unused_,
>       }
>     else
>       if (ctf_dynhash_cinsert (d->cd_output_mapping_guard, id, hval) < 0)
> -      return ctf_set_errno (fp, errno);
> +      {
> +	ctf_set_errno (fp, errno);
> +	return -1;
> +      }
>   #endif
>   
>     /* Record the type in the output mapping: if this is the first time this type
> @@ -1197,17 +1206,24 @@ ctf_dedup_populate_mappings (ctf_dict_t *fp, ctf_dict_t *input _libctf_unused_,
>   				      hval)) == NULL)
>       {
>         if (ctf_dynhash_cinsert (d->cd_output_first_gid, hval, id) < 0)
> -	return ctf_set_errno (fp, errno);
> +	{
> +	  ctf_set_errno (fp, errno);
> +	  return -1;
> +	}
>   
>         if ((type_ids = ctf_dynset_create (htab_hash_pointer,
>   					 htab_eq_pointer,
>   					 NULL)) == NULL)
> -	return ctf_set_errno (fp, errno);
> +	{
> +	  ctf_set_errno (fp, errno);
> +	  return -1;
> +	}
>         if (ctf_dynhash_insert (d->cd_output_mapping, (void *) hval,
>   			      type_ids) < 0)
>   	{
>   	  ctf_dynset_destroy (type_ids);
> -	  return ctf_set_errno (fp, errno);
> +	  ctf_set_errno (fp, errno);
> +	  return -1;
>   	}
>       }
>   #ifdef ENABLE_LIBCTF_HASH_DEBUGGING
> @@ -1248,7 +1264,10 @@ ctf_dedup_populate_mappings (ctf_dict_t *fp, ctf_dict_t *input _libctf_unused_,
>   	    }
>   	}
>         if (err != ECTF_NEXT_END)
> -	return ctf_set_errno (fp, err);
> +	{
> +	  ctf_set_errno (fp, err);
> +	  return -1;
> +	}
>       }
>   #endif
>   
> @@ -1256,7 +1275,10 @@ ctf_dedup_populate_mappings (ctf_dict_t *fp, ctf_dict_t *input _libctf_unused_,
>        don't waste time reinserting the same keys in that case.  */
>     if (!ctf_dynset_exists (type_ids, id, NULL)
>         && ctf_dynset_insert (type_ids, id) < 0)
> -    return ctf_set_errno (fp, errno);
> +    {
> +      ctf_set_errno (fp, errno);
> +      return -1;
> +    }
>   
>     /* The rest only needs to happen for types with names.  */
>     if (!decorated_name)
> @@ -1273,12 +1295,16 @@ ctf_dedup_populate_mappings (ctf_dict_t *fp, ctf_dict_t *input _libctf_unused_,
>         if ((name_counts = ctf_dynhash_create (ctf_hash_string,
>   					     ctf_hash_eq_string,
>   					     NULL, NULL)) == NULL)
> -	  return ctf_set_errno (fp, errno);
> +	{
> +	  ctf_set_errno (fp, errno);
> +	  return -1;
> +	}
>         if (ctf_dynhash_cinsert (d->cd_name_counts, decorated_name,
>   			       name_counts) < 0)
>   	{
>   	  ctf_dynhash_destroy (name_counts);
> -	  return ctf_set_errno (fp, errno);
> +	  ctf_set_errno (fp, errno);
> +	  return -1;
>   	}
>       }
>   
> @@ -1287,7 +1313,10 @@ ctf_dedup_populate_mappings (ctf_dict_t *fp, ctf_dict_t *input _libctf_unused_,
>   
>     if (ctf_dynhash_cinsert (name_counts, hval,
>   			   (const void *) (uintptr_t) (count + 1)) < 0)
> -    return ctf_set_errno (fp, errno);
> +    {
> +      ctf_set_errno (fp, errno);
> +      return -1;
> +    }
>   
>     return 0;
>   }
> @@ -1339,10 +1368,11 @@ ctf_dedup_mark_conflicting_hash (ctf_dict_t *fp, const char *hval)
>   	  return -1;				/* errno is set for us.  */
>   	}
>       }
> -  if (err != ECTF_NEXT_END)
> -    return ctf_set_errno (fp, err);
> +  if (err == ECTF_NEXT_END)
> +    return 0;
>   
> -  return 0;
> +  ctf_set_errno (fp, err);
> +  return -1;
>   }
>   
>   /* Look up a type kind from the output mapping, given a type hash value.  */
> @@ -1366,7 +1396,8 @@ ctf_dedup_hash_kind (ctf_dict_t *fp, ctf_dict_t **inputs, const char *hash)
>     if (!type_ids)
>       {
>         ctf_dprintf ("Looked up type kind by nonexistent hash %s.\n", hash);
> -      return ctf_set_errno (fp, ECTF_INTERNAL);
> +      ctf_set_errno (fp, ECTF_INTERNAL);
> +      return -1;
>       }
>     id = ctf_dynset_lookup_any (type_ids);
>     if (!ctf_assert (fp, id))
> @@ -1585,7 +1616,8 @@ ctf_dedup_detect_name_ambiguity (ctf_dict_t *fp, ctf_dict_t **inputs)
>   
>    iterr:
>     ctf_err_warn (fp, 0, err, _("iteration failed: %s"), gettext (whaterr));
> -  return ctf_set_errno (fp, err);
> +  ctf_set_errno (fp, err);
> +  return -1;
>   
>    assert_err:
>     ctf_next_destroy (i);
> @@ -1683,7 +1715,8 @@ ctf_dedup_init (ctf_dict_t *fp)
>    oom:
>     ctf_err_warn (fp, 0, ENOMEM, _("ctf_dedup_init: cannot initialize: "
>   				 "out of memory"));
> -  return ctf_set_errno (fp, ENOMEM);
> +  ctf_set_errno (fp, ENOMEM);
> +  return -1;
>   }
>   
>   /* No ctf_dedup calls are allowed after this call other than starting a new
> @@ -1784,7 +1817,8 @@ ctf_dedup_multiple_input_dicts (ctf_dict_t *output, ctf_dict_t **inputs,
>       {
>         ctf_err_warn (output, 0, err, _("iteration error "
>   				      "propagating conflictedness"));
> -      return ctf_set_errno (output, err);
> +      ctf_set_errno (output, err);
> +      return -1;
>       }
>   
>     if (multiple)
> @@ -1879,7 +1913,8 @@ ctf_dedup_conflictify_unshared (ctf_dict_t *output, ctf_dict_t **inputs)
>    iterr:
>     ctf_dynset_destroy (to_mark);
>     ctf_err_warn (output, 0, err, _("conflictifying unshared types"));
> -  return ctf_set_errno (output, err);
> +  ctf_set_errno (output, err);
> +  return -1;
>   }
>   
>   /* The core deduplicator.  Populate cd_output_mapping in the output ctf_dedup
> @@ -2219,7 +2254,8 @@ ctf_dedup_rwalk_output_mapping (ctf_dict_t *output, ctf_dict_t **inputs,
>       {
>         ctf_err_warn (output, 0, ECTF_INTERNAL,
>   		    _("looked up type kind by nonexistent hash %s"), hval);
> -      return ctf_set_errno (output, ECTF_INTERNAL);
> +      ctf_set_errno (output, ECTF_INTERNAL);
> +      return -1;
>       }
>   
>     /* Have we seen this type before?  */
> @@ -2237,7 +2273,8 @@ ctf_dedup_rwalk_output_mapping (ctf_dict_t *output, ctf_dict_t **inputs,
>   	{
>   	  ctf_err_warn (output, 0, ENOMEM,
>   			_("out of memory tracking already-visited types"));
> -	  return ctf_set_errno (output, ENOMEM);
> +	  ctf_set_errno (output, ENOMEM);
> +	  return -1;
>   	}
>       }
>   
> @@ -2274,7 +2311,8 @@ ctf_dedup_rwalk_output_mapping (ctf_dict_t *output, ctf_dict_t **inputs,
>     if (err != ECTF_NEXT_END)
>       {
>         ctf_err_warn (output, 0, err, _("cannot walk conflicted type"));
> -      return ctf_set_errno (output, err);
> +      ctf_set_errno (output, err);
> +      return -1;
>       }
>   
>     return 0;
> @@ -2376,7 +2414,10 @@ ctf_dedup_walk_output_mapping (ctf_dict_t *output, ctf_dict_t **inputs,
>     if ((already_visited = ctf_dynset_create (htab_hash_string,
>   					    htab_eq_string,
>   					    NULL)) == NULL)
> -    return ctf_set_errno (output, ENOMEM);
> +    {
> +      ctf_set_errno (output, ENOMEM);
> +      return -1;
> +    }
>   
>     sort_arg.inputs = inputs;
>     sort_arg.ninputs = ninputs;
> @@ -2664,7 +2705,8 @@ ctf_dedup_emit_type (const char *hval, ctf_dict_t *output, ctf_dict_t **inputs,
>   	      ctf_err_warn (output, 0, err,
>   			    _("cannot create per-CU CTF archive for CU %s"),
>   			    ctf_link_input_name (input));
> -	      return ctf_set_errno (output, err);
> +	      ctf_set_errno (output, err);
> +	      return -1;
>   	    }
>   
>   	  ctf_import_unref (target, output);
> @@ -2687,7 +2729,8 @@ ctf_dedup_emit_type (const char *hval, ctf_dict_t *output, ctf_dict_t **inputs,
>         ctf_err_warn (output, 0, ctf_errno (input),
>   		    _("%s: lookup failure for type %lx"),
>   		    ctf_link_input_name (real_input), type);
> -      return ctf_set_errno (output, ctf_errno (input));
> +      ctf_set_errno (output, ctf_errno (input));
> +      return -1;
>       }
>   
>     name = ctf_strraw (real_input, tp->ctt_name);
> @@ -2765,7 +2808,8 @@ ctf_dedup_emit_type (const char *hval, ctf_dict_t *output, ctf_dict_t **inputs,
>   			      ctf_link_input_name (input), input_num, name,
>   			      type);
>   		ctf_next_destroy (i);
> -		return ctf_set_errno (output, ctf_errno (target));
> +		ctf_set_errno (output, ctf_errno (target));
> +		return -1;
>   	      }
>   	  }
>   	if (ctf_errno (input) != ECTF_NEXT_END)
> @@ -2921,7 +2965,8 @@ ctf_dedup_emit_type (const char *hval, ctf_dict_t *output, ctf_dict_t **inputs,
>         ctf_err_warn (output, 0, ECTF_CORRUPT, _("%s: unknown type kind for "
>   					       "input type %lx"),
>   		    ctf_link_input_name (input), type);
> -      return ctf_set_errno (output, ECTF_CORRUPT);
> +      ctf_set_errno (output, ECTF_CORRUPT);
> +      return -1;
>       }
>   
>     if (!emission_hashed
> @@ -2931,7 +2976,8 @@ ctf_dedup_emit_type (const char *hval, ctf_dict_t *output, ctf_dict_t **inputs,
>       {
>         ctf_err_warn (output, 0, ENOMEM, _("out of memory tracking deduplicated "
>   					 "global type IDs"));
> -	return ctf_set_errno (output, ENOMEM);
> +      ctf_set_errno (output, ENOMEM);
> +      return -1;
>       }
>   
>     if (!emission_hashed && new_type != 0)
> @@ -2944,21 +2990,24 @@ ctf_dedup_emit_type (const char *hval, ctf_dict_t *output, ctf_dict_t **inputs,
>    oom_hash:
>     ctf_err_warn (output, 0, ENOMEM, _("out of memory creating emission-tracking "
>   				     "hashes"));
> -  return ctf_set_errno (output, ENOMEM);
> +  ctf_set_errno (output, ENOMEM);
> +  return -1;
>   
>    err_input:
>     ctf_err_warn (output, 0, ctf_errno (input),
>   		_("%s (%i): while emitting deduplicated %s, error getting "
>   		  "input type %lx"), ctf_link_input_name (input),
>   		input_num, errtype, type);
> -  return ctf_set_errno (output, ctf_errno (input));
> +  ctf_set_errno (output, ctf_errno (input));
> +  return -1;
>    err_target:
>     ctf_err_warn (output, 0, ctf_errno (target),
>   		_("%s (%i): while emitting deduplicated %s, error emitting "
>   		  "target type from input type %lx"),
>   		ctf_link_input_name (input), input_num,
>   		errtype, type);
> -  return ctf_set_errno (output, ctf_errno (target));
> +  ctf_set_errno (output, ctf_errno (target));
> +  return -1;
>   }
>   
>   /* Traverse the cd_emission_struct_members and emit the members of all
> @@ -3051,11 +3100,13 @@ ctf_dedup_emit_struct_members (ctf_dict_t *output, ctf_dict_t **inputs,
>     ctf_err_warn (output, 0, ctf_errno (err_fp),
>   		_("%s (%i): error emitting members for structure type %lx"),
>   		ctf_link_input_name (input_fp), input_num, err_type);
> -  return ctf_set_errno (output, ctf_errno (err_fp));
> +  ctf_set_errno (output, ctf_errno (err_fp));
> +  return -1;
>    iterr:
>     ctf_err_warn (output, 0, err, _("iteration failure emitting "
>   				  "structure members"));
> -  return ctf_set_errno (output, err);
> +  ctf_set_errno (output, err);
> +  return -1;
>   }
>   
>   /* Emit deduplicated types into the outputs.  The shared type repository is
> diff --git a/libctf/ctf-dump.c b/libctf/ctf-dump.c
> index 686951a9869..b5de3c76709 100644
> --- a/libctf/ctf-dump.c
> +++ b/libctf/ctf-dump.c
> @@ -56,7 +56,10 @@ ctf_dump_append (ctf_dump_state_t *state, char *str)
>     ctf_dump_item_t *cdi;
>   
>     if ((cdi = malloc (sizeof (struct ctf_dump_item))) == NULL)
> -    return (ctf_set_errno (state->cds_fp, ENOMEM));
> +    {
> +      ctf_set_errno (state->cds_fp, ENOMEM);
> +      return -1;
> +    }
>   
>     cdi->cdi_item = str;
>     ctf_list_append (&state->cds_items, cdi);
> @@ -261,7 +264,8 @@ ctf_dump_header_strfield (ctf_dict_t *fp, ctf_dump_state_t *state,
>     return 0;
>   
>    err:
> -  return (ctf_set_errno (fp, errno));
> +  ctf_set_errno (fp, errno);
> +  return -1;
>   }
>   
>   /* Dump one section-offset field from the file header into the cds_items.  */
> @@ -281,7 +285,8 @@ ctf_dump_header_sectfield (ctf_dict_t *fp, ctf_dump_state_t *state,
>     return 0;
>   
>    err:
> -  return (ctf_set_errno (fp, errno));
> +  ctf_set_errno (fp, errno);
> +  return -1;
>   }
>   
>   /* Dump the file header into the cds_items.  */
> @@ -398,7 +403,8 @@ ctf_dump_header (ctf_dict_t *fp, ctf_dump_state_t *state)
>     return 0;
>    err:
>     free (flagstr);
> -  return (ctf_set_errno (fp, errno));
> +  ctf_set_errno (fp, errno);
> +  return -1;
>   }
>   
>   /* Dump a single label into the cds_items.  */
> @@ -412,7 +418,10 @@ ctf_dump_label (const char *name, const ctf_lblinfo_t *info,
>     ctf_dump_state_t *state = arg;
>   
>     if (asprintf (&str, "%s -> ", name) < 0)
> -    return (ctf_set_errno (state->cds_fp, errno));
> +    {
> +      ctf_set_errno (state->cds_fp, errno);
> +      return -1;
> +    }
>   
>     if ((typestr = ctf_dump_format_type (state->cds_fp, info->ctb_type,
>   				       CTF_ADD_ROOT | CTF_FT_REFS)) == NULL)
> @@ -487,7 +496,10 @@ ctf_dump_var (const char *name, ctf_id_t type, void *arg)
>     ctf_dump_state_t *state = arg;
>   
>     if (asprintf (&str, "%s -> ", name) < 0)
> -    return (ctf_set_errno (state->cds_fp, errno));
> +    {
> +      ctf_set_errno (state->cds_fp, errno);
> +      return -1;
> +    }
>   
>     if ((typestr = ctf_dump_format_type (state->cds_fp, type,
>   				       CTF_ADD_ROOT | CTF_FT_REFS)) == NULL)
> @@ -540,7 +552,8 @@ ctf_dump_member (const char *name, ctf_id_t id, unsigned long offset,
>    oom:
>     free (typestr);
>     free (bit);
> -  return (ctf_set_errno (state->cdm_fp, errno));
> +  ctf_set_errno (state->cdm_fp, errno);
> +  return -1;
>   }
>   
>   /* Report the number of digits in the hexadecimal representation of a type
> @@ -569,7 +582,10 @@ ctf_dump_type (ctf_id_t id, int flag, void *arg)
>   
>     /* Indent neatly.  */
>     if (asprintf (&indent, "    %*s", type_hex_digits (id), "") < 0)
> -    return (ctf_set_errno (state->cds_fp, ENOMEM));
> +    {
> +      ctf_set_errno (state->cds_fp, ENOMEM);
> +      return -1;
> +    }
>   
>     /* Dump the type itself.  */
>     if ((str = ctf_dump_format_type (state->cds_fp, id,
> @@ -654,7 +670,8 @@ ctf_dump_type (ctf_id_t id, int flag, void *arg)
>    oom:
>     free (indent);
>     free (str);
> -  return ctf_set_errno (state->cds_fp, ENOMEM);
> +  ctf_set_errno (state->cds_fp, ENOMEM);
> +  return -1;
>   }
>   
>   /* Dump the string table into the cds_items.  */
> @@ -671,7 +688,10 @@ ctf_dump_str (ctf_dict_t *fp, ctf_dump_state_t *state)
>         if (asprintf (&str, "0x%lx: %s",
>   		    (unsigned long) (s - fp->ctf_str[CTF_STRTAB_0].cts_strs),
>   		    s) < 0)
> -	return (ctf_set_errno (fp, errno));
> +	{
> +	  ctf_set_errno (fp, errno);
> +	  return -1;
> +	}
>         ctf_dump_append (state, str);
>         s += strlen (s) + 1;
>       }
> diff --git a/libctf/ctf-labels.c b/libctf/ctf-labels.c
> index 16b111b14df..8aa2edc1847 100644
> --- a/libctf/ctf-labels.c
> +++ b/libctf/ctf-labels.c
> @@ -74,7 +74,10 @@ ctf_label_iter (ctf_dict_t *fp, ctf_label_f *func, void *arg)
>       return -1;			/* errno is set for us.  */
>   
>     if (num_labels == 0)
> -    return (ctf_set_errno (fp, ECTF_NOLABELDATA));
> +    {
> +      ctf_set_errno (fp, ECTF_NOLABELDATA);
> +      return -1;
> +    }
>   
>     for (i = 0; i < num_labels; i++, ctlp++)
>       {
> @@ -84,7 +87,8 @@ ctf_label_iter (ctf_dict_t *fp, ctf_label_f *func, void *arg)
>   	  ctf_err_warn (fp, 0, ECTF_CORRUPT,
>   			"failed to decode label %u with type %u",
>   			ctlp->ctl_label, ctlp->ctl_type);
> -	  return (ctf_set_errno (fp, ECTF_CORRUPT));
> +	  ctf_set_errno (fp, ECTF_CORRUPT);
> +	  return -1;
>   	}
>   
>         linfo.ctb_type = ctlp->ctl_type;
> @@ -133,8 +137,9 @@ ctf_label_info (ctf_dict_t *fp, const char *lname, ctf_lblinfo_t *linfo)
>     if ((rc = ctf_label_iter (fp, label_info_cb, &cb_arg)) < 0)
>       return rc;
>   
> -  if (rc != 1)
> -    return (ctf_set_errno (fp, ECTF_NOLABEL));
> +  if (rc == 1)
> +    return 0;
>   
> -  return 0;
> +  ctf_set_errno (fp, ECTF_NOLABEL);
> +  return -1;
>   }
> diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
> index 9babec2aa37..97beb36be83 100644
> --- a/libctf/ctf-link.c
> +++ b/libctf/ctf-link.c
> @@ -142,7 +142,8 @@ ctf_link_add_ctf_internal (ctf_dict_t *fp, ctf_archive_t *ctf,
>    oom1:
>     free (filename);
>    oom:
> -  return ctf_set_errno (fp, ENOMEM);
> +  ctf_set_errno (fp, ENOMEM);
> +  return -1;
>   }
>   
>   /* Add a file, memory buffer, or unopened file (by name) to a link.
> @@ -173,12 +174,18 @@ ctf_link_add (ctf_dict_t *fp, ctf_archive_t *ctf, const char *name,
>   	      void *buf _libctf_unused_, size_t n _libctf_unused_)
>   {
>     if (buf)
> -    return (ctf_set_errno (fp, ECTF_NOTYET));
> +    {
> +      ctf_set_errno (fp, ECTF_NOTYET);
> +      return -1;
> +    }
>   
>     if (!((ctf && name && !buf)
>   	|| (name && !buf && !ctf)
>   	|| (buf && name && !ctf)))
> -    return (ctf_set_errno (fp, EINVAL));
> +    {
> +      ctf_set_errno (fp, EINVAL);
> +      return -1;
> +    }
>   
>     /* We can only lazily open files if libctf.so is in use rather than
>        libctf-nobfd.so.  This is a little tricky: in shared libraries, we can use
> @@ -187,21 +194,33 @@ ctf_link_add (ctf_dict_t *fp, ctf_archive_t *ctf, const char *name,
>   
>   #if defined (PIC)
>     if (!buf && !ctf && name && !ctf_open)
> -    return (ctf_set_errno (fp, ECTF_NEEDSBFD));
> +    {
> +      ctf_set_errno (fp, ECTF_NEEDSBFD);
> +      return -1;
> +    }
>   #elif NOBFD
>     if (!buf && !ctf && name)
> -    return (ctf_set_errno (fp, ECTF_NEEDSBFD));
> +    {
> +      ctf_set_errno (fp, ECTF_NEEDSBFD);
> +      return -1;
> +    }
>   #endif
>   
>     if (fp->ctf_link_outputs)
> -    return (ctf_set_errno (fp, ECTF_LINKADDEDLATE));
> +    {
> +      ctf_set_errno (fp, ECTF_LINKADDEDLATE);
> +      return -1;
> +    }
>     if (fp->ctf_link_inputs == NULL)
>       fp->ctf_link_inputs = ctf_dynhash_create (ctf_hash_string,
>   					      ctf_hash_eq_string, free,
>   					      ctf_link_input_close);
>   
>     if (fp->ctf_link_inputs == NULL)
> -    return (ctf_set_errno (fp, ENOMEM));
> +    {
> +      ctf_set_errno (fp, ENOMEM);
> +      return -1;
> +    }
>   
>     return ctf_link_add_ctf_internal (fp, ctf, NULL, name);
>   }
> @@ -378,7 +397,10 @@ ctf_link_add_cu_mapping (ctf_dict_t *fp, const char *from, const char *to)
>   
>     /* Mappings cannot be set up if per-CU output dicts already exist.  */
>     if (fp->ctf_link_outputs && ctf_dynhash_elements (fp->ctf_link_outputs) != 0)
> -      return (ctf_set_errno (fp, ECTF_LINKADDEDLATE));
> +    {
> +      ctf_set_errno (fp, ECTF_LINKADDEDLATE);
> +      return -1;
> +    }
>   
>     if (fp->ctf_link_in_cu_mapping == NULL)
>       fp->ctf_link_in_cu_mapping = ctf_dynhash_create (ctf_hash_string,
> @@ -582,7 +604,10 @@ ctf_link_one_variable (ctf_dict_t *fp, ctf_dict_t *in_fp, const char *name,
>   
>     if (check_variable (name, per_cu_out_fp, dst_type, &dvd))
>       if (ctf_add_variable (per_cu_out_fp, name, dst_type) < 0)
> -      return (ctf_set_errno (fp, ctf_errno (per_cu_out_fp)));
> +      {
> +	ctf_set_errno (fp, ctf_errno (per_cu_out_fp));
> +	return -1;
> +      }
>     return 0;
>   }
>   
> @@ -915,7 +940,10 @@ ctf_link_deduplicating_variables (ctf_dict_t *fp, ctf_dict_t **inputs,
>   	    }
>   	}
>         if (ctf_errno (inputs[i]) != ECTF_NEXT_END)
> -	return ctf_set_errno (fp, ctf_errno (inputs[i]));
> +	{
> +	  ctf_set_errno (fp, ctf_errno (inputs[i]));
> +	  return -1;
> +	}
>   
>         /* Next the symbols.  We integrate data symbols even though the compiler
>   	 is currently doing the same, to allow the compiler to stop in
> @@ -930,7 +958,10 @@ ctf_link_deduplicating_variables (ctf_dict_t *fp, ctf_dict_t **inputs,
>   	    }
>   	}
>         if (ctf_errno (inputs[i]) != ECTF_NEXT_END)
> -	return ctf_set_errno (fp, ctf_errno (inputs[i]));
> +	{
> +	  ctf_set_errno (fp, ctf_errno (inputs[i]));
> +	  return -1;
> +	}
>   
>         /* Finally the function symbols.  */
>   
> @@ -943,7 +974,10 @@ ctf_link_deduplicating_variables (ctf_dict_t *fp, ctf_dict_t **inputs,
>   	    }
>   	}
>         if (ctf_errno (inputs[i]) != ECTF_NEXT_END)
> -	return ctf_set_errno (fp, ctf_errno (inputs[i]));
> +	{
> +	  ctf_set_errno (fp, ctf_errno (inputs[i]));
> +	  return -1;
> +	}
>       }
>     return 0;
>   }
> @@ -1070,7 +1104,8 @@ ctf_link_deduplicating_one_symtypetab (ctf_dict_t *fp, ctf_dict_t *input,
>   			_("symbol %s in input file %s found conflicting "
>   			  "even when trying in per-CU dict."), name,
>   			ctf_unnamed_cuname (input));
> -	  return (ctf_set_errno (fp, ECTF_DUPLICATE));
> +	  ctf_set_errno (fp, ECTF_DUPLICATE);
> +	  return -1;
>   	}
>       }
>     if (ctf_errno (input) != ECTF_NEXT_END)
> @@ -1330,7 +1365,8 @@ ctf_link_deduplicating_per_cu (ctf_dict_t *fp)
>       {
>         ctf_err_warn (fp, 0, err, _("iteration error in CU-mapped deduplicating "
>   				  "link"));
> -      return ctf_set_errno (fp, err);
> +      ctf_set_errno (fp, err);
> +      return -1;
>       }
>   
>     return 0;
> @@ -1503,7 +1539,10 @@ ctf_link (ctf_dict_t *fp, int flags)
>   					       ctf_dict_close);
>   
>     if (fp->ctf_link_outputs == NULL)
> -    return ctf_set_errno (fp, ENOMEM);
> +    {
> +      ctf_set_errno (fp, ENOMEM);
> +      return -1;
> +    }
>   
>     fp->ctf_flags |= LCTF_LINKING;
>     ctf_link_deduplicating (fp);
> diff --git a/libctf/ctf-lookup.c b/libctf/ctf-lookup.c
> index c65849118cb..7ad07e3bf84 100644
> --- a/libctf/ctf-lookup.c
> +++ b/libctf/ctf-lookup.c
> @@ -30,7 +30,10 @@ grow_pptrtab (ctf_dict_t *fp, size_t new_len)
>   
>     if ((new_pptrtab = realloc (fp->ctf_pptrtab, sizeof (uint32_t)
>   			      * new_len)) == NULL)
> -    return (ctf_set_errno (fp, ENOMEM));
> +    {
> +      ctf_set_errno (fp, ENOMEM);
> +      return -1;
> +    }
>   
>     fp->ctf_pptrtab = new_pptrtab;
>   
> @@ -1046,7 +1049,10 @@ ctf_func_info (ctf_dict_t *fp, unsigned long symidx, ctf_funcinfo_t *fip)
>       return -1;					/* errno is set for us.  */
>   
>     if (ctf_type_kind (fp, type) != CTF_K_FUNCTION)
> -    return (ctf_set_errno (fp, ECTF_NOTFUNC));
> +    {
> +      ctf_set_errno (fp, ECTF_NOTFUNC);
> +      return -1;
> +    }
>   
>     return ctf_func_type_info (fp, type, fip);
>   }
> @@ -1064,7 +1070,10 @@ ctf_func_args (ctf_dict_t *fp, unsigned long symidx, uint32_t argc,
>       return -1;					/* errno is set for us.  */
>   
>     if (ctf_type_kind (fp, type) != CTF_K_FUNCTION)
> -    return (ctf_set_errno (fp, ECTF_NOTFUNC));
> +    {
> +      ctf_set_errno (fp, ECTF_NOTFUNC);
> +      return -1;
> +    }
>   
>     return ctf_func_type_args (fp, type, argc, argv);
>   }
> diff --git a/libctf/ctf-open.c b/libctf/ctf-open.c
> index 35f635b6559..fa5f2aab732 100644
> --- a/libctf/ctf-open.c
> +++ b/libctf/ctf-open.c
> @@ -1935,7 +1935,10 @@ ctf_parent_name_set (ctf_dict_t *fp, const char *name)
>       free (fp->ctf_dynparname);
>   
>     if ((fp->ctf_dynparname = strdup (name)) == NULL)
> -    return (ctf_set_errno (fp, ENOMEM));
> +    {
> +      ctf_set_errno (fp, ENOMEM);
> +      return -1;
> +    }
>     fp->ctf_parname = fp->ctf_dynparname;
>     return 0;
>   }
> @@ -1956,7 +1959,10 @@ ctf_cuname_set (ctf_dict_t *fp, const char *name)
>       free (fp->ctf_dyncuname);
>   
>     if ((fp->ctf_dyncuname = strdup (name)) == NULL)
> -    return (ctf_set_errno (fp, ENOMEM));
> +    {
> +      ctf_set_errno (fp, ENOMEM);
> +      return -1;
> +    }
>     fp->ctf_cuname = fp->ctf_dyncuname;
>     return 0;
>   }
> @@ -1969,10 +1975,16 @@ int
>   ctf_import (ctf_dict_t *fp, ctf_dict_t *pfp)
>   {
>     if (fp == NULL || fp == pfp || (pfp != NULL && pfp->ctf_refcnt == 0))
> -    return (ctf_set_errno (fp, EINVAL));
> +    {
> +      ctf_set_errno (fp, EINVAL);
> +      return -1;
> +    }
>   
>     if (pfp != NULL && pfp->ctf_dmodel != fp->ctf_dmodel)
> -    return (ctf_set_errno (fp, ECTF_DMODEL));
> +    {
> +      ctf_set_errno (fp, ECTF_DMODEL);
> +      return -1;
> +    }
>   
>     if (fp->ctf_parent && !fp->ctf_parent_unreffed)
>       ctf_dict_close (fp->ctf_parent);
> @@ -2008,10 +2020,16 @@ int
>   ctf_import_unref (ctf_dict_t *fp, ctf_dict_t *pfp)
>   {
>     if (fp == NULL || fp == pfp || (pfp != NULL && pfp->ctf_refcnt == 0))
> -    return (ctf_set_errno (fp, EINVAL));
> +    {
> +      ctf_set_errno (fp, EINVAL);
> +      return -1;
> +    }
>   
>     if (pfp != NULL && pfp->ctf_dmodel != fp->ctf_dmodel)
> -    return (ctf_set_errno (fp, ECTF_DMODEL));
> +    {
> +      ctf_set_errno (fp, ECTF_DMODEL);
> +      return -1;
> +    }
>   
>     if (fp->ctf_parent && !fp->ctf_parent_unreffed)
>       ctf_dict_close (fp->ctf_parent);
> @@ -2052,7 +2070,8 @@ ctf_setmodel (ctf_dict_t *fp, int model)
>   	}
>       }
>   
> -  return (ctf_set_errno (fp, EINVAL));
> +  ctf_set_errno (fp, EINVAL);
> +  return -1;
>   }
>   
>   /* Return the data model constant for the CTF dict.  */
> diff --git a/libctf/ctf-serialize.c b/libctf/ctf-serialize.c
> index ba830a2b095..258dec62366 100644
> --- a/libctf/ctf-serialize.c
> +++ b/libctf/ctf-serialize.c
> @@ -123,7 +123,10 @@ symtypetab_density (ctf_dict_t *fp, ctf_dict_t *symfp, ctf_dynhash_t *symhash,
>   
>         if ((linker_known = ctf_dynhash_create (ctf_hash_string, ctf_hash_eq_string,
>   					      NULL, NULL)) == NULL)
> -	return (ctf_set_errno (fp, ENOMEM));
> +	{
> +	  ctf_set_errno (fp, ENOMEM);
> +	  return -1;
> +	}
>   
>         while ((err = ctf_dynhash_cnext (symfp->ctf_dynsyms, &i,
>   				       &name, &ctf_sym)) == 0)
> @@ -147,7 +150,8 @@ symtypetab_density (ctf_dict_t *fp, ctf_dict_t *symfp, ctf_dynhash_t *symhash,
>   	  if (ctf_dynhash_cinsert (linker_known, name, ctf_sym) < 0)
>   	    {
>   	      ctf_dynhash_destroy (linker_known);
> -	      return (ctf_set_errno (fp, ENOMEM));
> +	      ctf_set_errno (fp, ENOMEM);
> +	      return -1;
>   	    }
>   	}
>         if (err != ECTF_NEXT_END)
> @@ -155,7 +159,8 @@ symtypetab_density (ctf_dict_t *fp, ctf_dict_t *symfp, ctf_dynhash_t *symhash,
>   	  ctf_err_warn (fp, 0, err, _("iterating over linker-known symbols during "
>   				  "serialization"));
>   	  ctf_dynhash_destroy (linker_known);
> -	  return (ctf_set_errno (fp, err));
> +	  ctf_set_errno (fp, err);
> +	  return -1;
>   	}
>       }
>   
> @@ -219,7 +224,8 @@ symtypetab_density (ctf_dict_t *fp, ctf_dict_t *symfp, ctf_dynhash_t *symhash,
>         ctf_err_warn (fp, 0, err, _("iterating over CTF symtypetab during "
>   				  "serialization"));
>         ctf_dynhash_destroy (linker_known);
> -      return (ctf_set_errno (fp, err));
> +      ctf_set_errno (fp, err);
> +      return -1;
>       }
>   
>     if (!(flags & CTF_SYMTYPETAB_FORCE_INDEXED))
> @@ -236,7 +242,8 @@ symtypetab_density (ctf_dict_t *fp, ctf_dict_t *symfp, ctf_dynhash_t *symhash,
>   	  ctf_err_warn (fp, 0, err, _("iterating over linker-known symbols "
>   				      "during CTF serialization"));
>   	  ctf_dynhash_destroy (linker_known);
> -	  return (ctf_set_errno (fp, err));
> +	  ctf_set_errno (fp, err);
> +	  return -1;
>   	}
>       }
>   
> @@ -970,7 +977,10 @@ ctf_serialize (ctf_dict_t *fp)
>     memset (&symstate, 0, sizeof (emit_symtypetab_state_t));
>   
>     if (!(fp->ctf_flags & LCTF_RDWR))
> -    return (ctf_set_errno (fp, ECTF_RDONLY));
> +    {
> +      ctf_set_errno (fp, ECTF_RDONLY);
> +      return -1;
> +    }
>   
>     /* Update required?  */
>     if (!(fp->ctf_flags & LCTF_DIRTY))
> @@ -1026,7 +1036,10 @@ ctf_serialize (ctf_dict_t *fp)
>     buf_size = sizeof (ctf_header_t) + hdr.cth_stroff + hdr.cth_strlen;
>   
>     if ((buf = malloc (buf_size)) == NULL)
> -    return (ctf_set_errno (fp, EAGAIN));
> +    {
> +      ctf_set_errno (fp, EAGAIN);
> +      return -1;
> +    }
>   
>     memcpy (buf, &hdr, sizeof (ctf_header_t));
>     t = (unsigned char *) buf + sizeof (ctf_header_t) + hdr.cth_objtoff;
> @@ -1106,7 +1119,8 @@ ctf_serialize (ctf_dict_t *fp)
>   				       1, &err)) == NULL)
>       {
>         free (buf);
> -      return (ctf_set_errno (fp, err));
> +      ctf_set_errno (fp, err);
> +      return -1;
>       }
>   
>     (void) ctf_setmodel (nfp, ctf_getmodel (fp));
> @@ -1221,7 +1235,8 @@ ctf_serialize (ctf_dict_t *fp)
>   
>   oom:
>     free (buf);
> -  return (ctf_set_errno (fp, EAGAIN));
> +  ctf_set_errno (fp, EAGAIN);
> +  return -1;
>   err:
>     free (buf);
>     return -1;					/* errno is set for us.  */
> @@ -1248,7 +1263,10 @@ ctf_gzwrite (ctf_dict_t *fp, gzFile fd)
>     while (resid != 0)
>       {
>         if ((len = gzwrite (fd, buf, resid)) <= 0)
> -	return (ctf_set_errno (fp, errno));
> +	{
> +	  ctf_set_errno (fp, errno);
> +	  return -1;
> +	}
>         resid -= len;
>         buf += len;
>       }
> @@ -1258,7 +1276,10 @@ ctf_gzwrite (ctf_dict_t *fp, gzFile fd)
>     while (resid != 0)
>       {
>         if ((len = gzwrite (fd, buf, resid)) <= 0)
> -	return (ctf_set_errno (fp, errno));
> +	{
> +	  ctf_set_errno (fp, errno);
> +	  return -1;
> +	}
>         resid -= len;
>         buf += len;
>       }
> diff --git a/libctf/ctf-string.c b/libctf/ctf-string.c
> index 911e94700f1..ec8532fc47c 100644
> --- a/libctf/ctf-string.c
> +++ b/libctf/ctf-string.c
> @@ -298,7 +298,10 @@ ctf_str_move_pending (ctf_dict_t *fp, uint32_t *new_ref, ptrdiff_t bytes)
>       return 0;
>   
>     if (ctf_dynset_insert (fp->ctf_str_pending_ref, (void *) new_ref) < 0)
> -    return (ctf_set_errno (fp, ENOMEM));
> +    {
> +      ctf_set_errno (fp, ENOMEM);
> +      return -1;
> +    }
>   
>     ctf_dynset_remove (fp->ctf_str_pending_ref,
>   		     (void *) ((signed char *) new_ref - bytes));
> diff --git a/libctf/ctf-types.c b/libctf/ctf-types.c
> index c20ff825d9a..22848a8c66b 100644
> --- a/libctf/ctf-types.c
> +++ b/libctf/ctf-types.c
> @@ -119,7 +119,10 @@ ctf_member_next (ctf_dict_t *fp, ctf_id_t type, ctf_next_t **it,
>   	return -1;			/* errno is set for us.  */
>   
>         if ((i = ctf_next_create ()) == NULL)
> -	return ctf_set_errno (ofp, ENOMEM);
> +	{
> +	  ctf_set_errno (ofp, ENOMEM);
> +	  return -1;
> +	}
>         i->cu.ctn_fp = ofp;
>         i->ctn_tp = tp;
>   
> @@ -129,7 +132,8 @@ ctf_member_next (ctf_dict_t *fp, ctf_id_t type, ctf_next_t **it,
>         if (kind != CTF_K_STRUCT && kind != CTF_K_UNION)
>   	{
>   	  ctf_next_destroy (i);
> -	  return (ctf_set_errno (ofp, ECTF_NOTSOU));
> +	  ctf_set_errno (ofp, ECTF_NOTSOU);
> +	  return -1;
>   	}
>   
>         if ((dtd = ctf_dynamic_type (fp, type)) != NULL)
> @@ -150,14 +154,23 @@ ctf_member_next (ctf_dict_t *fp, ctf_id_t type, ctf_next_t **it,
>       }
>   
>     if ((void (*) (void)) ctf_member_next != i->ctn_iter_fun)
> -    return (ctf_set_errno (ofp, ECTF_NEXT_WRONGFUN));
> +    {
> +      ctf_set_errno (ofp, ECTF_NEXT_WRONGFUN);
> +      return -1

The semicolon got lost when I prepared the mail. It will obviously be 
there when pushed.

> +    }
>   
>     if (ofp != i->cu.ctn_fp)
> -    return (ctf_set_errno (ofp, ECTF_NEXT_WRONGFP));
> +    {
> +      ctf_set_errno (ofp, ECTF_NEXT_WRONGFP);
> +      return -1;
> +    }
>   
>     /* Resolve to the native dict of this type.  */
>     if ((fp = ctf_get_dict (ofp, type)) == NULL)
> -    return (ctf_set_errno (ofp, ECTF_NOPARENT));
> +    {
> +      ctf_set_errno (ofp, ECTF_NOPARENT);
> +      return -1;
> +    }
>   
>     max_vlen = LCTF_INFO_VLEN (fp, i->ctn_tp->ctt_info);
>   
> @@ -177,7 +190,10 @@ ctf_member_next (ctf_dict_t *fp, ctf_id_t type, ctf_next_t **it,
>   
>         if (ctf_struct_member (fp, &memb, i->ctn_tp, i->u.ctn_vlen, i->ctn_size,
>   			     i->ctn_n) < 0)
> -        return (ctf_set_errno (ofp, ctf_errno (fp)));
> +	{
> +	  ctf_set_errno (ofp, ctf_errno (fp));
> +	  return -1;
> +	}
>   
>         membname = ctf_strptr (fp, memb.ctlm_name);
>   
> @@ -221,7 +237,10 @@ ctf_member_next (ctf_dict_t *fp, ctf_id_t type, ctf_next_t **it,
>   	}
>   
>         if (!ctf_assert (fp, (i->ctn_next == NULL)))
> -        return (ctf_set_errno (ofp, ctf_errno (fp)));
> +	{
> +	  ctf_set_errno (ofp, ctf_errno (fp));
> +	  return -1;
> +	}
>   
>         i->ctn_type = 0;
>         /* This sub-struct has ended: on to the next real member.  */
> @@ -233,7 +252,8 @@ ctf_member_next (ctf_dict_t *fp, ctf_id_t type, ctf_next_t **it,
>    end_iter:
>     ctf_next_destroy (i);
>     *it = NULL;
> -  return ctf_set_errno (ofp, ECTF_NEXT_END);
> +  ctf_set_errno (ofp, ECTF_NEXT_END);
> +  return -1;
>   }
>   
>   /* Iterate over the members of an ENUM.  We pass the string name and associated
> @@ -956,7 +976,8 @@ ctf_type_size (ctf_dict_t *fp, ctf_id_t type)
>   
>       case CTF_K_FORWARD:
>         /* Forwards do not have a meaningful size.  */
> -      return (ctf_set_errno (ofp, ECTF_INCOMPLETE));
> +      ctf_set_errno (ofp, ECTF_INCOMPLETE);
> +      return -1;
>   
>       default: /* including slices of enums, etc */
>         return (ctf_get_ctt_size (fp, tp, NULL, NULL));
> @@ -1039,7 +1060,8 @@ ctf_type_align (ctf_dict_t *fp, ctf_id_t type)
>   
>       case CTF_K_FORWARD:
>         /* Forwards do not have a meaningful alignment.  */
> -      return (ctf_set_errno (ofp, ECTF_INCOMPLETE));
> +      ctf_set_errno (ofp, ECTF_INCOMPLETE);
> +      return -1;
>   
>       default:  /* including slices of enums, etc */
>         return (ctf_get_ctt_size (fp, tp, NULL, NULL));
> @@ -1235,7 +1257,8 @@ ctf_type_encoding (ctf_dict_t *fp, ctf_id_t type, ctf_encoding_t *ep)
>   	break;
>         }
>       default:
> -      return (ctf_set_errno (ofp, ECTF_NOTINTFP));
> +      ctf_set_errno (ofp, ECTF_NOTINTFP);
> +      return -1;
>       }
>   
>     return 0;
> @@ -1370,7 +1393,10 @@ ctf_member_count (ctf_dict_t *fp, ctf_id_t type)
>     kind = LCTF_INFO_KIND (fp, tp->ctt_info);
>   
>     if (kind != CTF_K_STRUCT && kind != CTF_K_UNION && kind != CTF_K_ENUM)
> -    return (ctf_set_errno (ofp, ECTF_NOTSUE));
> +    {
> +      ctf_set_errno (ofp, ECTF_NOTSUE);
> +      return -1;
> +    }
>   
>     return LCTF_INFO_VLEN (fp, tp->ctt_info);
>   }
> @@ -1398,7 +1424,10 @@ ctf_member_info (ctf_dict_t *fp, ctf_id_t type, const char *name,
>     kind = LCTF_INFO_KIND (fp, tp->ctt_info);
>   
>     if (kind != CTF_K_STRUCT && kind != CTF_K_UNION)
> -    return (ctf_set_errno (ofp, ECTF_NOTSOU));
> +    {
> +      ctf_set_errno (ofp, ECTF_NOTSOU);
> +      return -1;
> +    }
>   
>     n = LCTF_INFO_VLEN (fp, tp->ctt_info);
>     if ((dtd = ctf_dynamic_type (fp, type)) != NULL)
> @@ -1418,7 +1447,10 @@ ctf_member_info (ctf_dict_t *fp, ctf_id_t type, const char *name,
>         const char *membname;
>   
>         if (ctf_struct_member (fp, &memb, tp, vlen, vbytes, i) < 0)
> -        return (ctf_set_errno (ofp, ctf_errno (fp)));
> +	{
> +	  ctf_set_errno (ofp, ctf_errno (fp));
> +	  return -1;
> +	}
>   
>         membname = ctf_strptr (fp, memb.ctlm_name);
>   
> @@ -1439,7 +1471,8 @@ ctf_member_info (ctf_dict_t *fp, ctf_id_t type, const char *name,
>   	}
>       }
>   
> -  return (ctf_set_errno (ofp, ECTF_NOMEMBNAM));
> +  ctf_set_errno (ofp, ECTF_NOMEMBNAM);
> +  return -1;
>   }
>   
>   /* Return the array type, index, and size information for the specified ARRAY.  */
> @@ -1457,7 +1490,10 @@ ctf_array_info (ctf_dict_t *fp, ctf_id_t type, ctf_arinfo_t *arp)
>       return -1;			/* errno is set for us.  */
>   
>     if (LCTF_INFO_KIND (fp, tp->ctt_info) != CTF_K_ARRAY)
> -    return (ctf_set_errno (ofp, ECTF_NOTARRAY));
> +    {
> +      ctf_set_errno (ofp, ECTF_NOTARRAY);
> +      return -1;
> +    }
>   
>     if ((dtd = ctf_dynamic_type (ofp, type)) != NULL)
>       ap = (const ctf_array_t *) dtd->dtd_vlen;
> @@ -1584,7 +1620,10 @@ ctf_func_type_info (ctf_dict_t *fp, ctf_id_t type, ctf_funcinfo_t *fip)
>     kind = LCTF_INFO_KIND (fp, tp->ctt_info);
>   
>     if (kind != CTF_K_FUNCTION)
> -    return (ctf_set_errno (ofp, ECTF_NOTFUNC));
> +    {
> +      ctf_set_errno (ofp, ECTF_NOTFUNC);
> +      return -1;
> +    }
>   
>     fip->ctc_return = tp->ctt_type;
>     fip->ctc_flags = 0;
> @@ -1697,7 +1736,10 @@ ctf_type_rvisit (ctf_dict_t *fp, ctf_id_t type, ctf_visit_f *func,
>         ctf_lmember_t memb;
>   
>         if (ctf_struct_member (fp, &memb, tp, vlen, vbytes, i) < 0)
> -        return (ctf_set_errno (ofp, ctf_errno (fp)));
> +	{
> +	  ctf_set_errno (ofp, ctf_errno (fp));
> +	  return -1;
> +	}
>   
>         if ((rc = ctf_type_rvisit (fp, memb.ctlm_type,
>   				 func, arg, ctf_strptr (fp, memb.ctlm_name),
  

On Wed, Aug 30, 2023 at 10:34:05AM +0200, Torbjorn SVENSSON wrote:
> @Alan, any additional comments on this updated patch (except the missing
> semicolon that I mention below)?

I'm leaving it to Nick Alcock to decide what to do here.
  

On 30 Aug 2023, Alan Modra via Binutils outgrape:

> On Wed, Aug 30, 2023 at 10:34:05AM +0200, Torbjorn SVENSSON wrote:
>> @Alan, any additional comments on this updated patch (except the missing
>> semicolon that I mention below)?
>
> I'm leaving it to Nick Alcock to decide what to do here.

I agree that we should indeed be returning -1 from all functions that
return an int (it used to, but ctf_id_t has to be an unsigned long). But
I think it might be less disruptive to do so via a new
ctf_set_errno_int() which is just like ctf_set_errno but returns an int
rather than an unsigned long. That eliminates a lot of {}ery and makes
each individual hunk smaller.

My concern is that it's really hard to validate all this -- can anyone
think of a trick that would emit *consistent* warnings if we called
return (ctf_set_errno()) from a function returning int? I mean this
returning-int thing is a change I made ages ago, and despite making it
*and* attempting to validate on 64-bit Windows I have clearly not got it
right because it's drifted right out of correctness again.

(Similarly, does anyone have a build/target triplet on which this goes
wrong? because it's not going wrong on any of my mingw64 or cygwin tests
as far as I can tell.)

I kinda wish we could rely on having C11 -- type-generic macros are made
for cases like this :(
  

On 2023-09-07 14:10, Nick Alcock wrote:
> On 30 Aug 2023, Alan Modra via Binutils outgrape:
> 
>> On Wed, Aug 30, 2023 at 10:34:05AM +0200, Torbjorn SVENSSON wrote:
>>> @Alan, any additional comments on this updated patch (except the missing
>>> semicolon that I mention below)?
>>
>> I'm leaving it to Nick Alcock to decide what to do here.
> 
> I agree that we should indeed be returning -1 from all functions that
> return an int (it used to, but ctf_id_t has to be an unsigned long). But
> I think it might be less disruptive to do so via a new
> ctf_set_errno_int() which is just like ctf_set_errno but returns an int
> rather than an unsigned long. That eliminates a lot of {}ery and makes
> each individual hunk smaller.

Ok, I can do that instead if that's considered the proper way.

> My concern is that it's really hard to validate all this -- can anyone
> think of a trick that would emit *consistent* warnings if we called
> return (ctf_set_errno()) from a function returning int? I mean this
> returning-int thing is a change I made ages ago, and despite making it
> *and* attempting to validate on 64-bit Windows I have clearly not got it
> right because it's drifted right out of correctness again.
>
> (Similarly, does anyone have a build/target triplet on which this goes
> wrong? because it's not going wrong on any of my mingw64 or cygwin tests
> as far as I can tell.)

I discovered the issue using the GCC12 package for arm-none-eabi that 
Arm released 
(https://developer.arm.com/downloads/-/arm-gnu-toolchain-downloads), but 
built using the x86_64-w64-mingw GCC compiler on Linux.

I've opened a ticket for the issue where I've attached 2 object files 
that you can use to reproduce the issue without needing to rebuild GCC + 
multlibs to verify the problem.
https://sourceware.org/bugzilla/show_bug.cgi?id=30836

> I kinda wish we could rely on having C11 -- type-generic macros are made
> for cases like this :(

Looks like it would be a nice fix indeed, but is there anything else 
that could be done to improve the situation without needing to go to C11?

Kind regards,
Torbjörn
  

On 8 Sep 2023, Torbjorn SVENSSON told this:

> On 2023-09-07 14:10, Nick Alcock wrote:
>> On 30 Aug 2023, Alan Modra via Binutils outgrape:
>> 
>>> On Wed, Aug 30, 2023 at 10:34:05AM +0200, Torbjorn SVENSSON wrote:
>>>> @Alan, any additional comments on this updated patch (except the missing
>>>> semicolon that I mention below)?
>>>
>>> I'm leaving it to Nick Alcock to decide what to do here.
>> I agree that we should indeed be returning -1 from all functions that
>> return an int (it used to, but ctf_id_t has to be an unsigned long). But
>> I think it might be less disruptive to do so via a new
>> ctf_set_errno_int() which is just like ctf_set_errno but returns an int
>> rather than an unsigned long. That eliminates a lot of {}ery and makes
>> each individual hunk smaller.
>
> Ok, I can do that instead if that's considered the proper way.

I think it might be a good bit neater and a good bit less work :)

>> My concern is that it's really hard to validate all this -- can anyone
>> think of a trick that would emit *consistent* warnings if we called
>> return (ctf_set_errno()) from a function returning int? I mean this
>> returning-int thing is a change I made ages ago, and despite making it
>> *and* attempting to validate on 64-bit Windows I have clearly not got it
>> right because it's drifted right out of correctness again.
>>
>> (Similarly, does anyone have a build/target triplet on which this goes
>> wrong? because it's not going wrong on any of my mingw64 or cygwin tests
>> as far as I can tell.)
>
> I discovered the issue using the GCC12 package for arm-none-eabi that Arm released
> (https://developer.arm.com/downloads/-/arm-gnu-toolchain-downloads), but built using the x86_64-w64-mingw GCC compiler on Linux.

I'm wondering what the configure options were for binutils itself --
anything? was it a cross at all? (I'll admit this description has
confused me a bit: is this a cross from mingw64 to aarch64 or what?)

> I've opened a ticket for the issue where I've attached 2 object files that you can use to reproduce the issue without needing to
> rebuild GCC + multlibs to verify the problem.
> https://sourceware.org/bugzilla/show_bug.cgi?id=30836

Thanks! I hope I won't need them, but they might well come in handy...

>> I kinda wish we could rely on having C11 -- type-generic macros are made
>> for cases like this :(
>
> Looks like it would be a nice fix indeed, but is there anything else
> that could be done to improve the situation without needing to go to
> C11?

I've been trying to think of something other than adding whatever the
build is you saw this on to my regular test matrix.
  

On 2023-09-12 16:23, Nick Alcock wrote:
> On 8 Sep 2023, Torbjorn SVENSSON told this:
> 
>> On 2023-09-07 14:10, Nick Alcock wrote:
>>> On 30 Aug 2023, Alan Modra via Binutils outgrape:
>>>
>>>> On Wed, Aug 30, 2023 at 10:34:05AM +0200, Torbjorn SVENSSON wrote:
>>>>> @Alan, any additional comments on this updated patch (except the missing
>>>>> semicolon that I mention below)?
>>>>
>>>> I'm leaving it to Nick Alcock to decide what to do here.
>>> I agree that we should indeed be returning -1 from all functions that
>>> return an int (it used to, but ctf_id_t has to be an unsigned long). But
>>> I think it might be less disruptive to do so via a new
>>> ctf_set_errno_int() which is just like ctf_set_errno but returns an int
>>> rather than an unsigned long. That eliminates a lot of {}ery and makes
>>> each individual hunk smaller.
>>
>> Ok, I can do that instead if that's considered the proper way.
> 
> I think it might be a good bit neater and a good bit less work :)

Ok, I'll send a revised patch in the next few days.

>>> My concern is that it's really hard to validate all this -- can anyone
>>> think of a trick that would emit *consistent* warnings if we called
>>> return (ctf_set_errno()) from a function returning int? I mean this
>>> returning-int thing is a change I made ages ago, and despite making it
>>> *and* attempting to validate on 64-bit Windows I have clearly not got it
>>> right because it's drifted right out of correctness again.
>>>
>>> (Similarly, does anyone have a build/target triplet on which this goes
>>> wrong? because it's not going wrong on any of my mingw64 or cygwin tests
>>> as far as I can tell.)
>>
>> I discovered the issue using the GCC12 package for arm-none-eabi that Arm released
>> (https://developer.arm.com/downloads/-/arm-gnu-toolchain-downloads), but built using the x86_64-w64-mingw GCC compiler on Linux.
> 
> I'm wondering what the configure options were for binutils itself --
> anything? was it a cross at all? (I'll admit this description has
> confused me a bit: is this a cross from mingw64 to aarch64 or what?)

Sorry for not being more clear on this point.
This is the configure line used to reproduce the issue outside a full 
build of an arm-none-eabi toolchain. I'm sure that a few of the 
arguments to configure can be dropped without impact on the problem, but 
I leave that to the reader to decide.

INSTALL=/tmp/pr30836-libctf-inifinit-loop
./configure \
   --build=x86_64-linux-gnu \
   --host=x86_64-w64-mingw32 \
   --target=arm-none-eabi \
   --prefix=$INSTALL \
   --infodir=$INSTALL/share/doc/gcc-arm-none-eabi/info \
   --mandir=$INSTALL/share/doc/gcc-arm-none-eabi/man \
   --htmldir=$INSTALL/share/doc/gcc-arm-none-eabi/html \
   --pdfdir=$INSTALL/share/doc/gcc-arm-none-eabi/pdf \
   --disable-nls \
   --disable-werror \
   --disable-sim \
   --disable-gdb \
   --enable-interwork \
   --enable-plugins \
   --with-sysroot=$INSTALL/arm-none-eabi \
   --with-pkgversion=foo
make -j8 V=1 CFLAGS="-O0 -g"
make install


>> I've opened a ticket for the issue where I've attached 2 object files that you can use to reproduce the issue without needing to
>> rebuild GCC + multlibs to verify the problem.
>> https://sourceware.org/bugzilla/show_bug.cgi?id=30836
> 
> Thanks! I hope I won't need them, but they might well come in handy...

Using the above built binutils (ld.exe in this case) with the 
pr41893-1.o object file is enough to trigger the loop.

>>> I kinda wish we could rely on having C11 -- type-generic macros are made
>>> for cases like this :(
>>
>> Looks like it would be a nice fix indeed, but is there anything else
>> that could be done to improve the situation without needing to go to
>> C11?
> 
> I've been trying to think of something other than adding whatever the
> build is you saw this on to my regular test matrix.

You know both the code and the CTF format better than I do, so maybe you 
can take a look at the .ctf section in pr41893-1.o and see why it is 
wrongly handled. It might well be that the error that I stumbled on (not 
returning -1 on failure) is not the real problem... Maybe the real 
problem is something less obvious in the parsing of the object file itself?