Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)

[Jakub Jelinek <jakub@redhat.com>]

On Tue, Feb 16, 2021 at 08:34:41PM -0700, Martin Sebor via Gcc-patches wrote:
> +	  if (integer_all_onesp (nelts))
> +	    /* Zero length array.  */
> +	    eltsize = 0;
> +	  else
>  	    {
> -	      tree bnds[] = { TYPE_MIN_VALUE (dom), TYPE_MAX_VALUE (dom) };
> -	      if (TREE_CODE (arg) == COMPONENT_REF)
> -		{
> -		  offset_int size = maxobjsize;
> -		  if (tree fldsize = component_ref_size (arg))
> -		    size = wi::to_offset (fldsize);
> -		  arrbounds[1] = wi::lrshift (size, eltsizelog2);
> -		}
> -	      else if (array_at_struct_end_p (arg) || !bnds[0] || !bnds[1])
> -		arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
> -	      else
> -		arrbounds[1] = (wi::to_offset (bnds[1]) - wi::to_offset (bnds[0])
> -				+ 1) * eltsize;
> +	      tree esz = TYPE_SIZE_UNIT (TREE_TYPE (reftype));
> +	      if (TREE_CODE (esz) == INTEGER_CST)
> +		/* Array element is not a VLA.  */
> +		eltsize = wi::to_offset (esz);
>  	    }
> +
> +	  if (!array_at_struct_end_p (arg)
> +	      && TREE_CODE (nelts) == INTEGER_CST)
> +	    arrbounds[1] = (wi::to_offset (nelts) + 1) * eltsize;
>  	  else
> -	    arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);
> +	    {
> +	      /* Use log2 of size to convert the array byte size in to its
> +		 upper bound in elements.  */
> +	      const offset_int eltsizelog2 = wi::floor_log2 (eltsize);
> +	      arrbounds[1] = wi::lrshift (maxobjsize, eltsizelog2);

So, what will this do for zero length arrays at the end of struct?
eltsize will be 0 and wi::floor_log2 (eltsize) is -1, and shifting by -1,
while maybe not UB in wide_int computations, is certainly just weird.
Why do you use eltsize = 0 for the [0] arrays?  They can still have their
element size and if array_at_struct_end_p and the element type is not a variable
length type, using the actual eltsize seems better.
Only when !array_at_struct_end_p we should ensure arrbounds[1] will be 0
and indeed that (wi::to_offset (nelts) + 1) * eltsize would likely not do
that because wi::to_offset is -1ULL or so, not -1.

Also, I'm not sure I understand the right shift by floor_log2 of eltsize,
why can't you simply divide maxobjsize by eltsize (if eltsize is not 0).

	Jakub