Bug 24851 - [4.1 Regression] f2c miscompilation
Summary: [4.1 Regression] f2c miscompilation
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.1.0
: P3 critical
Target Milestone: 4.1.0
Assignee: Andrew Pinski
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2005-11-14 15:26 UTC by Richard Biener
Modified: 2005-11-17 11:36 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-11-14 15:42:29


Attachments
testcase (7.27 KB, text/plain)
2005-11-14 15:27 UTC, Richard Biener
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2005-11-14 15:26:53 UTC
We miscompile yyparse of f2c to just

yyparse ()
{
...

<bb 0>:
  save1 = yylval;
  save2 = yyval;
  save3 = yynerrs;
  save4 = yyerrflag;
  yynerrs = 0;
  yyerrflag = 0;
  yyerror (&"yacc stack overflow"[0]);

ret:;
  yylval = save1;
  yyval = save2;
  yynerrs = save3;
  yyerrflag = save4;
  return 1;

}

which makes f2c pretty useless.  This happens at -O1.
Comment 1 Richard Biener 2005-11-14 15:27:39 UTC
Created attachment 10233 [details]
testcase

self-contained (but complete) yyparse function.
Comment 2 Richard Biener 2005-11-14 15:29:29 UTC
The problem is that with 31.ccp we introduce:

<bb 0>:
  save1 = yylval;
  save2 = yyval;
  save3_92 = yynerrs;
  save4_94 = yyerrflag;
  yystate_95 = 0;
  yychar_96 = -1;
  yynerrs = 0;
  yyerrflag = 0;
  yyp_99 = &yys[536870911];
  goto <bb 4> (yystack);

...

  # yychar_56 = PHI <-1(0), -1(13), yychar_61(31), ...
  # yystate_52 = PHI <0(0), yystate_2511(13), yystate_2467(31), ...
  # yyp_48 = PHI <&yys[536870911](0), yyp_129(13), yyp_49(31), ...
yystack:;
  yyp_129 = yyp_48 + 8B;
  D.1934_130 = &yys[150];
  if (yyp_129 >= &yys[150]) goto <L4>; else goto <L5>;

<L4>:;
  yyerror (&"yacc stack overflow"[0]);
  goto <bb 2> (ret1);

...

and later we will say that (yyp_129 >= &yys[150]) is always true because
of this.
Comment 3 Richard Biener 2005-11-14 15:40:46 UTC
Reduced testcase:

void abort(void);
int main()
{
  int a[10], *p;
  p = &a[-1];
  if (p >= &a[9])
    abort ();
  return 0;
}
Comment 4 Richard Biener 2005-11-14 15:42:01 UTC
C++ frontend is fine.  With C out of

(.03.gimple)
  p = &a + -4B;
  D.1282 = &a + 36B;

we (fold?) get

(.10.cleanup_cfg)
  p = &a[1073741823];
  D.1282 = &a[9];
Comment 5 Andrew Pinski 2005-11-14 15:42:29 UTC
Confirmed, short testcase:
int f(void)
{
  int i = -1;
  int a[10];
  int *b = &a[2];
  return &a[i]>=b;
}



int main(void)
{
  if (f())
    abort();
}
Comment 6 Andrew Pinski 2005-11-14 15:56:38 UTC
The problem here is that we are comparing in unsigned when we shoud be comparing in a signed type.

This was introduced by:
 2005-01-29  Richard Guenther <richard.guenther@uni-tuebingen.de>

        PR tree-optimization/15791
        * fold-const.c (extract_array_ref): New function.
        (fold): Fold comparisons between &a[i] and &a[j] or
        semantically equivalent trees.
Comment 7 Richard Biener 2005-11-14 16:27:47 UTC
Valid testcase (?):

void abort(void);
int main()
{
  int a[10], *p, *q;
  q = &a[1];
  p = &q[-1];
  if (p >= &a[9])
    abort ();
  return 0;
}

fold_stmt is doing the &a[0] + -4B to &a[1073741823] transformation.  But it
somehow looks the C representation of &a[X] with pointer addition is not a
good idea.  The C++ frontend makes out of the above

int main() ()
{
  int a[10];

<bb 0>:
  if (&a[9] <= &a[1073741824]) goto <L0>; else goto <L1>;

<L0>:;
  abort ();

<L1>:;
  return 0;

}

I'm unsure how to fix this without completely disabling the comparison
folding for anything but == and !=.
Comment 8 Richard Biener 2005-11-14 17:03:34 UTC
Patch to avoid the situation posted.
Comment 9 Andrew Pinski 2005-11-14 17:04:18 UTC
(In reply to comment #8)
> Patch to avoid the situation posted.

So this patch makes the real bug latent.
Comment 10 Andrew Pinski 2005-11-14 17:29:40 UTC
Actually I was wrong on which patch caused/exposed this,
This was caused by:
2005-05-14  Richard Guenther  <rguenth@gcc.gnu.org>

        * fold-const.c (div_if_zero_remainder): New function.
        (try_move_mult_to_index): Use it.

div_if_zero_remainder uses the unsigned ness of the domain, which is wrong.
Comment 11 Andrew Pinski 2005-11-14 17:31:44 UTC
div_if_zero_remainder assumes that the type of the agruments comming in are the same.
Comment 12 Andrew Pinski 2005-11-14 18:47:08 UTC
Note we have                        yyp = &yys[-1]; (where yys is an array) in the orginal testcase so that is undefined.
Comment 13 Andrew Pinski 2005-11-14 19:08:23 UTC
I have a fix for the only valid testcase (comment #7) here.
Comment 14 Andrew Pinski 2005-11-14 19:09:20 UTC
(In reply to comment #13)
> I have a fix for the only valid testcase (comment #7) here.

s/valid/defined/ 
Comment 15 Ian Lance Taylor 2005-11-16 01:09:44 UTC
Does it bother us that legal ISO C programs are not permitted to say &a[-1]?  You are permitted to take the address of the element immediately after the array, but you aren't permitted to take the address of the element before the array.

I suppose we should generally try to avoid breaking existing code, though.
Comment 16 Richard Biener 2005-11-16 09:39:22 UTC
Is the second reduced testcase not fine from a standards POV?  I.e.

void abort(void);
int main()
{
  int a[10], *p, *q;
  q = &a[1];
  p = &q[-1];
  if (p >= &a[9])
    abort ();
  return 0;
}

or does "array" in the standard refer to q[0]..q[8] here?
Comment 17 paolo.bonzini@lu.unisi.ch 2005-11-16 09:41:49 UTC
Subject: Re:  [4.1 Regression] f2c miscompilation

rguenth at gcc dot gnu dot org wrote:

>------- Comment #16 from rguenth at gcc dot gnu dot org  2005-11-16 09:39 -------
>Is the second reduced testcase not fine from a standards POV?  I.e.
>
>void abort(void);
>int main()
>{
>  int a[10], *p, *q;
>  q = &a[1];
>  p = &q[-1];
>  if (p >= &a[9])
>    abort ();
>  return 0;
>}
>
>or does "array" in the standard refer to q[0]..q[8] here?
>  
>
No, this is fine.

Paolo
Comment 18 Richard Biener 2005-11-17 11:35:05 UTC
Subject: Bug 24851

Author: rguenth
Date: Thu Nov 17 11:35:00 2005
New Revision: 107117

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=107117
Log:
2005-11-16  Richard Guenther  <rguenther@suse.de>

	PR middle-end/24851
	* fold-const.c (extract_array_ref): Return byte offset
	in all cases.
	(fold_binary): Fold &x[a] CMP &x[b] to
	a*sizeof(*x) CMP b*sizeof(*x) to get correct overflow
	behavior.

	* gcc.c-torture/execute/pr24851.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr24851.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog

Comment 19 Richard Biener 2005-11-17 11:36:59 UTC
Fixed.