[Bug go/64876] New: Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)

[Bug go/64876] New: Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)

[boger at us dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64876

            Bug ID: 64876
           Summary: Regressions in gcc-testresults for powerpc64 gccgo in
                    5.0 due to change for static chain for closures
                    (219776)
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: go
          Assignee: ian at airs dot com
          Reporter: boger at us dot ibm.com
                CC: cmang at google dot com
            Target: powerpc64

Many new failures appear in gcc-testresults for gccgo on powerpc64 after the
change was added to use the static chain for closures.  Regressions do not
occur with powerpc64le.  (Probably due to the different ABIs?) There are 30+
regressions in the go tests and 100+ regressions in the libgo testsuite.

The first gcc-testresults page with these regressions is:
 https://gcc.gnu.org/ml/gcc-testresults/2015-01/msg01797.html

The failures I've seen have a message with a signal 11:

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x8]

goroutine 16 [running]:
main.$nested0
       
/home/boger/gccgo.work/r219749/src/gcc/testsuite/go.test/test/escape.go:152
main.for_escapes3
       
/home/boger/gccgo.work/r219749/src/gcc/testsuite/go.test/test/escape.go:155
main.main
       
/home/boger/gccgo.work/r219749/src/gcc/testsuite/go.test/test/escape.go:204
created by main
        /home/boger/gccgo.work/r219749/bld/../src/libgo/runtime/go-main.c:42
FAIL: go.test/test/escape.go execution,  -O2 -g

[Bug go/64876] Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)

[boger at us dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64876

boger at us dot ibm.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amodra at gcc dot gnu.org,
                   |                            |amodra at gmail dot com

--- Comment #1 from boger at us dot ibm.com ---
I found some notes I had from Alan on his change related to GO closures for
powerpc and found that all his changes for libffi are in but this gcc patch is
not in gcc trunk:

Index: gcc/config/rs6000/linux64.h
===================================================================
--- gcc/config/rs6000/linux64.h    (revision 217330)
+++ gcc/config/rs6000/linux64.h    (working copy)
@@ -115,6 +115,14 @@
           if (dot_symbols)                    \
         error ("-mcall-aixdesc incompatible with -mabi=elfv2"); \
         }                            \
+      if (DEFAULT_ABI == ABI_AIX                \
+          && strcmp (lang_hooks.name, "GNU Go") == 0)    \
+        {                            \
+          if (global_options_set.x_TARGET_POINTERS_TO_NESTED_FUNCTIONS \
+          && TARGET_POINTERS_TO_NESTED_FUNCTIONS)    \
+        error ("-mpointers-to-nested-functions is incompatible with Go"); \
+          TARGET_POINTERS_TO_NESTED_FUNCTIONS = 0;        \
+        }                            \
       if (rs6000_isa_flags & OPTION_MASK_RELOCATABLE)    \
         {                            \
           rs6000_isa_flags &= ~OPTION_MASK_RELOCATABLE;    \

After applying this patch and rebuilding the regressions go away.

[Bug go/64876] Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)

[ian at airs dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64876

--- Comment #2 from Ian Lance Taylor <ian at airs dot com> ---
It seems to me that with the current code that issue should be handled in
libffi.  With the current code it's hard for me to see why checking for "GNU
Go" is correct.

[Bug go/64876] Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)

[amodra at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64876

Alan Modra <amodra at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-02-03
     Ever confirmed|0                           |1

--- Comment #3 from Alan Modra <amodra at gmail dot com> ---
Confirmed.  libffi does not appear to be involved here.  If I look at the
escape.go testcase Lynn singles out, the code generated with the patch (or with
-mno-pointers-to-nested-functions) is the following for the call that dies
without the patch.

Breakpoint 2, main.for_escapes3 (y=106, x=105) at
/home/amodra/src/gcc-virgin/gcc/testsuite/go.test/test/escape.go:155
155        return f[0](), f[1]()
(gdb) disass $pc,$pc+0x3f
Dump of assembler code from 0x1000267c to 0x100026bb:
=> 0x000000001000267c <main.for_escapes3+172>:    ld      r11,112(r1)
   0x0000000010002680 <main.for_escapes3+176>:    ld      r9,0(r11)
   0x0000000010002684 <main.for_escapes3+180>:    std     r2,40(r1)
   0x0000000010002688 <main.for_escapes3+184>:    ld      r10,0(r9)
   0x000000001000268c <main.for_escapes3+188>:    mtctr   r10
   0x0000000010002690 <main.for_escapes3+192>:    ld      r2,8(r9)
   0x0000000010002694 <main.for_escapes3+196>:    bctrl
   0x0000000010002698 <main.for_escapes3+200>:    ld      r2,40(r1)
(gdb) stepi 7
main.$nested0 () at
/home/amodra/src/gcc-virgin/gcc/testsuite/go.test/test/escape.go:152
152            f[n] = func() *int { return p }
(gdb) disass
Dump of assembler code for function main.$nested0:
=> 0x0000000010001960 <+0>:    ld      r9,8(r11)
   0x0000000010001964 <+4>:    ld      r3,0(r9)
   0x0000000010001968 <+8>:    blr

Clearly r11 is expected to hold the address of a struct on entry to
main.$nested0.

(gdb) x/2xg $r11
0xc2080000e0:    0x0000000010014470    0x000000c20903c000
(gdb) x 0x000000c20903c000
0xc20903c000:    0x000000c2080000d8
(gdb) x 0x000000c2080000d8
0xc2080000d8:    0x0000000000000069

Yup, there's our 105, so the value returned is as expected.

If I compile the testcase without -mno-pointers-to-nested-functions then the
code for the call in main.for_escapes3 is:

   0x000000001000267c <+172>:    ld      r9,112(r1)
---Type <return> to continue, or q <return> to quit---
   0x0000000010002680 <+176>:    ld      r9,0(r9)
   0x0000000010002684 <+180>:    std     r2,40(r1)
   0x0000000010002688 <+184>:    ld      r10,0(r9)
   0x000000001000268c <+188>:    ld      r11,16(r9)
   0x0000000010002690 <+192>:    mtctr   r10
   0x0000000010002694 <+196>:    ld      r2,8(r9)
   0x0000000010002698 <+200>:    bctrl
=> 0x000000001000269c <+204>:    ld      r2,40(r1)

So r11 is not loaded with the proper parameter from 112(r1), but instead set up
with a zero from the function descriptor.

[Bug go/64876] Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)

[ian at airs dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64876

--- Comment #4 from Ian Lance Taylor <ian at airs dot com> ---
This code is going to call main.$nested0 with the static chain pointer set to
the address of a struct created in main.for_escapes3.  In this code

        p := new(int)
        *p = i
        f[n] = func() *int { return p }

the struct will have a single field which will hold the address of p, and
main.$nested0, which is the "func() *int { return p }", will simply return an
indirection through the first field of that struct.

The static chain is set in gcc/go/go-gcc.cc in the function
Gcc_backend::call_expression by setting CALL_EXPR_STATIC_CHAIN.  The value is
picked up in the nested function by a parameter created by
Gcc_backend::static_chain_variable, a parameter stored in the functions
DECL_STATIC_CHAIN field.

So for some reason the value passed in CALL_EXPR_STATIC_CHAIN is not being
picked up by the parameter stored in the function's DECL_STATIC_CHAIN.  That
seems like a problem, but it doesn't seem like a problem in the Go front end.

[Bug target/64876] Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)

[amodra at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64876

Alan Modra <amodra at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
          Component|go                          |target
           Assignee|ian at airs dot com                |amodra at gmail dot com

--- Comment #5 from Alan Modra <amodra at gmail dot com> ---
Oh yeah, this is really an rs6000 backend bug, not a go bug.  In 185.r.expand I
see

;; _12 = _11 (); [static-chain: _9]

(insn 78 77 79 (set (reg:DI 11 11)
        (reg/f:DI 181 [ D.1382 ]))
/home/amodra/src/gcc-virgin/gcc/testsuite/go.test/test/escape.go:155 -1
     (nil))

So there's the static chain being loaded.  Trouble is, indirect calls on ELFv1
load r11 from the function descriptor so the proper value is overwritten..

(insn 79 78 80 (set (mem/v/c:DI (plus:DI (reg/f:DI 1 1)
                (const_int 40 [0x28])) [28  S8 A8])
        (reg:DI 2 2))
/home/amodra/src/gcc-virgin/gcc/testsuite/go.test/test/escape.go:155 -1
     (nil))

(insn 80 79 81 (set (reg:DI 198)
        (mem:DI (reg/f:DI 182 [ D.1381 ]) [0  S8 A8]))
/home/amodra/src/gcc-virgin/gcc/testsuite/go.test/test/escape.go:155 -1
     (nil))

(insn 81 80 82 (set (reg:DI 11 11)
        (mem:DI (plus:DI (reg/f:DI 182 [ D.1381 ])
                (const_int 16 [0x10])) [0  S8 A8]))
/home/amodra/src/gcc-virgin/gcc/testsuite/go.test/test/escape.go:155 -1
     (nil))

..above.

(call_insn 82 81 83 (parallel [
            (set (reg:DI 3 3)
                (call (mem:SI (reg:DI 198) [0 *_11 S4 A8])
                    (const_int 64 [0x40])))
            (use (mem:DI (plus:DI (reg/f:DI 182 [ D.1381 ])
                        (const_int 8 [0x8])) [0  S8 A8]))
            (set (reg:DI 2 2)
                (unspec:DI [
                        (const_int 40 [0x28])
                    ] UNSPEC_TOCSLOT))
            (clobber (reg:DI 65 lr))
        ]) /home/amodra/src/gcc-virgin/gcc/testsuite/go.test/test/escape.go:155
-1
     (expr_list:REG_CALL_DECL (nil)
        (nil))
    (expr_list (use (reg:DI 11 11))
        (expr_list (use (reg:DI 11 11))
            (nil))))

Thus the patch to clear TARGET_POINTERS_TO_NESTED_FUNCTIONS which turns off
loading of r11 from function descriptors.

[Bug target/64876] Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)

[amodra at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64876

--- Comment #6 from Alan Modra <amodra at gcc dot gnu.org> ---
Author: amodra
Date: Thu Feb  5 22:52:24 2015
New Revision: 220463

URL: https://gcc.gnu.org/viewcvs?rev=220463&root=gcc&view=rev
Log:
    PR target/64876
    * config/rs6000/rs6000.c (chain_already_loaded): New function.
    (rs6000_call_aix): Use it.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c

[Bug target/64876] Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64876

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bergner at gcc dot gnu.org

--- Comment #7 from Peter Bergner <bergner at gcc dot gnu.org> ---
Lynn and Alan, can we mark this as fixed now?

[Bug target/64876] Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)

[boger at us dot ibm.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64876

--- Comment #8 from boger at us dot ibm.com ---
Yes, I can see that these regressions have gone away in the latest gcc
testresults on ppc64.

[Bug target/64876] Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)

[ian at airs dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64876

Ian Lance Taylor <ian at airs dot com> changed:

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

--- Comment #9 from Ian Lance Taylor <ian at airs dot com> ---
Fixed.