[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874 Uroš Bizjak changed: What|Removed |Added Target Milestone|4.10.0 |4.9.2
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874 Uroš Bizjak changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED Target Milestone|--- |4.10.0 --- Comment #13 from Uroš Bizjak --- (In reply to Ian Lance Taylor from comment #12) > Thanks for the analysis. See also PR 60406. > > Dominik sent me a patch for 60406 but 1) he has no copyright assignment; 2) > I think that his patch does not work for SJLJ exceptions. Actually, this remaining problem is the same as PR60406. The original problem is fixed.
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874 --- Comment #12 from Ian Lance Taylor --- Thanks for the analysis. See also PR 60406. Dominik sent me a patch for 60406 but 1) he has no copyright assignment; 2) I think that his patch does not work for SJLJ exceptions.
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874 --- Comment #11 from Uroš Bizjak --- (In reply to Andreas Schwab from comment #10) > If you never use goto *exp in the same function the value of &&label is > undefined. I did try adding goto bla: just before label, but never managed to get something around return address. Care to post and example, please?
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874 --- Comment #10 from Andreas Schwab --- If you never use goto *exp in the same function the value of &&label is undefined.
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874
--- Comment #9 from Uroš Bizjak ---
To illustrate unreliable approach, please compile following test:
--cut here--
extern void foo (void *);
int test(void)
{
__label__ bla;
foo (&&bla);
bla:
return 0;
}
--cut here--
gcc -O2:
test:
.L2:
pushl %ebp
movl%esp, %ebp
subl$8, %esp
movl$.L2, (%esp)
callfoo
xorl%eax, %eax
leave
ret
The passed argument doesn't represent return address.
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874
--- Comment #8 from Uroš Bizjak ---
After lots of debugging...
The problem is with the label that is passed as an argument to
__go_set_defer_retaddr. In function main.$thunk0, in _.179r.cse1 dump, we have:
...
(insn 7 3 8 2 (set (reg:DI 84)
(high:DI (label_ref:DI 47))) rr.go:57 236 {*movdi}
(insn_list:REG_LABEL_OPERAND 47 (nil)))
(insn 8 7 9 2 (set (reg:DI 83)
(lo_sum:DI (reg:DI 84)
(label_ref:DI 47))) rr.go:57 230 {*movdi_er_low_l}
(insn_list:REG_LABEL_OPERAND 47 (expr_list:REG_EQUAL (label_ref:DI 47)
(nil
(insn 9 8 10 2 (set (reg:DI 16 $16)
(reg:DI 83)) rr.go:57 236 {*movdi}
(nil))
(call_insn 10 9 11 2 (parallel [
(set (reg:DI 0 $0)
(call (mem:DI (symbol_ref:DI ("__go_set_defer_retaddr") [flags
0x41] ) [0
__go_set_defer_retaddr S8 A64])
(const_int 0 [0])))
(use (reg:DI 29 $29))
(clobber (reg:DI 26 $26))
]) rr.go:57 357 {*call_value_osf_1_er}
(expr_list:REG_CALL_DECL (symbol_ref:DI ("__go_set_defer_retaddr") [flags
0x41] )
(nil))
(expr_list:DI (use (reg:DI 16 $16))
(nil)))
...
(call_insn 46 45 47 5 (parallel [
(call (mem:DI (reg/f:DI 80 [ D.1014 ]) [0 *_42 S8 A64])
(const_int 4048 [0xfd0]))
(use (reg:DI 29 $29))
(clobber (reg:DI 26 $26))
]) rr.go:57 210 {*call_osf_1_er}
(expr_list:REG_DEAD (reg/f:DI 80 [ D.1014 ])
(expr_list:REG_DEAD (reg:DI 21 $21)
(expr_list:REG_DEAD (reg:DI 20 $20)
(expr_list:REG_DEAD (reg:DI 19 $19)
(expr_list:REG_DEAD (reg:DI 18 $18)
(expr_list:REG_DEAD (reg:DI 17 $17)
(expr_list:REG_DEAD (reg:DI 16 $16)
(expr_list:REG_CALL_DECL (nil)
(nil)
(expr_list (use (reg:DI 21 $21))
(expr_list (use (reg:DI 20 $20))
(expr_list (use (reg:DI 19 $19))
(expr_list (use (reg:DI 18 $18))
(expr_list (use (reg:DI 17 $17))
(expr_list (use (reg:DI 16 $16))
(expr_list:BLK (use (mem:BLK (reg/f:DI 30 $30) [0
S4048 A64]))
(nil)
;; succ: 6 [100.0%] (FALLTHRU)
;; lr out 15 [$15] 26 [$26] 29 [$29] 30 [$30] 31 [AP] 63 [FP]
;; live out 15 [$15] 29 [$29] 30 [$30] 31 [AP] 63 [FP]
;; basic block 6, loop depth 0, count 0, freq 1, maybe hot
;; prev block 5, next block 7, flags: (REACHABLE, RTL, MODIFIED)
;; pred: 2 [39.0%]
;; 5 [100.0%] (FALLTHRU)
;; bb 6 artificial_defs: { }
;; bb 6 artificial_uses: { u-1(15){ }u-1(29){ }u-1(30){ }u-1(31){ }u-1(63){ }}
;; lr in15 [$15] 26 [$26] 29 [$29] 30 [$30] 31 [AP] 63 [FP]
;; lr use 15 [$15] 29 [$29] 30 [$30] 31 [AP] 63 [FP]
;; lr def 81
;; live in 15 [$15] 29 [$29] 30 [$30] 31 [AP] 63 [FP]
;; live gen 81
;; live kill
(code_label/s 47 46 48 6 2 ("retaddr") [5 uses])
(note 48 47 49 6 [bb 6] NOTE_INSN_BASIC_BLOCK)
(insn 49 48 52 6 (set (reg:DI 81 [ ])
(const_int 0 [0])) rr.go:57 236 {*movdi}
(nil))
succ: 8 [100.0%] (FALLTHRU)
Unfortunatelly, this is not a robust approach, since in a follow-up
_.180r.fwprop1 pass (insn 49) propagates to function return value, leaving:
(code_label/s 47 46 48 6 2 ("retaddr") [5 uses])
(note 48 47 52 6 [bb 6] NOTE_INSN_BASIC_BLOCK)
;; succ: 8 [100.0%] (FALLTHRU)
The following 181r.cprop pass removes the label (also updates passed argument
to __go_set_defer_retaddr in (insn 7) and (insn 8)) and merges bb after a thunk
call:
(insn 7 3 8 2 (set (reg/f:DI 84)
(high:DI (label_ref:DI [47 deleted]))) rr.go:57 236 {*movdi}
(insn_list:REG_LABEL_OPERAND 47 (nil)))
(insn 8 7 9 2 (set (reg/f:DI 83)
(lo_sum:DI (reg/f:DI 84)
(label_ref:DI [47 deleted]))) rr.go:57 230 {*movdi_er_low_l}
(expr_list:REG_DEAD (reg/f:DI 84)
(insn_list:REG_LABEL_OPERAND 47 (expr_list:REG_EQUAL (label_ref:DI [47
deleted])
(nil)
...
(call_insn 46 45 47 5 (parallel [
(call (mem:DI (reg/f:DI 80 [ D.1014 ]) [0 *_42 S8 A64])
(const_int 4048 [0xfd0]))
(use (reg:DI 29 $29))
(clobber (reg:DI 26 $26))
]) rr.go:57 210 {*call_osf_1_er}
(expr_list:REG_DEAD (reg/f:DI 80 [ D.1014 ])
(expr_list:REG_DEAD (reg:DI 21 $21)
(expr_list:REG_DEAD (reg:DI 20 $20)
(expr_list:REG_DEAD (reg:DI 19 $19)
(expr_list:REG_DEAD (reg:DI 18 $18)
(expr_list:REG_DEAD (reg:DI 17 $17)
(expr_list:REG_DEAD (reg:DI 16 $16)
(expr_list:REG_CALL_DECL (nil)
(nil)
(expr_list (use (reg:D
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874 --- Comment #7 from Uroš Bizjak --- (In reply to Ian Lance Taylor from comment #6) > I don't know why making the types smaller in comment #4 makes any > difference. On a system that does not use split stacks, the runtime will > allocate a 2M stack for each goroutine (see StackMin in > libgo/runtime/proc.c). That is clearly more than large enough to hold a > 8192 byte value. The comments in the test about splitting the stack only > apply to systems that support stack splitting. > > This will need more investigation by somebody with access to an Alpha. I > guess one simple first step would be to verify that USING_SPLIT_STACK and > LINKER_SUPPORTS_SPLIT_STACK are both 0 in the file TARGET/libgo/config.h. I can verify that they are undefined. Maybe the effects of split stack can be ruled out on x86 by forcing it to use libffi closures with non-split stack configuration?
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874 --- Comment #6 from Ian Lance Taylor --- I don't know why making the types smaller in comment #4 makes any difference. On a system that does not use split stacks, the runtime will allocate a 2M stack for each goroutine (see StackMin in libgo/runtime/proc.c). That is clearly more than large enough to hold a 8192 byte value. The comments in the test about splitting the stack only apply to systems that support stack splitting. This will need more investigation by somebody with access to an Alpha. I guess one simple first step would be to verify that USING_SPLIT_STACK and LINKER_SUPPORTS_SPLIT_STACK are both 0 in the file TARGET/libgo/config.h.
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874 --- Comment #5 from Uroš Bizjak --- I forgot to tell that reflect test from libgo testsuite now passes: PASS: reflect
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874
--- Comment #4 from Uroš Bizjak ---
(In reply to Uroš Bizjak from comment #3)
> (In reply to Ian Lance Taylor from comment #2)
> > The patch is committed so this bug may be fixed. I haven't tested it on
> > Alpha, though.
>
> The testcase recover.go doesn't fail outright with not-implemented error.
> However, tests test11reflect2, test13reflect2 and test14reflect2 currently
> fail.
The problem with test11reflect2 was due to unhandled FFI_TYPE_VOID case in
ffi_closure_osf_inner, fixed by following patch:
--cut here-
Index: src/alpha/ffi.c
===
--- src/alpha/ffi.c (revision 212882)
+++ src/alpha/ffi.c (working copy)
@@ -237,6 +237,7 @@ ffi_closure_osf_inner(ffi_closure *closure, void *
switch (arg_types[i]->type)
{
+ case FFI_TYPE_VOID:
case FFI_TYPE_SINT8:
case FFI_TYPE_UINT8:
case FFI_TYPE_SINT16:
--cut here--
It looks that test13reflect2 and test14reflect2 tests fail on non-split stack
targets, since everything works OK with slightly modified testcase:
--cut here--
Index: testsuite/go.test/test/recover.go
===
--- testsuite/go.test/test/recover.go (revision 212882)
+++ testsuite/go.test/test/recover.go (working copy)
@@ -432,7 +432,7 @@
}
// enormous receiver, so wrapper splits stack to call M
-type T5 [8192]byte
+type T5 [2048]byte
func (T5) M() {
mustRecoverBody(doubleRecover(), recover(), recover(), 13)
@@ -459,12 +459,12 @@
// enormous receiver + enormous method frame, so wrapper splits stack to call
M,
// and then M splits stack to allocate its frame.
// recover must look back two frames to find the panic.
-type T6 [8192]byte
+type T6 [2048]byte
var global byte
func (T6) M() {
- var x [8192]byte
+ var x [2048]byte
x[0] = 1
x[1] = 2
for i := range x {
--cut here--
It looks that the later problem points to a generic problem with non
split-stack targets.
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874 Uroš Bizjak changed: What|Removed |Added Status|WAITING |NEW --- Comment #3 from Uroš Bizjak --- (In reply to Ian Lance Taylor from comment #2) > The patch is committed so this bug may be fixed. I haven't tested it on > Alpha, though. The testcase recover.go doesn't fail outright with not-implemented error. However, tests test11reflect2, test13reflect2 and test14reflect2 currently fail.
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874 Ian Lance Taylor changed: What|Removed |Added Status|UNCONFIRMED |WAITING Last reconfirmed||2014-07-20 Ever confirmed|0 |1 --- Comment #2 from Ian Lance Taylor --- The patch is committed so this bug may be fixed. I haven't tested it on Alpha, though.
[Bug go/60874] FAIL: go.test/test/recover.go execution
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60874 --- Comment #1 from Uroš Bizjak --- Patch that extends libgo to use libffi closures is at [1]. [1] https://gcc.gnu.org/ml/gcc-patches/2014-07/msg01355.html
