[Bug go/60406] recover.go: test13reflect2 test failure

[vogt at linux dot vnet.ibm.com]

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

--- Comment #23 from Dominik Vogt <vogt at linux dot vnet.ibm.com> ---
Regarding the new patch:

1) We need to call __builtin_extract_return_address(retaddr) in
__go_set_defer_retaddr() too.  (On s390 (31 bit) this is necessary to remove
the most significant bit of the address which indicates the addressing mode.)

I.e.

--snip--
-    g->defer->__retaddr = retaddr;
+    g->defer->__retaddr = __builtin_extract_return_addr (retaddr);
--snip--


2) The new patch does not compile for me:

--
In file included from ../../../libgo/runtime/go-check-interface.c:8:0:
../../../libgo/runtime/go-panic.h:43:13: error: conflicting types for
‘__go_makefunc_can_recover’
 extern void __go_makefunc_can_recover (void *retaddr);
             ^
In file included from ../../../libgo/runtime/go-check-interface.c:7:0:
../../../libgo/runtime/runtime.h:845:9: note: previous declaration of
‘__go_makefunc_can_recover’ was here
 void    __go_makefunc_can_recover (const void *);
         ^
--

In runtime.h, __go_makefunc_can_recover still has a "const" argument:

--snip--
-void    __go_makefunc_can_recover (const void *);
+void    __go_makefunc_can_recover (void *);
--snip--


3) Couldn't this be written more efficiently by passing a flag?

+  /* If we are called from __go_makefunc_can_recover, then we need to
+     look one level higher.  */
+  if (locs[0].function.len > 0
+      && __builtin_strcmp ((const char *) locs[0].function.str,
+               "__go_makefunc_can_recover") == 0)

E.g.

  _Bool __go_can_recover (void *retaddr, _Bool called_by_makefunc_can_recover)
  {
    ...
    if (called_by_makefunc_can_recover)
      { do something }
    else
      { do something else }
    ...
  }


4) With the suggested changes from 1 and 2 above:

s390x (64 bit):

* PASS: test/recover.go
* the test from #4 in my earlier posting works as expected
* my private defer/recover/panic testsuite works as expected

s390 (31 bit):

* PASS: test/recover.go
* the test from #4 in my earlier posting works as expected
* my private defer/recover/panic testsuite works as expected


5) I find the assumption in the loop at the end of __go_can_recover() that if a
caller's name begins with "__go_" that means the panic can be recovered, a bit
hairy.  Even if it is with the current libgo, an unrelated change elsewhere
could break this logic.