[Bug c/38587] New: psim miscompiled #2 [regression]

[Bug c/38587] New: psim miscompiled #2 [regression]

[joel at gcc dot gnu dot org]

GNATLS 4.4.0 20081209 (experimental) [trunk revision 142610]

idecode.c:6591: assertion failed - current_cpu >= 0 && current_cpu < nr_cpus

It may be after the new year before I can investigate further.  I have
a test sweep of 20081219 running now on gcc12 and once it gets past the powerpc
I will try to narrow it down using the techniques suggested on the previous
PR like this and update the PR.


-- 
           Summary: psim miscompiled #2 [regression]
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: joel at gcc dot gnu dot org
  GCC host triplet: x86_64-unknown-linux-gnu
GCC target triplet: x86_64-unknown-linux-gnu


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] psim miscompiled #2 [regression]

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2008-12-20 18:46 -------
This sounds more likely an overflow issue.  Does -fno-strict-overflow make this
work?


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |middle-end


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug c/38587] psim miscompiled #2 [regression]

[joel at gcc dot gnu dot org]

------- Comment #2 from joel at gcc dot gnu dot org  2008-12-21 02:15 -------
(In reply to comment #1)
> This sounds more likely an overflow issue.  Does -fno-strict-overflow make this
> work?
> 

No.  "-O2 -fno-strict-overflow" did not work.
I also tried with -O1 and it did not work.
With -O0, it worked.  

If I recompile idecode.c with -O2 and relink psim, then the failure returns. 
So the problem is with the generation of that single file. What can I
do to help more?


-- 

joel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|middle-end                  |c


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] psim miscompiled #2 [regression]

[joel at gcc dot gnu dot org]

------- Comment #3 from joel at gcc dot gnu dot org  2009-01-05 14:32 -------
Issue appears to have been independently resolved as of:

gcc (GCC) 4.4.0 20090105 (experimental) [trunk revision 143068]

I will make a full test run on the powerpc to make sure and close this if that
run is OK.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] psim miscompiled #2 [regression]

[joel at gcc dot gnu dot org]

------- Comment #4 from joel at gcc dot gnu dot org  2009-01-05 22:09 -------
Grrr.. it looks like I was just sleepy this morning.  The problem is
still there as originally described.

gcc (GCC) 4.4.0 20090105 (experimental) [trunk revision 143092]

Running on gcc12 in the CFARM.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] psim miscompiled #2 [regression]

[joel at gcc dot gnu dot org]

------- Comment #5 from joel at gcc dot gnu dot org  2009-01-08 22:07 -------
Problem still present:

gcc (GCC) 4.4.0 20090108 (experimental) [trunk revision 143192]


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[rguenth at gcc dot gnu dot org]

------- Comment #6 from rguenth at gcc dot gnu dot org  2009-01-19 14:07 -------
Can you attach preprocessed source of idecode.c at least?  Can you track down
where in idecode.c the bug is and wrap a main () around that piece, making it
an executable testcase?

Does 4.3.[23] work?  Pleas adjust the Known-to-work fields.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
            Summary|psim miscompiled #2         |[4.4 Regression] psim
                   |[regression]                |miscompiled #2
   Target Milestone|---                         |4.4.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[joel at gcc dot gnu dot org]

------- Comment #7 from joel at gcc dot gnu dot org  2009-01-19 14:38 -------
Works with 4.3.2 as shipped with Fedora 10 (RPM gcc-4.3.2-7.x86_64)


-- 

joel at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.3.2


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[joel at gcc dot gnu dot org]

------- Comment #8 from joel at gcc dot gnu dot org  2009-01-19 14:45 -------
$ gcc --version
gcc (GCC) 4.4.0 20090116 (experimental) [trunk revision 143442]

broken still.  Test script 
=================================================
rm -rf b-gdb && mkdir b-gdb && cd b-gdb && \
/home/joel/gcc-pr38587/gdb-6.8/configure --target=powerpc-rtems4.10 \
  --disable-werror --enable-sim --enable-sim-hardware --enable-timebase \
    --enable-sim-trace 2>&1 && \
make -j6  2>&1 && \
../psim-4.10 /home/joel/powerpc-psim-ticker.ralf

=================================================


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[joel at gcc dot gnu dot org]

------- Comment #9 from joel at gcc dot gnu dot org  2009-01-19 14:51 -------
Created an attachment (id=17144)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17144&action=view)
PSIM test executable that produces assertion

PowerPC PSIM executable used by previous test script.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[joel at gcc dot gnu dot org]

------- Comment #10 from joel at gcc dot gnu dot org  2009-01-19 14:54 -------
Created an attachment (id=17145)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17145&action=view)
preprocessed idecode.c

Preprocessed version of idecode.i using this gcc version on Fedora 10:

4.4.0 20090116 (experimental) [trunk revision 143442]

gcc -E -g -O2 -DDEFAULT_INLINE=PSIM_INLINE_LOCALS    
-DWITH_HOST_BYTE_ORDER=LITTLE_ENDIAN -DWITH_SMP=5        -DWITH_TRACE=1     
-DHAVE_TERMIOS_STRUCTURE -DHAVE_TERMIOS_CLINE -DHAVE_DEVZERO     -I.
-I/home/joel/gcc-pr38587/gdb-6.8/sim/ppc
-I/home/joel/gcc-pr38587/gdb-6.8/sim/ppc/../../include -I../../bfd
-I/home/joel/gcc-pr38587/gdb-6.8/sim/ppc/../../bfd -I../../gdb
-I/home/joel/gcc-pr38587/gdb-6.8/sim/ppc/../../gdb 
-I/home/joel/gcc-pr38587/gdb-6.8/sim/ppc/../../gdb/config  -DHAVE_COMMON_FPU
-I../common -I/home/joel/gcc-pr38587/gdb-6.8/sim/ppc/../common idecode.c
>idecode.i


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[joel at gcc dot gnu dot org]

------- Comment #11 from joel at gcc dot gnu dot org  2009-01-19 15:00 -------
(In reply to comment #6)
> Can you attach preprocessed source of idecode.c at least?  Can you track down
> where in idecode.c the bug is and wrap a main () around that piece, making it
> an executable testcase?

psim is automatically generated code.  I have no idea how to narrow it down
to an executable test case.  I included my gdb configuration, psim
test program, and preprocessed source.  I will upload the script I
am using to run it since you have to have a device tree.

Hmmm... this is the code where the assert happens:

  /* now establish the restart target */
  psim_set_halt_and_restart(system, &halt, &restart);
  if (setjmp(restart)) {
    current_cpu = psim_last_cpu(system);
    ASSERT(current_cpu >= 0 && current_cpu < nr_cpus);
  }

Is it possible that setjmp/longjmp is not preserving a register that
gcc x86_64 is now using?

> Does 4.3.[23] work?  Pleas adjust the Known-to-work fields.

Confirmed 4.3.2.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #12 from hjl dot tools at gmail dot com  2009-01-19 19:01 -------
How can I reproduce it on Linux/x86-64, assuming I know nothing
about rtems and sim?


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl dot tools at gmail dot
                   |                            |com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[joel at gcc dot gnu dot org]

------- Comment #13 from joel at gcc dot gnu dot org  2009-01-19 19:09 -------
Created an attachment (id=17147)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17147&action=view)
script to run psim

script to run psim to reproduce bug.  Specifying the device tree is painful.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[joel at gcc dot gnu dot org]

------- Comment #14 from joel at gcc dot gnu dot org  2009-01-19 19:15 -------
(In reply to comment #12)
> How can I reproduce it on Linux/x86-64, assuming I know nothing
> about rtems and sim?
> 

You don't have to know much of anything.  It should be reproducible with 
powerpc-elf but this is the known way to the goal.  I configure gdb 
6.8 this way using a GCC SVN native compiler.

rm -rf b-gdb && mkdir b-gdb && cd b-gdb && \
../gdb-6.8/configure --target=powerpc-rtems4.10 \
  --disable-werror --enable-sim --enable-sim-hardware --enable-timebase \
    --enable-sim-trace 2>&1 && \
make

That should give you an executable named "run" in the sim/ppc subdirectory
of the build tree.  There are two attachments to this PR:
 + powerpc-ticker-ticker.ralf.bz2
 + psim-4.10

uncompress the ticker executable anywhere.

The psim-4.10 scripts creates a psim device tree and runs the executable
that is $1.  Edit to fix the definition of "RUN" to get the "run" executable
in the build tree.

I place the psim-4.10 program in my "gcc-pr38587" and it picks up the
run executable from the build.  It assumes you are in the build tree
when run.  

../psim-4.10 /home/joel/powerpc-psim-ticker.ralf

It is a hack to make this easier to check since it has been 
broken for over a month and I have checked it every few days.

It will print a few lines from tasks TA1, TA2, and TA3 and when it gets to
the end, you get an assertion.

Looking at the code in idecode.c, I suspect a setjmp/longjmp interacting
with the register usage.  But who knows.  

If you have trouble, I will try to answer quickly.  Thanks.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #15 from hjl dot tools at gmail dot com  2009-01-19 21:38 -------
Revision 142516 gave:

[hjl@gnu-28 pr38587]$ ./psim-4.10 ./powerpc-psim-ticker.ralf
events.c:329: assertion failed - !events->processing

[hjl@gnu-28 pr38587]$ 

Revision 142517 gave:

[hjl@gnu-34 pr38587]$ ./psim-4.10 ./powerpc-psim-ticker.ralf


*** CLOCK TICK TEST ***
TA1  - rtems_clock_get - 09:00:00   12/31/1988
TA2  - rtems_clock_get - 09:00:00   12/31/1988
TA3  - rtems_clock_get - 09:00:00   12/31/1988
TA1  - rtems_clock_get - 09:00:05   12/31/1988
TA2  - rtems_clock_get - 09:00:10   12/31/1988
TA1  - rtems_clock_get - 09:00:10   12/31/1988
TA3  - rtems_clock_get - 09:00:15   12/31/1988
TA1  - rtems_clock_get - 09:00:15   12/31/1988
TA2  - rtems_clock_get - 09:00:20   12/31/1988
TA1  - rtems_clock_get - 09:00:20   12/31/1988
TA1  - rtems_clock_get - 09:00:25   12/31/1988
TA3  - rtems_clock_get - 09:00:30   12/31/1988
TA1  - rtems_clock_get - 09:00:30   12/31/1988
TA2  - rtems_clock_get - 09:00:30   12/31/1988
*** END OF CLOCK TICK TEST ***
idecode.c:6591: assertion failed - current_cpu >= 0 && current_cpu < nr_cpus

[hjl@gnu-34 pr38587]$ 

Is that possible a bug in sim?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[joel at gcc dot gnu dot org]

------- Comment #16 from joel at gcc dot gnu dot org  2009-01-19 22:20 -------
(In reply to comment #15)
> Revision 142516 gave:
> 
> [hjl@gnu-28 pr38587]$ ./psim-4.10 ./powerpc-psim-ticker.ralf
> events.c:329: assertion failed - !events->processing

That sounds like the previous psim miscompiled bug.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38387

I closed it when the problem went away with no explanation.

I first saw the bug around this revision.

gcc (GCC) 4.4.0 20081205 (experimental) [trunk revision 142492]

So you have to go back further than that to miss psim regression #1

> 
> [hjl@gnu-28 pr38587]$ 
> 
> Revision 142517 gave:
> 
> [hjl@gnu-34 pr38587]$ ./psim-4.10 ./powerpc-psim-ticker.ralf
> 
> 
> *** CLOCK TICK TEST ***
> TA1  - rtems_clock_get - 09:00:00   12/31/1988
> TA2  - rtems_clock_get - 09:00:00   12/31/1988
> TA3  - rtems_clock_get - 09:00:00   12/31/1988
> TA1  - rtems_clock_get - 09:00:05   12/31/1988
> TA2  - rtems_clock_get - 09:00:10   12/31/1988
> TA1  - rtems_clock_get - 09:00:10   12/31/1988
> TA3  - rtems_clock_get - 09:00:15   12/31/1988
> TA1  - rtems_clock_get - 09:00:15   12/31/1988
> TA2  - rtems_clock_get - 09:00:20   12/31/1988
> TA1  - rtems_clock_get - 09:00:20   12/31/1988
> TA1  - rtems_clock_get - 09:00:25   12/31/1988
> TA3  - rtems_clock_get - 09:00:30   12/31/1988
> TA1  - rtems_clock_get - 09:00:30   12/31/1988
> TA2  - rtems_clock_get - 09:00:30   12/31/1988
> *** END OF CLOCK TICK TEST ***
> idecode.c:6591: assertion failed - current_cpu >= 0 && current_cpu < nr_cpus
> 
> [hjl@gnu-34 pr38587]$ 
> 
> Is that possible a bug in sim?
> 

psim is very old code written by Andrew Cagney over a decade ago.  It has
compiled and worked with all gcc versions since that time.  My test script
on CFARM builds a native GCC from SVN and uses that to build the crosses.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #17 from hjl dot tools at gmail dot com  2009-01-19 23:29 -------
*** Bug 38387 has been marked as a duplicate of this bug. ***


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #18 from hjl dot tools at gmail dot com  2009-01-19 23:35 -------
(In reply to comment #11)
> (In reply to comment #6)
> > Can you attach preprocessed source of idecode.c at least?  Can you track down
> > where in idecode.c the bug is and wrap a main () around that piece, making it
> > an executable testcase?
> 
> psim is automatically generated code.  I have no idea how to narrow it down
> to an executable test case.  I included my gdb configuration, psim
> test program, and preprocessed source.  I will upload the script I
> am using to run it since you have to have a device tree.
> 
> Hmmm... this is the code where the assert happens:
> 
>   /* now establish the restart target */
>   psim_set_halt_and_restart(system, &halt, &restart);
>   if (setjmp(restart)) {
>     current_cpu = psim_last_cpu(system);
>     ASSERT(current_cpu >= 0 && current_cpu < nr_cpus);
>   }
> 
> Is it possible that setjmp/longjmp is not preserving a register that
> gcc x86_64 is now using?
> 

This is an IRA bug. -fno-ira fixes it. This may be caused by revision
139996:

http://gcc.gnu.org/ml/gcc-patches/2008-09/msg00315.html


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vmakarov at redhat dot com
           Keywords|                            |ra, wrong-code
   Last reconfirmed|0000-00-00 00:00:00         |2009-01-19 23:35:49
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #19 from hjl dot tools at gmail dot com  2009-01-19 23:41 -------
Revert revision revision 139996 doesn't solve the problem.


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|2009-01-19 23:35:49         |2009-01-19 23:41:45
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #20 from hjl dot tools at gmail dot com  2009-01-20 02:00 -------
The problem may something to do with

setjmp
...
while (1)
{
  foo (); // foo calls longjmp and foo is inlined.
}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #21 from hjl dot tools at gmail dot com  2009-01-20 05:00 -------
(In reply to comment #20)
> The problem may something to do with
> 
> setjmp
> ...
> while (1)
> {
>   foo (); // foo calls longjmp and foo is inlined.
> }
> 

The problem is

  if (setjmp (buf))
    {
      /* Restore registers from stack.  */
    }

  while (1)
    {
      bar ();   /* Reuse stack slots used to restore registers after
                   longjmp.  */
      foo ();   /* Call longjmp.  */
    }

IRA should avoid reusing stack slots used to restore registers after
longjmp.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[rguenth at gcc dot gnu dot org]

------- Comment #22 from rguenth at gcc dot gnu dot org  2009-01-20 09:13 -------
>From tree-inline.c:

          case BUILT_IN_LONGJMP:
            /* We can't inline functions that call __builtin_longjmp at
               all.  The non-local goto machinery really requires the
               destination be in a different function.  If we allow the
               function calling __builtin_longjmp to be inlined into the
               function calling __builtin_setjmp, Things will Go Awry.  */

so IMHO we shouldn't inline foo ().  See inline_forbidden_p_stmt.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #23 from hjl dot tools at gmail dot com  2009-01-20 14:24 -------
(In reply to comment #22)
> From tree-inline.c:
> 
>           case BUILT_IN_LONGJMP:
>             /* We can't inline functions that call __builtin_longjmp at
>                all.  The non-local goto machinery really requires the
>                destination be in a different function.  If we allow the
>                function calling __builtin_longjmp to be inlined into the
>                function calling __builtin_setjmp, Things will Go Awry.  */
> 
> so IMHO we shouldn't inline foo ().  See inline_forbidden_p_stmt.
> 

foo () isn't inlined. We inline bar () which reuses stack slot used
to restore registers after longjmp, which is called from foo ().


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[rguenther at suse dot de]

------- Comment #24 from rguenther at suse dot de  2009-01-20 15:01 -------
Subject: Re:  [4.4 Regression] psim miscompiled #2

On Tue, 20 Jan 2009, hjl dot tools at gmail dot com wrote:

> ------- Comment #23 from hjl dot tools at gmail dot com  2009-01-20 14:24 -------
> (In reply to comment #22)
> > From tree-inline.c:
> > 
> >           case BUILT_IN_LONGJMP:
> >             /* We can't inline functions that call __builtin_longjmp at
> >                all.  The non-local goto machinery really requires the
> >                destination be in a different function.  If we allow the
> >                function calling __builtin_longjmp to be inlined into the
> >                function calling __builtin_setjmp, Things will Go Awry.  */
> > 
> > so IMHO we shouldn't inline foo ().  See inline_forbidden_p_stmt.
> > 
> 
> foo () isn't inlined. We inline bar () which reuses stack slot used
> to restore registers after longjmp, which is called from foo ().

Still I don't think this is an IRA bug.  Either this code is sort-of
non-conforming or we need to throttle down inlining more.

Richard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #25 from hjl dot tools at gmail dot com  2009-01-20 15:08 -------
(In reply to comment #24)
> Subject: Re:  [4.4 Regression] psim miscompiled #2
> 
> On Tue, 20 Jan 2009, hjl dot tools at gmail dot com wrote:
> 
> > ------- Comment #23 from hjl dot tools at gmail dot com  2009-01-20 14:24 -------
> > (In reply to comment #22)
> > > From tree-inline.c:
> > > 
> > >           case BUILT_IN_LONGJMP:
> > >             /* We can't inline functions that call __builtin_longjmp at
> > >                all.  The non-local goto machinery really requires the
> > >                destination be in a different function.  If we allow the
> > >                function calling __builtin_longjmp to be inlined into the
> > >                function calling __builtin_setjmp, Things will Go Awry.  */
> > > 
> > > so IMHO we shouldn't inline foo ().  See inline_forbidden_p_stmt.
> > > 
> > 
> > foo () isn't inlined. We inline bar () which reuses stack slot used
> > to restore registers after longjmp, which is called from foo ().
> 
> Still I don't think this is an IRA bug.  Either this code is sort-of
> non-conforming or we need to throttle down inlining more.
> 

Since -fno-ira works, I think it may be fixed in IRA if we can
properly mark stack slots used to restore registers after setjmp.

FWIW, bar () is marked as inline in sim.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[rguenth at gcc dot gnu dot org]

------- Comment #26 from rguenth at gcc dot gnu dot org  2009-01-20 15:29 -------
Well, ISTR something about local variables need to be marked volatile if you
use setjmp/longjmp.  Is psim really in compliance with what the standard says
here?  See 7.13.2.1/3 "..., except that the values of objects of automatic
storage duration that are local to the function containing the invocation
of the corresponding setjmp macro that do not have volatile-qualified type and
have been changed between the setjmp invocation and longjmp call are
indeterminate"

Which also means that if GCC may introduce violations of that criterion then
GCC needs to stop inlining into setjmp functions and/or disable any other
optimization that may result in violations of that.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #27 from hjl dot tools at gmail dot com  2009-01-20 15:49 -------
(In reply to comment #26)
> Well, ISTR something about local variables need to be marked volatile if you
> use setjmp/longjmp.  Is psim really in compliance with what the standard says
> here?  See 7.13.2.1/3 "..., except that the values of objects of automatic
> storage duration that are local to the function containing the invocation
> of the corresponding setjmp macro that do not have volatile-qualified type and
> have been changed between the setjmp invocation and longjmp call are
> indeterminate"
> 

This patch for sim:
Index: sim/ppc/gen-idecode.c
===================================================================
RCS file: /cvs/src/src/sim/ppc/gen-idecode.c,v
retrieving revision 1.4
diff -u -p -r1.4 gen-idecode.c
--- sim/ppc/gen-idecode.c       19 Jun 2003 18:42:30 -0000      1.4
+++ sim/ppc/gen-idecode.c       20 Jan 2009 15:48:18 -0000
@@ -708,11 +708,11 @@ print_run_until_stop_body(lf *file,
   }
   lf_putstr(file, "int last_cpu;\n");
   if (generate_smp) {
-    lf_putstr(file, "int current_cpu;\n");
+    lf_putstr(file, "volatile int current_cpu;\n");
   }

   if ((code & generate_with_icache)) {
-    lf_putstr(file, "int cpu_nr;\n");
+    lf_putstr(file, "volatile int cpu_nr;\n");
     lf_putstr(file, "\n");
     lf_putstr(file, "/* flush the icache of a possible break insn */\n");
     lf_putstr(file, "for (cpu_nr = 0; cpu_nr < nr_cpus; cpu_nr++)\n");

seems to work for me.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[joel at gcc dot gnu dot org]

------- Comment #28 from joel at gcc dot gnu dot org  2009-01-20 15:55 -------

I am starting a test sweep with this patch applied.

I will report back when the testing is done.

Thanks.

--joel

(In reply to comment #27)
> (In reply to comment #26)
> > Well, ISTR something about local variables need to be marked volatile if you
> > use setjmp/longjmp.  Is psim really in compliance with what the standard says
> > here?  See 7.13.2.1/3 "..., except that the values of objects of automatic
> > storage duration that are local to the function containing the invocation
> > of the corresponding setjmp macro that do not have volatile-qualified type and
> > have been changed between the setjmp invocation and longjmp call are
> > indeterminate"
> > 
> 
> This patch for sim:
> Index: sim/ppc/gen-idecode.c
> ===================================================================
> RCS file: /cvs/src/src/sim/ppc/gen-idecode.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 gen-idecode.c
> --- sim/ppc/gen-idecode.c       19 Jun 2003 18:42:30 -0000      1.4
> +++ sim/ppc/gen-idecode.c       20 Jan 2009 15:48:18 -0000
> @@ -708,11 +708,11 @@ print_run_until_stop_body(lf *file,
>    }
>    lf_putstr(file, "int last_cpu;\n");
>    if (generate_smp) {
> -    lf_putstr(file, "int current_cpu;\n");
> +    lf_putstr(file, "volatile int current_cpu;\n");
>    }
> 
>    if ((code & generate_with_icache)) {
> -    lf_putstr(file, "int cpu_nr;\n");
> +    lf_putstr(file, "volatile int cpu_nr;\n");
>      lf_putstr(file, "\n");
>      lf_putstr(file, "/* flush the icache of a possible break insn */\n");
>      lf_putstr(file, "for (cpu_nr = 0; cpu_nr < nr_cpus; cpu_nr++)\n");
> 
> seems to work for me.
> 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[schwab at suse dot de]

------- Comment #29 from schwab at suse dot de  2009-01-20 16:30 -------
(In reply to comment #27)
> This patch for sim:
> Index: sim/ppc/gen-idecode.c
> ===================================================================
> RCS file: /cvs/src/src/sim/ppc/gen-idecode.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 gen-idecode.c
> --- sim/ppc/gen-idecode.c       19 Jun 2003 18:42:30 -0000      1.4
> +++ sim/ppc/gen-idecode.c       20 Jan 2009 15:48:18 -0000
> @@ -708,11 +708,11 @@ print_run_until_stop_body(lf *file,
>    }
>    lf_putstr(file, "int last_cpu;\n");
>    if (generate_smp) {
> -    lf_putstr(file, "int current_cpu;\n");
> +    lf_putstr(file, "volatile int current_cpu;\n");
>    }
> 
>    if ((code & generate_with_icache)) {
> -    lf_putstr(file, "int cpu_nr;\n");
> +    lf_putstr(file, "volatile int cpu_nr;\n");
>      lf_putstr(file, "\n");
>      lf_putstr(file, "/* flush the icache of a possible break insn */\n");
>      lf_putstr(file, "for (cpu_nr = 0; cpu_nr < nr_cpus; cpu_nr++)\n");
> 
> seems to work for me.

Neither of these volatile qualifiers are necessary for defined operation.  


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #30 from hjl dot tools at gmail dot com  2009-01-20 16:36 -------
(In reply to comment #28)
> I am starting a test sweep with this patch applied.
> 
> I will report back when the testing is done.
> 

I am not sure if those volatile are really needed. I am trying
to create a testcase.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #31 from hjl dot tools at gmail dot com  2009-01-20 21:17 -------
Created an attachment (id=17154)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17154&action=view)
A testcase

Revision 143498 gave:

[hjl@gnu-34 ppc]$ /export/gnu/import/rrs/143498/usr/bin/gcc -S bar.c -O2; egrep
"32\(%rsp\)" bar.s
        movq    %rdi, 32(%rsp)
        movq    32(%rsp), %rdi
        movq    32(%rsp), %rdi
        movq    %rdx, 32(%rsp)
        movq    32(%rsp), %rcx
        movq    %r14, 32(%rsp)
        movq    32(%rsp), %rax
        movq    32(%rsp), %rsi
        movq    32(%rsp), %rsi
        movq    32(%rsp), %rdi
[hjl@gnu-34 ppc]$ 

The stack slot 32(%rsp) is used to save RDI.  But it is also
use for something else.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[pinskia at gcc dot gnu dot org]

------- Comment #32 from pinskia at gcc dot gnu dot org  2009-01-20 21:19 -------
PR 38660 might the same issue too.  It is an issue with longjmp and setjmp. 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #33 from hjl dot tools at gmail dot com  2009-01-21 02:14 -------
For

 psim_set_halt_and_restart(system, &halt, ((void *)0));
  if (_setjmp (halt))
    return;
  last_cpu = psim_last_cpu(system);
  psim_set_halt_and_restart(system, &halt, &restart);
  if (_setjmp (restart)) {
    current_cpu = psim_last_cpu(system);
    do { if (1) { if (!(current_cpu >= 0 && current_cpu < nr_cpus))
        { error("%s:%d: assertion failed - %s\n",
                filter_filename("xxx.c"), 59, "current_cpu >= 0 && current_cpu
< nr_cpus");
        } } } while (0);
  }
  else {
    current_cpu = last_cpu;
    do { if (1) { if (!(current_cpu >= -1 && current_cpu < nr_cpus))
        {
          error("%s:%d: assertion failed - %s\n",
                filter_filename("xxx.c"), 63, "current_cpu >= -1 && current_cpu
< nr_cpus");
        } } } while (0);
  }
  while (1) {
...
    {     
      cpu *processor = processors[current_cpu];
      unsigned_word cia =
        cpu_get_program_counter(processor);
      idecode_cache *cache_entry =
        cpu_icache_entry(processor, cia);
      if (cache_entry->address == cia) {
        idecode_semantic *semantic = cache_entry->semantic;
        cia = semantic(processor, cache_entry, cia);
        cpu_set_program_counter(processor, cia);
      }       
}

IRA allocates stack slots

init_insns for 186: (insn_list:REG_DEP_TRUE 349 (nil))
      Coalescing spilled allocnos a32r60->a94r74
      Coalescing spilled allocnos a33r87->a50r73
      Coalescing spilled allocnos a10r91->a94r74
      Coalescing spilled allocnos a13r100->a50r73
      Slot 1 (freq,size): a10r91(5,4) a32r60(292,4) a94r74(412,8)
      Slot 2 (freq,size): a13r100(5,8) a33r87(123,8) a50r73(255,8)
      Slot 3 (freq,size): a9r103(65,4)
      Slot 4 (freq,size): a8r102(63,8)
      Slot 5 (freq,size): a7r101(2,8)
      Assigning 101(freq=2) a new slot 4
      Assigning 102(freq=63) a new slot 3
      Assigning 103(freq=65) a new slot 2
      Assigning 73(freq=255) a new slot 1
      Assigning 87(freq=123) slot 1 of 73
      Assigning 100(freq=5) slot 1 of 73 87
      Assigning 74(freq=412) a new slot 0
      Assigning 60(freq=292) slot 0 of 74
      Assigning 91(freq=5) slot 0 of 60 74

for

(note:HI 37 36 38 4 [bb 4] NOTE_INSN_BASIC_BLOCK)

(insn:HI 38 37 39 4 ../bar.c:613 (set (reg:DI 5 di)
        (reg/v/f:DI 100 [ system ])) 89 {*movdi_1_rex64} (expr_list:REG_DEAD
(reg/v/f:DI 100 [ system ])
        (nil)))

(call_insn:HI 39 38 40 4 ../bar.c:613 (set (reg:SI 0 ax)
        (call (mem:QI (symbol_ref:DI ("psim_last_cpu") [flags 0x41]
<function_decl 0x7f2d2d62e400 psim_last_cpu>) [0 S1 A8])
            (const_int 0 [0x0]))) 904 {*call_value_0_rex64} (expr_list:REG_DEAD
(reg:DI 5 di)
        (nil))
    (expr_list:REG_DEP_TRUE (use (reg:DI 5 di))
        (nil)))
...
(insn:HI 308 307 315 42 ../bar.c:631 (set (reg/v/f:DI 87 [ cache_entry ])
        (plus:DI (plus:DI (reg/v/f:DI 89 [ processor ])
                (reg:DI 136))
            (const_int 4632 [0x1218]))) 273 {*lea_2_rex64} (nil))

(insn:HI 315 308 316 42 ../bar.c:632 (parallel [
            (set (reg:DI 145)
                (plus:DI (reg/v/f:DI 89 [ processor ])
                    (reg:DI 136)))
            (clobber (reg:CC 17 flags))
        ]) 280 {*adddi_1_rex64} (expr_list:REG_DEAD (reg:DI 136)
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

It generates:

(insn:HI 38 37 39 4 ../bar.c:613 (set (reg:DI 5 di)
        (mem/c:DI (plus:DI (reg/f:DI 7 sp)
                (const_int 32 [0x20])) [42 %sfp+-448 S8 A64])) 89
{*movdi_1_rex64} (nil))

(call_insn:HI 39 38 40 4 ../bar.c:613 (set (reg:SI 0 ax)
        (call (mem:QI (symbol_ref:DI ("psim_last_cpu") [flags 0x41]
<function_decl 0x7f2d2d62e400 psim_last_cpu>) [0 S1 A8])
            (const_int 0 [0x0]))) 904 {*call_value_0_rex64} (nil)
    (expr_list:REG_DEP_TRUE (use (reg:DI 5 di))
        (nil)))
...
(insn:HI 308 307 525 43 ../bar.c:631 (set (reg:DI 1 dx)
        (plus:DI (plus:DI (reg/v/f:DI 43 r14 [orig:89 processor ] [89])
                (reg:DI 0 ax [136]))
            (const_int 4632 [0x1218]))) 273 {*lea_2_rex64} (nil))

(insn 525 308 315 43 ../bar.c:631 (set (mem/c:DI (plus:DI (reg/f:DI 7 sp)
                (const_int 32 [0x20])) [42 %sfp+-448 S8 A64])
        (reg:DI 1 dx)) 89 {*movdi_1_rex64} (nil))

(insn:HI 315 525 316 43 ../bar.c:632 (parallel [
            (set (reg:DI 0 ax [145])
                (plus:DI (reg/v/f:DI 43 r14 [orig:89 processor ] [89])
                    (reg:DI 0 ax [136])))
            (clobber (reg:CC 17 flags))
        ]) 280 {*adddi_1_rex64} (nil))

The problem is

      Assigning 73(freq=255) a new slot 1
      Assigning 87(freq=123) slot 1 of 73
      Assigning 100(freq=5) slot 1 of 73 87

100 needs a separate slot. We can't share it with 73 and 87.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[vmakarov at redhat dot com]

------- Comment #34 from vmakarov at redhat dot com  2009-01-21 03:15 -------
C standard guarantees that automatic variable only with volatile will be
restored after longjmp. Gcc puts volatiles on stack and don't use pseudos for
them.  So they will be never shared on stack.

I think the code (its expected behavior) does not conform with the standard.

Unfortunately there are a lot of code which does not conform with the standard.
 So I think we should provide the same functionality.  It can be done by
prohibiting stack slot sharing in IRA if there are setjmps in the function.

I'll make a patch and send it for a review after some testing most probably
tomorrow.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #35 from hjl dot tools at gmail dot com  2009-01-21 03:27 -------
(In reply to comment #34)
> C standard guarantees that automatic variable only with volatile will be
> restored after longjmp. Gcc puts volatiles on stack and don't use pseudos for
> them.  So they will be never shared on stack.
> 
> I think the code (its expected behavior) does not conform with the standard.
> 
> Unfortunately there are a lot of code which does not conform with the standard.
>  So I think we should provide the same functionality.  It can be done by
> prohibiting stack slot sharing in IRA if there are setjmps in the function.
> 

Does C standard say anything about function argument? "system", which
is a function argument, is trashed here.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[pinskia at gcc dot gnu dot org]

------- Comment #36 from pinskia at gcc dot gnu dot org  2009-01-21 03:28 -------
(In reply to comment #35)
> (In reply to comment #34)
> > C standard guarantees that automatic variable only with volatile will be
> > restored after longjmp. Gcc puts volatiles on stack and don't use pseudos for
> > them.  So they will be never shared on stack.
> > 
> > I think the code (its expected behavior) does not conform with the standard.
> > 
> > Unfortunately there are a lot of code which does not conform with the standard.
> >  So I think we should provide the same functionality.  It can be done by
> > prohibiting stack slot sharing in IRA if there are setjmps in the function.
> > 
> 
> Does C standard say anything about function argument? "system", which
> is a function argument, is trashed here.

function arguments are just local variables ....


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[pinskia at gcc dot gnu dot org]

------- Comment #37 from pinskia at gcc dot gnu dot org  2009-01-21 03:35 -------
To be more explict:
6.9.1/9:
Each parameter has automatic storage duration.

7.13.2.1/3:
All accessible objects have values, and all other components of the abstract
machine209)
have state, as of the time the longjmp function was called, except that the
values of
objects of automatic storage duration that are local to the function containing
the
invocation of the corresponding setjmp macro that do not have
volatile-qualified type
and have been changed between the setjmp invocation and longjmp call are
indeterminate.

So yes arguments are treated the same as local variables in this case.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[schwab at suse dot de]

------- Comment #38 from schwab at suse dot de  2009-01-21 09:16 -------
(In reply to comment #35)
> "system", which is a function argument, is trashed here.

But it's not modified between setjmp and longjmp, so its value must be
preserved.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[vmakarov at redhat dot com]

------- Comment #39 from vmakarov at redhat dot com  2009-01-21 15:39 -------
Yes, right.  I should have read the standard itself (not gcc manual which
states "ISO C says that automatic variables that are not declared `volatile'
have undefined values after a `longjmp'").  That is good that Andrew checked
and posted what the current standard saying about setjmps.

So the code is actually valid and it is an IRA bug.

Working on the PR, I found that GCC even guarantees that modified local
variable will have the right values after longjmp because GCC always puts
pseudos intersected setjmps on the stack.  Probably it is the safest behaviour.

In any way the patch solving the PR is ready and I'll submit it today after
checking bootstraps on a few machines.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] psim miscompiled #2

[hjl dot tools at gmail dot com]

------- Comment #40 from hjl dot tools at gmail dot com  2009-01-21 20:03 -------
*** Bug 38660 has been marked as a duplicate of this bug. ***


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kurt at roeckx dot be


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] IRA doesn't preserve local variables after setjmp

[vmakarov at gcc dot gnu dot org]

------- Comment #41 from vmakarov at gcc dot gnu dot org  2009-01-21 20:18 -------
Subject: Bug 38587

Author: vmakarov
Date: Wed Jan 21 20:18:03 2009
New Revision: 143554

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=143554
Log:
2009-01-21  Vladimir Makarov  <vmakarov@redhat.com>

        PR middle-end/38587
        * ira-color.c (coalesce_spill_slots): Don't coalesce allocnos
        crossing setjmps.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ira-color.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] IRA doesn't preserve local variables after setjmp

[rguenth at gcc dot gnu dot org]

------- Comment #42 from rguenth at gcc dot gnu dot org  2009-01-21 21:56 -------
Fixed.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] IRA doesn't preserve local variables after setjmp

[joel at gcc dot gnu dot org]

------- Comment #43 from joel at gcc dot gnu dot org  2009-01-22 19:23 -------
(In reply to comment #42)
> Fixed.
> 

Did the test case get added to the suite?

This does appear to be fixed to me as well.

C/C++: http://gcc.gnu.org/ml/gcc-testresults/2009-01/msg02243.html
Ada:   http://gcc.gnu.org/ml/gcc-testresults/2009-01/msg02255.html


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587

[Bug middle-end/38587] [4.4 Regression] IRA doesn't preserve local variables after setjmp

[hjl dot tools at gmail dot com]

------- Comment #44 from hjl dot tools at gmail dot com  2009-01-22 19:33 -------
(In reply to comment #43)
> (In reply to comment #42)
> > Fixed.
> > 
> 
> Did the test case get added to the suite?
> 
> This does appear to be fixed to me as well.

There are no suitable testcases in this PR. Maybe someone can extract a
testcase from PR 38660.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38587