[Bug middle-end/83758] ICE building gccgo on powerpc64le --with-cpu=power8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83758
boger at us dot ibm.com changed:
What|Removed |Added
CC||boger at us dot ibm.com
--- Comment #5 from boger at us dot ibm.com ---
The gccgo command that actually results in the ICE is not what is shown above.
The original posting must have been based on a build using a -j option so some
of the compile output was interspersed. Here is the actual command that results
in the ICE:
/home/boger/gccgo.work/trunk/bld/./gcc/gccgo
-B/home/boger/gccgo.work/trunk/bld/./gcc/
-B/usr/local/gccgo.trunk/powerpc64le-linux/bin/
-B/usr/local/gccgo.trunk/powerpc64le-linux/lib/ -isystem
/usr/local/gccgo.trunk/powerpc64le-linux/include -isystem
/usr/local/gccgo.trunk/powerpc64le-linux/sys-include -O2 -g -I . -c
-fgo-pkgpath=cmd/go/internal/work
../../../src/libgo/go/cmd/go/internal/work/action.go
../../../src/libgo/go/cmd/go/internal/work/build.go
../../../src/libgo/go/cmd/go/internal/work/buildid.go
../../../src/libgo/go/cmd/go/internal/work/exec.go
../../../src/libgo/go/cmd/go/internal/work/gc.go
../../../src/libgo/go/cmd/go/internal/work/gccgo.go
../../../src/libgo/go/cmd/go/internal/work/init.go -o cmd/go/internal/work.o
during RTL pass: vartrack
../../../src/libgo/go/cmd/go/internal/work/gccgo.go: In function
\u2018work.gc.N35_cmd_go_internal_work.gccgoToolchain\u2019:
../../../src/libgo/go/cmd/go/internal/work/gccgo.go:54:1: internal compiler
error: in vt_expand_var_loc_chain, at var-tracking.c:8331
func (tools gccgoToolchain) gc(b *Builder, a *Action, archive string,
importcfg []byte, asmhdr bool, gofiles []string) (ofile string, output []byte,
err error) {
^
0x10d37b27 vt_expand_var_loc_chain
../../src/gcc/var-tracking.c:8331
0x10d37b27 vt_expand_loc_callback
../../src/gcc/var-tracking.c:8527
0x1041aad3 cselib_expand_value_rtx_1
../../src/gcc/cselib.c:1715
0x1041a797 cselib_expand_value_rtx_1
../../src/gcc/cselib.c:1753
0x1041a797 cselib_expand_value_rtx_1
../../src/gcc/cselib.c:1753
0x1041d837 cselib_expand_value_rtx_cb(rtx_def*, bitmap_head*, int, rtx_def*
(*)(rtx_def*, bitmap_head*, int, void*), void*)
../../src/gcc/cselib.c:1561
0x10d375d3 vt_expand_var_loc_chain
../../src/gcc/var-tracking.c:8365
0x10d375d3 vt_expand_loc_callback
../../src/gcc/var-tracking.c:8527
0x1041aad3 cselib_expand_value_rtx_1
../../src/gcc/cselib.c:1715
0x1041a797 cselib_expand_value_rtx_1
../../src/gcc/cselib.c:1753
0x1041d837 cselib_expand_value_rtx_cb(rtx_def*, bitmap_head*, int, rtx_def*
(*)(rtx_def*, bitmap_head*, int, void*), void*)
../../src/gcc/cselib.c:1561
0x10d375d3 vt_expand_var_loc_chain
../../src/gcc/var-tracking.c:8365
0x10d375d3 vt_expand_loc_callback
../../src/gcc/var-tracking.c:8527
0x1041aad3 cselib_expand_value_rtx_1
../../src/gcc/cselib.c:1715
0x1041d837 cselib_expand_value_rtx_cb(rtx_def*, bitmap_head*, int, rtx_def*
(*)(rtx_def*, bitmap_head*, int, void*), void*)
../../src/gcc/cselib.c:1561
0x10d38263 vt_expand_var_loc_chain
../../src/gcc/var-tracking.c:8365
0x10d38263 vt_expand_1pvar
../../src/gcc/var-tracking.c:8640
0x10d38263 emit_note_insn_var_location(variable**, emit_note_data*)
../../src/gcc/var-tracking.c:8695
0x10d396df void hash_table::traverse_noresize(emit_note_data*)
../../src/gcc/hash-table.h:969
0x10d396df void hash_table::traverse(emit_note_data*)
../../src/gcc/hash-table.h:990
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
Also note that the file it is complaining about
src/libgo/go/cmd/go/internal/work/gccgo.go is brand new with the latest update
to the go1.10beta1.
[Bug debug/83758] ICE building gccgo on powerpc64le --with-cpu=power8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83758 --- Comment #8 from boger at us dot ibm.com --- An update was made to go1.10beta2, so I rebuilt with the updates but the same error happens at the same statement.
[Bug debug/83758] ICE building gccgo on powerpc64le --with-cpu=power8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83758 --- Comment #21 from boger at us dot ibm.com --- (In reply to Alexandre Oliva from comment #19) > I was copied, presumably because the problem occurred in var-tracking. > > I've tried to duplicate the problem on gcc112. I bootstrapped the trunk > (without any --with-cpu flag), and then build attachment 43208 [details] > with stage3: > > $ ./gccgo -B./ -I../../libgo/go -I../powerpc64le-unknown-linux-gnu/libgo > -I../powerpc64le-unknown-linux-gnu/libgo/go -O2 -g t.go -mcpu=power8 -c > [success] > > Given this, I conclude that the problem only occurs if the compiler is > bootstrapped --with-cpu=power[89]. This suggests to me that, although the > ICE is triggered in var-tracking, the problem is unlikely to be there: the > compiler is getting miscompiled. I suppose that, by now, you already knew > that ;-) The problem only occurs when configuring --with-cpu=power8 or 9 AND compiling for split stack. If you are on a system where glibc is 2.17 or older then split stack is not enabled and you won't see the failure even with --with-cpu=power8 or 9.
[Bug debug/83758] ICE building gccgo on powerpc64le --with-cpu=power8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83758
--- Comment #25 from boger at us dot ibm.com ---
A few other variations that enable it to work even with a power8 configuration:
Compiling with -fdisable-ipa-cp prevents the ICE.
OR
Using the //go:nosplit directive before the function identified in the error
message prevents the ICE.
OR making various source changes to the function in some cases allows it to
work.
for example, in function:
func (tools gccgoToolchain) gc(b *Builder, a *Action, archive string, importcfg
[]byte, asmhdr bool, gofiles []string) (ofile string, output []byte, err error)
Commenting out this loop prevents the error:
for _, f := range gofiles {
args = append(args, mkAbs(p.Dir, f))
}
[Bug other/80803] libgo appears to be miscompiled on powerpc64le since r247923
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80803 boger at us dot ibm.com changed: What|Removed |Added CC||boger at us dot ibm.com --- Comment #10 from boger at us dot ibm.com --- Bill, I've been out for a few days but can help debug this now that I'm back. It looks like the test case gets the correct answer but the string it thinks is correct is wrong (always nil). Either the initialization of the expected values is wrong to begin with or they are being corrupted during the run. We should be able to figure that out with gdb.
[Bug other/80803] libgo appears to be miscompiled on powerpc64le since r247923
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80803
--- Comment #11 from boger at us dot ibm.com ---
The first failure happens in TestParseIP from ip_test.go because the "out"
entries in the var parseIPTests are not initialized correctly. This causes the
failures because the actual value (which is correct) doesn't match the expected
value (which is incorrect).
var parseIPTests = []struct {
in string
out IP
}{
{"127.0.1.2", IPv4(127, 0, 1, 2)},
{"127.0.0.1", IPv4(127, 0, 0, 1)},
{"127.001.002.003", IPv4(127, 1, 2, 3)},
{":::127.1.2.3", IPv4(127, 1, 2, 3)},
{":::127.001.002.003", IPv4(127, 1, 2, 3)},
{":::7f01:0203", IPv4(127, 1, 2, 3)},
{"0:0:0:0:::127.1.2.3", IPv4(127, 1, 2, 3)},
{"0:0:0:0:00::127.1.2.3", IPv4(127, 1, 2, 3)},
{"0:0:0:0:::127.1.2.3", IPv4(127, 1, 2, 3)},
.
I believe this is a static var, and the initialization of "out" is done through
a call to IPv4 (which does a make) but I'm not sure where and when this
initialization is supposed to occur? I tried to use gdb and set a watch where
I thought it should be initialized and it didn't trigger.
Another oddity, when the entire net.test is run, the output from the call to
UnmarshalText is extremely long and that is what causes the output file to get
so large, but if the test ParseIP is run by itself, it fails but does not
generate the excessive output. But in both cases, the initial failure is due
to bad initialization of the "out" entries.
[Bug other/80803] libgo appears to be miscompiled on powerpc64le since r247848
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80803 --- Comment #14 from boger at us dot ibm.com --- I have found the incorrect code that is causing this test to fail but not sure what causes it. My previous theory was not correct. The problem is that the second argument being passed to runtime.DeepEqual in net.TestParseIP is wrong. The storage for the arguments are being generated by calls to runtime.newobject, followed by code to fill in the contents. The arguments are slices so should have 3 doublewords including the address and 2 lengths. The first argument is initialized correctly but the second is not storing the length values. Here is a snippet of code from the bad a.out: 10066158: 69 91 fc 4b bl 1002f2c0 <3287.plt_call.runtime.newobject> 1006615c: 18 00 41 e8 ld r2,24(r1) 10066160: 10 02 01 e9 ld r8,528(r1) 10066164: 18 02 41 e9 ld r10,536(r1) 10066168: 20 02 21 e9 ld r9,544(r1) 1006616c: f8 01 a1 38 addir5,r1,504 10066170: 78 1b 72 7c mr r18,r3 10066174: 78 fb e3 7f mr r3,r31 10066178: 78 93 44 7e mr r4,r18 1006617c: f8 01 01 f9 std r8,504(r1) 10066180: 00 02 41 f9 std r10,512(r1) 10066184: 08 02 21 f9 std r9,520(r1) 10066188: d9 92 fc 4b bl 1002f460 <3287.plt_call.runtime.typedmemmove> 1006618c: 18 00 41 e8 ld r2,24(r1) 10066190: 78 fb e3 7f mr r3,r31 10066194: 2d 91 fc 4b bl 1002f2c0 <3287.plt_call.runtime.newobject> 10066198: 18 00 41 e8 ld r2,24(r1) 1006619c: e0 01 a1 38 addir5,r1,480 100661a0: e0 01 c1 fb std r30,480(r1) 100661a4: 78 1b 70 7c mr r16,r3 100661a8: 78 fb e3 7f mr r3,r31 100661ac: 78 83 04 7e mr r4,r16 100661b0: b1 92 fc 4b bl 1002f460 <3287.plt_call.runtime.typedmemmove> 100661b4: 18 00 41 e8 ld r2,24(r1) 100661b8: 78 fb e5 7f mr r5,r31 100661bc: 78 83 06 7e mr r6,r16 100661c0: 78 fb e3 7f mr r3,r31 100661c4: 78 93 44 7e mr r4,r18 100661c8: 49 9b fc 4b bl 1002fd10 <3287.plt_call.reflect.DeepEqual> In gdb I can see that the length field in the slice passed to DeepEqual and then deepValueEqual is 0 causing them to appear not equal, and the length 0 causes other problems later on. I tried to reproduce this in a small testcase but it did not fail. In that case the code looks like this: 10002c48: 19 f9 ff 4b bl 10002560 <00be.plt_call.runtime.newobject> 10002c4c: 18 00 41 e8 ld r2,24(r1) 10002c50: 78 1b 69 7c mr r9,r3 10002c54: c0 02 3f f9 std r9,704(r31) 10002c58: 48 03 1f e9 ld r8,840(r31) 10002c5c: 50 03 5f e9 ld r10,848(r31) 10002c60: 58 03 3f e9 ld r9,856(r31) 10002c64: a8 02 1f f9 std r8,680(r31) 10002c68: b0 02 5f f9 std r10,688(r31) 10002c6c: b8 02 3f f9 std r9,696(r31) 10002c70: c0 02 3f e9 ld r9,704(r31) 10002c74: a8 02 5f 39 addir10,r31,680 10002c78: 78 53 45 7d mr r5,r10 10002c7c: 78 4b 24 7d mr r4,r9 10002c80: 58 80 62 e8 ld r3,-32680(r2) 10002c84: fd fa ff 4b bl 10002780 <00be.plt_call.runtime.typedmemmove> 10002c88: 18 00 41 e8 ld r2,24(r1) 10002c8c: c0 02 3f e9 ld r9,704(r31) 10002c90: 78 4b 33 7d mr r19,r9 10002c94: 58 80 82 ea ld r20,-32680(r2) 10002c98: 58 80 62 e8 ld r3,-32680(r2) 10002c9c: c5 f8 ff 4b bl 10002560 <00be.plt_call.runtime.newobject> 10002ca0: 18 00 41 e8 ld r2,24(r1) 10002ca4: 78 1b 69 7c mr r9,r3 10002ca8: a0 02 3f f9 std r9,672(r31) 10002cac: c8 02 1f e9 ld r8,712(r31) 10002cb0: d0 02 5f e9 ld r10,720(r31) 10002cb4: d8 02 3f e9 ld r9,728(r31) 10002cb8: 88 02 1f f9 std r8,648(r31) 10002cbc: 90 02 5f f9 std r10,656(r31) 10002cc0: 98 02 3f f9 std r9,664(r31) 10002cc4: a0 02 3f e9 ld r9,672(r31) 10002cc8: 88 02 5f 39 addir10,r31,648 10002ccc: 78 53 45 7d mr r5,r10 10002cd0: 78 4b 24 7d mr r4,r9 10002cd4: 58 80 62 e8 ld r3,-32680(r2) 10002cd8: a9 fa ff 4b bl 10002780 <00be.plt_call.runtime.typedmemmove> 10002cdc: 18 00 41 e8 ld r2,24(r1) 10002ce0: a0 02 3f e9 ld r9,672(r31) 10002ce4: 78 4b 35 7d mr r21,r9 10002ce8: 78 a3 85 7e mr r5,r20 10002cec: 78 ab a6 7e mr r6,r21 10002cf0: 78 93 43 7e mr r3,r18 10002cf
[Bug other/80803] libgo appears to be miscompiled on powerpc64le since r247848
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80803 --- Comment #16 from boger at us dot ibm.com --- After further investigation I found the original testcase failures started happening with an earlier commit, which I have verified as 247497. That commit caused 7 new libgo testcase failures, but at the time, it didn't cause the error log to grow excessively so went unnoticed. When the large gofrontend change went in, then the same failures occurred but it was at that point when the information written to the error log grew to an excessive size. Probably due to the random nature of the original bug. One additional note, when debugging a few other failures, I found that when calling runtime.typedmemmove from reflect.typedmemmove, the second argument is not being initialized, which results in a SEGV for some tests (reflect, archive/tar, mime/multipart). It might be easier to compile the file containing reflect.typedmemmove and dump out the intermediate output?
[Bug other/80803] libgo appears to be miscompiled on powerpc64le since r247497
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80803
--- Comment #17 from boger at us dot ibm.com ---
Here's more info on the failures and how to reproduce them. Starting with
commit 247497 there are 7 new failures in the libgo testsuite.
There are 4 that fail with a SEGV at runtime: reflect, archive/tar,
mime/multipart, and net/mail. I looked at 2 and their stacks look similar but
not quite the same. The SEGV happens because a 0 is passed as the address of
the src argument.
archive/tar shows a stack like this at the point of the SEGV:
Program received signal SIGSEGV, Segmentation fault.
__memcpy_power7 () at ../sysdeps/powerpc/powerpc64/power7/memcpy.S:219
219 ../sysdeps/powerpc/powerpc64/power7/memcpy.S: No such file or
directory.
(gdb) bt
#0 __memcpy_power7 () at ../sysdeps/powerpc/powerpc64/power7/memcpy.S:219
#1 0x3fffb6350918 in __GI_memmove (dest=0xc208086600, src=,
len=) at ../sysdeps/powerpc/memmove.c:54
#2 0x3fffb71567bc in runtime.memmove (p1=, p2=, len=)
at ../../../src/libgo/runtime/go-memmove.c:15
#3 0x3fffb7620fd8 in runtime.typedmemmove (typ=,
dst=, src=)
at ../../../src/libgo/go/runtime/stubs.go:298
#4 0x3fffb76215bc in reflect.typedmemmove (typ=,
dst=, src=)
at ../../../src/libgo/go/runtime/stubs.go:304
#5 0x3fffb75c8384 in reflect.packEface (v=...) at
../../../src/libgo/go/reflect/value.go:113
#6 reflect.valueInterface (param=..., safe=true) at
../../../src/libgo/go/reflect/value.go:821
#7 0x3fffb75c8760 in reflect.Interface.N13_reflect.Value
(pointer=) at ../../../src/libgo/go/reflect/value.go:781
#8 0x3fffb72e570c in fmt.printValue.pN6_fmt.pp (p=,
param=..., verb=, depth=)
at ../../../src/libgo/go/fmt/print.go:694
#9 0x3fffb72e623c in fmt.printValue.pN6_fmt.pp (p=,
param=..., verb=, depth=)
With gdb I found that when reflect.typedmemmove calls runtime.typedmemmove, it
is passing a bad argument for the src.
Something similar happens in the reflect test, although that SEGV stack looks
like this:
288 ../sysdeps/powerpc/powerpc64/power7/memcpy.S: No such file or
directory.
(gdb) bt
#0 __memcpy_power7 () at ../sysdeps/powerpc/powerpc64/power7/memcpy.S:288
#1 0x3fffb6350918 in __GI_memmove (dest=0xc20800b6d0, src=,
len=) at ../sysdeps/powerpc/memmove.c:54
#2 0x3fffb71567bc in runtime.memmove (p1=, p2=, len=)
at ../../../src/libgo/runtime/go-memmove.c:15
#3 0x3fffb762165c in runtime.typedslicecopy (typ=, dst=...,
src=...) at ../../../src/libgo/go/runtime/stubs.go:329
#4 0x3fffb76216cc in reflect.typedslicecopy (elemType=,
dst=..., src=...) at ../../../src/libgo/go/runtime/stubs.go:336
#5 0x10052c6c in reflect.Copy (param=..., param=...) at value.go:1747
#6 0x100529f4 in reflect.AppendSlice (param=..., param=...) at
value.go:1702
#7 0x10069514 in reflect_test.TestAppend (t=) at
all_test.go:534
And the bad address is passed for the src argument for the call from
reflect.typedslicecopy to runtime.typedslicecopy.
The tests for fmt, net, and debug/dwarf fail when processing code that calls
reflect.DeepEqual followed by a call to Errorf, something like this:
if out := ParseIP(tt.in); !reflect.DeepEqual(out, tt.out) {
t.Errorf("ParseIP(%q) = %v, want %v", tt.in, out, tt.out)
}
The argument for tt.out is passed incorrectly to DeepEqual, causing it to
return false when it shouldn't, and then passes bad information to the Errorf
function.
I've tried to reproduce this in a smaller testcase but haven't been able to. I
run these tests after a build by first editing the src/libgo/testsuite/gotest
to set keep=true and trace=true. Then I go to my bld directory:
cd bld/powerpc64le-linux/libgo
make fmt/check
That should give you output with the full gccgo command to build the test, the
directory containing the a.out and files from the test. The name of the
directory is gotestx.
FAIL
Keeping gotest32163
FAIL: fmt
Makefile:3331: recipe for target 'fmt/check' failed
[Bug other/80803] libgo appears to be miscompiled on powerpc64le since r247497
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80803 --- Comment #19 from boger at us dot ibm.com --- Is someone building and testing gccgo on x86_64 on a regular basis? If I look at the gcc-testresults output for x86_64 I don't see the go or libgo test results like I do for ppc64le. Maybe it does fail there too.
[Bug go/78700] New: gccgo testcases stack.go, recover.go, crypto/tls start failing with r241222
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78700 Bug ID: 78700 Summary: gccgo testcases stack.go, recover.go, crypto/tls start failing with r241222 Product: gcc Version: 7.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 Milestone: --- There are 3 failures in the gccgo testcases starting with commit 241222. I built with 241215 and none of these tests failed there, and the two between are simple and relate to fortran and sparc. In the go tests for gcc, stack.go failure looks like this: bad func 21 at level 8000 panic: fail goroutine 16 []: __go_panic /home/boger/gccgo.work/trunk/bld/../src/libgo/runtime/go-panic.c:110main.recur /home/boger/gccgo.work/trunk/src/gcc/testsuite/go.test/test/stack.go:87main.main /home/boger/gccgo.work/trunk/src/gcc/testsuite/go.test/test/stack.go:99FAIL: go.test/test/stack.go execution, -O2 -g testcase /home/boger/gccgo.work/trunk/src/gcc/testsuite/go.test/go-test.exp completed in 1 seconds recover.go: panic: 13 panic: runtime error: invalid memory address or nil pointer dereference [signal 0xb code=0x1 addr=0xa] goroutine 16 [running]: __go_panic /home/boger/gccgo.work/trunk/bld/../src/libgo/runtime/go-panic.c:110__go_panic /home/boger/gccgo.work/trunk/bld/../src/libgo/runtime/go-panic.c:66main.test13reflect2 /home/boger/gccgo.work/trunk/src/gcc/testsuite/go.test/test/recover.go:456main.main /home/boger/gccgo.work/trunk/src/gcc/testsuite/go.test/test/recover.go:58 goroutine 18 [finalizer wait]: Also, libgo testcase crypto/tls starts failing with this commit as well: unexpected fault address 0xc02f00010313 fatal error: fault [signal 0xb code=0x1 addr=0xc02f00010313] goroutine 77 [running]: runtime_dopanic /home/boger/gccgo.work/trunk/bld/../src/libgo/runtime/panic.c:133runtime_throw /home/boger/gccgo.work/trunk/bld/../src/libgo/runtime/panic.c:195sig_panic_info_handler /home/boger/gccgo.work/trunk/bld/../src/libgo/runtime/go-signal.c:296 :0tls.$nested12 /home/boger/gccgo.work/trunk/bld/powerpc64le-linux/libgo/gotest16088/test/handshake_client_test.go:755tls.$nested3 /home/boger/gccgo.work/trunk/bld/powerpc64le-linux/libgo/gotest16088/test/handshake_client_test.go:370 /home/boger/gccgo.work/trunk/bld/powerpc64le-linux/libgo/gotest16088/test/handshake_client_test.go:273kickoff /home/boger/gccgo.work/trunk/bld/../src/libgo/runtime/proc.c:257created by crypto_tls.run.pN21_crypto_tls.clientTest /home/boger/gccgo.work/trunk/bld/powerpc64le-linux/libgo/gotest16088/test/handshake_client_test.go:302 +828 goroutine 16 [chan receive]: testing.Run.pN9_testing.T /home/boger/gccgo.work/trunk/bld/../src/libgo/go/testing/testing.go:646testing.$nested9 /home/boger/gccgo.work/trunk/bld/../src/libgo/go/testing/testing.go:792testing.tRunner /home/boger/gccgo.work/trunk/bld/../src/libgo/go/testing/testing.go:609testing.RunTests /home/boger/gccgo.work/trunk/bld/../src/libgo/go/testing/testing.go:790testing.Run.pN9_testing.M /home/boger/gccgo.work/trunk/bld/../src/libgo/go/testing/testing.go:742main.main /home/boger/gccgo.work/trunk/bld/powerpc64le-linux/libgo/gotest16088/test/_testmain.go:109 goroutine 18 [finalizer wait]: goroutine 20 [syscall]: goroutine in C code; stack unavailable goroutine 76 [semacquire]: sync.runtime_notifyListWait /home/boger/gccgo.work/trunk/bld/../src/libgo/go/runtime/sema.go:271sync.Wait.pN9_sync.Cond /home/boger/gccgo.work/trunk/bld/../src/libgo/go/sync/cond.go:57io.read.pN7_io.pipe /home/boger/gccgo.work/trunk/bld/../src/libgo/go/io/pipe.go:47io.Read.pN13_io.PipeReader /home/boger/gccgo.work/trunk/bld/../src/libgo/go/io/pipe.go:129io.ReadAtLeast /home/boger/gccgo.work/trunk/bld/../src/libgo/go/io/io.go:307io.ReadFull /home/boger/gccgo.work/trunk/bld/../src/libgo/go/io/io.go:325crypto_tls.run.pN21_crypto_tls.clientTest /home/boger/gccgo.work/trunk/bld/powerpc64le-linux/libgo/gotest16088/test/handshake_client_test.go:387tls.runClientTestForVersion /home/boger/gccgo.work/trunk/bld/powerpc64le-linux/libgo/gotest16088/test/handshake_client_test.go:428tls.runClientTestTLS12 /home/boger/gccgo.work/trunk/bld/powerpc64le-linux/libgo/gotest16088/test/handshake_client_test.go:440crypto_tls.TestHandshakClientSCTs /home/boger/gccgo.work/trunk/bld/powerpc64le-linux/libgo/gotest16088/test/handshake_client_test.go:770testing.tRunner /home/boger/gccgo.work/trunk/bld/../src/libgo/go/testing/testing.go:609created by testing.Run.pN9_testing.T /home/boger/gccgo.work/trunk/bld/../src/libgo/go/testing/testing.go:645 +660 /home/boger
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #42 from boger at us dot ibm.com --- (In reply to Ian Lance Taylor from comment #41) > I really don't want libbacktrace to be processor-dependent. That makes all > uses of it more complex for no significant gain. Maybe we should change > libbacktrace, but definitely not by making it processor-dependent. I'm sorry but I have to disagree. In my opinion what libbacktrace does now for Power and s390/s390x is wrong. It does not return valid PC values for those platforms. Everyone who calls it has to know that they must look through the list of PC values and change them back. In addition, libbacktrace has already looked up the line number information with those invalid PCs. So essentially libbacktrace is returning invalid PCs and usually incorrect line numbers which the caller has to know about and fix up and that seems significant to me. Maybe the PCs should never be decremented for any platform? > > I would prefer that changes to files like runtime/pprof/pprof.go and > log/log.go go through the master repo first. If gccgo's runtime.Callers > acts the same as gc's runtime.Callers, then there shouldn't be any > gccgo-specific need to change those files. Yes I agree that is the way it should be done. I mainly wanted to show all the changes in one place for review and didn't think through the process for getting that code in place. > > Can somebody please try just editing the function callback in > runtime/go-callers.c to add one to pc when storing it in loc->pc? I'd like > to understand why that simple change doesn't work before trying something > more complex. Yes I've already tried that. But I ran into the issues I described in my first comment above, especially seeing that all the stack information from runtime_callers had incorrect line information and it is not obvious to the caller that they should fix that information before using it. In addition, there is evidence that looking up the line number information is not cheap, so doing it in libbacktrace and then having to look it up again if it is needed seems like a waste especially in those cases when the line number information is not even used.
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #44 from boger at us dot ibm.com --- If we do the increment of the pc to fix it in the callback, here is how that happens: - backtrace_full gets the pc and decrements by 1 - backtrace_full calls backtrace_pcinfo to look up the file, function, line number information based on the decremented pc - backtrace_pcinfo calls the callback function after the line number has been determined. If the pc is incremented here then it might not correspond to the line number anymore. A common example is if you have a statement that does a function call with no return value, the line number will be wrong because the call instruction and the instruction following the call correspond to different line numbers.
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #48 from boger at us dot ibm.com --- (In reply to Ian Lance Taylor from comment #47) > We have to separate backtrace_full and backtrace_simple, which are part of > the libbacktrace library, from functions like runtime_callers which are part > of libgo. The libbacktrace library is used by several different projects. > It is not part of libgo. > > It may be correct that libbacktrace should never decrement pc. But the > argument for that can't be the behaviour of the gc library. > The backtraces I'm familiar with are those where the pc values represent the instruction after the call. Regardless of what the gc library does I don't see why you'd want to decrement the pcs in backtrace_full. Actually to me it seems like decrementing it would be wrong for all platforms (based on what is done in pprof.go -- doesn't that mean in some cases it would get decremented twice?) but I just haven't had time to investigate that further. > The bit in comment #9 about not decrementing the pc for a signal frame is > specific to libbacktrace. You can see that in unwind in > libbacktrace/backtrace.c and in simple_unwind in libbacktrace/simple.c. > > Perhaps we need to change libbacktrace so that backtrace_full_callback and > backtrace_simple_callback take an additional parameter, which is whether the > PC is in a signal frame. From libgo's perspective, though, that would be > exactly the same as simply always adding to pc in callback in go-callers.c. > You said that did not work, and I'm still trying to understand why. I'm not exactly sure what you are describing -- which functions would have an additional argument and what would you do with that argument. Here is the problem with incrementing the pc in the callback. In unwind the pc is decremented, and the line number is looked up for that pc and saved away. Then callback is called, and the pc is incremented, but now the line number that was saved away is stale. That means when on return to runtime_callers, the pcs are all correct but some line numbers might not be right for those pcs. I tried adding code to look up the line number again in the callback after the pc was modified but that caused the program to hang.
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #50 from boger at us dot ibm.com --- (In reply to Ian Lance Taylor from comment #49) > libbacktrace returns the line number that you actually care about: the line > number of the call instruction. There is no question that that is correct. > > You say that it is a problem if the PC does not match the line number, but > to me that sounds like a conceptual problem. What is the actual problem? > > The pprof.go code expects to get the PC after the call instruction, and > tries to turn it into the PC of the call instruction. I think we can all > agree that pprof.go should in fact see the PC after the call instruction. > The simple way to do that is for callback in libgo/runtime/go-callers.c to > increment pc before storing it in loc->pc. Apparently there is some problem > with that but I do not know what that problem is. > > The additional argument I mentioned would be for the function types > backtrace_fulL_callback and backtrace_simple_callback defined in > libbacktrace/backtrace.h. The additional argument would be, essentially, > the value of ip_before_insn in libbacktrace/backtrace.c and > libbacktrace/simple.c. The argument would tell you whether the PC follows a > call instruction, or whether it is the correct PC for a signal frame > instruction OK. I didn't realize that backtrace_full is expected to return an array of Location structures which contain the PC for the instruction following the call but the line number for the call. When I saw there was a single structure containing a PC and lineno, I assumed that it was intended to hold information for a single location, i.e., a PC and the lineno that corresponded to it. I will go back and try this again as you suggested. I had tried several variations and hit different errors and when I saw how the line number and PC were out of sync that steered me down the wrong path. Now that I understand what it really should be I think it should probably work.
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #46 from boger at us dot ibm.com --- (In reply to Ian Lance Taylor from comment #45) > If we change the PC returned by backtrace_full, and then use that changed PC > to look up file/line information, we might get different results. That > seems clear. My next question is: when does this matter? > > There are two ways that we use the result of runtime_callers: either we use > the file/line information, or we use the PC. If we use the file/line > information, all is well, and there is nothing to discuss. If we use the > PC, as in runtime.Callers, then it's true that if we pass that PC back to > libbacktrace we may get different file/line information. But, so what? > We've already lost the original file/line information anyhow. > > The idea is that the changed PC will be the same as the PC returned by the > gc library. Therefore, we should get approximately the same file/line > information as the gc library gets. That is why runtime/pprof/pprof.go in > the gc library backs up the PC to the call instruction: because it knows > that it has the PC after the call instruction. Let me just respond to this last paragraph for now. If the pc values in gc are always the instruction following the call (or the excepting instruction), and runtime_callers expects it to be that way, and we want gc and gccgo to return the same results, then I don't understand why backtrace_full and backtrace_simple should ever decrement the pc for any platform. There is a brief mention of this in comment 9 -- that we can make use the fact that the pc had been decremented by inspecting it and making use of the fact that is an abnormal pc value; however I don't see anywhere in the libgo code where it does this, and besides, for some platforms after the pc is decremented it is still a normal looking pc so there is no way to know if it had been decremented. Getting the right pc and the right instruction seems more important than preserving some information that is currently not being used and can't be used on all platforms. That means, if the decrement was removed completely from backtrace_full and backtrace_simple then the pc values would be correct for each platform and the line numbers would be determined based on the correct pc. And then we would still need the BackupPC function for those cases where the call instruction was needed as is done in pprof.go and some extra stuff for s390 and s390x for their various call instructions to get the right number of bytes to back up.
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #51 from boger at us dot ibm.com --- Here is the change I made to go-callers.c. This patch along with my previous changes to extern.go and pprof.go allows the test identified in this issue to report the correct line number on ppc64le. I wanted to post it here for comments before I do the bootstrap build and test on ppc64 and ppc64le in case there is a concern with using an #if defined. This is in libgo/runtime and I didn't see any other examples where it used the GOARCH value in C code so that is why I did it this way. === --- libgo/runtime/go-callers.c (revision 221039) +++ libgo/runtime/go-callers.c (working copy) @@ -83,6 +83,10 @@ callback (void *data, uintptr_t pc, const char *fi } loc = &arg->locbuf[arg->index]; +#if defined(__powerpc64__) || defined(__s390__) || defined(__s390x__) + if ((pc & 1) == 1) +pc++; +#endif loc->pc = pc; /* The libbacktrace library says that these strings might disappear,
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #53 from boger at us dot ibm.com --- I was taking the approach of only fixing what was known to be broken, and I was not aware that this was broken on other platforms. Minimizing risk. I can change it for all platforms but these tests are not broken on x86 today with the pc decremented in both libbacktrace and in pprof.go and I don't understand why not so I didn't want to mess with it. I'm not sure if I understand how it would work to add an argument to callback. The call to callback happens much later after the ip_before_insn flag has been set: from inside unwind -> backtrace_pcinfo -> dwarf_fileline -> dwarf_lookup_pc -> callback. Do you mean pass this flag as an argument to each of these functions so it can be passed to callback?
[Bug go/65462] New: Use of 'go get' with gccgo is not finding dependencies correctly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65462
Bug ID: 65462
Summary: Use of 'go get' with gccgo is not finding dependencies
correctly
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
When using the go tool provided with gccgo in gcc 5.0, the use of 'go get'
doesn't always find and build the dependent packages correctly.
Here is an example:
go get -u github.com/mitchellh/gox
There is a dependent package github.com/mitchellh/iochan but that never gets
downloaded or built and as a result neither does github.com/mitchellh/gox. I
can make it work by downloading them in separate steps this way:
go get -u github.com/mitchellh/uchan
go get -u github.com/mitchellh/gox
After further debugging I found the problem has to do with the code in
libgo/go/cmd/go/pkg.go in the load function where it does this check:
p1 := loadImport(path, p.Dir, stk, p.build.ImportPos[path])
if !reqPkgSrc && p1.Root == "" {
continue
}
When it tries to load the github.com/mitchellh/uchan package, p1.Root == "" so
it skips the rest of the code in the loop that loads the package. Isn't this a
case where p1.Root should have a non-null string and therefore not
continue/skip the rest of the body of the loop and get it loaded?
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #31 from boger at us dot ibm.com --- Here are two suggestions to solve this issue without having to use the -a and -tags netgo options to rebuild packages at build time. Since this is a common problem, it seems best to provide a way to have the packages built and provided with the gccgo build instead of requiring users to know how to rebuild the net package with netgo. 1) Since libgo is provided for both static (libgo.a) and dynamic (libgo.so.N) linking, and the problem only happens with static linking, we could build libgo.a with netgo enabled, so that would be the expected/default behavior with static linking with gccgo. 2) As part of the gccgo and libgo build, build just the net package with netgo enabled and install it somewhere that is easily found, using the normal directory conventions for GO. Then if someone builds a program for static linking and want the GO DNS resolver, they could link in that package before they link in libgo.a.
[Bug go/65462] Use of 'go get' with gccgo is not finding dependencies correctly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65462 --- Comment #1 from boger at us dot ibm.com --- Created attachment 35077 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35077&action=edit Fix dependencies in go tool for gccgo The code to determine when to import packages was not quite catching the cases it should. It really needs to know what packages are part of the GO standard packages and not try to load those -- if they aren't then they should be loaded. This makes a few changes to the previous: - change the name of reqPkgSrc to reqStdPkgSrc to indicate when we should require the source for GO standard packages when loading packages. - generate the list of standard packages found in libgo and make that available to build into the go tool - set the flag Standard in the Package structure correctly for gccgo, based on the list of expected libgo standard packages - use the reqStdPkgSrc flag and the package.Standard flag to determine if the load of the package should be attempted
[Bug go/65462] Use of 'go get' with gccgo is not finding dependencies correctly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65462 --- Comment #2 from boger at us dot ibm.com --- Created attachment 35081 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35081&action=edit Updated patch to add unsafe to the list of std package names
[Bug go/65462] Use of 'go get' with gccgo is not finding dependencies correctly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65462 --- Comment #4 from boger at us dot ibm.com --- Do you mean as if submitting to gofrontend-dev?
[Bug go/65462] Use of 'go get' with gccgo is not finding dependencies correctly
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65462 --- Comment #6 from boger at us dot ibm.com --- I decided that you meant the gofrontend so here it is (just did the hg mail) https://codereview.appspot.com/213570043/
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #32 from boger at us dot ibm.com --- I have a prototype working for #2. I am assuming #1 would not be accepted. This fix consists of building a library called libnetgo.a which consists of the net files that would be built if the netgo tag was used. This new library was installed into the same directory as libgo.a. Once this library has been built and installed in the correct location, I was able to get this to work by explicitly linking in this lib: go build -gccgoflags '-static -lnetgo' lookup.go I will attach a patch after some more testing.
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #33 from boger at us dot ibm.com --- Created attachment 35195 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35195&action=edit "Fallback" netgo solution for gccgo This patch updates the libgo Makefile to build and install the library libnetgo.a into the same install directory as libgo.a. When libnetgo.a is available then the user can link it into their statically linked program and get the same result as when using the netgo fallback mechanism that is available with golang as mentioned in this bugzilla. Once libnetgo.a is built and installed then a statically linked program can link this into their program as follows: go build -gccgoflags '-static -lnetgo' lookup.go
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #34 from boger at us dot ibm.com --- Created a codereview: https://codereview.appspot.com/217620043
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #37 from boger at us dot ibm.com --- Created attachment 35260 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35260&action=edit libgo/go/go/build/doc.go documentation update Adding comments about the use of the netgo tag and the equivalent method for use with gccgo.
[Bug go/65755] New: incorrect reflection of struct fields with gccgo
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65755
Bug ID: 65755
Summary: incorrect reflection of struct fields with gccgo
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: ppc64le, ppc64, x86_64
packer testcase fails due to incorrect reflected struct field when test is
built with gccgo. Failure occurs with gccgo on ppc64le and x86_64 but works
when built with gc for both.
I've tried to reproduce this in a smaller testcase but have not been able to.
I'm not 100% sure the code is correct go code but I couldn't find anything to
tell me otherwise, and since it works with gc and works in my smaller testcase
with gccgo it seems like it must be correct.
To build the app and the testsuite follow the directions here
https://github.com/mitchellh/packer#developing-packer
When doing 'make' the testsuite is run, and there are two failures. The
failure documented in this bugzilla is:
FAILgithub.com/mitchellh/packer/fix
In github.com/mitchellh/packer/fix there are several files which declare the
template struct within a function as follows:
// Our template type we'll use for this fixer only
type template struct {
Builders []map[string]interface{}
}
However in fixer_pp_vagrant_override.go the template struct is declared like
this:
type template struct {
PostProcessors []interface{} `mapstructure:"post-processors"`
}
In all cases, the template struct is passed to another function which uses the
reflect package to look at the fields of the struct. The field obtained using
these functions is inccorect and causes the test to fail because of unexpected
results. It should see PostProcessors as the field but instead it sees the
field as Builders.
I added the following print statements to display the field before the call
which demonstrate the incorrect fields. This is in
fixer_pp_vagrant_override.go:
func (FixerVagrantPPOverride) Fix(input map[string]interface{})
(map[string]interface{}, error) {
// Our template type we'll use for this fixer only
type template struct {
PostProcessors []interface{} `mapstructure:"post-processors"`
}
// Decode the input into our structure, if we can
var tpl template
val := reflect.ValueOf(tpl)
field := val.Type().Field(0)
fmt.Printf("### field: %s ###\n", field)
And the output looks like this:
### field: {Builders []map[string]interface {} %!s(uintptr=0) [%!s(int=0)]
%!s(bool=false)} ###
--- FAIL: TestFixerVagrantPPOverride_Fix (0.00s)
testing.go:278: unexpected: map[string]interface
{}{"post-processors":[]interface {}(nil)}
expected: map[string]interface {}{"post-processors":[]interface
{}{"foo", map[string]interface {}{"override":map[string]interface
{}{"aws":map[string]interface {}{"foo":"bar"}}, "type":"vagrant"},
map[string]interface {}{"type":"vsphere"}, []interface {}{map[string]interface
{}{"override":map[string]interface {}{"vmware":map[string]interface
{}{"foo":"bar"}}, "type":"vagrant"
FAIL
[Bug go/65772] New: With multiple return values including a function with side effects, incorrect value is returned
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65772 Bug ID: 65772 Summary: With multiple return values including a function with side effects, incorrect value is returned 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: ppc64le, x86_64
[Bug go/65772] With multiple return values including a function with side effects, incorrect value is returned
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65772 --- Comment #1 from boger at us dot ibm.com --- Created attachment 35321 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35321&action=edit testcase for bad return values
[Bug go/65772] With multiple return values including a function with side effects, incorrect value is returned
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65772 --- Comment #2 from boger at us dot ibm.com --- When running the attached testcase on a platform with gccgo (ppc64le, x86_64), the test fails due to incorrect return values from the function getList. The source line for the return looks like this: return files, filepath.Walk(dir, visit) The problem occurs because the array "files" is declared in the getList function, and updated by the "visit" function which is passed to filepath.Walk. The array that is returned from getList does not reflect the updates made by the visit. When build with gc for either ppc64le or x86_64, the test case passes. Expected output: ./myreturn ### Trying to list file: /tmp/mytest465364283/stuff in dir: /tmp/mytest465364283 ### ### When walking: found file: /tmp/mytest465364283/stuff ### ### PASS: files returned !### Incorrect output: ./myreturn ### Trying to list file: /tmp/mytest819936920/stuff in dir: /tmp/mytest819936920 ### ### When walking: found file: /tmp/mytest819936920/stuff ### ### FAIL: no files returned ###
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #55 from boger at us dot ibm.com --- Created attachment 35344 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35344&action=edit Increment the pc in the callback routine for backtrace_full Always increment the pc in the callback, knowing that it was decremented during the unwind function to find the line number number of the call instruction. This leaves the pc with the correct value for the call stack. If a signal had occurred, and it wasn't a call instruction, the pc will get incremented when it hadn't been decremented during unwind but that is OK for this fix.
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #56 from boger at us dot ibm.com --- Here is a bit more detail. Now that I think I understand all the considerations, I'm proposing this simple fix for gcc 5. Maybe longer term a more thorough solution could be done but not sure it is worth changing multiple functions to add an additional argument and changing external header files in gcc 5. The problems to be fixed are: - the pc in the call stack returned by runtime_callers is wrong for some platforms, such as Power and Z and possibly others - the pc adjustment and line number that is looked up in go/runtime/pprof.go sometimes provides the wrong line number if based on the bad pc The proposed fix is to increment the pc in the callback routine that is passed to backtrace_full when trying to find the runtime_callers all the time. During the unwind the pc was decremented for the normal call case. If it was a signal, the pc would not have been decremented so it is being incremented unnecessarily for that situation but I think that is OK since the line number will be right and the pc will still refer to the signalling instruction. As far as the change to the pprof code, at least in gccgo for Power and Z the pc and the line number should be correct so no adjustment is even needed, but my understanding from previous posts is that in the gc compiled case an adjustment is needed and this code is common for gc and gccgo? With the pprof code that is there now, the pprof code is OK for Power, i.e., the line number in the pprof testcase matches the expected line number. For Z, I think that the amount to decrement should be the minimum size for a call instruction (not sure what that value is) and then the tracepc value would refer to the previous instruction. Note that the pprof testcase does not yet work even with this fix because there is a new problem that fails on all platforms with gccgo described here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64683.
[Bug go/64683] FAIL: runtime/pprof -- testing.go:278: The entry did not match
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64683 boger at us dot ibm.com changed: What|Removed |Added CC||boger at us dot ibm.com --- Comment #10 from boger at us dot ibm.com --- I see this same failure on ppc64le & ppc64. I found that this particular failure started with commit id 221230. It passes with commit id 220481.
[Bug go/65180] regression in gccgo testcase runtime/pprof on ppc64le, ppc64 following move to go 1.4 from 1.3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65180 --- Comment #2 from boger at us dot ibm.com --- We've been putting most of the discussion on this in the bugzilla mentioned in the previous comment. However there is a simple fix for Power which I will add here: ndex: libgo/runtime/go-callers.c === --- libgo/runtime/go-callers.c (revision 222128) +++ libgo/runtime/go-callers.c (working copy) @@ -83,8 +83,20 @@ callback (void *data, uintptr_t pc, const char *fi } loc = &arg->locbuf[arg->index]; - loc->pc = pc; + /* On the call to backtrace_full the pc value was most likely decremented + if there was a normal call, since the pc referred to the instruction + where the call returned and not the call itself. This was done so that + the line number referred to the call instruction. To make sure + the actual pc from the call stack is used, it is incremented here. + + In the case of a signal, the pc was not decremented by backtrace_full but + still incremented here. That doesn't really hurt anything since the line + number is right and the pc refers to the same instruction. + */ + + loc->pc = pc+1; + /* The libbacktrace library says that these strings might disappear, but with the current implementation they won't. We can't easily allocate memory here, so for now assume that we can save a Since 64999 was written against Z there might be an additional change needed to make that work.
[Bug go/65798] New: runtime.Caller returns ok=true when return data is invalid
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65798 Bug ID: 65798 Summary: runtime.Caller returns ok=true when return data is invalid 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: ppc64le, x86_64, s390x Created attachment 35347 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35347&action=edit Don't return ok=true from runtime.Caller if the pc is 0 or the filename has 0 len. This came from a user trying to build Docker on Fedora 22 using gccgo. The they found when calling runtime.Caller, the return value for ok could be true even though the other returned values could be invalid. Because ok was true they assumed the data was valid and tried to use those return values from runtime.Caller to pass onto runtime.FuncForPC which then resulted in the signal 11 displayed below. The failure occurs on every platform they tried to use with gccgo. Here is the original information: Description of problem: docker using gcc-go crashes Version-Release number of selected component (if applicable): gcc-5.0.1-0.1.fc22 How reproducible: Always(reproduced on x86_64, ppc64le, s390x, but expecting ppc64 too) Steps to Reproduce: 1.install docker from https://repos.fedorapeople.org/repos/jcajka/docker-gccgo/ (ppc64(le), s390x) or https://copr.fedoraproject.org/coprs/jcajka/docker-gccgo/ (x86_64) 2.systemctl start docker 3.mkdir /root/dir 4.chcon -Rt svirt_sandbox_file_t /root/dir/ (just in case) 5.1 for x86: docker run fedora -itv /root/dir/:/root/dir/ /bin/bash 5.2 for ppc64(le),s390x(replace arch in docker.io/jcajka/fedora22-...): docker run docker.io/jcajka/fedora22-ppc64 -itv /root/dir/:/root/dir/ /bin/bash Actual results on ppc64le/x86: docker run docker.io/jcajka/fedora22-ppc64le -itv /root/dir/:/root/dir/ /bin/bash panic: runtime error: invalid memory address or nil pointer dereference on ppc64le: [signal 0xb code=0x1 addr=0x8] on x86_64: [signal 0xb code=0x1 addr=0x0] goroutine 16 [running]: github_com_docker_libcontainer_stacktrace.NewFrame /builddir/build/BUILD/docker-1dcc59a0b40f7d29ab2e0b8fdeb0adf537dbcbaf/vendor/src/github.com/docker/libcontainer/stacktrace/frame.go:12 github_com_docker_libcontainer_stacktrace.Capture /builddir/build/BUILD/docker-1dcc59a0b40f7d29ab2e0b8fdeb0adf537dbcbaf/vendor/src/github.com/docker/libcontainer/stacktrace/capture.go:18 libcontainer.newSystemError /builddir/build/BUILD/docker-1dcc59a0b40f7d29ab2e0b8fdeb0adf537dbcbaf/vendor/src/github.com/docker/libcontainer/generic_error.go:48 libcontainer.$nested4 /builddir/build/BUILD/docker-1dcc59a0b40f7d29ab2e0b8fdeb0adf537dbcbaf/vendor/src/github.com/docker/libcontainer/factory_linux.go:212 github_com_docker_libcontainer.StartInitialization.pN43_github_com_docker_libcontainer.LinuxFactory /builddir/build/BUILD/docker-1dcc59a0b40f7d29ab2e0b8fdeb0adf537dbcbaf/vendor/src/github.com/docker/libcontainer/factory_linux.go:224 native.initializer /builddir/build/BUILD/docker-1dcc59a0b40f7d29ab2e0b8fdeb0adf537dbcbaf/_build/src/github.com/docker/docker/daemon/execdriver/native/init.go:35 github_com_docker_docker_pkg_reexec.Init /builddir/build/BUILD/docker-1dcc59a0b40f7d29ab2e0b8fdeb0adf537dbcbaf/_build/src/github.com/docker/docker/pkg/reexec/reexec.go:26 main.main /builddir/build/BUILD/docker-1dcc59a0b40f7d29ab2e0b8fdeb0adf537dbcbaf/docker/docker.go:29 goroutine 18 [finalizer wait]: created by runtime_createfing ../../../libgo/runtime/mgc0.c:2572 goroutine 19 [syscall]: goroutine in C code; stack unavailable created by os_signal..import ../../../libgo/go/os/signal/signal_unix.go:25 Expected results: docker run docker.io/jcajka/fedora22-s390x -itv /root/dir/:/root/dir/ /bin/bash FATA[] Error response from daemon: Cannot start container fb6ed2f3efd57fa07ddf5c4d8576408d0f4cb95fe645a335063f7bed014a0654: [8] System error: exec: "-itv": executable file not found in $PATH Additional info: Crash seems to be triggered by creating new Frame using invalid pc/filename/line in libcontainer(seen this crash from time to time, but haven't got reliable reproducer, until now). As libgo's runtime.Caller(capture.go:14) returns seemingly invalid data(0,"",0) later used in runtime.FuncForPC/parseFunctionName(frame.go). This could be workaround by extending/fixing check in capture.go:15, but I think that runtime.Caller shouldn't return ok state when returned data seems invalid. Patch in attachment prevents this crash. -- I'm attaching their suggested patch.
[Bug go/64683] FAIL: runtime/pprof -- testing.go:278: The entry did not match
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64683 --- Comment #12 from boger at us dot ibm.com --- Sorry I did not intend to reopen a closed bugzilla, I must not have looked carefully enough and thought it was still open. Just wanted to document what I found since their log output was the same. I will be more careful.
[Bug go/65180] regression in gccgo testcase runtime/pprof on ppc64le, ppc64 following move to go 1.4 from 1.3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65180 --- Comment #6 from boger at us dot ibm.com --- I have verified this testcase now passes on ppc64le with today's gcc-5-branch and with gcc trunk.
[Bug go/66138] New: json decoder Decode function fails for some structure return values
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66138
Bug ID: 66138
Summary: json decoder Decode function fails for some structure
return values
Product: gcc
Version: 5.1.1
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 Milestone: ---
Host: ppc64le, ppc64, x86_64
Target: ppc64le, ppc64, x86_64
Created attachment 35538
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35538&action=edit
failing testcase
I'm attaching a testcase that fails when built with gccgo on ppc64le and x86_64
from the gcc5 branch. The test also fails using gccgo built from gcc trunk on
ppc64le. The same testcase works if compiled with gc for ppc64le and x86_64.
The problem occurs when creating a json decoder. The difference in the cases
that pass and fail is the format of the structure being passed to the Decode
function to hold the return value. The calls to DecodeContainerConfig2
demonstrate structures that work. The calls to DecodeContainerConfig use
structures that fail with gccgo.
Incorrect output looks like this:
./resconf
HC &{1000 []}
CONFIG &{host with memory at top level}
HC &{1000 [/tmp:/tmp]}
CONFIG &{host with memory at top level and a hostconfig}
HC &{1000 [/tmp:/tmp]}
CONFIG &{host with memory inside hostconfig}
ORIGINAL CODE CAUSE
CONFIG &{host with memory at top level}
HC &{0 []}
CONFIG &{host with memory at top level and a hostconfig}
HC &{0 []}
CONFIG &{host with memory inside hostconfig}
HC &{0 []}
Correct output:
./resconf
HC &{1000 []}
CONFIG &{host with memory at top level}
HC &{1000 [/tmp:/tmp]}
CONFIG &{host with memory at top level and a hostconfig}
HC &{1000 [/tmp:/tmp]}
CONFIG &{host with memory inside hostconfig}
ORIGINAL CODE CAUSE
CONFIG &{host with memory at top level}
HC &{1000 []}
CONFIG &{host with memory at top level and a hostconfig}
HC &{1000 [/tmp:/tmp]}
CONFIG &{host with memory inside hostconfig}
HC &{1000 [/tmp:/tmp]}
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #25 from boger at us dot ibm.com --- (In reply to Andreas Schwab from comment #24) > ../../gcc/go/gospec.c: In function 'void > lang_specific_driver(cl_decoded_option**, unsigned int*, int*)': > ../../gcc/go/gospec.c:161:7: error: 'OPT_m32' was not declared in this scope > case OPT_m32: >^ > Makefile:1077: recipe for target 'go/gospec.o' failed > make[3]: *** [go/gospec.o] Error 1 What target are you building this on? bootstrap, with or without multilib?
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #28 from boger at us dot ibm.com --- I could put back the #ifdef TARGET_CAN_SPLIT_STACK_64BIT around the OPT_m32 case if that is OK. Doesn't fail on the builds for ppc64le or ppc64 either.
[Bug go/68420] New: Errors with go escape analysis
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68420 Bug ID: 68420 Summary: Errors with go escape analysis Product: gcc Version: 6.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 Milestone: --- Escape analysis for Go was added to gcc trunk, but there are errors when using it. It is not enabled by default, but requires the option -fgo-optimize-allocs to enable it. Details on the problem are documented in the golang issue tracker https://github.com/golang/go/issues/12965. I was asked to open this as a bugzilla in addition to the Go issue for gcc tracking purposes.
[Bug go/68420] Errors with go escape analysis
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68420 boger at us dot ibm.com changed: What|Removed |Added Target||ppc64le, x86_64 --- Comment #1 from boger at us dot ibm.com --- Fails on ppc64le and x86_64.
[Bug go/66574] Time is provided in millisecond precision instead of nanoseconds as described in go documentation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66574 --- Comment #6 from boger at us dot ibm.com --- (In reply to Ian Lance Taylor from comment #5) > Fixed on mainline. Can this be backported to the gcc 5 branch?
[Bug go/64876] New: Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)
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)
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 target/64876] Regressions in gcc-testresults for powerpc64 gccgo in 5.0 due to change for static chain for closures (219776)
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 go/65180] New: regression in gccgo testcase runtime/pprof on ppc64le, ppc64 following move to go 1.4 from 1.3
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65180 Bug ID: 65180 Summary: regression in gccgo testcase runtime/pprof on ppc64le, ppc64 following move to go 1.4 from 1.3 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: bergner at gcc dot gnu.org, cmang at google dot com Target: ppc64le, ppc64 A regression appeared in the nightly gcc testresults for the runtime/pprof in gcc 5.0 when the GO language level moved from go 1.3 to 1.4 which completed with commit id 219629. The regression appears occurs because the output is compared and the line number for one of the entries on the stack that is displayed does not match with the expected output. In inspecting the source file, it appears that the expected output is correct. The test output is off by 1 or 2. When determining the backtrace, a call is made to runtime_callers, which calls functions in libbacktrace. In two of these procedures it looks like the decrement of the pc is incorrect for Power and probably other platforms. The decremented amount for Power should be 4, but it is being decremented by 1. Once I made this change, the testcase passed but another then started failing. Here is the current failure: --- FAIL: TestMemoryProfiler (0.24s) testing.go:278: The entry did not match: 32: 1024 \[32: 1024\] @ 0x[0-9,a-f x]+ # 0x[0-9,a-f]+ pprof_test\.allocatePersistent1K\+0x[0-9,a-f]+ .*/mprof_test\.go:43 # 0x[0-9,a-f]+ runtime_pprof_test\.TestMemoryProfiler\+0x[0-9,a-f]+.*/mprof_test\.go:66 ... 1: 1024 [1024: 1048576] @ 0x100281db 0x100281db 0x100113fb 0x10011493 0x1000b73f 0x100c6c9f 0x1001ff53 # 0x100c6c9f testing.$thunk15+0xff /home/boger/gccgo.work/go1.4/150218/bld/../src/libgo/go/testing/testing.go:555 # 0x1001ff53 kickoff+0x43 /home/boger/gccgo.work/go1.4/150218/bld/../src/libgo/runtime/proc.c:235 32: 1024 [32: 1024] @ 0x100281db 0x100281db 0x10011aa7 0x1000b5cb 0x1000b793 0x100c6c9f 0x1001ff53 # 0x1000b5cb pprof_test.allocatePersistent1K+0x4b /home/boger/gccgo.work/go1.4/150218/bld/powerpc64le-linux/libgo/gotest49070/test/mprof_test.go:43 # 0x1000b793 runtime_pprof_test.TestMemoryProfiler+0x173 /home/boger/gccgo.work/go1.4/150218/bld/powerpc64le-linux/libgo/gotest49070/test/mprof_test.go:65 The mismatch is due to the line number specified for mprof_test.go. After some initial investigation, I found that the code in libbacktrace does not look correct for Power (ppc64, ppc64le). In both backtrace.c and simple.c, there is a decrement on the variable 'pc' which is declared as a uintptr. On Power this becomes a long int, so the decrement ends up subtracting 1 from the pointer. On Power this is not correct, because the decrement of the 'pc' should be done with multiples of 4. However, I tried to make this change but it didn't seem to help the failure. I also see that in pprof.go function printStackRecord, there is some decrementing done on the pc value, not sure if that correctly corresponds to the decrement that is done by libbacktrace. I mention these potential issues because they don't look right but they don't seem to affect the mismatch in line number and I'm not sure what is causing that.
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999
boger at us dot ibm.com changed:
What|Removed |Added
CC||boger at us dot ibm.com
--- Comment #24 from boger at us dot ibm.com ---
In my opinion, the way the decrement of pc is done in libbacktrace is
incorrect. The code in libbacktrace looks similar to code in libgcc, where the
pc type is _Unwind_Ptr. Since the libgcc has been around for quite a while I
assume it is correct, and I see no reason why the code to do the pc decrement
in libbacktrace should be different from what's in libgcc.
I made this change to libbacktrace/backtrace.c and simple.c:
if (!ip_before_insn)
pc = (uintptr_t)((_Unwind_Ptr)pc-1);
And then in pprof.go, added this so that the pc value did not get decremented
again for ppc64 & ppc64le:
tracepc := pc
// Back up to call instruction.
if i > 0 && pc > f.Entry() && !wasPanic {
if runtime.GOARCH == "386" || runtime.GOARCH ==
"amd64" {
tracepc--
} else if runtime.GOARCH == "s390" ||
runtime.GOARCH == "s390x" {
// only works if function was called
// with the brasl instruction (or a
// different 6-byte instruction).
tracepc -= 6
} else if runtime.GOARCH != "ppc64" &&
runtime.GOARCH != "ppc64le" {
tracepc -= 4 // arm, etc
}
}
After making this change the regression went away and no further failures
occurred on ppc64 and ppc64le. I am not familiar with other platforms (i.e.,
other supported GOARCHes) to understand how they might be affected by this.
However, since the code that appears in libgcc has existed and been used for
quite some time, that code should be right and decrementing the pc in
libbacktrace in the same way as is done in libgcc should be correct for all
platforms that use it. Then that leads to the question of what is done in
pprof.go, which looks like a hack to undo what was done incorrectly in
libbacktrace at least for some platforms.
I will continue to do further testing, but would be curious to know if this
change to libbacktrace would fix s390, assuming the special check in pprof.go
would be removed? And then I don't think you should have to make changes in
other locations, once libbacktrace has been corrected.
[Bug go/65134] gccgo ignores the attribute "constructor" in a subdirectory
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65134 --- Comment #5 from boger at us dot ibm.com --- (In reply to Tatsushi Inagaki from comment #0) > Created attachment 34813 [details] > Example to reproduce the constructor problem > > Gccgo ignores a C-function with the "constructor" attribute in a > subdirectory. In the attached example, gc executes a C-function init() in > sub.go before a Go function main(), but gccgo does not: > > ~/example$ export GOPATH=$(pwd) > ~/example$ /usr/local/go/bin/go version > go version go1.4.2 linux/amd64 > ~/example$ /usr/local/go/bin/go run src/main/main.go > Hello from constructor > Hello from main > > ~/example$ export LD_LIBRARY_PATH=/usr/local/gccgo/lib64:$LD_LIBRARY_PATH > ~/example$ /usr/local/gccgo/bin/go version > go version gccgo (GCC) 5.0.0 20150115 (experimental) linux/amd64 > ~/example$ /usr/local/gccgo/bin/go run src/main/main.go > Hello from main > > ~/example$ > > It seems this difference is due to that gccgo cannot detect a function with > the constructor attribute in an archived objects created in a subdirectory. > Is there any workaround to avoid this problem? Just to clarify this -- are you saying the problem that the gccgo linker is smart enough to determine that the function is not being used so it is discarded, but with gc it is not? So this is a difference in behavior but not actually a bug?
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #26 from boger at us dot ibm.com --- (In reply to Ian Lance Taylor from comment #25) > To which code in libgcc are you referring? I don't see it. > Here is the code I was referring to, but I was wrong when I thought it fixed the problem. Even though the testcase passed on further inspection I see that the pc values are still wrong. ip a.k.a. pc unwind-c.c: #ifdef HAVE_GETIPINFO ip = _Unwind_GetIPInfo (context, &ip_before_insn); #else ip = _Unwind_GetIP (context); #endif if (! ip_before_insn) --ip; > Our goal has to be for runtime.Callers to return the same sort of values in > gccgo as it does in gc. That means that your change to > go/runtime/pprof/pprof.go looks wrong to me. It suggests that the ppc64 > values are different from those of other architectures. What's the > justification for that? Sorry I was only experimenting with a solution for Power, just to understand what could be changed to make it work so it was not meant to be a solution for other platforms. I could see that the pc was being decremented wrong for Power so once I fixed that it didn't make sense to decrement it again in pprof.go. That was my thinking. After rereading all you've wrote I think we are coming to same result (somewhat). If I read the documentation for runtime.Callers, it says that the pc values are the return program counters, but in gccgo it is program counter for the call instruction, so runtime.Callers does need to be changed. But in one place you are just incrementing the pc, so won't that be wrong for the cases where libbacktrace didn't decrement it?
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #28 from boger at us dot ibm.com --- I'm not concerned about the inline case. The user could build without inlining if it was important. To me it seems like you don't want libbacktrace to decrement the pc when it is being called by runtime.Callers. Then runtime.Callers should be correct and no changes needed for pprof.go. But I suppose that messes up other users of libbacktrace unless you somehow had a way to indicate not to decrement in this case or to create a different unwind callback function.
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #30 from boger at us dot ibm.com --- I don't think it is a good idea to add code in multiple places to fix up the pc values after calling runtime.Callers. That seems prone to error and confusing for future updates to the code. >From what I can tell, the problem originates with the use of backtrace_full from the libgo code to find the pc stack values for some platforms. I think if the pc was never decremented in the backtrace code then I think the values are right. However I understand you can't just change that behavior due to other existing users of this code that might expect it to continue working as before. Here are some options: - Add a new function to libbacktrace for this situation where the pc is never decremented. - Add a flag to one of the backtrace structures to indicate you don't want the pc decremented in libbacktrace and then use that flag in libbacktrace to control whether the pc is decremented. - Add a wrapper function to the libgo code to call backtrace_full and then adjust the pc value based on the GOARCH value, understanding what backtrace_full might have done and what the GO code expects. Then there should be no direct calls to backtrace_full in libgo, but only calls to this wrapper function. Once the stack pc values are generated as expected then runtime.Callers has the correct values according to what its callers would expect, how it is documented, and matches gc behavior. I think the pc mapping for inlined functions is a separate issue. Inlining happens in gccgo and not gc and happens on all gcc compilers so the mapping of pc values to line numbers for inlined code is not an issue specific to gccgo and that is not the issue in this testcase. Maybe it just needs to be documented so users understand that can happen or maybe inlining should be disabled by default for gccgo and then if users enable it they understand what could happen.
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #33 from boger at us dot ibm.com --- (In reply to Ian Lance Taylor from comment #32) > > I don't think it is a good idea to add code in multiple places to fix up > > the pc values after calling runtime.Callers. That seems prone to error and > > confusing for future updates to the code. > > Right, which is why I never suggested that. I suggested changing > runtime.Callers itself to adjust the values that it gets from libbacktrace. > > > > - Add a wrapper function to the libgo code to call backtrace_full and then > > adjust the pc value based on the GOARCH value, understanding what > > backtrace_full might have done and what the GO code expects. Then there > > should be no direct calls to backtrace_full in libgo, but only calls to > > this wrapper function. > > Yes, that is exactly what I have been trying to say, but we don't need to > introduce a new function. We already only call backtrace_full from a single > place in libgo: runtime_callers (which, to be clear, is not the same as > runtime.Callers). > In comments 21 & 23 it sounded there were plans to make changes in other locations too. I agree with what you just said here, that you could just make a change in runtime_callers after it calls backtrace_full to adjust the pc and then no other changes should be needed anywhere else. Yes I realize runtime_callers and runtime.Callers are different and I was being sloppy when I mentioned them. > > > I think the pc mapping for inlined functions is a separate issue. Inlining > > happens in gccgo and not gc and happens on all gcc compilers so the mapping > > of pc values to line numbers for inlined code is not an issue specific to > > gccgo and that is not the issue in this testcase. Maybe it just needs to > > be documented so users understand that can happen or maybe inlining should > > be disabled by default for gccgo and then if users enable it they > > understand what could happen. > > To be clear, libbacktrace can handle inlined functions just fine, and libgo > does the right thing for things like the stack traces dumped when a program > crashes. It also does the right thing when handling the skip parameter to > runtime.Caller and runtime.Callers. The problem arises when converting the > libbacktrace values into the single PC value expected by Go functions like > runtime.Callers. > > I am not going to disable inlining by default for gccgo. That would be a > performance killer.
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #29 from boger at us dot ibm.com --- Yohei noted in comment 20 that this is also broken with gc in 1.4 when using static linking. That was a while ago -- is that no longer a problem?
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #36 from boger at us dot ibm.com --- I'd like to take a step back and clarify what the functions in question, runtime_callers, runtime.Caller, and runtime.Callers should be returning: the pc values for the call instruction, or the pc values from the instruction following the call? The golang documentation for runtime.Callers says it is the pc for the instruction after the call so I will assume that is what we want for that one. For runtime.Caller I believe it is supposed to return the call instruction itself but I'm not 100% sure on that. Since runtime.Callers calls runtime_callers, I assume runtime_callers should be providing the instruction after the call. That means all callers of runtime_callers should be decrementing pc values to the previous instruction if the call instruction is what they want. Does this sound right?
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #38 from boger at us dot ibm.com --- I've spent some time thinking about a fix for this and made a few other observations... I've noticed that when runtime_callers calls backtrace_full, it locks the thread using runtime_xadd and then looks up the line number and function name information and returns it to runtime_callers. However, in some cases, the caller of runtime callers doesn't use the line and file information, as happens with runtime.Callers and others. And actually if the caller of runtime_callers (or runtime.Caller) wants to back up the pc, then line number information that was determined by backtrace_full might not be correct so has to be looked up again. That has me wondering if runtime_callers should be changed so that it calls backtrace_simple instead of backtrace_full and returns just the pc values. I don't think backtrace_simple would require the thread to be locked since it doesn't care about the backtrace_state; then the code in libbacktrace wouldn't have to read in and save all the debug information as it does now. And then it would be up to the callers of runtime_callers to determine if the pc should be backed up and determine the line information and file information after it has the correct pc using FuncForPC. And then to complete the fix for this, we could add a simple function BackupPC which would be similar to what is in pprof.go to move the pc back to the previous instruction based on the GOARCH value. Unfortunately I am not familiar with all that needs to be done for the s390 and s390x case but whatever has to be done for that would be done in this function. And then any time the pc needs to be backed up it would be done by calling this function. Summarizing the changes: - runtime_callers would call backtrace_simple instead of backtrace_full, returning an array of just the pc values for the stack - backtrace_simple would not decrement the pc for Power and s390. I'm assuming it is OK to do the decrement for other platforms because they don't have an issue with the way it is now? This could be controlled by an #if defined - a new function BackupPC would be added to take a pc value as input and return the pc for the previous instruction, as described above - callers of runtime_callers, runtime.Caller or runtime.Callers would call BackupPC if that was needed, and then use FuncForPC to find the file and line number information only if needed. I have started to work on a patch for this but wanted to put this idea out there.
[Bug go/64999] s390x libgo test failure in TestMemoryProfiler
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64999 --- Comment #40 from boger at us dot ibm.com --- Created attachment 34995 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34995&action=edit Proposed fix Here is my proposed fix to correct the problem on Power, and mostly fix it for s390/s390x. Since I don't know the details on the s390/s390x call instructions that have different sizes, I didn't add the code to fix the problem where the call instruction might not be 6 bytes. Description: - I made a change to avoid doing the decrement in libbacktrace for Power and s390/s390x. I think it is better to do it this way than having libbacktrace decrement it and then on return or through the callback increment it. If it is fixed up in the callback or after the return then the line number information has already been determined in libbacktrace and it could be incorrect. My assumption is that no other platform has the issue with the decrement of the PC since there are no bugzillas for them so the change in libbacktrace is only needed for Power and s390/s390x. - I added a new function BackupPC to back up the PC by one instruction by subtracting the correct instruction size for the platform. As mentioned above, some extra work is needed here to handle different call instructions with different sizes. - All callers of runtime_callers, runtime.Caller, and runtime.Callers have to recognize that the PC values being returned represent the instruction following the call instruction, so if the call instruction is what is needed, the code will do the back up of the PC and determine the line number based on the new PC. I had to make a change to log.go to do this otherwise log_test.go failed. I changed the code in pprof.go to call the new function. With my patch, the line number information is now correct but the mprof testcase fails due to other differences in the mprof output. My patch applied to commit 220481 works and it fails on commit 221230, but have not yet narrowed it down to the exact commit where it starts to fail. I am investigating further, but here is where the output differs: testing.go:370: The entry did not match: (0|1): (0|2097152) \[1: 2097152\] @ 0x[0-9,a-f x]+ # 0x[0-9,a-f]+ pprof_test\.allocateTransient2M\+0x[0-9,a-f]+ .*/mprof_test.go:30 # 0x[0-9,a-f]+ runtime_pprof_test\.TestMemoryProfiler\+0x[0-9,a-f]+.*/mprof_test.go:65 0: 0 [1: 32] @ 0x1002815c 0x1002815c 0x10011a48 0x1000b510 0x1000b7b0 0x100c5a00 0x1001fed4 # 0x1000b510 pprof_test.allocateTransient2M+0x50 /home/boger/gccgo.work/go1.4/150121/bld/powerpc64le-linux/libgo/gotest8260/test/mprof_test.go:30 # 0x1000b7b0 runtime_pprof_test.TestMemoryProfiler+0x170 /home/boger/gccgo.work/go1.4/150121/bld/powerpc64le-linux/libgo/gotest8260/test/mprof_test.go:65 On the first line, the actual output contains [1: 32] but the expected output is [1: 2097152]. I built the latest gccgo on an x86 with no extra patches and this same failure occurs there.
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #6 from boger at us dot ibm.com --- I understand why some functions like getaddrinfo don't work with static linking unless the LD_LIBRARY_PATH is set to find libc.so, but then what should happen if it isn't?
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #9 from boger at us dot ibm.com --- My question was: what is supposed to happen on fallback? Sounds like some code gets rebuilt and used instead?
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #11 from boger at us dot ibm.com --- What I was asking is: what does it mean to call the pure-Go goLookupIP?
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #13 from boger at us dot ibm.com --- Then my question is: why isn't this "fallback" code always built for GO and available to run instead of waiting until it hits this situation and then built and run? In the situation you are describing it sounds like it is related to whether or not there is a static libnss available which could be determined at GO build time.
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #15 from boger at us dot ibm.com --- I think what Ian is saying is that mechanism to rebuild packages in this way doesn't work with gccgo (and probably never should?) Now I'm finally understanding this. Originally with gc the net package is built with netgo off, but the netgo tag says to rebuild the GO standard library with the netgo tag set on and then build the program. Yohei's original fix used the code that was built with netgo off but allowed the go resolver to be called if the call to getaddrinfo failed. There are probably a small set of errnos that could be checked to determine if the go resolver should be called after getaddrinfo failed, but is that important? I would expect the errnos are consistent across platforms but don't know for sure. To me it seems like Yohei's fix is probably OK for gccgo with some additional checks for errno if needed, but would change existing behavior for gc and because of that should not change there?
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #17 from boger at us dot ibm.com --- Can you clarify how using -a -tags netgo actually works. I know it requires that the source be available, but it must mean that it rebuilds the package for the current link only, throws it away after using it and next time it has to rebuild it? Couldn't some of your concerns about unexpected failures be resolved by providing better error information when there were failures, or provide other ways to indicate that the go resolver shouldn't be called (like an environment variable)? It just seems that rebuilding the package every time is a heavy hammer to resolve this problem. I think if someone wants to have their GO program use libnss if it is present and then the go resolver if not, that should be an option and currently it is not.
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #19 from boger at us dot ibm.com --- (In reply to Ian Lance Taylor from comment #18) > The -a option to "go build" means to rebuild all packages rather than using > the installed versions (see http://golang.org/cmd/go for documentation). > The "-tags netgo" option means to build with the build tag netgo (see the > build constraints section in http://golang.org/pkg/go/build/). So, yes, it > rebuilds the packages for the current link only. This is more reasonable > with the gc compiler than with gccgo, since the gc compiler is so fast. > Most of the examples in the documentation show that the built packages are put into the same directories as the source. I assume that for an official release with a binary distribution, that is not the way it works. That's how it would have to work with gccgo. In that case everyone must share the same copy of the source but then if build options are used that would cause packages to be rebuilt, they must go somewhere that is only used for the curent build. And I don't understand what 'go install' would mean in that case. The 'go install' command documentation has very little information on where built packages are stored or if there are cases when 'go install' can't be used. > I'm OK in principle with coming up with some other approach to direct the Go > library to use the Go DNS lookup rather than calling getaddrinfo. I don't > think that can be the default. I don't think we want a program to > unpredictably sometimes use one and sometimes use the other. I don't think > an environment variable would work well, since Yohei presumably wants the > statically linked binary to work this way by default. Unfortunately all I > can think of would be adding another function to the net package directing > lookups to use pure DNS; this is unfortunate because the net package already > has the most complex and confusing API of all the standard Go packages. I think providing another function that called the pure GO resolver would be best. Then the GO programmer can decide how to handle it if the first call failed.
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #21 from boger at us dot ibm.com --- I'm confused by the description of -a in the go1.4 documentation. I asked about this before and the answer was that each invocation of 'go build' would create a copy of the built package which was then used for the current build but then thrown away. But that must not be the way it works?
[Bug go/63731] Fallback to netgo does not work
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63731 --- Comment #23 from boger at us dot ibm.com --- If I look at this documentation: http://tip.golang.org/doc/go1.4#gocmd It says this: The behavior of the go build subcommand's -a flag has been changed for non-development installations. For installations running a released distribution, the -a flag will no longer rebuild the standard library and commands, to avoid overwriting the installation's files. When I read this it sounds like the previous behavior with the -a option was to rebuild the packages and put the newly built packages into the installed directories, including the standard library. If everyone who used 'go build' with -a generated their own copy of the built packages and then threw them away, how would the installation's files ever get overwritten?
[Bug go/63170] New: gccgo testcase recover.go fails execution with message 'missing recover' on ppc64 and ppc64le
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63170 Bug ID: 63170 Summary: gccgo testcase recover.go fails execution with message 'missing recover' on ppc64 and ppc64le 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 gccgo was built from trunk on power8 machines for both ppc64 and ppc64le with the same results ./gccgo --version gccgo (GCC) 5.0.0 20140904 (experimental) Copyright (C) 2014 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. This error occurs during make check testing for go for testcase: gcc/testsuite/go.test/test/recover.go Failing output for this testcase from go.log after running 'make check-gcc-go' missing recover Trace/breakpoint trap main.die /home/boger/gccgo.work/orig/trunk-src/gcc/testsuite/go.test/test/recover.go:69 main.mustRecoverBody /home/boger/gccgo.work/orig/trunk-src/gcc/testsuite/go.test/test/recover.go:81 main.M.N7_main.T5 /home/boger/gccgo.work/orig/trunk-src/gcc/testsuite/go.test/test/recover.go:437 reflect.call /home/boger/gccgo.work/orig/trunk-src/libgo/runtime/go-reflect-call.c:226 reflect.call.N13_reflect.Value /home/boger/gccgo.work/orig/trunk-src/libgo/go/reflect/value.go:576 reflect.Call.N13_reflect.Value /home/boger/gccgo.work/orig/trunk-src/libgo/go/reflect/value.go:412 reflect.$nested2 /home/boger/gccgo.work/orig/trunk-src/libgo/go/reflect/makefunc.go:175 reflect.ffiCall /home/boger/gccgo.work/orig/trunk-src/libgo/go/reflect/makefunc_ffi.go:64 reflect.$nested3 /home/boger/gccgo.work/orig/trunk-src/libgo/go/reflect/makefunc_ffi.go:36 main.test13reflect2 /home/boger/gccgo.work/orig/trunk-src/gcc/testsuite/go.test/test/recover.go:456 main.main /home/boger/gccgo.work/orig/trunk-src/gcc/testsuite/go.test/test/recover.go:58 goroutine 18 [finalizer wait]: created by runtime_createfing /home/boger/gccgo.work/orig/trunk-src/libgo/runtime/mgc0.c:2572 FAIL: go.test/test/recover.go execution, -O2 -g
[Bug go/63172] New: gccgo testcase cplx2.go execution provides incorrect answers on trunk for powerpc64, powerpc64le
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63172
Bug ID: 63172
Summary: gccgo testcase cplx2.go execution provides incorrect
answers on trunk for powerpc64, powerpc64le
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: bergner at vnet dot ibm.com, cmang at google dot com
Built gccgo from trunk.
Testcase cplx2.go in the go testsuite fails with incorrect answers.
>From the go.log when running 'make check-gcc-go'
opcode x (+7.218935e-001-5.325444e-001i) (+7.218935e-001-5.325444e-001i)
panic: fail
goroutine 16 [running]:
main.main
/home/boger/gccgo.work/orig/trunk-src/gcc/testsuite/go.test/test/ken/cplx2.go:102
created by main
/home/boger/gccgo.work/orig/trunk-src/libgo/runtime/go-main.c:42
FAIL: go.test/test/ken/cplx2.go execution, -O2 -g
Looking at an objdump of the executable, the compiler has determined at compile
time that the values are not correct, omits the code to do the conversion and
compare and generates the printlns and panic directly.
r64 := real(complex128(ce))
if r64 != real(Ce) {
println("real(complex128(ce))", r64, real(Ce))
10001318: 30 00 9f e8 ld r4,48(r31)
1000131c: 28 00 7f e8 ld r3,40(r31)
10001320: 41 fc ff 4b bl 1f60
<005f.plt_call.__go_print_string>
10001324: 18 00 41 e8 ld r2,24(r1)
10001328: 09 fc ff 4b bl 1f30
<005f.plt_call.__go_print_space>
1000132c: 18 00 41 e8 ld r2,24(r1)
10001330: 90 f8 20 fc fmr f1,f31
10001334: 5d fc ff 4b bl 1f90
<005f.plt_call.__go_print_double>
10001338: 18 00 41 e8 ld r2,24(r1)
1000133c: f5 fb ff 4b bl 1f30
<005f.plt_call.__go_print_space>
10001340: 18 00 41 e8 ld r2,24(r1)
10001344: 90 e8 20 fc fmr f1,f29
10001348: 49 fc ff 4b bl 1f90
<005f.plt_call.__go_print_double>
1000134c: 18 00 41 e8 ld r2,24(r1)
10001350: d1 fb ff 4b bl 1f20
<005f.plt_call.__go_print_nl>
10001354: 18 00 41 e8 ld r2,24(r1)
/home/boger/gccgo.work/orig/trunk-src/gcc/testsuite/go.test/test/ken/cplx2.go:120
panic("fail")
10001358: 10 00 60 38 li r3,16
1000135c: f5 fb ff 4b bl 1f50 <005f.plt_call.__go_new>
10001360: 18 00 41 e8 ld r2,24(r1)
10001364: 04 00 20 39 li r9,4
10001368: 78 1b 64 7c mr r4,r3
1000136c: 00 00 e3 fb std r31,0(r3)
10001370: 08 00 23 f9 std r9,8(r3)
10001374: ff ff 62 3c addis r3,r2,-1
10001378: 98 7c 63 38 addir3,r3,31896
1000137c: c5 fb ff 4b bl 1f40 <005f.plt_call.__go_panic>
10001380: 18 00 41 e8 ld r2,24(r1)
[Bug go/60406] recover.go: test13reflect2 test failure
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60406 --- Comment #7 from boger at us dot ibm.com --- I've built with Dominik's patches against trunk on ppc64le and have been trying to run the gcc testsuite for go and libgo. The recover.go testcase continues to fail in my build. I did some debugging on this and found that the problem occurs in ffi_callback when it calls __go_makefunc_can_recover. Based on the comments I think it should be called with the program address for the most recent caller on the stack which is not ffi, but instead it is called with the address for ffi_closer_helper_LINUX64. I suspect this is happening because runtime_callers is not returning the complete set of callers. This error leads to an incorrect setting of __makefunc_can_recover in the __go_defer_stack structure so the test prints the "missing recover" error.
[Bug go/60406] recover.go: test13reflect2 test failure
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60406 --- Comment #8 from boger at us dot ibm.com --- Update on my previous work: 1) My previous update referred to a build that was done with the patches that were submitted to gcc and patches that Dominik provided me for libffi. I found that if I only build with the gcc patches and don't use the libffi patches then the recover.go testcase passes on ppc64le.
[Bug go/60406] recover.go: test13reflect2 test failure
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60406
--- Comment #9 from boger at us dot ibm.com ---
The patch to fix the recover.go problem causes significant regression in gccgo
when built for ppc64 (BE). There are 32 unexpected failures in the gcc go
testsuite with the patch 32 unexpected failures in the gcc libgo testsuite.
Without this patch on trunk there are 2 failures in the go testsuites and 1
failure in the libgo testsuite.
I did some debugging on one of the regression failures: bug113.go. I found
that the problem occurs in the new code in the patch in
go/gofrontend/statements.cc when creating the expression tree to call
__go_set_defering_fn. The argument that is being passed is generated through a
call to make_func_code_reference:
Expression* arg =
Expression::make_func_code_reference(function, location);
Expression* call = Runtime::make_call(Runtime::SET_DEFERING_FN,
location, 1, arg);
Statement* s = Statement::make_statement(call, true);
s->determine_types();
gogo->add_statement(s);
On ppc64le, this works as expected but on ppc64(be) the code that is generated
from this is not the address of the function but the address of the .opd entry
that is used to call the function. As a result the setting of the deferring
function is incorrect and it does not appear as if it can recover because of
the way __go_can_recover uses the deferring function's address to see if it is
in the correct range.
When I debug this using gdb:
Breakpoint 1, __go_set_defering_fn (defering_fn=0x10020150 ) at
/home/boger/gccgo.work/140922/src/libgo/runtime/go-defer.c:78
78 {
Missing separate debuginfos, use: debuginfo-install glibc-2.17-55.el7.ppc64
(gdb) info reg $r3
r3 0x10020150 268566864
>From the objdump, I see this address is in the .opd:
10020150 :
10020150: 00 00 00 00 .long 0x0
10020154: 10 00 1c 4c vsrov0,v0,v3
10020158: 00 00 00 00 .long 0x0
1002015c: 10 02 81 c0 .long 0x100281c0
but the code is actually here, which is the address that should be passed to
__go_set_defering_fn:
10001c4c <.main.$thunk0>:
main.$thunk0():
10001c4c: 7c 08 02 a6 mflrr0
10001c50: f8 01 00 10 std r0,16(r1)
10001c54: fb e1 ff f8 std r31,-8(r1)
10001c58: f8 21 ff 71 stdur1,-144(r1)
Note that the actual function code address is in the first 8 bytes of the .opd
entry for the function.
[Bug go/60406] recover.go: test13reflect2 test failure
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60406 --- Comment #11 from boger at us dot ibm.com --- On ppc64 BE, the call to make_code_func_reference returns the function pointer in the .opd and not the actual address of the function. That is what causes the failures with this patch https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00710.html The information in this reply fixes the regressions from this patch on ppc64 BE: https://gcc.gnu.org/ml/gcc-patches/2014-09/msg02282.html Essentially the change is to dereference the function pointer in __go_set_defering_fn when building for ppc64 BE as follows: +#if defined(__powerpc64__) && _CALL_ELF != 2 +g->defer->__defering_fn = *(void **)defering_fn; +#else +g->defer->__defering_fn = defering_fn; +#endif
[Bug go/63172] gccgo testcase cplx2.go execution provides incorrect answers on trunk for powerpc64, powerpc64le
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63172 boger at us dot ibm.com changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |DUPLICATE --- Comment #5 from boger at us dot ibm.com --- This is the same problem as described in 60181. *** This bug has been marked as a duplicate of bug 60181 ***
[Bug target/60181] constant folding of complex number incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60181 boger at us dot ibm.com changed: What|Removed |Added CC||boger at us dot ibm.com --- Comment #5 from boger at us dot ibm.com --- *** Bug 63172 has been marked as a duplicate of this bug. ***
[Bug go/63172] gccgo testcase cplx2.go execution provides incorrect answers on trunk for powerpc64, powerpc64le
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63172 boger at us dot ibm.com changed: What|Removed |Added Status|RESOLVED|CLOSED --- Comment #6 from boger at us dot ibm.com --- Comments for this bug will appear in 60181.
[Bug target/60181] constant folding of complex number incorrect
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60181 --- Comment #6 from boger at us dot ibm.com --- If the last comment is true, does that mean the fold_const.c file in gcc should be built in a way so that it doesn't use the fma, like using some kind of option during the build of gcc at least for this file on the s390 and Power?
[Bug go/60406] recover.go: test13reflect2 test failure
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60406 --- Comment #15 from boger at us dot ibm.com --- The testcase recover.go continues to fail on both ppc64 LE & BE with the new patch https://codereview.appspot.com/153950043.
[Bug go/60406] recover.go: test13reflect2 test failure
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60406 --- Comment #20 from boger at us dot ibm.com --- The latest patch worked on ppc64 for LE & BE. That is, the testcase recover.go now works and no new regressions were introduced.
[Bug go/67198] [5/6 Regression] change of type of syscall.RawSockaddr.Data on ppc64 breaks compilation of existing programs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67198 --- Comment #8 from boger at us dot ibm.com --- This fix is already in the gcc 5 branch commit r226595. It was tracked in the golang issues database https://github.com/golang/go/issues/11469.
[Bug go/66368] [5 Regression] go tool crashes on powerpc-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66368 boger at us dot ibm.com changed: What|Removed |Added CC||boger at us dot ibm.com --- Comment #3 from boger at us dot ibm.com --- Let's get the obvious set up questions out of the way first... If you are building on Ubuntu, I assume you must be trying to build powerpc64le-linux-gnu, and not powerpc-linux-gnu. Is your LD_LIBRARY_PATH being set to the correct path for the libgo that corresponds to the go and gccgo you are using? That is, you aren't using libgo from a gcc trunk build but go and gccgo from a gcc5 build?
[Bug go/66368] [5 Regression] go tool crashes on powerpc-linux-gnu
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66368 --- Comment #6 from boger at us dot ibm.com --- I think you've said the problem only occurs when using -fstack-protector-strong and when building with glibc-2.21. Have you tried using gdb to see where the segv actually occurs? >gdb go . >run version (Once it hits the segv) >x/20ni $pc-24 >bt The panic stacktrace is showing a line in proc.c:2328 which is making a call to __builtin_return_address according to my source. Does that match your source? If that is the case then I have a feeling this isn't go specific, but a problem with calling __builtin_return_address on ppc32 when using -fstack-protector-strong and possibly glibc-2.21 has some affect.
[Bug go/66138] json decoder Decode function fails for some structure return values
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66138 --- Comment #1 from boger at us dot ibm.com --- I am not sure, but this appears to be similar to the golang issue I opened yesterday https://github.com/golang/go/issues/11236 which was closed as a duplicate of https://github.com/golang/go/issues/7247.
[Bug go/66574] New: Time is provided in millisecond precision instead of nanoseconds as described in go documentation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66574 Bug ID: 66574 Summary: Time is provided in millisecond precision instead of nanoseconds as described in go documentation 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 Milestone: --- Target: ppc64le, ppc64, x86_64 Created attachment 35794 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35794&action=edit testcase demonstrating microsecond precision of time.Now() According to http://golang.org/pkg/time/#Time A Time represents an instant in time with nanosecond precision. The precision provided by gccgo when calling time.Now() is in microseconds, not nanoseconds, since it invokes gettimeofday to retrieve the time. If clock_gettime was called instead, that would provide nanosecond precision. A testcase is attached. This is a mirror of an issue that was reported to the golang issue tracker: https://github.com/golang/go/issues/11222.
[Bug go/66574] Time is provided in millisecond precision instead of nanoseconds as described in go documentation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66574 --- Comment #2 from boger at us dot ibm.com --- I was asked to do that by my team leader in order to track these changes. If you prefer it not be done that way I prefer that too and won't in the future.
[Bug go/66574] Time is provided in millisecond precision instead of nanoseconds as described in go documentation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66574 boger at us dot ibm.com changed: What|Removed |Added Severity|normal |critical
[Bug go/66138] json decoder Decode function fails for some structure return values
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66138 boger at us dot ibm.com changed: What|Removed |Added Severity|normal |critical --- Comment #2 from boger at us dot ibm.com --- Increasing priority since it is affecting correct execution of Docker when built with gccgo.
[Bug go/66574] Time is provided in millisecond precision instead of nanoseconds as described in go documentation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66574 boger at us dot ibm.com changed: What|Removed |Added Severity|critical|normal
[Bug go/66870] New: split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 Bug ID: 66870 Summary: split stack issues on ppc64le and ppc64 Product: gcc Version: 6.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: go Assignee: ian at airs dot com Reporter: boger at us dot ibm.com CC: amodra at gmail dot com, cmang at google dot com, sch...@linux-m68k.org Target Milestone: --- Target: ppc64le, ppc64 Created attachment 35977 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35977&action=edit Detect no_split_stack attribute when generating split stack prologue. There are a few issues to be resolved with the split stack implementation on ppc64le, ppc64, and disabling it correctly when building for ppc (32 bit BE). 1) When building on a system with glibc >= 2.18 and doing a multilib build, the ppc 32 build incorrectly has the split stack flag set, causing an error during the build. This was identified in gcc-patches and a proposed fix suggested: https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00963.html https://gcc.gnu.org/ml/gcc-patches/2015-06/msg01038.html (Note: this seems to fix the problem but I don't understand why. The setting of the flag was simply moved past a few statements and I don't see why that should affect the outcome, since the opt argument passed in this case is &global_options.) 2) For functions with the _no_split_stack attribute set, the split stack code generation was not always correct on ppc64le & ppc64. I have a patch for that which is attached. This problem was not obvious when building on ppc64le (where most of my testing was done) but occurs often when built on ppc64. 3) The file gcc/go/gospec.c generates options to be passed to the linker based on a define of #TARGET_CAN_SPLIT_STACK but this define is never set for a build for ppc64le or ppc64. The first occurrence in the function lang_specific_driver generates the fsplit-stack option when calling the linker, the second occurrence sets -Wl,-u pthread_create. I believe the setting of this define has to be consistent with the result of rs6000_supports_split_stack in gcc/common/config/rs6000/rs6000-common.c -- but that function depends on the setting of rs6000_isa_flags which has not been updated by the time lang_specific_driver is called. I'm assuming it is wrong for these options not to be generated and passed to the linker when using split stack? 4) I believe we always want to use the gold linker with gccgo on ppc64le and ppc64 and probably ppc is OK too. This is necessary to get the minimum storage allocated for goroutines (which is the whole point of providing this option) and also prevents errors with split stack that could happen in some cases. I've exchanged some notes with iant on this topic and he suggested this: The -fuse-ld=gold option is newer than gccgo. I suppose it would be nice if: * the GCC configure process checks whether -fuse-ld=gold works; if so: * -fuse-ld=gold is passed to the libgo configure/build >>> I think this is already done by the libgo configure -- or at least it attempts to see if the linker associated with the gccgo it is using to build libgo supports split stack and then sets the flag libgo_cv_c_linker_supports_split_stack and that makes it all work. So if we build gccgo with the gold linker as its linker then this should just work. * -fuse-ld=gold is used by default by the gccgo driver program >>> Not sure how best to do this just for gccgo and not the other gcc compilers. I can keep looking but if anyone has a suggestion let me know. There are probably other creative ways to do this by setting spec defines that are only available for ppc64le, ppc64 builds but haven't figured it out yet.
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #2 from boger at us dot ibm.com --- (In reply to Ian Lance Taylor from comment #1) > The various issues in this bug are in different parts of the code base. The > bug is assigned to me, but, to be clear, I have no plans to work on anything > in the PPC-specific code. > Understood. > For x86, TARGET_CAN_SPLIT_STACK is defined in > gcc/config/i386/gnu-user-common.h. The intent of the code in gospec.c is to > pass -fsplit-stack and -Wl,-u,pthread_create when linking. When compiling, > split stack is turned on by the code in go_langhook_init_options_struct in > gcc/go/go-lang.c. When linking, we want to pass -fsplit-stack so that > gcc/gcc.c uses STACK_SPLIT_SPEC. If the target defines > TARGET_CAN_SPLIT_STACK for all cases where it might support it, I think the > code will more or less work even for cases where split stack is not > supported. If we don't pass those options, then the split-stack code will > not work entirely correctly because new threads won't know their stack size. In gcc.c there is STACK_SPLIT_SPEC which sets --wrap=pthread_create in the LINK_COMMAND_SPEC if -fsplit-stack is set. Is there a reason there are 2 different split stack dependent link options are being set in two different ways?
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #5 from boger at us dot ibm.com --- (In reply to Andreas Schwab from comment #4) > > past a few statements > > Huh?? Here is your patch diff --git a/gcc/go/go-lang.c b/gcc/go/go-lang.c index ce4dd9b..d952e0f 100644 --- a/gcc/go/go-lang.c +++ b/gcc/go/go-lang.c @@ -158,10 +158,6 @@ go_langhook_init_options_struct (struct gcc_options *opts) opts->x_flag_errno_math = 0; opts->frontend_set_flag_errno_math = true; - /* We turn on stack splitting if we can. */ - if (targetm_common.supports_split_stack (false, opts)) -opts->x_flag_split_stack = 1; - /* Exceptions are used to handle recovering from panics. */ opts->x_flag_exceptions = 1; opts->x_flag_non_call_exceptions = 1; @@ -295,6 +291,11 @@ go_langhook_post_options (const char **pfilename ATTRIBUTE_UNUSED) && global_options.x_write_symbols == NO_DEBUG) global_options.x_write_symbols = PREFERRED_DEBUGGING_TYPE; + /* We turn on stack splitting if we can. */ + if (!global_options_set.x_flag_split_stack + && targetm_common.supports_split_stack (false, &global_options)) +global_options.x_flag_split_stack = 1; + /* Returning false means that the backend should be used. */ return false; } Your change moved the if statement containing the call to targetm_common.supports_split_stack to a different location in the file (past a few statements) and re-added them along with a check for global_options_set.x_flag_split_stack. It looks to me that the values in the rs6000_isa_flags are still the same whether you call supports_split_stack where it was or where you moved it to. I'm trying to fix some of the other issues mentioned in this bugzilla and even with your fix I sometimes hit the original problem when building with m32.
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #8 from boger at us dot ibm.com --- Created attachment 35989 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35989&action=edit Clean up checks for flag_split_stack and attribute no_split_stack Made the change related to Alan's comment for my patch to rs6000.c.
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #9 from boger at us dot ibm.com --- (In reply to Andreas Schwab from comment #6) > > (past a few statements) > > Huh? > > @@ -158,10 +158,6 @@ go_langhook_init_options_struct (struct gcc_options > *opts) > @@ -295,6 +291,11 @@ go_langhook_post_options (const char **pfilename Sorry about that. Would you submit your change?
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #10 from boger at us dot ibm.com --- Now that I am building on ppc64 with a new enough glibc, using my latest patches for rs6000.c and Andreas' patch, and forcing the gold linker to be used, I am hitting a testcase failure in the libgo fmt test. The failure is a segv, gdb shows this: 0x10024aa0 :ld r0,-28736(r13) 0x10024aa4 : addir12,r1,-160 0x10024aa8 : nop 0x10024aac : cmpld cr7,r12,r0 0x10024ab0 : blt-cr7,0x10024b78 0x10024ab4 : rldicl. r9,r4,8,56 => 0x10024ab8 : std r4,56(r12) 0x10024abc : std r5,64(r12) 0x10024ac0 : std r6,72(r12) 0x10024ac4 : std r7,80(r12) r12 does not contain the correct value. If look below in this function where the call to __morestack was, I see this: 0x10024b78 : mflrr0 0x10024b7c :std r0,16(r1) 0x10024b80 :bl 0x1004ed18 <.__morestack> 0x10024b84 :ld r0,16(r1) 0x10024b88 :mtlrr0 0x10024b8c :blr 0x10024b90 :b 0x10024ab4 For it to get to the code where the segv occurs, it must have taken the last branch after the call to __morestack. In reading the comments in the emit prologue code in rs6000.c, it says that __morestack returns the value in r29 and that should be moved to r12, but I don't see that happening in this code. Alan, can you look at this? I will send you the details on where it is.
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #13 from boger at us dot ibm.com --- Use of the gold linker on ppc64 (BE) with static linking results in these warnings: /usr/local/gold/bin/ld.gold: warning: /usr/lib/gcc/ppc64-redhat-linux/4.8.3/../../../../lib64/libc.a(malloc.o): .opd is not a regular array of opd entries /usr/local/gold/bin/ld.gold: warning: /usr/lib/gcc/ppc64-redhat-linux/4.8.3/../../../../lib64/libc.a(dl-lookup.o): .opd is not a regular array of opd entries /usr/local/gold/bin/ld.gold: warning: /usr/lib/gcc/ppc64-redhat-linux/4.8.3/../../../../lib64/libc.a(printf_fp.o): .opd is not a regular array of opd entries I'm assuming these shouldn't be ignored? Perhaps at this time we should not allow split stack (with the gold linker) for ppc64 BE and only for ppc64 LE.
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #14 from boger at us dot ibm.com --- I did bootstrap a build on ppc64 multilib, using Alan's latest and my patch and Andreas' patch on a system with glibc >= 2.18. (Without Andreas' patch it won't bootstrap on the 32 bit build on this system.)
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #15 from boger at us dot ibm.com --- I have submitted my patch to gcc-patches to check for the no_split_stack attribute after revising it based on Alan's comments.
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #17 from boger at us dot ibm.com --- Created attachment 36132 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36132&action=edit Configure gold linker with split stack if available Attaching my patch to detect for split stack support within the gold linker that is available by the gcc build, and then use it with gccgo compiles. Here are the steps: - When configuring gcc, check for split stack support in the gold linker either identified as the linker to use in the gcc build or found in the path at build time. This check currently only works on ppc64le; I couldn't get it to fail on ppc64 even with older (2.24) gold linkers. Not sure what might be wanted for other targets. If the support is there then HAVE_LD_GOLD_SUPPORTS_SPLIT_STACK is defined. - Modify the check in libgo configure to detect that the gold linker used with the gccgo that was just built supports split stack, defining LINKER_SUPPORTS_SPLIT_STACK if it is. - Add -fuse-ld=gold to the linker spec if HAVE_LD_GOLD_SUPPORTS_SPLIT_STACK is defined (not for m32). - Define TARGET_CAN_SPLIT_STACK for Power targets if the glibc level is correct. This is needed by go/gospec.c to enable the -fsplit-stack option when linking. Comments?
[Bug go/66870] split stack issues on ppc64le and ppc64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66870 --- Comment #19 from boger at us dot ibm.com --- (In reply to Ian Lance Taylor from comment #18) > In the patch in comment #17, the code in gcc/configure.ac looks misplaced: > shouldn't it be before the ";;", and not add another ";;"? > > Can you explain why the test in libgo/configure.ac will fail for a linker > that does not support split-stack? I'm not quite seeing it. I haven't > tried it, though. For ppc64le, some split stack support was recently added to the gold linker by Alan, so if you try to use a gold linker that is too old, you will get an error like this: /usr/bin/ld.gold -o sss ss-callee.o ss-call.o --defsym __morestack=0 /usr/bin/ld.gold: error: linker does not include stack split support required by ss-call.o But with one after he added the support, it does not get this error. If it is not gold then it doesn't get the error either. However on ppc64 (BE, not LE) it never gets the error, old or new gold. Alan would have to answer why. I don't know about other targets, I haven't tried it. I suppose the test could be on the version number for the gold linker at least for ppc64 LE or BE. Sorry I probably messed up the ;; I will double check on that.
