[Bug go/61877] New: [4.10 Regression]: reflect: cannot use []string as type string in Call

[Bug go/61877] New: [4.10 Regression]: reflect: cannot use []string as type string in Call

[lists at kambanaria dot org]

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

            Bug ID: 61877
           Summary: [4.10 Regression]: reflect: cannot use []string as
                    type string in Call
           Product: gcc
           Version: 4.10.0
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: go
          Assignee: ian at airs dot com
          Reporter: lists at kambanaria dot org
                CC: cmang at google dot com

Created attachment 33171
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33171&action=edit
test case that can be executed with "go test -compiler gccgo -test.v 
reflect_test"

There is a regression in the runtime behavior of the reflection module.

Attached is a very simple ( < 30 lines) test case (that can be execured via go
test command). I hope this can be included in the normal test suite

Expected behavior:
     go run -compiler gccgo src/ref_main.go go test -compiler gccgo -test.v 
reflect_test
should result in:
     === RUN TestFindingVarargMethod
     --- PASS: TestFindingVarargMethod (0.00 seconds)
     PASS
     ok
Actual behavior:

     === RUN TestFindingVarargMethod
     --- FAIL: TestFindingVarargMethod (0.00 seconds)
     panic: reflect: cannot use []string as type string in Call [recovered]
         panic: reflect: cannot use []string as type string in Call

     goroutine 20 [running]:
     testing.$nested5
         ../.././libgo/go/testing/testing.go:406
     reflect.call.N13_reflect.Value
         ../.././libgo/go/reflect/value.go:494
     reflect.Call.N13_reflect.Value
         ../.././libgo/go/reflect/value.go:412
     reflect.call.pN20_reflect.makeFuncImpl
         ../.././libgo/go/reflect/makefunc.go:198
     reflect.MakeFuncStubGo
         ../.././libgo/go/reflect/makefuncgo_amd64.go:322
     reflect_test_test.TestFindingVarargMethod
         /home/ashopov/WORK/GO/src/reflect_test/finder_test.go:19
     testing.tRunner
         ../.././libgo/go/testing/testing.go:422
     created by testing.RunTests
         ../.././libgo/go/testing/testing.go:504

     goroutine 16 [chan receive]:
     testing.RunTests
         ../.././libgo/go/testing/testing.go:505
     testing.Main
         ../.././libgo/go/testing/testing.go:435
     main.main
         /tmp/go-build886615920/reflect_test/_test/_testmain.go:47
     created by main
         ../.././libgo/runtime/go-main.c:42

     goroutine 18 [finalizer wait]:
     created by runtime_createfing
         ../.././libgo/runtime/mgc0.c:2596
     exit status 2
     FAIL    reflect_test    0.058s





This test case is WORKING fine with:
1. golang-pkg-bin-linux-amd64-1.2.2-7 distributed with Fedora 20
which corresponds to 
2. gcc 4.9:
gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/home/ashopov/WORK/GCC/libexec/gcc/x86_64-linux-gnu/4.9.1/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../configure --prefix=/home/ashopov/WORK/GCC
--build=x86_64-linux-gnu --disable-browser-plugin --disable-libgcj
--disable-libmudflap --disable-vtable-verify --disable-libunwind-exceptions
--enable-checking=release --disable-bootstrap --enable-__cxa_atexit
--enable-gnu-unique-object --enable-languages=c,c++,go,lto
--enable-linker-build-id --enable-multilib --enable-plugin --enable-shared
--enable-threads=posix --with-linker-hash-style=gnu --with-system-zlib
--with-tune=generic --enable-multiarch
Thread model: posix
gcc version 4.9.1 20140709 (prerelease) (GCC) 



This test case is FAILING with:
1. gcc 4.10
gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/home/ashopov/WORK/GCC-TRUNK/libexec/gcc/x86_64-linux-gnu/4.10.0/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ./configure --prefix=/home/ashopov/WORK/GCC-TRUNK
--build=x86_64-linux-gnu --disable-browser-plugin --disable-libgcj
--disable-libmudflap --disable-vtable-verify --disable-libunwind-exceptions
--enable-checking=release --disable-bootstrap --enable-__cxa_atexit
--enable-gnu-unique-object --enable-languages=c,c++,go,lto
--enable-linker-build-id --enable-multilib --enable-plugin --enable-shared
--enable-threads=posix --with-linker-hash-style=gnu --with-system-zlib
--with-tune=generic --enable-multiarch
Thread model: posix
gcc version 4.10.0 20140721 (experimental) (GCC) 


Machine used for tests:
uname -a
Linux ashopov-dev 3.15.6-200.fc20.x86_64 #1 SMP Fri Jul 18 02:36:27 UTC 2014
x86_64 x86_64 x86_64 GNU/Linux

[Bug go/61877] [4.10 Regression]: reflect: cannot use []string as type string in Call

[lists at kambanaria dot org]

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

--- Comment #1 from Alexander Shopov <lists at kambanaria dot org> ---
I will provide additional information if necessary

[Bug go/61877] [5 Regression]: reflect: cannot use []string as type string in Call

[rth at gcc dot gnu.org]

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

Richard Henderson <rth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-09-25
                 CC|                            |rth at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #2 from Richard Henderson <rth at gcc dot gnu.org> ---
Confirmed.

[Bug go/61877] [5 Regression]: reflect: cannot use []string as type string in Call

[michael.hudson at linaro dot org]

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

--- Comment #3 from Michael Hudson-Doyle <michael.hudson at linaro dot org> ---
The problem is that the call to "m.Call" at
https://gcc.gnu.org/viewcvs/gcc/trunk/libgo/go/reflect/makefunc.go?revision=212853&view=markup#l133
needs, in this case, to be a call to "m.CallSlice".  I don't know how it can
know when it needs to do that though.

[Bug go/61877] [5 Regression]: reflect: cannot use []string as type string in Call

[michael.hudson at linaro dot org]

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

--- Comment #4 from Michael Hudson-Doyle <michael.hudson at linaro dot org> ---
Created attachment 33639
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33639&action=edit
proposed fix

This fix works for me.  I can't find any tests of this behaviour -- casting the
result of (*reflect.Value).Interface to an actual pointer type and then calling
it.  Am I just being blind?

[Bug go/61877] [5 Regression]: reflect: cannot use []string as type string in Call

[ian at airs dot com]

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

--- Comment #5 from Ian Lance Taylor <ian at airs dot com> ---
Hmmm, I don't quite see how that patch could be correct.  Does it do the right
thing for both f(a, b, c) and f(a, sliceval...)?

Any test would look like TestMethodValue in all_test.go, but it looks like that
doesn't test a variadic function.  Or in TestMakeFuncVariadic, but it looks
like that doesn't test a method value.

[Bug go/61877] [5 Regression]: reflect: cannot use []string as type string in Call

[michael.hudson at linaro dot org]

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

--- Comment #6 from Michael Hudson-Doyle <michael.hudson at linaro dot org> ---
Created attachment 33640
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33640&action=edit
my test cases

I think the patch works because when the compiler sees a call to a variadic
function, it generates a regular call with a slice as the last parameter.

I'm attaching a test case that I wrote -- it passes with my patch and fails
without it, so I think my patch has at last something going for it...

It also seems the intel case is broken on this test case on mainline.  If I
hack it to take the ffi case, it works with my patch.  Also 4.9 works, which
confuses me a little -- I didn't think the intel code path had changed here
(but haven't actually checked).

[Bug go/61877] [5 Regression]: reflect: cannot use []string as type string in Call

[lists at kambanaria dot org]

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

--- Comment #7 from Alexander Shopov <lists at kambanaria dot org> ---
> It also seems the intel case is broken on this test case on mainline.  If I
> hack it to take the ffi case, it works with my patch.  Also 4.9 works, which

Not sure how relevant this would be, but makefuncgo_amd64.go seems quite
changed between 4.9 and master/trunk

[Bug go/61877] [5 Regression]: reflect: cannot use []string as type string in Call

[ian at airs dot com]

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

--- Comment #8 from Ian Lance Taylor <ian at airs dot com> ---
I see only minor changes to makefuncgo_amd64.go between 4.9 and mainline--are
you sure you are looking at the right files?

[Bug go/61877] [5 Regression]: reflect: cannot use []string as type string in Call

[ian at airs dot com]

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

--- Comment #9 from Ian Lance Taylor <ian at airs dot com> ---
Thanks for the test case.  I wrote a completely different test case that is
more like the existing reflect tests: https://codereview.appspot.com/151280043/
.  This test passes with gc but fails with gccgo at present on amd64:

--- FAIL: TestVariadicMethodValue (0.00 seconds)
panic: reflect: cannot use []reflect_test.Point as type reflect_test.Point in
Call [recovered]
    panic: reflect: cannot use []reflect_test.Point as type reflect_test.Point
in Call

[Bug go/61877] [5 Regression]: reflect: cannot use []string as type string in Call

[michael.hudson at linaro dot org]

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

--- Comment #10 from Michael Hudson-Doyle <michael.hudson at linaro dot org> ---
I've proposed a fix https://codereview.appspot.com/152840043

[Bug go/61877] [5 Regression]: reflect: cannot use []string as type string in Call

[ian at gcc dot gnu.org]

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

--- Comment #11 from ian at gcc dot gnu.org <ian at gcc dot gnu.org> ---
Author: ian
Date: Fri Oct  3 15:51:38 2014
New Revision: 215859

URL: https://gcc.gnu.org/viewcvs?rev=215859&root=gcc&view=rev
Log:
    PR go/61877
refect: fix direct call of variadic method value

As reported in bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61877
gcc mainline has regressed in this.  This CL adds the tests proposed
for the main Go repository:

        https://codereview.appspot.com/151280043/
        https://codereview.appspot.com/152060043/

restores the code from the amd64/386 path that makes this work and
was lost when the Go 1.3 stdlib was merged and changes the FFI path
to call into the same helper code as the amd64/386 path.

I've only tested this on amd64 but I did test a version that was
patched to unconditionally take the FFI path.

Modified:
    trunk/libgo/go/reflect/all_test.go
    trunk/libgo/go/reflect/makefunc.go

[Bug go/61877] [5 Regression]: reflect: cannot use []string as type string in Call

[ian at airs dot com]

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

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

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

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