Bug 49079 - [4.6/4.7 Regression] Bogus constant folding
[4.6/4.7 Regression] Bogus constant folding
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: tree-optimization
4.6.1
: P3 normal
: 4.6.1
Assigned To: Not yet assigned to anyone
: wrong-code
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-20 12:13 UTC by Richard Biener
Modified: 2011-05-20 15:11 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work: 4.5.3, 4.6.1
Known to fail: 4.6.0
Last reconfirmed: 2011-05-20 13:28:05


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2011-05-20 12:13:15 UTC
libustr testing fails because we constant fold the s2 initializer in the
following testcase despite the fact it can be modified by ustr_sc_ensure_owner.

extern void abort (void);

typedef long unsigned int size_t;

struct Ustr
{
  unsigned char data[1];
};

size_t ustr_xi__pow2(int, unsigned char);
int ustr_sized(const struct Ustr *);
int ustr_cmp_fast_buf(const struct Ustr*, const void*,size_t);
const char *ustr_cstr(const struct Ustr *);
int ustr_sc_ensure_owner(struct Ustr **);

extern inline __attribute__ ((__gnu_inline__))
size_t ustr_xi__embed_val_get(const unsigned char *data, size_t len)
{
  size_t ret = 0;

  switch (len)
    {
      case 0: return (-1);

      case 8:
              ret |= (((size_t)data[7]) << 56);
              ret |= (((size_t)data[6]) << 48);
              ret |= (((size_t)data[5]) << 40);
              ret |= (((size_t)data[4]) << 32);

      case 4:
              ret |= (((size_t)data[3]) << 24);
              ret |= (((size_t)data[2]) << 16);
      case 2:
              ret |= (((size_t)data[1]) << 8);
      case 1:
              ret |= (((size_t)data[0]) << 0);

              break;

      default: abort();
    }

  return (ret);
}

extern inline __attribute__ ((__gnu_inline__))
size_t ustr_len(const struct Ustr *s1)
{
  int usized;
  if (!s1->data[0])
    return (0);

  usized = ustr_sized(s1);
  return (ustr_xi__embed_val_get(s1->data + 1 + ustr_xi__pow2(usized, (s1)->data[0] >> 2),
                                 ustr_xi__pow2(usized, (s1)->data[0])));
}

extern inline __attribute__ ((__gnu_inline__))
int ustr_cmp_fast(const struct Ustr *s1,const struct Ustr*s2)
{
  if (s1 == s2)
    return (0);


  return (ustr_cmp_fast_buf(s1, ustr_cstr(s2), ustr_len(s2)));
}


extern inline __attribute__ ((__gnu_inline__))
int ustr_cmp_eq(const struct Ustr *s1, const struct Ustr *s2)
{ return (!ustr_cmp_fast(s1, s2)); }

static struct Ustr *s2 = ((struct Ustr *) "\x01" "\x02" "s2" "\0<ii-CONST_EOS-ii>");

int
main()
{
  ustr_sc_ensure_owner(&s2);

  if (!ustr_cmp_eq(s2, ((struct Ustr *) "\x01" "\x0002" "s2" "\0<ii-CONST_EOS-ii>")))
    abort ();

  return 0;
}

It seems to need the several steps of inlining to reproduce, sofar
I wasn't successfull in making a very simple testcase.  The folding
happens when inlining ustr_cmp_eq to main.
Comment 1 Richard Biener 2011-05-20 12:34:27 UTC
Hm, no, it's not constant-folding from s2, I misread.
Comment 2 Richard Biener 2011-05-20 13:05:23 UTC
We call ustr_cmp_fast_buf with
  s1, $3 = 0x6101b0 "\245\001\002s2", 1
when compiled with -O1 instead of with
  s1, $1 = 0x40ac06 "s2", 2
when compiled with -O0.
Comment 3 Richard Biener 2011-05-20 13:21:23 UTC
I think it goes wrong when folding during inlining of ustr_len.

  D.2729_11 = xi1_6 + 1;
  switch (xi2_9) <default: <L5>, case 0: <L6>, case 1: <L4>, case 2: <L3>, case 4: <L2>, case 8: <L1>>
...
<L4>:
  D.2772_47 = MEM[(const unsigned char *)s1_1(D)].data[D.2729_11]{lb: 0 sz: 1};

at runtime xi1_6 is zero.  After inlining we get

  D.2830_13 = xi1_9 + 1;
  switch (xi2_12) <default: <L7>, case 0: <L8>, case 1: <L6>, case 2: <L5>, case 4: <L4>, case 8: <L3>>
...
  # ret_45 = PHI <0(2), ret_42(5)>
<L6>:
  D.2828_43 = 1;

but we are called with s1_1 = "\1\2s2" so somehow the offset from the array
ref around the MEM is ignored.
Comment 4 Richard Biener 2011-05-20 13:28:05 UTC
Because the array length is of size 1 and get_ref_base_and_extent returns an
exact match in that case.  We have to account for trailing flexible arrays.

Testcase:

extern void abort (void);

struct Ustr
{
  unsigned char data[1];
};

static unsigned int
ustr_xi__embed_val_get(const unsigned char *data)
{
  return (unsigned int)data[0];
}

int __attribute__((noinline)) zero(void) { return 0; }

static unsigned int
ustr_len(const struct Ustr *s1)
{
  return ustr_xi__embed_val_get(s1->data + 1 + zero());
}

int
main()
{
  if (ustr_len (((struct Ustr *) "\x01" "\x0002" "s2")) != 2)
    abort ();

  return 0;
}
Comment 5 Jakub Jelinek 2011-05-20 13:45:32 UTC
Slightly deobfuscated testcase:
extern void abort (void);

struct U { unsigned char u[1]; };

static unsigned int
foo (const unsigned char *x)
{
  return (unsigned int) x[0];
}

__attribute__ ((noinline)) int
bar (void)
{
  return 0;
}

static unsigned int
baz (const struct U *x)
{
  return foo (x->u + 1 + bar ());
}

int
main ()
{
  if (baz (((struct U *) "abcd")) != 'b')
    abort ();
  return 0;
}

bisecting when it started...
Comment 6 Richard Biener 2011-05-20 14:13:27 UTC
I'm testing a patch already (probably caused by the mem-ref merge).
Comment 7 Jakub Jelinek 2011-05-20 14:29:04 UTC
Started in:
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164688
Comment 8 Richard Biener 2011-05-20 15:02:51 UTC
Author: rguenth
Date: Fri May 20 15:02:49 2011
New Revision: 173954

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=173954
Log:
2011-05-20  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/49079
	* tree-dfa.c (get_ref_base_and_extent): Handle view-converting
	MEM_REFs correctly for the trailing array access detection.
	Special case constants the same way as decls for overall size
	constraining.

	* gcc.dg/torture/pr49079.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr49079.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-dfa.c
Comment 9 Richard Biener 2011-05-20 15:08:59 UTC
Author: rguenth
Date: Fri May 20 15:08:56 2011
New Revision: 173955

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=173955
Log:
2011-05-20  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/49079
	* tree-dfa.c (get_ref_base_and_extent): Handle view-converting
	MEM_REFs correctly for the trailing array access detection.
	Special case constants the same way as decls for overall size
	constraining.

	* gcc.dg/torture/pr49079.c: New testcase.

Added:
    branches/gcc-4_6-branch/gcc/testsuite/gcc.dg/torture/pr49079.c
Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_6-branch/gcc/tree-dfa.c
Comment 10 Richard Biener 2011-05-20 15:11:35 UTC
Fixed.