RE: [Patch ARM] Fix PR target/56846
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > ow...@gcc.gnu.org] On Behalf Of Tony Wang > > > Hi all, > > The bug is reported at > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56846, and it’s about the > problem that > when exception handler is involved in the function, then > _Unwind_Backtrace function will run into deadloop on > arm target. The patch (in r215101) can be backported without any change on 4.8 and 4.9 branches. I checked in QEMU with and without the patch on both branches and it indeed solves the problem. Testsuite run without regression when compiled with arm-none-eabi cross compiler and executed on QEMU emulating Cortex-M3. I also bootstrapped gcc on x86_64-linux-gnu and run the testsuite without regressions. Is it ok for backport? Best regards, Thomas
RE: [Patch ARM] Fix PR target/56846
> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-ow...@gcc.gnu.org] On
> Behalf Of Jonathan Wakely
> Sent: Tuesday, September 09, 2014 5:16 PM
> To: Ramana Radhakrishnan
> Cc: Tony Wang; gcc-patches; d...@debian.org; aph-...@littlepinkcloud.com;
> Richard Earnshaw; Ramana
> Radhakrishnan; libstd...@gcc.gnu.org
> Subject: Re: [Patch ARM] Fix PR target/56846
>
> On 09/09/14 09:33 +0100, Ramana Radhakrishnan wrote:
> >I'd like another review here however it looks sane to me. You need to
> >CC libstd...@gcc.gnu.org for libstdc++ patches. Your email doesn't say
> >how you tested this patch. Can you make sure you've run this through a
> >bootstrap and regression test on GNU/Linux and a cross regression test
> >on arm-none-eabi with no regressions ?
>
> Thanks for forwarding this, Ramana.
>
> I don't know the EABI unwinder code so if Ramana is OK with it and no
> other ARM maintainers have any comments then the patch is OK with me
> too, with a couple of small tweaks ...
Thanks for your comment, Jonathan.
I will send a new patch to cover your comment.
BR,
Tony
>
> >>
> >> gcc/libstdc++-v3/ChangeLog:
> >> 2014-8-25 Tony Wang
> >>
> >> PR target/56846
> >> * libsupc++/eh_personality.cc: Return with
> >> CONTINUE_UNWINDING
> >> when meet with the unwind state pattern:
> >> _US_VIRTUAL_UNWIND_FRAME |
> >> _US_FORCE_UNWIND
>
> The changelog should say which function is being changed:
>
> * libsupc++/eh_personality.cc (__gxx_personality_v0): ...
>
> Or maybe:
>
> * libsupc++/eh_personality.cc (PERSONALITY_FUNCTION): ...
>
> Instead of "when meet with the unwind state pattern" please say "when the
> state
> pattern contains"
>
> >> diff --git a/libstdc++-v3/libsupc++/eh_personality.cc
> >> b/libstdc++-v3/libsupc++/eh_personality.cc
> >> index f315a83..c2b30e9 100644
> >> --- a/libstdc++-v3/libsupc++/eh_personality.cc
> >> +++ b/libstdc++-v3/libsupc++/eh_personality.cc
> >> @@ -378,6 +378,11 @@ PERSONALITY_FUNCTION (int version,
> >>switch (state & _US_ACTION_MASK)
> >> {
> >> case _US_VIRTUAL_UNWIND_FRAME:
> >> + // If the unwind state pattern is _US_VIRTUAL_UNWIND_FRAME |
> >> + // _US_FORCE_UNWIND, we don't need to search for any handler
> >> + // as it is not a real exception. Just unwind the stack.
>
> I think this comment would be easier to read if the expression with the two
> constants was all on one line:
>
> // If the unwind state pattern is
> // _US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND
> // then we don't need to search for any handler as it is not a real
> // exception. Just unwind the stack.
>
> >> + if (state & _US_FORCE_UNWIND)
> >> +CONTINUE_UNWINDING;
> >>actions = _UA_SEARCH_PHASE;
> >>break;
> >>
> >>
Re: [Patch ARM] Fix PR target/56846
On 09/09/14 09:33 +0100, Ramana Radhakrishnan wrote:
I'd like another review here however it looks sane to me. You need to
CC libstd...@gcc.gnu.org for libstdc++ patches. Your email doesn't say
how you tested this patch. Can you make sure you've run this through a
bootstrap and regression test on GNU/Linux and a cross regression test
on arm-none-eabi with no regressions ?
Thanks for forwarding this, Ramana.
I don't know the EABI unwinder code so if Ramana is OK with it and no
other ARM maintainers have any comments then the patch is OK with me
too, with a couple of small tweaks ...
gcc/libstdc++-v3/ChangeLog:
2014-8-25 Tony Wang
PR target/56846
* libsupc++/eh_personality.cc: Return with CONTINUE_UNWINDING
when meet with the unwind state pattern:
_US_VIRTUAL_UNWIND_FRAME |
_US_FORCE_UNWIND
The changelog should say which function is being changed:
* libsupc++/eh_personality.cc (__gxx_personality_v0): ...
Or maybe:
* libsupc++/eh_personality.cc (PERSONALITY_FUNCTION): ...
Instead of "when meet with the unwind state pattern" please say "when the state
pattern contains"
diff --git a/libstdc++-v3/libsupc++/eh_personality.cc
b/libstdc++-v3/libsupc++/eh_personality.cc
index f315a83..c2b30e9 100644
--- a/libstdc++-v3/libsupc++/eh_personality.cc
+++ b/libstdc++-v3/libsupc++/eh_personality.cc
@@ -378,6 +378,11 @@ PERSONALITY_FUNCTION (int version,
switch (state & _US_ACTION_MASK)
{
case _US_VIRTUAL_UNWIND_FRAME:
+ // If the unwind state pattern is _US_VIRTUAL_UNWIND_FRAME |
+ // _US_FORCE_UNWIND, we don't need to search for any handler
+ // as it is not a real exception. Just unwind the stack.
I think this comment would be easier to read if the expression with the two
constants was all on one line:
// If the unwind state pattern is
// _US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND
// then we don't need to search for any handler as it is not a real
// exception. Just unwind the stack.
+ if (state & _US_FORCE_UNWIND)
+CONTINUE_UNWINDING;
actions = _UA_SEARCH_PHASE;
break;
RE: [Patch ARM] Fix PR target/56846
> -Original Message-
> From: Ramana Radhakrishnan [mailto:ramana@googlemail.com]
> Sent: Tuesday, September 09, 2014 4:33 PM
> To: Tony Wang
> Cc: gcc-patches; d...@debian.org; aph-...@littlepinkcloud.com; Richard
> Earnshaw; Ramana Radhakrishnan;
> libstd...@gcc.gnu.org
> Subject: Re: [Patch ARM] Fix PR target/56846
>
> On Mon, Aug 25, 2014 at 11:32 AM, Tony Wang wrote:
> > Hi all,
> >
> > The bug is reported at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56846,
> > and it’s about the problem that
> > when exception handler is involved in the function, then _Unwind_Backtrace
> > function will run into deadloop
> on
> > arm target.
>
> You mean an infinite loop.
>
> >
> > Cmd line: arm-none-eabi-g++ -mthumb -mcpu=cortex-m3 -O0 -g -std=c++11
> > -specs=rdimon.specs main.c -o
> main.exe
> > #include
> > #include
> > _Unwind_Reason_Code trace_func(struct _Unwind_Context * context, void* arg)
> > {
> > void *ip = (void *)_Unwind_GetIP(context);
> > printf("Address: %p\n", ip);
> > return _URC_NO_REASON;
> > }
> > void bar()
> > {
> > puts("This is in bar");
> > _Unwind_Backtrace((_Unwind_Trace_Fn)&trace_func, 0);
> > }
> > void foo()
> > {
> > try
> > {
> > bar();
> > }
> > catch (...)
> > {
> > puts("Exception");
> > }
> > }
> >
> > The potential of such a bug is discussed long time ago in mail:
> > https://gcc.gnu.org/ml/gcc/2007-08/msg00235.html. Basically, as the ARM
> > EHABI does not define how to
> implement
> > the Unwind_Backtrace, Andrew give control to the personality routine to
> > unwind the stack, and use the
> unwind
> > state combination of “_US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND” to
> > represent that the caller is
> asking the
> > personality routine to only unwind the stack for it.
> >
> > However, the pr in the libstdc++-v3 doesn’t handle such a unwind state
> > pattern correctly. When the backtrace
> > function passes such a pattern to it, it will still return
> > _URC_HANDLER_FOUND to the caller in some cases.
> > It’s because the pr will think that the _Unwind_Backtrace is raising a none
> > type exception to it, so if the
> > exception handler in current stack frame can catch anything(like catch(…)),
> > the pr will return
> > _URC_HANDLER_FOUND to the caller and ask for next step. But definitely, the
> > unwind backtrace function
> don’t
> > know what to do when pr return an exception handler to it.
> >
> > So this patch just evaluate such a unwind state pattern at the beginning of
> > the personality routine in
> > libstdc++-v3, if we meet with “_US_VIRTUAL_UNWIND_FRAME |
> > _US_FORCE_UNWIND”, then we directly call
> macro
> > CONTINUE_UNWINDING to unwind the stack and return.
> >
> > Is this a reasonable fix?
>
> I'd like another review here however it looks sane to me. You need to
> CC libstd...@gcc.gnu.org for libstdc++ patches. Your email doesn't say
> how you tested this patch. Can you make sure you've run this through a
> bootstrap and regression test on GNU/Linux and a cross regression test
> on arm-none-eabi with no regressions ?
Hi Ramana,
Thanks for you review. After this patch, the infinite loop will be fixed for
the above test case, and I can make sure that no regression is happen
during bootstrap and regression test on Linux and a cross regression test
on arm-none-eabi.
Regards,
Tony
>
>
> regards
> Ramana
>
>
>
> >
> > gcc/libstdc++-v3/ChangeLog:
> > 2014-8-25 Tony Wang
> >
> > PR target/56846
> > * libsupc++/eh_personality.cc: Return with
> > CONTINUE_UNWINDING
> > when meet with the unwind state pattern:
> > _US_VIRTUAL_UNWIND_FRAME |
> > _US_FORCE_UNWIND
> >
> > diff --git a/libstdc++-v3/libsupc++/eh_personality.cc
> > b/libstdc++-v3/libsupc++/eh_personality.cc
> > index f315a83..c2b30e9 100644
> > --- a/libstdc++-v3/libsupc++/eh_personality.cc
> > +++ b/libstdc++-v3/libsupc++/eh_personality.cc
> > @@ -378,6 +378,11 @@ PERSONALITY_FUNCTION (int version,
> >switch (state & _US_ACTION_MASK)
> > {
> > case _US_VIRTUAL_UNWIND_FRAME:
> > + // If the unwind state pattern is _US_VIRTUAL_UNWIND_FRAME |
> > + // _US_FORCE_UNWIND, we don't need to search for any handler
> > + // as it is not a real exception. Just unwind the stack.
> > + if (state & _US_FORCE_UNWIND)
> > +CONTINUE_UNWINDING;
> >actions = _UA_SEARCH_PHASE;
> >break;
> >
> >
Re: [Patch ARM] Fix PR target/56846
On Mon, Aug 25, 2014 at 11:32 AM, Tony Wang wrote:
> Hi all,
>
> The bug is reported at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56846,
> and it’s about the problem that
> when exception handler is involved in the function, then _Unwind_Backtrace
> function will run into deadloop on
> arm target.
You mean an infinite loop.
>
> Cmd line: arm-none-eabi-g++ -mthumb -mcpu=cortex-m3 -O0 -g -std=c++11
> -specs=rdimon.specs main.c -o main.exe
> #include
> #include
> _Unwind_Reason_Code trace_func(struct _Unwind_Context * context, void* arg)
> {
> void *ip = (void *)_Unwind_GetIP(context);
> printf("Address: %p\n", ip);
> return _URC_NO_REASON;
> }
> void bar()
> {
> puts("This is in bar");
> _Unwind_Backtrace((_Unwind_Trace_Fn)&trace_func, 0);
> }
> void foo()
> {
> try
> {
> bar();
> }
> catch (...)
> {
> puts("Exception");
> }
> }
>
> The potential of such a bug is discussed long time ago in mail:
> https://gcc.gnu.org/ml/gcc/2007-08/msg00235.html. Basically, as the ARM EHABI
> does not define how to implement
> the Unwind_Backtrace, Andrew give control to the personality routine to
> unwind the stack, and use the unwind
> state combination of “_US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND” to
> represent that the caller is asking the
> personality routine to only unwind the stack for it.
>
> However, the pr in the libstdc++-v3 doesn’t handle such a unwind state
> pattern correctly. When the backtrace
> function passes such a pattern to it, it will still return _URC_HANDLER_FOUND
> to the caller in some cases.
> It’s because the pr will think that the _Unwind_Backtrace is raising a none
> type exception to it, so if the
> exception handler in current stack frame can catch anything(like catch(…)),
> the pr will return
> _URC_HANDLER_FOUND to the caller and ask for next step. But definitely, the
> unwind backtrace function don’t
> know what to do when pr return an exception handler to it.
>
> So this patch just evaluate such a unwind state pattern at the beginning of
> the personality routine in
> libstdc++-v3, if we meet with “_US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND”,
> then we directly call macro
> CONTINUE_UNWINDING to unwind the stack and return.
>
> Is this a reasonable fix?
I'd like another review here however it looks sane to me. You need to
CC libstd...@gcc.gnu.org for libstdc++ patches. Your email doesn't say
how you tested this patch. Can you make sure you've run this through a
bootstrap and regression test on GNU/Linux and a cross regression test
on arm-none-eabi with no regressions ?
regards
Ramana
>
> gcc/libstdc++-v3/ChangeLog:
> 2014-8-25 Tony Wang
>
> PR target/56846
> * libsupc++/eh_personality.cc: Return with CONTINUE_UNWINDING
> when meet with the unwind state pattern:
> _US_VIRTUAL_UNWIND_FRAME |
> _US_FORCE_UNWIND
>
> diff --git a/libstdc++-v3/libsupc++/eh_personality.cc
> b/libstdc++-v3/libsupc++/eh_personality.cc
> index f315a83..c2b30e9 100644
> --- a/libstdc++-v3/libsupc++/eh_personality.cc
> +++ b/libstdc++-v3/libsupc++/eh_personality.cc
> @@ -378,6 +378,11 @@ PERSONALITY_FUNCTION (int version,
>switch (state & _US_ACTION_MASK)
> {
> case _US_VIRTUAL_UNWIND_FRAME:
> + // If the unwind state pattern is _US_VIRTUAL_UNWIND_FRAME |
> + // _US_FORCE_UNWIND, we don't need to search for any handler
> + // as it is not a real exception. Just unwind the stack.
> + if (state & _US_FORCE_UNWIND)
> +CONTINUE_UNWINDING;
>actions = _UA_SEARCH_PHASE;
>break;
>
>
Re: [Patch ARM] Fix PR target/56846
On 25/08/14 11:32, Tony Wang wrote:
> Hi all,
>
> The bug is reported at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56846,
> and it’s about the problem that
> when exception handler is involved in the function, then _Unwind_Backtrace
> function will run into deadloop on
> arm target.
>
> Cmd line: arm-none-eabi-g++ -mthumb -mcpu=cortex-m3 -O0 -g -std=c++11
> -specs=rdimon.specs main.c -o main.exe
> #include
> #include
> _Unwind_Reason_Code trace_func(struct _Unwind_Context * context, void* arg)
> {
> void *ip = (void *)_Unwind_GetIP(context);
> printf("Address: %p\n", ip);
> return _URC_NO_REASON;
> }
> void bar()
> {
> puts("This is in bar");
> _Unwind_Backtrace((_Unwind_Trace_Fn)&trace_func, 0);
> }
> void foo()
> {
> try
> {
> bar();
> }
> catch (...)
> {
> puts("Exception");
> }
> }
>
> The potential of such a bug is discussed long time ago in mail:
> https://gcc.gnu.org/ml/gcc/2007-08/msg00235.html. Basically, as the ARM EHABI
> does not define how to implement
> the Unwind_Backtrace, Andrew give control to the personality routine to
> unwind the stack, and use the unwind
> state combination of “_US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND” to
> represent that the caller is asking the
> personality routine to only unwind the stack for it.
>
> However, the pr in the libstdc++-v3 doesn’t handle such a unwind state
> pattern correctly. When the backtrace
> function passes such a pattern to it, it will still return _URC_HANDLER_FOUND
> to the caller in some cases.
> It’s because the pr will think that the _Unwind_Backtrace is raising a none
> type exception to it, so if the
> exception handler in current stack frame can catch anything(like catch(…)),
> the pr will return
> _URC_HANDLER_FOUND to the caller and ask for next step. But definitely, the
> unwind backtrace function don’t
> know what to do when pr return an exception handler to it.
>
> So this patch just evaluate such a unwind state pattern at the beginning of
> the personality routine in
> libstdc++-v3, if we meet with “_US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND”,
> then we directly call macro
> CONTINUE_UNWINDING to unwind the stack and return.
>
> Is this a reasonable fix?
>
> gcc/libstdc++-v3/ChangeLog:
> 2014-8-25 Tony Wang
>
> PR target/56846
> * libsupc++/eh_personality.cc: Return with CONTINUE_UNWINDING
> when meet with the unwind state pattern:
> _US_VIRTUAL_UNWIND_FRAME |
> _US_FORCE_UNWIND
That looks very sensible, but it's not my call to approve it.
Andrew.
RE: [Patch ARM] Fix PR target/56846
Ping?
> -Original Message-
> From: Tony Wang [mailto:tony.w...@arm.com]
> Sent: Monday, August 25, 2014 6:33 PM
> To: 'gcc-patches@gcc.gnu.org'; 'd...@debian.org';
> 'aph-...@littlepinkcloud.com'; Richard Earnshaw; Ramana
> Radhakrishnan
> Subject: [Patch ARM] Fix PR target/56846
>
> Hi all,
>
> The bug is reported at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56846,
> and it’s about the problem that
> when exception handler is involved in the function, then _Unwind_Backtrace
> function will run into deadloop
on
> arm target.
>
> Cmd line: arm-none-eabi-g++ -mthumb -mcpu=cortex-m3 -O0 -g -std=c++11
> -specs=rdimon.specs main.c -o
> main.exe
> #include
> #include
> _Unwind_Reason_Code trace_func(struct _Unwind_Context * context, void* arg)
> {
> void *ip = (void *)_Unwind_GetIP(context);
> printf("Address: %p\n", ip);
> return _URC_NO_REASON;
> }
> void bar()
> {
> puts("This is in bar");
> _Unwind_Backtrace((_Unwind_Trace_Fn)&trace_func, 0);
> }
> void foo()
> {
> try
> {
> bar();
> }
> catch (...)
> {
> puts("Exception");
> }
> }
>
> The potential of such a bug is discussed long time ago in mail:
> https://gcc.gnu.org/ml/gcc/2007-
> 08/msg00235.html. Basically, as the ARM EHABI does not define how to
> implement the Unwind_Backtrace,
> Andrew give control to the personality routine to unwind the stack, and use
> the unwind state combination of
> “_US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND” to represent that the caller is
> asking the personality
> routine to only unwind the stack for it.
>
> However, the pr in the libstdc++-v3 doesn’t handle such a unwind state
> pattern correctly. When the backtrace
> function passes such a pattern to it, it will still return _URC_HANDLER_FOUND
> to the caller in some cases.
It’s
> because the pr will think that the _Unwind_Backtrace is raising a none type
> exception to it, so if the
exception
> handler in current stack frame can catch anything(like catch(…)), the pr will
> return _URC_HANDLER_FOUND to
> the caller and ask for next step. But definitely, the unwind backtrace
> function don’t know what to do when
pr
> return an exception handler to it.
>
> So this patch just evaluate such a unwind state pattern at the beginning of
> the personality routine in
libstdc++-v3,
> if we meet with “_US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND”, then we
> directly call macro
> CONTINUE_UNWINDING to unwind the stack and return.
>
> Is this a reasonable fix?
>
> gcc/libstdc++-v3/ChangeLog:
> 2014-8-25 Tony Wang
>
> PR target/56846
> * libsupc++/eh_personality.cc: Return with CONTINUE_UNWINDING
> when meet with the unwind state pattern:
> _US_VIRTUAL_UNWIND_FRAME |
> _US_FORCE_UNWIND
>
> diff --git a/libstdc++-v3/libsupc++/eh_personality.cc
> b/libstdc++-v3/libsupc++/eh_personality.cc
> index f315a83..c2b30e9 100644
> --- a/libstdc++-v3/libsupc++/eh_personality.cc
> +++ b/libstdc++-v3/libsupc++/eh_personality.cc
> @@ -378,6 +378,11 @@ PERSONALITY_FUNCTION (int version,
> switch (state & _US_ACTION_MASK)
> {
> case _US_VIRTUAL_UNWIND_FRAME:
> + // If the unwind state pattern is _US_VIRTUAL_UNWIND_FRAME |
> + // _US_FORCE_UNWIND, we don't need to search for any handler
> + // as it is not a real exception. Just unwind the stack.
> + if (state & _US_FORCE_UNWIND)
> + CONTINUE_UNWINDING;
> actions = _UA_SEARCH_PHASE;
> break;
