diff mbox

Fix for PR libstdc++/60758

Message ID 533E8CFF.2090606@samsung.com
State New
Headers show

Commit Message

Alexey Merzlyakov April 4, 2014, 10:44 a.m. UTC
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,
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.

Comments

Alexey Merzlyakov April 9, 2014, 8:07 a.m. UTC | #1
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,
> 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"
>  "    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"
>  "    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.

Best regards,
Merzlyakov Alexey
Ramana Radhakrishnan April 9, 2014, 11:12 a.m. UTC | #2
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
>
diff mbox

Patch

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"
  "	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"
  "	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