Re: libctf warnings

[Nick Alcock <nick.alcock@oracle.com>]

On 9 Apr 2024, Alan Modra said:

> Seen with every compiler I have:
> home/alan/src/binutils-gdb/libctf/ctf-create.c: In function ‘ctf_add_encoded’:
> /home/alan/src/binutils-gdb/libctf/ctf-create.c:555:3: warning: ‘encoding’ may be used uninitialized [-Wmaybe-uninitialized]
>   555 |   memcpy (dtd->dtd_vlen, &encoding, sizeof (encoding));

Never seen this one. Does it apply to prehistoric compilers only?

> Seen with gcc-4.9 and probably others at lower optimisation levels:
> home/alan/src/binutils-gdb/libctf/ctf-serialize.c: In function 'symtypetab_density':
> /home/alan/src/binutils-gdb/libctf/ctf-serialize.c:211:18: warning: 'sym' may be used uninitialized in this function [-Wmaybe-uninitialized]
>     if (*max < sym->st_symidx)

I'd like to fix this warning, but your fix is broken :( see below.

> Seen with gcc-4.5 and probably others at lower optimisation levels:
> /home/alan/src/binutils-gdb/libctf/ctf-types.c:1649:21: warning: 'tp' may be used uninitialized in this function
> /home/alan/src/binutils-gdb/libctf/ctf-link.c:765:16: warning: 'parent_i' may be used uninitialized in this function

Do we care about compilers that old? I know I never test with them. GCC
4.6 was released *ten years ago*. I'd be inclined to just ignore
all warnings on anything of remotely that vintage.

> Also with gcc-4.5:
> In file included from /home/alan/src/binutils-gdb/libctf/ctf-endian.h:25:0,
>                  from /home/alan/src/binutils-gdb/libctf/ctf-archive.c:24:
> /home/alan/src/binutils-gdb/libctf/swap.h:70:0: warning: "_Static_assert" redefined
> /usr/include/sys/cdefs.h:568:0: note: this is the location of the previous definition
>
> 	* swap.h (_Static_assert): Undef before defining.

Doesn't this break _Static_assert() if the compiler actually provides
it? I'd rather not do that.

> 	* ctf-create.c (ctf_add_encoded): Avoid "encoding" may be used
> 	uninitialized warning.

... hm, the warning is definitely an fp: ack. (Does this only happen
with really old GCCs?)

> 	* ctf-link.c (ctf_link_deduplicating_open_inputs): Avoid
> 	"parent_i" may be used uninitialized warning.
> 	* ctf-types.c (ctf_type_rvisit): Avoid "tp" may be used
> 	uninitialized warning.
>
> I'll apply this in a few days unless Nick objects.

*waves objection flag* though honestly the only bit that's really
objectionable is the bit that's actually broken, below (the rest is just
me wondering why on earth anyone would care about warnings seen only
with decade-old compilers).

> diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c
> index d0255e5ba7f..99ff4882fd9 100644
> --- a/libctf/ctf-create.c
> +++ b/libctf/ctf-create.c
> @@ -551,6 +551,10 @@ ctf_add_encoded (ctf_dict_t *fp, uint32_t flag,
>      case CTF_K_FLOAT:
>        encoding = CTF_FP_DATA (ep->cte_format, ep->cte_offset, ep->cte_bits);
>        break;
> +    default:
> +      /* ctf_assert is opaque to compilers.  This dead code avoids a
> +	 warning about "encoding" being used uninitialized.  */

Hmm. Do you know of any way to make it *less* opaque? :/

> diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
> index d5433b9d9bd..360bc1a0e63 100644
> --- a/libctf/ctf-link.c
> +++ b/libctf/ctf-link.c
> @@ -762,7 +762,7 @@ ctf_link_deduplicating_open_inputs (ctf_dict_t *fp, ctf_dynhash_t *cu_names,
>        ctf_link_input_t *one_input;
>        ctf_dict_t *one_fp;
>        ctf_dict_t *parent_fp = NULL;
> -      uint32_t parent_i;
> +      uint32_t parent_i = 0;
>        ctf_next_t *j = NULL;
>        /* If we are processing CU names, get the real input.  All the inputs

Again, if this is only seen with GCC 4.5, I'm tempted to just ignore
warnings from a compiler this prehistoric.

> diff --git a/libctf/ctf-serialize.c b/libctf/ctf-serialize.c
> index 8645f32ab20..11cbe75601e 100644
> --- a/libctf/ctf-serialize.c
> +++ b/libctf/ctf-serialize.c
> @@ -202,17 +202,15 @@ symtypetab_density (ctf_dict_t *fp, ctf_dict_t *symfp, ctf_dynhash_t *symhash,
>  	    }
>  
>  	  ctf_dynhash_remove (linker_known, name);
> -	}
> -      *unpadsize += sizeof (uint32_t);
> -      (*count)++;
>  
> -      if (!(flags & CTF_SYMTYPETAB_FORCE_INDEXED))
> -	{
>  	  if (*max < sym->st_symidx)
>  	    *max = sym->st_symidx;
>  	}
>        else
>  	(*max)++;
> +
> +      *unpadsize += sizeof (uint32_t);
> +      (*count)++;

I think this is a change in semantics. (That (*max++) branch is no
longer executed when sym->st_type != STT_FUNC, whether or not (flags &
CTF_SYMTYPETAB_EMIT_FUNCTION) is turned on.

I think just initializing sym to NULL to quiet the warning would be
safer.

> diff --git a/libctf/swap.h b/libctf/swap.h
> index d004dc14348..2dd20a27390 100644
> --- a/libctf/swap.h
> +++ b/libctf/swap.h
> @@ -67,6 +67,7 @@ bswap_64 (uint64_t v)
>  /* < C11? define away static assertions.  */
>  
>  #if !defined (__STDC_VERSION__) || __STDC_VERSION__ < 201112L
> +#undef _Static_assert
>  #define _Static_assert(cond, err)
>  #endif

I do wish there was some way to say 'use the compiler's _Static_assert'
if available', but there's no requirement it's a macro...

-- 
NULL && (void)