This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix PR61762, reads from string constants


On Fri, Jul 11, 2014 at 11:13:12AM +0200, Richard Biener wrote:
> +      if (offset >= off)
> +       ptr[offset - off] = value;

For original off != 0, you aren't checking whether offset - off < len
though (various places), you don't want to write beyond end of buffer.

> -  return total_bytes;
> +  return total_bytes - off;

What will happen with originally off is bigger than total_bytes?
Returning negative value sounds wrong, IMHO you should in that case return
early 0.  So like:
if ((off == -1 && total_bytes > len) || off >= total_bytes)
  return 0;

> @@ -7290,7 +7293,8 @@ native_encode_fixed (const_tree expr, un
>    FIXED_VALUE_TYPE value;
>    tree i_value, i_type;
>  
> -  if (total_bytes * BITS_PER_UNIT > HOST_BITS_PER_DOUBLE_INT)
> +  if (off == -1
> +      && total_bytes * BITS_PER_UNIT > HOST_BITS_PER_DOUBLE_INT)
>      return 0;

This isn't comparing total_bytes to len, so IMHO shouldn't be changed.

> @@ -7324,8 +7328,11 @@ native_encode_real (const_tree expr, uns
>       up to 192 bits.  */
>    long tmp[6];
>  
> -  if (total_bytes > len)
> +  if (off == -1
> +      && total_bytes > len)

This can fit onto one line.

> -  rsize = native_encode_expr (part, ptr, len);
> +  rsize = native_encode_expr (part, ptr, len, off);
>    if (rsize == 0)
>      return 0;

If off is not -1 and len is too short, the above will do a partial
store.  But:
a) if it is a partial store, because some bytes didn't fit, then the
second native_encode_expr should probably not be invoked
b) what about the case when the first one returns 0 because you are asking
for few bytes from the imag part?

>    part = TREE_IMAGPART (expr);
> -  isize = native_encode_expr (part, ptr+rsize, len-rsize);
> -  if (isize != rsize)
> +  if (off != -1)
> +    off = MAX (0, off - rsize);
> +  isize = native_encode_expr (part, ptr+rsize, len-rsize, off);
> +  if (off == -1
> +      && isize != rsize)
>      return 0;
>    return rsize + isize;
>  }

> @@ -7396,9 +7408,13 @@ native_encode_vector (const_tree expr, u
>    for (i = 0; i < count; i++)
>      {
>        elem = VECTOR_CST_ELT (expr, i);
> -      if (native_encode_expr (elem, ptr+offset, len-offset) != size)
> +      int res = native_encode_expr (elem, ptr+offset, len-offset, off);
> +      if (off == -1
> +	  && res != size)
>  	return 0;

I don't think this will work correctly if off is not -1.

>    if (TREE_STRING_LENGTH (expr) < total_bytes)

No verification that you are not accessing beyond end of string here.
>      {
> -      memcpy (ptr, TREE_STRING_POINTER (expr), TREE_STRING_LENGTH (expr));
> -      memset (ptr + TREE_STRING_LENGTH (expr), 0,
> -	      total_bytes - TREE_STRING_LENGTH (expr));
> +      memcpy (ptr, TREE_STRING_POINTER (expr) + off,
> +	      TREE_STRING_LENGTH (expr) - off);
> +      memset (ptr + TREE_STRING_LENGTH (expr) - off, 0,
> +	      MIN (total_bytes, len) - TREE_STRING_LENGTH (expr) + off);
>      }
>    else
> -    memcpy (ptr, TREE_STRING_POINTER (expr), total_bytes);
> -  return total_bytes;
> +    memcpy (ptr, TREE_STRING_POINTER (expr) + off,
> +	    MIN (total_bytes, len));
> +  return MIN (total_bytes - off, len);
>  }

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]