This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [gofrontend-dev] Re: libgo patch committed: Add precise stack scan support
Finally I found an ARM machine and was able to build ARM32 gcc on it, and
reproduced the failures. The Go programs aborted, in parse_lsda_header,
called from probestackmaps in the runtime startup. It seems that
_Unwind_GetLanguageSpecificData doesn't return a valid LSDA when called
from a callback from _Unwind_Backtrace.
Reading the unwinder's source code in libgcc, it seems that a function may
have a "predefined" personality function, and in this case an ARM-specific
"compact" model is used, which doesn't use the standard LSDA.
_Unwind_GetLanguageSpecificData doesn't distinguish this and simply assumes
the "generic" model is used, i.e. not used with a "predefined" personality
function. This works fine if _Unwind_GetLanguageSpecificData is called from
a non-predefined personality function, but it doesn't work if it is called
during _Unwind_Backtrace.
The patch below (also CL
https://go-review.googlesource.com/c/gofrontend/+/155758) will fix the
problem, by checking which model is used before calling
_Unwind_GetLanguageSpecificData.
Alternatively, we could change _Unwind_GetLanguageSpecificData in libgcc to
returning NULL when a predefined personality function is used.
Let me know what you think.
Thanks,
Cherry
diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
index f4bbfb60..388d7c70 100644
--- a/libgo/runtime/go-unwind.c
+++ b/libgo/runtime/go-unwind.c
@@ -646,6 +646,17 @@ findstackmaps (struct _Unwind_Context *context,
_Unwind_Ptr *ip, _Unwind_Ptr *sp
_sleb128_t index;
int size;
+#ifdef __ARM_EABI_UNWINDER__
+ {
+ _Unwind_Control_Block *ucbp;
+ ucbp = (_Unwind_Control_Block *) _Unwind_GetGR (context, 12);
+ if (*ucbp->pr_cache.ehtp & (1u << 31))
+ // The "compact" model is used, with one of the predefined
+ // personality functions. It doesn't have standard LSDA.
+ return NOTFOUND_OK;
+ }
+#endif
+
language_specific_data = (const unsigned char *)
_Unwind_GetLanguageSpecificData (context);
On Tue, Dec 18, 2018 at 10:24 PM Matthias Klose <doko@ubuntu.com> wrote:
> Cherry, see
> https://gcc.gnu.org/ml/gcc-testresults/2018-12/msg02241.html
> https://gcc.gnu.org/ml/gcc-testresults/2018-12/msg02240.html
>
> still shows ~180 test failures on ARM32. Sorry, no access to an ARM32 box
> until
> next year.
>
> Matthias
>
> On 13.12.18 00:27, Ian Lance Taylor wrote:
> > On Wed, Dec 12, 2018 at 8:10 AM Cherry Zhang <cherryyz@google.com>
> wrote:
> >>
> >> Thank you, Matthias!
> >>
> >> From the log, essentially all the tests aborted. The only place the new
> code can cause abort on all programs that I can think of is in the runtime
> startup code, probestackmaps, which calls value_size, which aborts due to
> an unhandled case. I haven't been able to try out on an ARM machine, but I
> managed to cross-compile a Go program and visually inspect the exception
> table. The type table's encoding is DW_EH_PE_absptr, which is indeed not
> handled, which will cause abort.
> >>
> >> I send https://go-review.googlesource.com/c/gofrontend/+/153857 (also
> as below). Hopefully this will fix the problem.
> >>
> >> Thanks,
> >> Cherry
> >>
> >> diff --git a/libgo/runtime/go-unwind.c b/libgo/runtime/go-unwind.c
> >> index c44755f9..f4bbfb60 100644
> >> --- a/libgo/runtime/go-unwind.c
> >> +++ b/libgo/runtime/go-unwind.c
> >> @@ -318,6 +318,8 @@ value_size (uint8_t encoding)
> >> case DW_EH_PE_sdata8:
> >> case DW_EH_PE_udata8:
> >> return 8;
> >> + case DW_EH_PE_absptr:
> >> + return sizeof(uintptr);
> >> default:
> >> break;
> >> }
> >
> >
> > Thanks.
> >
> > Committed to mainline.
> >
> > Ian
> >
> >
> >
> >> On Tue, Dec 11, 2018 at 7:03 PM Matthias Klose <doko@ubuntu.com> wrote:
> >>>
> >>> On 11.12.18 22:01, Cherry Zhang wrote:
> >>>> On Tue, Dec 11, 2018 at 3:51 PM Ian Lance Taylor <iant@golang.org>
> wrote:
> >>>>
> >>>>> On Tue, Dec 11, 2018 at 6:52 AM Matthias Klose <doko@ubuntu.com>
> wrote:
> >>>>>>
> >>>>>> On 10.12.18 16:54, Cherry Zhang wrote:
> >>>>>>> On Mon, Dec 10, 2018 at 1:41 AM Matthias Klose <doko@ubuntu.com>
> >>>>> wrote:
> >>>>>>>
> >>>>>>>> On 06.12.18 00:09, Ian Lance Taylor wrote:
> >>>>>>>>> This libgo patch by Cherry Zhang adds support for precise stack
> >>>>>>>>> scanning to the Go runtime. This uses per-function stack maps
> stored
> >>>>>>>>> in the exception tables in the language-specific data area. The
> >>>>>>>>> compiler needs to generate these stack maps; currently this is
> only
> >>>>>>>>> done by a version of LLVM, not by GCC. Each safepoint in a
> function
> >>>>>>>>> is associated with a (real or dummy) landing pad, and its "type
> info"
> >>>>>>>>> in the exception table is a pointer to the stack map. When a
> stack is
> >>>>>>>>> scanned, the stack map is found by the stack unwinding code.
> >>>>>>>>>
> >>>>>>>>> For precise stack scan we need to unwind the stack. There are
> three
> >>>>>>>> cases:
> >>>>>>>>>
> >>>>>>>>> - If a goroutine is scanning its own stack, it can unwind the
> stack
> >>>>>>>>> and scan the frames.
> >>>>>>>>>
> >>>>>>>>> - If a goroutine is scanning another, stopped, goroutine, it
> cannot
> >>>>>>>>> directly unwind the target stack. We handle this by switching
> >>>>>>>>> (runtime.gogo) to the target g, letting it unwind and scan the
> stack,
> >>>>>>>>> and switch back.
> >>>>>>>>>
> >>>>>>>>> - If we are scanning a goroutine that is blocked in a syscall, we
> >>>>> send
> >>>>>>>>> a signal to the target goroutine's thread, and let the signal
> handler
> >>>>>>>>> unwind and scan the stack. Extra care is needed as this races
> with
> >>>>>>>>> enter/exit syscall.
> >>>>>>>>>
> >>>>>>>>> Currently this is only implemented on GNU/Linux.
> >>>>>>>>>
> >>>>>>>>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.
> Committed
> >>>>>>>>> to mainline.
> >>>>>>>>
> >>>>>>>> this broke the libgo build on ARM32:
> >>>>>>>>
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c: In function
> >>>>>>>> 'scanstackwithmap_callback':
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: error:
> >>>>> '_URC_NORMAL_STOP'
> >>>>>>>> undeclared (first use in this function)
> >>>>>>>> 754 | return _URC_NORMAL_STOP;
> >>>>>>>> | ^~~~~~~~~~~~~~~~
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c:754:18: note: each
> undeclared
> >>>>>>>> identifier
> >>>>>>>> is reported only once for each function i
> >>>>>>>> t appears in
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c: In function
> >>>>>>>> 'probestackmaps_callback':
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c:802:10: error:
> >>>>> '_URC_NORMAL_STOP'
> >>>>>>>> undeclared (first use in this function)
> >>>>>>>> 802 | return _URC_NORMAL_STOP;
> >>>>>>>> | ^~~~~~~~~~~~~~~~
> >>>>>>>> ../../../src/libgo/runtime/go-unwind.c:803:1: warning: control
> >>>>> reaches end
> >>>>>>>> of
> >>>>>>>> non-void function [-Wreturn-type]
> >>>>>>>> 803 | }
> >>>>>>>> | ^
> >>>>>>>> make[6]: *** [Makefile:1474: runtime/go-unwind.lo] Error 1
> >>>>>>>> make[6]: Leaving directory
> >>>>>>>> '/<<PKGBUILDDIR>>/build/arm-linux-gnueabihf/libgo'
> >>>>>>>>
> >>>>>>>>
> >>>>>>> Hell Matthias,
> >>>>>>>
> >>>>>>> Thank you for the report. And sorry about the breakage. Does
> >>>>>>> https://go-review.googlesource.com/c/gofrontend/+/153417 (or the
> patch
> >>>>>>> below) fix ARM32 build? I don't have an ARM32 machine at hand to
> test.
> >>>>>>
> >>>>>> this fixes the build.
> >>>>>>
> >>>>>> currently running the testsuite, almost every test case core dumps
> on
> >>>>>> arm-linux-gnueabihf
> >>>>>
> >>>>> I committed Cherry's patch to trunk, since it looks reasonable to me.
> >>>>> Cherry, Matthias, let me know if you figure out why programs are
> >>>>> failing.
> >>>>>
> >>>>>
> >>>> Thank you.
> >>>>
> >>>> I don't know for the moment. I'm trying to find an ARM32 machine so I
> can
> >>>> test.
> >>>>
> >>>> Matthias, is it convenient for you to share a stack trace for the
> failing
> >>>> programs? That would be very helpful. Thanks!
> >>>
> >>> I'll do a local build this weekend. For now you find the build log at
> >>>
> https://launchpad.net/ubuntu/+source/gcc-snapshot/1:20181210-0ubuntu1/+build/15759748
>
>