User account creation filtered due to spam.

Bug 57915 - [4.8 Regression] ICE in set_address_disp, at rtlanal.c:5537
Summary: [4.8 Regression] ICE in set_address_disp, at rtlanal.c:5537
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.8.1
: P2 normal
Target Milestone: 4.9.0
Assignee: Jakub Jelinek
Depends on:
Reported: 2013-07-16 22:25 UTC by etienne_lorrain
Modified: 2015-06-23 08:38 UTC (History)
6 users (show)

See Also:
Target: i?86-*-*
Known to work: 4.7.3, 4.9.0
Known to fail: 4.8.1, 4.8.5
Last reconfirmed: 2013-09-12 00:00:00

gcc49-pr57915.patch (936 bytes, patch)
2014-01-31 15:24 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description etienne_lorrain 2013-07-16 22:25:13 UTC
On latest Fedora, with: gcc version 4.8.1 20130603 (Red Hat 4.8.1-1) (GCC)
I get:
  $ /usr/bin/gcc -m32 -Os bug.c
  bug.c: In function ‘menu’:
  bug.c:59:1: internal compiler error: in set_address_disp, at rtlanal.c:5537
  Please submit a full bug report,

possibly related to  asm(... "X" (*str)) , I use "X" mostly because I had problems using "m" when the pointer points to local variable on the stack.
The problem do not exists with previous compiler versions, not a new code.

The simplified source code I finished to get after few hours is:

$ cat bug.c
extern inline const char *
_strnchr (const char *str, char c, unsigned size)
  asm ("cld ; repne scasb %%es:(%%edi),%%al"
	: "+c" (size), "+D" (str): "a" (c), "X" (*str):"cc");
  return str - 1;

extern inline unsigned
strlen (const char *str)
  return _strnchr (str, '\0', (~0)) - str;

extern inline void
_strncpy (char *dst, const char *src, unsigned nb)

  unsigned len = strlen (src);
  if (len > nb)
    len = nb;

  __builtin_memcpy (dst, src, len);
  dst[len] = '\0';


extern inline char *
strcpy (char *dst, const char *src)
  _strncpy (dst, src, (~0));
  return dst;

typedef struct
  char go_msg[8];
  char scanpath[16];
} gujin_param_t;

extern gujin_param_t copy_gujin_param;
enum { kernel_bottom_menu, setup_bottom_menu } type;

unsigned short getkey(void);
unsigned timeout;
unsigned menu (void)
    char local_scanpath[sizeof (copy_gujin_param.scanpath)];
    strcpy (local_scanpath, copy_gujin_param.scanpath);

    for (;;)
	unsigned short key = getkey();
	if ((type == kernel_bottom_menu) ? (key == (0x1312)) : key == (0x3900 | ' '))
	    strcpy (local_scanpath, copy_gujin_param.scanpath);

Regards, Etienne.
Comment 1 Marek Polacek 2013-09-12 14:12:42 UTC
Confirmed with 4.8/trunk.
Comment 2 Marek Polacek 2013-09-12 14:48:07 UTC
Somewhat reduced...

bar (int *dst, int *src)
  asm ("cld ; repne scasb %%es:(%%edi),%%al": "+c" (dst), "+DaX" (*src));

  int a;
  int p[];
} s;

foo (void)
  while (1)
    bar (0, s.p);
Comment 3 Jakub Jelinek 2013-09-12 14:58:54 UTC
Started with r192719 aka LRA merge.
extern struct T { char a[8]; char b[16]; } t;
int c;
void foo (void);

extern inline char *
baz (char *x, const char *y)
  const char *e = y;
  unsigned long f, g;
  asm ("repne scasb" : "+c" (f), "+D" (e) : "a" ('\0'), "X" (*e));
  g = e - 1 - y;
  __builtin_memcpy (x, y, g);
  x[g] = '\0';
  return x;

bar (void)
  char d[16];
  baz (d, t.b);

  for (;;)
      foo ();
      if (c)
        baz (d, t.b);
Comment 4 Vladimir Makarov 2013-09-20 16:29:56 UTC
The address in question is

(plus (symbol_ref ...) (const_int 4))

LRA finds two displacements (symbol_ref and const_int) although only one displacement is allowed.

The correct canonical address should be:

(const (plus (symbol_ref ...) (const_int 4)))

Non-canonical address is created from

(reg ...)

by 1st constant propagation pass (cprop1).

I believe the problem should be fixed there.

As for reload pass, it has code transforming address (plus some const some const) into (const (plus some const some const)).  It was probably a problem fix in a wrong place.  There is no need to complicate LRA more and implement analogous code in LRA.  As I wrote I believe it should be fixed in cprop1 by generating the correct canonical address.
Comment 5 Richard Biener 2013-11-22 10:53:42 UTC
Still happens.
Comment 6 Andrey Belevantsev 2014-01-23 13:54:59 UTC
The cprop pass doesn't care much in this case, it only passes the resulting set to simplify_rtx which does nothing.  Fwprop does the right thing with the similar insn (insn 9) but doesn't work for insn in question (insn 58) because it is inside the loop:

1466      Do not forward propagate addresses into loops until after unrolling.
1467      CSE did so because it was able to fix its own mess, but we are not.  */

But come on, in this case the def/use are in the same basic block(!) so this stuff not fixed by fwprop is later fixed (wrongly) by _local_ cprop.  Fwprop2 is supposed to fix this but it surely runs after the 1st cprop so it doesn't have a chance.

So either we:

1) drop the loop check altogether -- forward_propagate_into already has code not to propagate into the inner loop; this might be too risky as I don't know all reasons of why the first fwprop does not propagate inside loops;

2) allow at least propagating into the same blocks to offset the local cprop code as in the below patch;

3) teach cprop to leave it for the second fwprop_addr -- but there's a lot of stuff happening in between;

4) teach cprop to simplify rtxes in line with the fwprop's propagate rtx code, i.e. diving inside and when looking at a mem, pass its address to simplify_gen_binary and other address-related things.

The below patch is the simplest option 2) and fixes the test case.

diff --git a/gcc/fwprop.c b/gcc/fwprop.c
index 4317f51..c62a501 100644
--- a/gcc/fwprop.c
+++ b/gcc/fwprop.c
@@ -1469,11 +1469,14 @@ fwprop (void)
   for (i = 0; i < DF_USES_TABLE_SIZE (); i++)
       df_ref use = DF_USES_GET (i);
+      df_ref def;
       if (use)
 	if (DF_REF_TYPE (use) == DF_REF_REG_USE
 	    || DF_REF_BB (use)->loop_father == NULL
 	    /* The outer most loop is not really a loop.  */
-	    || loop_outer (DF_REF_BB (use)->loop_father) == NULL)
+	    || loop_outer (DF_REF_BB (use)->loop_father) == NULL
+	    || ((def = get_def_for_use (use))
+		&& DF_REF_BB (use) == DF_REF_BB (def)))
 	  need_cleanup |= forward_propagate_into (use);
Comment 7 Jakub Jelinek 2014-01-31 15:24:12 UTC
I'd say if (plus:SI (symbol_ref:SI ...) (const_int ...)) not surrounded by (const:SI ) is considered invalid IL, then trying to workaround cprop bug in some other pass is still workaround rather than fix.
So, either we make sure that it is simplified (will attach an untested patch for that), or do some minimal sanity checking for 'X' operands in asm_operand_ok (basically verify it is valid RTL, say start with testing for this missing CONST), or both.
Comment 8 Jakub Jelinek 2014-01-31 15:24:59 UTC
Created attachment 32001 [details]

Untested fix.
Comment 9 Andrey Belevantsev 2014-02-03 09:28:29 UTC
(In reply to Jakub Jelinek from comment #7)
> I'd say if (plus:SI (symbol_ref:SI ...) (const_int ...)) not surrounded by
> (const:SI ) is considered invalid IL, then trying to workaround cprop bug in
> some other pass is still workaround rather than fix.

Sure, I agree that simplifying when inside cprop is the proper fix, though I wasn't sure which of the general interfaces to fix.  The question of why fwprop does not simplify this particular insn, i.e. whether the limitation of fwprop working within loops makes sense, still remains IMHO, but this is a separate issue.
Comment 10 Jakub Jelinek 2014-02-04 12:15:24 UTC
Author: jakub
Date: Tue Feb  4 12:14:52 2014
New Revision: 207460

	PR rtl-optimization/57915
	* recog.c (simplify_while_replacing): If all unary/binary/relational
	operation arguments are constant, attempt to simplify those.

	* New test.

Comment 11 Jakub Jelinek 2014-02-04 12:59:10 UTC
Fixed on the trunk so far.
Comment 12 Richard Biener 2014-05-22 09:03:03 UTC
GCC 4.8.3 is being released, adjusting target milestone.
Comment 13 Jakub Jelinek 2014-12-19 13:28:59 UTC
GCC 4.8.4 has been released.
Comment 14 Richard Biener 2015-06-23 08:38:29 UTC
Fixed for 4.9.0.