First Last Prev Next    No search results available      Search page      Enter new bug
Bug#: 38587
Product:  
Component:  
Status: RESOLVED
Resolution: FIXED
Assigned To: Not yet assigned to anyone <unassigned@gcc.gnu.org>
Host:
Reported against  
Priority:  
Severity:  
Target Milestone:  
 
 
Target:
Reporter: Joel Sherrill <joel@gcc.gnu.org>
Add CC:
CC:
Remove selected CCs
Build:
URL:
Summary:
Keywords:
Known to work:
Known to fail:

Attachment Description Type Created Size Actions
powerpc-psim-ticker.ralf.bz2 PSIM test executable that produces assertion application/octet-stream 2009-01-19 14:51 517.69 KB Edit
idecode.i.bz2 preprocessed idecode.c application/x-bzip2 2009-01-19 14:54 34.77 KB Edit
psim-4.10 script to run psim application/octet-stream 2009-01-19 19:09 1.74 KB Edit
bar.c A testcase text/plain 2009-01-20 21:17 3.91 KB Edit
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 38587 depends on: Show dependency tree
Show dependency graph
Bug 38587 blocks:

Additional Comments:






View Bug Activity   |   Format For Printing   |   Clone This Bug


Description:   Last confirmed: 2009-01-19 23:41 Opened: 2008-12-20 15:34
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.

------- Comment #1 From Andrew Pinski 2008-12-20 18:46 -------
This sounds more likely an overflow issue.  Does -fno-strict-overflow make this
work?

------- Comment #2 From Joel Sherrill 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?

------- Comment #3 From Joel Sherrill 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.

------- Comment #4 From Joel Sherrill 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.

------- Comment #5 From Joel Sherrill 2009-01-08 22:07 -------
Problem still present:

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

------- Comment #6 From Richard Guenther 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.

------- Comment #7 From Joel Sherrill 2009-01-19 14:38 -------
Works with 4.3.2 as shipped with Fedora 10 (RPM gcc-4.3.2-7.x86_64)

------- Comment #8 From Joel Sherrill 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

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

------- Comment #9 From Joel Sherrill 2009-01-19 14:51 -------
Created an attachment (id=17144) [edit]
PSIM test executable that produces assertion

PowerPC PSIM executable used by previous test script.

------- Comment #10 From Joel Sherrill 2009-01-19 14:54 -------
Created an attachment (id=17145) [edit]
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

------- Comment #11 From Joel Sherrill 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.

------- Comment #12 From H.J. Lu 2009-01-19 19:01 -------
How can I reproduce it on Linux/x86-64, assuming I know nothing
about rtems and sim?

------- Comment #13 From Joel Sherrill 2009-01-19 19:09 -------
Created an attachment (id=17147) [edit]
script to run psim

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

------- Comment #14 From Joel Sherrill 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.

------- Comment #15 From H.J. Lu 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?

------- Comment #16 From Joel Sherrill 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.

------- Comment #17 From H.J. Lu 2009-01-19 23:29 -------
*** Bug 38387 has been marked as a duplicate of this bug. ***

------- Comment #18 From H.J. Lu 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

------- Comment #19 From H.J. Lu 2009-01-19 23:41 -------
Revert revision revision 139996 doesn't solve the problem.

------- Comment #20 From H.J. Lu 2009-01-20 02:00 -------
The problem may something to do with

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

------- Comment #21 From H.J. Lu 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.

------- Comment #22 From Richard Guenther 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.

------- Comment #23 From H.J. Lu 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 ().

------- Comment #24 From rguenther@suse.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.

------- Comment #25 From H.J. Lu 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.

------- Comment #26 From Richard Guenther 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.

------- Comment #27 From H.J. Lu 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.

------- Comment #28 From Joel Sherrill 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.
> 

------- Comment #29 From Andreas Schwab 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.  

------- Comment #30 From H.J. Lu 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.

------- Comment #31 From H.J. Lu 2009-01-20 21:17 -------
Created an attachment (id=17154) [edit]
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.

------- Comment #32 From Andrew Pinski 2009-01-20 21:19 -------
PR 38660 might the same issue too.  It is an issue with longjmp and setjmp. 

------- Comment #33 From H.J. Lu 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.

------- Comment #34 From Vladimir Makarov 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.

------- Comment #35 From H.J. Lu 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.

------- Comment #36 From Andrew Pinski 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 ....

------- Comment #37 From Andrew Pinski 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.

------- Comment #38 From Andreas Schwab 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.

------- Comment #39 From Vladimir Makarov 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.


------- Comment #40 From H.J. Lu 2009-01-21 20:03 -------
*** Bug 38660 has been marked as a duplicate of this bug. ***

------- Comment #41 From Vladimir Makarov 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

------- Comment #42 From Richard Guenther 2009-01-21 21:56 -------
Fixed.

------- Comment #43 From Joel Sherrill 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

------- Comment #44 From H.J. Lu 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.

First Last Prev Next    No search results available      Search page      Enter new bug