public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch ARM] Fix PR target/56846
@ 2014-08-25 10:33 Tony Wang
  2014-09-05  7:42 ` Andrew Haley
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Tony Wang @ 2014-08-25 10:33 UTC (permalink / raw)
  To: gcc-patches, dan, aph-gcc, Richard Earnshaw, Ramana Radhakrishnan

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 <unwind.h>
#include <stdio.h>
_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 <tony.wang@arm.com>

                 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;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Patch ARM] Fix PR target/56846
  2014-08-25 10:33 [Patch ARM] Fix PR target/56846 Tony Wang
@ 2014-09-05  7:42 ` Andrew Haley
  2014-09-09  8:33 ` Ramana Radhakrishnan
  2014-11-19 18:06 ` Thomas Preud'homme
  2 siblings, 0 replies; 8+ messages in thread
From: Andrew Haley @ 2014-09-05  7:42 UTC (permalink / raw)
  To: Tony Wang, gcc-patches, dan, aph-gcc, Richard Earnshaw,
	Ramana Radhakrishnan

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 <unwind.h>
> #include <stdio.h>
> _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 <tony.wang@arm.com>
> 
>                  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.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Patch ARM] Fix PR target/56846
  2014-08-25 10:33 [Patch ARM] Fix PR target/56846 Tony Wang
  2014-09-05  7:42 ` Andrew Haley
@ 2014-09-09  8:33 ` Ramana Radhakrishnan
  2014-09-09  8:44   ` Tony Wang
  2014-09-09  9:16   ` Jonathan Wakely
  2014-11-19 18:06 ` Thomas Preud'homme
  2 siblings, 2 replies; 8+ messages in thread
From: Ramana Radhakrishnan @ 2014-09-09  8:33 UTC (permalink / raw)
  To: Tony Wang
  Cc: gcc-patches, dan, aph-gcc, Richard Earnshaw,
	Ramana Radhakrishnan, libstdc++

On Mon, Aug 25, 2014 at 11:32 AM, Tony Wang <tony.wang@arm.com> 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 <unwind.h>
> #include <stdio.h>
> _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 libstdc++@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 <tony.wang@arm.com>
>
>                  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;
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [Patch ARM] Fix PR target/56846
  2014-09-09  8:33 ` Ramana Radhakrishnan
@ 2014-09-09  8:44   ` Tony Wang
  2014-09-09  9:16   ` Jonathan Wakely
  1 sibling, 0 replies; 8+ messages in thread
From: Tony Wang @ 2014-09-09  8:44 UTC (permalink / raw)
  To: Ramana Radhakrishnan
  Cc: gcc-patches, dan, aph-gcc, Richard Earnshaw, libstdc++

> -----Original Message-----
> From: Ramana Radhakrishnan [mailto:ramana.gcc@googlemail.com]
> Sent: Tuesday, September 09, 2014 4:33 PM
> To: Tony Wang
> Cc: gcc-patches; dan@debian.org; aph-gcc@littlepinkcloud.com; Richard Earnshaw; Ramana Radhakrishnan;
> libstdc++@gcc.gnu.org
> Subject: Re: [Patch ARM] Fix PR target/56846
> 
> On Mon, Aug 25, 2014 at 11:32 AM, Tony Wang <tony.wang@arm.com> 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 <unwind.h>
> > #include <stdio.h>
> > _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 libstdc++@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 <tony.wang@arm.com>
> >
> >                  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;
> >
> >




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Patch ARM] Fix PR target/56846
  2014-09-09  8:33 ` Ramana Radhakrishnan
  2014-09-09  8:44   ` Tony Wang
@ 2014-09-09  9:16   ` Jonathan Wakely
  2014-09-09  9:24     ` Tony Wang
  1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2014-09-09  9:16 UTC (permalink / raw)
  To: ramrad01
  Cc: Tony Wang, gcc-patches, dan, aph-gcc, Richard Earnshaw,
	Ramana Radhakrishnan, libstdc++

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 libstdc++@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 <tony.wang@arm.com>
>>
>>                  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;
>>
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [Patch ARM] Fix PR target/56846
  2014-09-09  9:16   ` Jonathan Wakely
@ 2014-09-09  9:24     ` Tony Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Tony Wang @ 2014-09-09  9:24 UTC (permalink / raw)
  To: 'Jonathan Wakely', Ramana Radhakrishnan
  Cc: gcc-patches, dan, aph-gcc, Richard Earnshaw,
	Ramana Radhakrishnan, libstdc++

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-owner@gcc.gnu.org] On Behalf Of Jonathan Wakely
> Sent: Tuesday, September 09, 2014 5:16 PM
> To: Ramana Radhakrishnan
> Cc: Tony Wang; gcc-patches; dan@debian.org; aph-gcc@littlepinkcloud.com; Richard Earnshaw; Ramana
> Radhakrishnan; libstdc++@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 libstdc++@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 <tony.wang@arm.com>
> >>
> >>                  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;
> >>
> >>




^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [Patch ARM] Fix PR target/56846
  2014-08-25 10:33 [Patch ARM] Fix PR target/56846 Tony Wang
  2014-09-05  7:42 ` Andrew Haley
  2014-09-09  8:33 ` Ramana Radhakrishnan
@ 2014-11-19 18:06 ` Thomas Preud'homme
  2 siblings, 0 replies; 8+ messages in thread
From: Thomas Preud'homme @ 2014-11-19 18:06 UTC (permalink / raw)
  To: 'Tony Wang',
	gcc-patches, dan, aph-gcc, Richard Earnshaw,
	Ramana Radhakrishnan, libstdc++

> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@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 




^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [Patch ARM] Fix PR target/56846
@ 2014-09-04  2:15 Tony Wang
  0 siblings, 0 replies; 8+ messages in thread
From: Tony Wang @ 2014-09-04  2:15 UTC (permalink / raw)
  To: gcc-patches, dan, aph-gcc, Richard Earnshaw, Ramana Radhakrishnan

Ping?

> -----Original Message-----
> From: Tony Wang [mailto:tony.wang@arm.com]
> Sent: Monday, August 25, 2014 6:33 PM
> To: 'gcc-patches@gcc.gnu.org'; 'dan@debian.org'; 'aph-gcc@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 <unwind.h>
> #include <stdio.h>
> _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 <tony.wang@arm.com>
> 
>                  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;



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-11-19 18:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-25 10:33 [Patch ARM] Fix PR target/56846 Tony Wang
2014-09-05  7:42 ` Andrew Haley
2014-09-09  8:33 ` Ramana Radhakrishnan
2014-09-09  8:44   ` Tony Wang
2014-09-09  9:16   ` Jonathan Wakely
2014-09-09  9:24     ` Tony Wang
2014-11-19 18:06 ` Thomas Preud'homme
2014-09-04  2:15 Tony Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).