Re: [PATCH] Fix for PR libstdc++/60758

[Alexey Merzlyakov <alexey dot merzlyakov at samsung dot com>]

On 09.04.2014 15:12, Ramana Radhakrishnan wrote:
> On 04/09/14 09:07, Alexey Merzlyakov wrote:
> > On 04.04.2014 14:44, Alexey Merzlyakov wrote:
> > > Hi all,
> > >
> > > Here is a patch, that fixes infinite backtraces in __cxa_end_cleanup().
> > > The Bugzilla entry for
> > > this:http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60758
> > >
> > > The __cxa_end_cleanup() does not save/restore LR in function
> > > header/footer and does not provide any unwind info,
>
> So, your patch saves / restores LR to allow the prologue parser in GDB 
> to get this right and still doesn't provide unwind info. It would be 
> better to make that work correctly as well while you are here by 
> providing the appropriate cfi directives.
>
> > > which causes problems with GDB and other tools (e.g. unwind code in
> > > libgcc, libbacktrace, etc.).
> > >
> > > Best regards,
> > > Merzlyakov Alexey
> > >
> > > 2014-04-03  Alexey Merzlyakov <alexey.merzlyakov@samsung.com>
> > >
> > >      PR libstdc++/60758
> > >      * libsupc++/eh_arm.cc (__cxa_end_cleanup): Add LR save/restore.
> > >
> > > diff --git a/libstdc++-v3/libsupc++/eh_arm.cc
> > > b/libstdc++-v3/libsupc++/eh_arm.cc
> > > index aa453dd..ead1e61 100644
> > > --- a/libstdc++-v3/libsupc++/eh_arm.cc
> > > +++ b/libstdc++-v3/libsupc++/eh_arm.cc
> > > @@ -206,9 +206,9 @@ asm ("  .pushsection .text.__cxa_end_cleanup\n"
> > >   "    .type __cxa_end_cleanup, \"function\"\n"
> > >   "    .thumb_func\n"
> > >   "__cxa_end_cleanup:\n"
> > > -"    push\t{r1, r2, r3, r4}\n"
> > > +"    push\t{r1, r2, r3, r4, lr}\n"
>
> So if you are doing that please replace r4 by lr ? r4 is a callee save 
> register and is purely used here to keep stack alignment to 64 bits. 
> Not doing that isn't ideal here even though things will work because 
> __cxa_end_cleanup is part of this.
>
> > >   "    bl\t__gnu_end_cleanup\n"
> > > -"    pop\t{r1, r2, r3, r4}\n"
> > > +"    pop\t{r1, r2, r3, r4, lr}\n"
> > >   "    bl\t_Unwind_Resume @ Never returns\n"
> > >   "    .popsection\n");
> > >   #else
> > > @@ -216,9 +216,9 @@ asm ("  .pushsection .text.__cxa_end_cleanup\n"
> > >   "    .global __cxa_end_cleanup\n"
> > >   "    .type __cxa_end_cleanup, \"function\"\n"
> > >   "__cxa_end_cleanup:\n"
> > > -"    stmfd\tsp!, {r1, r2, r3, r4}\n"
> > > +"    stmfd\tsp!, {r1, r2, r3, r4, lr}\n"
>
> and likewise.
>
> > >   "    bl\t__gnu_end_cleanup\n"
> > > -"    ldmfd\tsp!, {r1, r2, r3, r4}\n"
> > > +"    ldmfd\tsp!, {r1, r2, r3, r4, lr}\n"
> > >   "    bl\t_Unwind_Resume @ Never returns\n"
> > >   "    .popsection\n");
> > >   #endif
> >
> > Forgot to mention:
> > the patch has been tested on ARM - no regressions.
>
> And by that what do you mean ?
>
> arm-eabi , arm-linux-gnueabi(hf) with / without Neon, ARM state / 
> Thumb state ?
>
>
>
> regards
> Ramana
>
> > Best regards,
> > Merzlyakov Alexey

Ok, got it. The patch fixed accordingly:
* Restored proper stack alignment.
* Added unwinding directives.

Updated patch re-tested again on arm-linux-gnueabi(sf):
$ arm-linux-gnueabi-gcc -v
  Using built-in specs.
  COLLECT_GCC=./arm-linux-gnueabi-gcc
COLLECT_LTO_WRAPPER=/home/alexey.merzlyakov/arm/libexec/gcc/arm-linux-gnueabi/4.9.0/lto-wrapper
  Target: arm-linux-gnueabi
  Configured with: ../gcc-src/configure --host=i686-pc-linux-gnu 
--target=arm-linux-gnueabi --prefix=/home/alexey.merzlyakov/arm 
--with-sysroot=/home/alexey.merzlyakov/arm/arm-linux-gnueabi/sys-root 
--disable-libmudflap --disable-libssp --with-mode=arm --with-fpu=vfpv3 
--with-cpu=cortex-a15 --with-tune=cortex-a15 --with-float=softfp 
--disable-libatomic --disable-libgomp --enable-languages=c,c++ 
--with-gmp=/home/alexey.merzlyakov/arm 
--with-mpfr=/home/alexey.merzlyakov/arm 
--with-mpc=/home/alexey.merzlyakov/arm
  Thread model: posix
  gcc version 4.9.0 20140403 (experimental) (GCC)

Is this enough or should I test other platforms?

Best regards,
Merzlyakov Alexey

2014-04-03  Alexey Merzlyakov <alexey.merzlyakov@samsung.com>

    PR libstdc++/60758
    * libsupc++/eh_arm.cc (__cxa_end_cleanup): Change r4 to lr in 
save/restore.

diff --git a/libstdc++-v3/libsupc++/eh_arm.cc 
b/libstdc++-v3/libsupc++/eh_arm.cc
index aa453dd..f49ad4f 100644
--- a/libstdc++-v3/libsupc++/eh_arm.cc
+++ b/libstdc++-v3/libsupc++/eh_arm.cc
@@ -199,27 +199,34 @@ asm (".global __cxa_end_cleanup\n"
 "    nop        5\n");
 #else
 // Assembly wrapper to call __gnu_end_cleanup without clobbering r1-r3.
-// Also push r4 to preserve stack alignment.
+// Additionally push lr to preserve stack alignment and to allow
+// backtracing.
 #ifdef __thumb__
 asm ("  .pushsection .text.__cxa_end_cleanup\n"
 "    .global __cxa_end_cleanup\n"
 "    .type __cxa_end_cleanup, \"function\"\n"
 "    .thumb_func\n"
 "__cxa_end_cleanup:\n"
-"    push\t{r1, r2, r3, r4}\n"
+"    .fnstart\n"
+"    push\t{r1, r2, r3, lr}\n"
+"    .save\t{r1, r2, r3, lr}\n"
 "    bl\t__gnu_end_cleanup\n"
-"    pop\t{r1, r2, r3, r4}\n"
+"    pop\t{r1, r2, r3, lr}\n"
 "    bl\t_Unwind_Resume @ Never returns\n"
+"    .fnend\n"
 "    .popsection\n");
 #else
 asm ("  .pushsection .text.__cxa_end_cleanup\n"
 "    .global __cxa_end_cleanup\n"
 "    .type __cxa_end_cleanup, \"function\"\n"
 "__cxa_end_cleanup:\n"
-"    stmfd\tsp!, {r1, r2, r3, r4}\n"
+"    .fnstart\n"
+"    stmfd\tsp!, {r1, r2, r3, lr}\n"
+"    .save\t{r1, r2, r3, lr}\n"
 "    bl\t__gnu_end_cleanup\n"
-"    ldmfd\tsp!, {r1, r2, r3, r4}\n"
+"    ldmfd\tsp!, {r1, r2, r3, lr}\n"
 "    bl\t_Unwind_Resume @ Never returns\n"
+"    .fnend\n"
 "    .popsection\n");
 #endif
 #endif