This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR61762, reads from string constants
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 11 Jul 2014 12:08:00 +0200 (CEST)
- Subject: Re: [PATCH] Fix PR61762, reads from string constants
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1407101710260 dot 5753 at zhemvz dot fhfr dot qr> <20140710152843 dot GM31640 at tucnak dot redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1407110955150 dot 5753 at zhemvz dot fhfr dot qr> <20140711081134 dot GP31640 at tucnak dot redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1407111022580 dot 5753 at zhemvz dot fhfr dot qr> <alpine dot LSU dot 2 dot 11 dot 1407111110260 dot 5753 at zhemvz dot fhfr dot qr> <20140711093848 dot GQ31640 at tucnak dot redhat dot com>
On Fri, 11 Jul 2014, Jakub Jelinek wrote:
> 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.
Indeed. Fixed.
> > - 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;
Fixed.
> > @@ -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.
Oh, indeed. (reading that and native_encode_int I think we want
a native_encode_wide_int and avoid creating an integer cst for
encoding fixed_csts)
> > @@ -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
len will get negative so that might just be handled fine (fingers
crossing). I suppose I need to come up with a testcase that ends
up native-encoding a vector partially (SCCVN also benefits from
this work).
> b) what about the case when the first one returns 0 because you are asking
> for few bytes from the imag part?
Yeah. Sth like
> > part = TREE_IMAGPART (expr);
> > - isize = native_encode_expr (part, ptr+rsize, len-rsize);
> > - if (isize != rsize)
> > + if (off != -1)
> > + off = MAX (0, off - rsize);
if (off != -1)
off = MAX (0, off - GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (part))));
instead. Same mistake in vector encoding - we need to adjust
off by the size of the sub-object not by what was encoded.
> > + 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.
I have
elem = VECTOR_CST_ELT (expr, i);
int res = native_encode_expr (elem, ptr+offset, len-offset, off);
if (off == -1
&& res != size)
return 0;
offset += res;
if (off != -1)
off = MAX (0, off - size);
now, so 'offset' tracks the number of encoded bytes and off is
decremented properly. Now it will of course break if we fail
to encode one vector element but not another. But I hate
complicating things here ... (maybe guard all recursive calls
with off < size and check for a 0 return). Like
for (i = 0; i < count; i++)
{
if (off >= size)
{
off -= size;
continue;
}
elem = VECTOR_CST_ELT (expr, i);
int res = native_encode_expr (elem, ptr+offset, len-offset, off);
if ((off == -1 && res != size)
|| res == 0)
return 0;
offset += res;
if (off != -1)
off = 0;
?
> > if (TREE_STRING_LENGTH (expr) < total_bytes)
>
> No verification that you are not accessing beyond end of string here.
Changed to
if (TREE_STRING_LENGTH (expr) - off < MIN (total_bytes, len))
{
int written = 0;
if (off < TREE_STRING_LENGTH (expr))
{
written = MIN (len, TREE_STRING_LENGTH (expr) - off);
memcpy (ptr, TREE_STRING_POINTER (expr) + off, written);
}
memset (ptr + written, 0,
MIN (total_bytes - written, len - written));
}
else
memcpy (ptr, TREE_STRING_POINTER (expr) + off, MIN (total_bytes,
len));
ugh.
Now I have to think about getting at least some testing coverage ...
Richard.
Index: gcc/fold-const.c
===================================================================
*** gcc/fold-const.c.orig 2014-07-11 11:13:22.380607780 +0200
--- gcc/fold-const.c 2014-07-11 12:05:02.352394351 +0200
*************** fold_plusminus_mult_expr (location_t loc
*** 7239,7253 ****
upon failure. */
static int
! native_encode_int (const_tree expr, unsigned char *ptr, int len)
{
tree type = TREE_TYPE (expr);
int total_bytes = GET_MODE_SIZE (TYPE_MODE (type));
int byte, offset, word, words;
unsigned char value;
! if (total_bytes > len)
return 0;
words = total_bytes / UNITS_PER_WORD;
for (byte = 0; byte < total_bytes; byte++)
--- 7239,7256 ----
upon failure. */
static int
! native_encode_int (const_tree expr, unsigned char *ptr, int len, int off)
{
tree type = TREE_TYPE (expr);
int total_bytes = GET_MODE_SIZE (TYPE_MODE (type));
int byte, offset, word, words;
unsigned char value;
! if ((off == -1 && total_bytes > len)
! || off >= total_bytes)
return 0;
+ if (off == -1)
+ off = 0;
words = total_bytes / UNITS_PER_WORD;
for (byte = 0; byte < total_bytes; byte++)
*************** native_encode_int (const_tree expr, unsi
*** 7270,7278 ****
}
else
offset = BYTES_BIG_ENDIAN ? (total_bytes - 1) - byte : byte;
! ptr[offset] = value;
}
! return total_bytes;
}
--- 7273,7283 ----
}
else
offset = BYTES_BIG_ENDIAN ? (total_bytes - 1) - byte : byte;
! if (offset >= off
! && offset - off < len)
! ptr[offset - off] = value;
}
! return MIN (len, total_bytes - off);
}
*************** native_encode_int (const_tree expr, unsi
*** 7282,7288 ****
upon failure. */
static int
! native_encode_fixed (const_tree expr, unsigned char *ptr, int len)
{
tree type = TREE_TYPE (expr);
enum machine_mode mode = TYPE_MODE (type);
--- 7287,7293 ----
upon failure. */
static int
! native_encode_fixed (const_tree expr, unsigned char *ptr, int len, int off)
{
tree type = TREE_TYPE (expr);
enum machine_mode mode = TYPE_MODE (type);
*************** native_encode_fixed (const_tree expr, un
*** 7302,7308 ****
value = TREE_FIXED_CST (expr);
i_value = double_int_to_tree (i_type, value.data);
! return native_encode_int (i_value, ptr, len);
}
--- 7307,7313 ----
value = TREE_FIXED_CST (expr);
i_value = double_int_to_tree (i_type, value.data);
! return native_encode_int (i_value, ptr, len, off);
}
*************** native_encode_fixed (const_tree expr, un
*** 7312,7318 ****
upon failure. */
static int
! native_encode_real (const_tree expr, unsigned char *ptr, int len)
{
tree type = TREE_TYPE (expr);
int total_bytes = GET_MODE_SIZE (TYPE_MODE (type));
--- 7317,7323 ----
upon failure. */
static int
! native_encode_real (const_tree expr, unsigned char *ptr, int len, int off)
{
tree type = TREE_TYPE (expr);
int total_bytes = GET_MODE_SIZE (TYPE_MODE (type));
*************** native_encode_real (const_tree expr, uns
*** 7324,7331 ****
up to 192 bits. */
long tmp[6];
! if (total_bytes > len)
return 0;
words = (32 / BITS_PER_UNIT) / UNITS_PER_WORD;
real_to_target (tmp, TREE_REAL_CST_PTR (expr), TYPE_MODE (type));
--- 7329,7339 ----
up to 192 bits. */
long tmp[6];
! if ((off == -1 && total_bytes > len)
! || off >= total_bytes)
return 0;
+ if (off == -1)
+ off = 0;
words = (32 / BITS_PER_UNIT) / UNITS_PER_WORD;
real_to_target (tmp, TREE_REAL_CST_PTR (expr), TYPE_MODE (type));
*************** native_encode_real (const_tree expr, uns
*** 7349,7357 ****
}
else
offset = BYTES_BIG_ENDIAN ? 3 - byte : byte;
! ptr[offset + ((bitpos / BITS_PER_UNIT) & ~3)] = value;
}
! return total_bytes;
}
/* Subroutine of native_encode_expr. Encode the COMPLEX_CST
--- 7357,7368 ----
}
else
offset = BYTES_BIG_ENDIAN ? 3 - byte : byte;
! offset = offset + ((bitpos / BITS_PER_UNIT) & ~3);
! if (offset >= off
! && offset - off < len)
! ptr[offset - off] = value;
}
! return MIN (len, total_bytes - off);
}
/* Subroutine of native_encode_expr. Encode the COMPLEX_CST
*************** native_encode_real (const_tree expr, uns
*** 7360,7377 ****
upon failure. */
static int
! native_encode_complex (const_tree expr, unsigned char *ptr, int len)
{
int rsize, isize;
tree part;
part = TREE_REALPART (expr);
! rsize = native_encode_expr (part, ptr, len);
! if (rsize == 0)
return 0;
part = TREE_IMAGPART (expr);
! isize = native_encode_expr (part, ptr+rsize, len-rsize);
! if (isize != rsize)
return 0;
return rsize + isize;
}
--- 7371,7392 ----
upon failure. */
static int
! native_encode_complex (const_tree expr, unsigned char *ptr, int len, int off)
{
int rsize, isize;
tree part;
part = TREE_REALPART (expr);
! rsize = native_encode_expr (part, ptr, len, off);
! if (off == -1
! && rsize == 0)
return 0;
part = TREE_IMAGPART (expr);
! if (off != -1)
! off = MAX (0, off - GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (part))));
! isize = native_encode_expr (part, ptr+rsize, len-rsize, off);
! if (off == -1
! && isize != rsize)
return 0;
return rsize + isize;
}
*************** native_encode_complex (const_tree expr,
*** 7383,7389 ****
upon failure. */
static int
! native_encode_vector (const_tree expr, unsigned char *ptr, int len)
{
unsigned i, count;
int size, offset;
--- 7398,7404 ----
upon failure. */
static int
! native_encode_vector (const_tree expr, unsigned char *ptr, int len, int off)
{
unsigned i, count;
int size, offset;
*************** native_encode_vector (const_tree expr, u
*** 7395,7404 ****
size = GET_MODE_SIZE (TYPE_MODE (itype));
for (i = 0; i < count; i++)
{
elem = VECTOR_CST_ELT (expr, i);
! if (native_encode_expr (elem, ptr+offset, len-offset) != size)
return 0;
! offset += size;
}
return offset;
}
--- 7410,7428 ----
size = GET_MODE_SIZE (TYPE_MODE (itype));
for (i = 0; i < count; i++)
{
+ if (off >= size)
+ {
+ off -= size;
+ continue;
+ }
elem = VECTOR_CST_ELT (expr, i);
! int res = native_encode_expr (elem, ptr+offset, len-offset, off);
! if ((off == -1 && res != size)
! || res == 0)
return 0;
! offset += res;
! if (off != -1)
! off = 0;
}
return offset;
}
*************** native_encode_vector (const_tree expr, u
*** 7410,7416 ****
upon failure. */
static int
! native_encode_string (const_tree expr, unsigned char *ptr, int len)
{
tree type = TREE_TYPE (expr);
HOST_WIDE_INT total_bytes;
--- 7434,7440 ----
upon failure. */
static int
! native_encode_string (const_tree expr, unsigned char *ptr, int len, int off)
{
tree type = TREE_TYPE (expr);
HOST_WIDE_INT total_bytes;
*************** native_encode_string (const_tree expr, u
*** 7421,7437 ****
|| !tree_fits_shwi_p (TYPE_SIZE_UNIT (type)))
return 0;
total_bytes = tree_to_shwi (TYPE_SIZE_UNIT (type));
! if (total_bytes > len)
return 0;
! if (TREE_STRING_LENGTH (expr) < total_bytes)
{
! memcpy (ptr, TREE_STRING_POINTER (expr), TREE_STRING_LENGTH (expr));
! memset (ptr + TREE_STRING_LENGTH (expr), 0,
! total_bytes - TREE_STRING_LENGTH (expr));
}
else
! memcpy (ptr, TREE_STRING_POINTER (expr), total_bytes);
! return total_bytes;
}
--- 7445,7469 ----
|| !tree_fits_shwi_p (TYPE_SIZE_UNIT (type)))
return 0;
total_bytes = tree_to_shwi (TYPE_SIZE_UNIT (type));
! if ((off == -1 && total_bytes > len)
! || off >= total_bytes)
return 0;
! if (off == -1)
! off = 0;
! if (TREE_STRING_LENGTH (expr) - off < MIN (total_bytes, len))
{
! int written = 0;
! if (off < TREE_STRING_LENGTH (expr))
! {
! written = MIN (len, TREE_STRING_LENGTH (expr) - off);
! memcpy (ptr, TREE_STRING_POINTER (expr) + off, written);
! }
! memset (ptr + written, 0,
! MIN (total_bytes - written, len - written));
}
else
! memcpy (ptr, TREE_STRING_POINTER (expr) + off, MIN (total_bytes, len));
! return MIN (total_bytes - off, len);
}
*************** native_encode_string (const_tree expr, u
*** 7441,7467 ****
placed in the buffer, or zero upon failure. */
int
! native_encode_expr (const_tree expr, unsigned char *ptr, int len)
{
switch (TREE_CODE (expr))
{
case INTEGER_CST:
! return native_encode_int (expr, ptr, len);
case REAL_CST:
! return native_encode_real (expr, ptr, len);
case FIXED_CST:
! return native_encode_fixed (expr, ptr, len);
case COMPLEX_CST:
! return native_encode_complex (expr, ptr, len);
case VECTOR_CST:
! return native_encode_vector (expr, ptr, len);
case STRING_CST:
! return native_encode_string (expr, ptr, len);
default:
return 0;
--- 7473,7499 ----
placed in the buffer, or zero upon failure. */
int
! native_encode_expr (const_tree expr, unsigned char *ptr, int len, int off)
{
switch (TREE_CODE (expr))
{
case INTEGER_CST:
! return native_encode_int (expr, ptr, len, off);
case REAL_CST:
! return native_encode_real (expr, ptr, len, off);
case FIXED_CST:
! return native_encode_fixed (expr, ptr, len, off);
case COMPLEX_CST:
! return native_encode_complex (expr, ptr, len, off);
case VECTOR_CST:
! return native_encode_vector (expr, ptr, len, off);
case STRING_CST:
! return native_encode_string (expr, ptr, len, off);
default:
return 0;
Index: gcc/fold-const.h
===================================================================
*** gcc/fold-const.h.orig 2014-07-11 11:13:22.397607779 +0200
--- gcc/fold-const.h 2014-07-11 11:14:59.472601095 +0200
*************** along with GCC; see the file COPYING3.
*** 25,31 ****
extern int folding_initializer;
/* Convert between trees and native memory representation. */
! extern int native_encode_expr (const_tree, unsigned char *, int);
extern tree native_interpret_expr (tree, const unsigned char *, int);
/* Fold constants as much as possible in an expression.
--- 25,31 ----
extern int folding_initializer;
/* Convert between trees and native memory representation. */
! extern int native_encode_expr (const_tree, unsigned char *, int, int off = -1);
extern tree native_interpret_expr (tree, const unsigned char *, int);
/* Fold constants as much as possible in an expression.