Re: [ARM/FDPIC 13/21] [ARM] FDPIC: Support unwinding across thumb2 signal trampoline

2018-06-11 Thread Christophe Lyon
On 8 June 2018 at 13:01, Kyrill  Tkachov  wrote:
> Hi Christophe,
>
> Similar comments to patch 11/21
>
>
> On 25/05/18 09:03, Christophe Lyon wrote:
>>
>> 2018-XX-XX  Christophe Lyon 
>> Mickaël Guêné 
>>
>> libgcc/
>> * unwind-arm-common.inc (FDPIC_T2_LDR_R12_WITH_FUNCDESC)
>> (FDPIC_T2_LDR_R9_WITH_GOT, FDPIC_T2_LDR_PC_WITH_RESTORER): New.
>> (__gnu_personality_sigframe_fdpic): Support Thumb address.
>> (get_eit_entry): Support Thumb code.
>>
>> Change-Id: I2bb8994e733e48a89c6f4e0682921186c086f8bc
>>
>> diff --git a/libgcc/unwind-arm-common.inc b/libgcc/unwind-arm-common.inc
>> index 80d1e88..7de4033 100644
>> --- a/libgcc/unwind-arm-common.inc
>> +++ b/libgcc/unwind-arm-common.inc
>> @@ -38,6 +38,9 @@
>>  #define FDPIC_LDR_R12_WITH_FUNCDESC 0xe59fc004
>>  #define FDPIC_LDR_R9_WITH_GOT   0xe59c9004
>>  #define FDPIC_LDR_PC_WITH_RESTORER  0xe59cf000
>> +#define FDPIC_T2_LDR_R12_WITH_FUNCDESC  0xc008f8df
>> +#define FDPIC_T2_LDR_R9_WITH_GOT   0x9004f8dc
>> +#define FDPIC_T2_LDR_PC_WITH_RESTORER   0xf000f8dc
>>  #define FDPIC_FUNCDESC_OFFSET   12
>>  /* Signal frame offsets.  */
>>  #define ARM_NEW_RT_SIGFRAME_UCONTEXT0x80
>> @@ -228,7 +231,7 @@ __gnu_personality_sigframe_fdpic (_Unwind_State state,
>>  _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP, _UVRSD_UINT32, );
>>  _Unwind_VRS_Get (context, _UVRSC_CORE, R_PC, _UVRSD_UINT32, );
>>
>> -funcdesc = *(unsigned int *)(pc + FDPIC_FUNCDESC_OFFSET);
>> +funcdesc = *(unsigned int *)((pc & ~1) + FDPIC_FUNCDESC_OFFSET);
>>  handler = *(unsigned int *)(funcdesc);
>>  first_handler_instruction = *(unsigned int *)(handler & ~1);
>>
>> @@ -277,10 +280,13 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw
>> return_address)
>>/* If we are unwinding a signal handler then perhaps we have
>>   reached a trampoline.  Try to detect jump to restorer
>>   sequence.  */
>> - _uw *pc = (_uw *)((return_address+2) & ~3);
>> - if (pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
>> - && pc[1] == FDPIC_LDR_R9_WITH_GOT
>> - && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
>> + _uw *pc = (_uw *)((return_address+2) & ~1);
>> + if ((pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
>> +  && pc[1] == FDPIC_LDR_R9_WITH_GOT
>> +  && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
>> + || (pc[0] == FDPIC_T2_LDR_R12_WITH_FUNCDESC
>> + && pc[1] == FDPIC_T2_LDR_R9_WITH_GOT
>> + && pc[2] == FDPIC_T2_LDR_PC_WITH_RESTORER))
>>  {
>
>
> This largely overwrites and extends code added in patch 11/21. Can't we just
> merge the two
> patches into a final sane one?
>

Sure. It was developed in two steps, and I thought it was easier to
review in small pieces, but I can merge them.

> Thanks,
> Kyrill
>
>
>>struct funcdesc_t *funcdesc = (struct funcdesc_t *)
>>  &__gnu_personality_sigframe_fdpic;
>> @@ -309,13 +315,16 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw
>> return_address)
>>/* If we are unwinding a signal handler then perhaps we have
>>   reached a trampoline.  Try to detect jump to restorer
>>   sequence.  */
>> -  _uw *pc = (_uw *)((return_address+2) & ~3);
>> -  if (pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
>> - && pc[1] == FDPIC_LDR_R9_WITH_GOT
>> - && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
>> +  _uw *pc = (_uw *)((return_address+2) & ~1);
>> +  if ((pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
>> +  && pc[1] == FDPIC_LDR_R9_WITH_GOT
>> +  && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
>> + || (pc[0] == FDPIC_T2_LDR_R12_WITH_FUNCDESC
>> + && pc[1] == FDPIC_T2_LDR_R9_WITH_GOT
>> + && pc[2] == FDPIC_T2_LDR_PC_WITH_RESTORER))
>>  {
>> - struct funcdesc_t *funcdesc = (struct funcdesc_t *)
>> -   &__gnu_personality_sigframe_fdpic;
>> + struct funcdesc_t *funcdesc
>> +   = (struct funcdesc_t *) &__gnu_personality_sigframe_fdpic;
>>
>>UCB_PR_ADDR (ucbp) = funcdesc->ptr;
>>UCB_PR_GOT (ucbp) = funcdesc->got;
>> @@ -335,13 +344,16 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw
>> return_address)
>>/* If we are unwinding a signal handler then perhaps we have
>>   reached a trampoline.  Try to detect jump to restorer
>>   sequence.  */
>> -  _uw *pc = (_uw *)((return_address+2) & ~3);
>> -  if (pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
>> - && pc[1] == FDPIC_LDR_R9_WITH_GOT
>> - && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
>> +  _uw *pc = (_uw *)((return_address+2) & ~1);
>> +  if ((pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
>> +  && pc[1] == FDPIC_LDR_R9_WITH_GOT
>> +  && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
>> + || (pc[0] == FDPIC_T2_LDR_R12_WITH_FUNCDESC
>> + && pc[1] == FDPIC_T2_LDR_R9_WITH_GOT
>> + 

Re: [ARM/FDPIC 13/21] [ARM] FDPIC: Support unwinding across thumb2 signal trampoline

2018-06-08 Thread Kyrill Tkachov

Hi Christophe,

Similar comments to patch 11/21

On 25/05/18 09:03, Christophe Lyon wrote:

2018-XX-XX  Christophe Lyon 
Mickaël Guêné 

libgcc/
* unwind-arm-common.inc (FDPIC_T2_LDR_R12_WITH_FUNCDESC)
(FDPIC_T2_LDR_R9_WITH_GOT, FDPIC_T2_LDR_PC_WITH_RESTORER): New.
(__gnu_personality_sigframe_fdpic): Support Thumb address.
(get_eit_entry): Support Thumb code.

Change-Id: I2bb8994e733e48a89c6f4e0682921186c086f8bc

diff --git a/libgcc/unwind-arm-common.inc b/libgcc/unwind-arm-common.inc
index 80d1e88..7de4033 100644
--- a/libgcc/unwind-arm-common.inc
+++ b/libgcc/unwind-arm-common.inc
@@ -38,6 +38,9 @@
 #define FDPIC_LDR_R12_WITH_FUNCDESC 0xe59fc004
 #define FDPIC_LDR_R9_WITH_GOT   0xe59c9004
 #define FDPIC_LDR_PC_WITH_RESTORER  0xe59cf000
+#define FDPIC_T2_LDR_R12_WITH_FUNCDESC  0xc008f8df
+#define FDPIC_T2_LDR_R9_WITH_GOT   0x9004f8dc
+#define FDPIC_T2_LDR_PC_WITH_RESTORER   0xf000f8dc
 #define FDPIC_FUNCDESC_OFFSET   12
 /* Signal frame offsets.  */
 #define ARM_NEW_RT_SIGFRAME_UCONTEXT0x80
@@ -228,7 +231,7 @@ __gnu_personality_sigframe_fdpic (_Unwind_State state,
 _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP, _UVRSD_UINT32, );
 _Unwind_VRS_Get (context, _UVRSC_CORE, R_PC, _UVRSD_UINT32, );

-funcdesc = *(unsigned int *)(pc + FDPIC_FUNCDESC_OFFSET);
+funcdesc = *(unsigned int *)((pc & ~1) + FDPIC_FUNCDESC_OFFSET);
 handler = *(unsigned int *)(funcdesc);
 first_handler_instruction = *(unsigned int *)(handler & ~1);

@@ -277,10 +280,13 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw 
return_address)
   /* If we are unwinding a signal handler then perhaps we have
  reached a trampoline.  Try to detect jump to restorer
  sequence.  */
- _uw *pc = (_uw *)((return_address+2) & ~3);
- if (pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
- && pc[1] == FDPIC_LDR_R9_WITH_GOT
- && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+ _uw *pc = (_uw *)((return_address+2) & ~1);
+ if ((pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
+  && pc[1] == FDPIC_LDR_R9_WITH_GOT
+  && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+ || (pc[0] == FDPIC_T2_LDR_R12_WITH_FUNCDESC
+ && pc[1] == FDPIC_T2_LDR_R9_WITH_GOT
+ && pc[2] == FDPIC_T2_LDR_PC_WITH_RESTORER))
 {


This largely overwrites and extends code added in patch 11/21. Can't we just 
merge the two
patches into a final sane one?

Thanks,
Kyrill


   struct funcdesc_t *funcdesc = (struct funcdesc_t *)
 &__gnu_personality_sigframe_fdpic;
@@ -309,13 +315,16 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw 
return_address)
   /* If we are unwinding a signal handler then perhaps we have
  reached a trampoline.  Try to detect jump to restorer
  sequence.  */
-  _uw *pc = (_uw *)((return_address+2) & ~3);
-  if (pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
- && pc[1] == FDPIC_LDR_R9_WITH_GOT
- && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+  _uw *pc = (_uw *)((return_address+2) & ~1);
+  if ((pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
+  && pc[1] == FDPIC_LDR_R9_WITH_GOT
+  && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+ || (pc[0] == FDPIC_T2_LDR_R12_WITH_FUNCDESC
+ && pc[1] == FDPIC_T2_LDR_R9_WITH_GOT
+ && pc[2] == FDPIC_T2_LDR_PC_WITH_RESTORER))
 {
- struct funcdesc_t *funcdesc = (struct funcdesc_t *)
-   &__gnu_personality_sigframe_fdpic;
+ struct funcdesc_t *funcdesc
+   = (struct funcdesc_t *) &__gnu_personality_sigframe_fdpic;

   UCB_PR_ADDR (ucbp) = funcdesc->ptr;
   UCB_PR_GOT (ucbp) = funcdesc->got;
@@ -335,13 +344,16 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw 
return_address)
   /* If we are unwinding a signal handler then perhaps we have
  reached a trampoline.  Try to detect jump to restorer
  sequence.  */
-  _uw *pc = (_uw *)((return_address+2) & ~3);
-  if (pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
- && pc[1] == FDPIC_LDR_R9_WITH_GOT
- && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+  _uw *pc = (_uw *)((return_address+2) & ~1);
+  if ((pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
+  && pc[1] == FDPIC_LDR_R9_WITH_GOT
+  && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+ || (pc[0] == FDPIC_T2_LDR_R12_WITH_FUNCDESC
+ && pc[1] == FDPIC_T2_LDR_R9_WITH_GOT
+ && pc[2] == FDPIC_T2_LDR_PC_WITH_RESTORER))
 {
- struct funcdesc_t *funcdesc = (struct funcdesc_t *)
-   &__gnu_personality_sigframe_fdpic;
+ struct funcdesc_t *funcdesc
+   = (struct funcdesc_t *) &__gnu_personality_sigframe_fdpic;

   UCB_PR_ADDR (ucbp) = funcdesc->ptr;
   UCB_PR_GOT (ucbp) = funcdesc->got;
--
2.6.3





[ARM/FDPIC 13/21] [ARM] FDPIC: Support unwinding across thumb2 signal trampoline

2018-05-25 Thread Christophe Lyon
2018-XX-XX  Christophe Lyon  
Mickaël Guêné 

libgcc/
* unwind-arm-common.inc (FDPIC_T2_LDR_R12_WITH_FUNCDESC)
(FDPIC_T2_LDR_R9_WITH_GOT, FDPIC_T2_LDR_PC_WITH_RESTORER): New.
(__gnu_personality_sigframe_fdpic): Support Thumb address.
(get_eit_entry): Support Thumb code.

Change-Id: I2bb8994e733e48a89c6f4e0682921186c086f8bc

diff --git a/libgcc/unwind-arm-common.inc b/libgcc/unwind-arm-common.inc
index 80d1e88..7de4033 100644
--- a/libgcc/unwind-arm-common.inc
+++ b/libgcc/unwind-arm-common.inc
@@ -38,6 +38,9 @@
 #define FDPIC_LDR_R12_WITH_FUNCDESC0xe59fc004
 #define FDPIC_LDR_R9_WITH_GOT  0xe59c9004
 #define FDPIC_LDR_PC_WITH_RESTORER 0xe59cf000
+#define FDPIC_T2_LDR_R12_WITH_FUNCDESC  0xc008f8df
+#define FDPIC_T2_LDR_R9_WITH_GOT   0x9004f8dc
+#define FDPIC_T2_LDR_PC_WITH_RESTORER   0xf000f8dc
 #define FDPIC_FUNCDESC_OFFSET  12
 /* Signal frame offsets.  */
 #define ARM_NEW_RT_SIGFRAME_UCONTEXT   0x80
@@ -228,7 +231,7 @@ __gnu_personality_sigframe_fdpic (_Unwind_State state,
 _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP, _UVRSD_UINT32, );
 _Unwind_VRS_Get (context, _UVRSC_CORE, R_PC, _UVRSD_UINT32, );
 
-funcdesc = *(unsigned int *)(pc + FDPIC_FUNCDESC_OFFSET);
+funcdesc = *(unsigned int *)((pc & ~1) + FDPIC_FUNCDESC_OFFSET);
 handler = *(unsigned int *)(funcdesc);
 first_handler_instruction = *(unsigned int *)(handler & ~1);
 
@@ -277,10 +280,13 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw 
return_address)
  /* If we are unwinding a signal handler then perhaps we have
 reached a trampoline.  Try to detect jump to restorer
 sequence.  */
- _uw *pc = (_uw *)((return_address+2) & ~3);
- if (pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
- && pc[1] == FDPIC_LDR_R9_WITH_GOT
- && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+ _uw *pc = (_uw *)((return_address+2) & ~1);
+ if ((pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
+  && pc[1] == FDPIC_LDR_R9_WITH_GOT
+  && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+ || (pc[0] == FDPIC_T2_LDR_R12_WITH_FUNCDESC
+ && pc[1] == FDPIC_T2_LDR_R9_WITH_GOT
+ && pc[2] == FDPIC_T2_LDR_PC_WITH_RESTORER))
{
  struct funcdesc_t *funcdesc = (struct funcdesc_t *)
&__gnu_personality_sigframe_fdpic;
@@ -309,13 +315,16 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw 
return_address)
   /* If we are unwinding a signal handler then perhaps we have
 reached a trampoline.  Try to detect jump to restorer
 sequence.  */
-  _uw *pc = (_uw *)((return_address+2) & ~3);
-  if (pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
- && pc[1] == FDPIC_LDR_R9_WITH_GOT
- && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+  _uw *pc = (_uw *)((return_address+2) & ~1);
+  if ((pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
+  && pc[1] == FDPIC_LDR_R9_WITH_GOT
+  && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+ || (pc[0] == FDPIC_T2_LDR_R12_WITH_FUNCDESC
+ && pc[1] == FDPIC_T2_LDR_R9_WITH_GOT
+ && pc[2] == FDPIC_T2_LDR_PC_WITH_RESTORER))
{
- struct funcdesc_t *funcdesc = (struct funcdesc_t *)
-   &__gnu_personality_sigframe_fdpic;
+ struct funcdesc_t *funcdesc
+   = (struct funcdesc_t *) &__gnu_personality_sigframe_fdpic;
 
  UCB_PR_ADDR (ucbp) = funcdesc->ptr;
  UCB_PR_GOT (ucbp) = funcdesc->got;
@@ -335,13 +344,16 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw 
return_address)
   /* If we are unwinding a signal handler then perhaps we have
 reached a trampoline.  Try to detect jump to restorer
 sequence.  */
-  _uw *pc = (_uw *)((return_address+2) & ~3);
-  if (pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
- && pc[1] == FDPIC_LDR_R9_WITH_GOT
- && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+  _uw *pc = (_uw *)((return_address+2) & ~1);
+  if ((pc[0] == FDPIC_LDR_R12_WITH_FUNCDESC
+  && pc[1] == FDPIC_LDR_R9_WITH_GOT
+  && pc[2] == FDPIC_LDR_PC_WITH_RESTORER)
+ || (pc[0] == FDPIC_T2_LDR_R12_WITH_FUNCDESC
+ && pc[1] == FDPIC_T2_LDR_R9_WITH_GOT
+ && pc[2] == FDPIC_T2_LDR_PC_WITH_RESTORER))
{
- struct funcdesc_t *funcdesc = (struct funcdesc_t *)
-   &__gnu_personality_sigframe_fdpic;
+ struct funcdesc_t *funcdesc
+   = (struct funcdesc_t *) &__gnu_personality_sigframe_fdpic;
 
  UCB_PR_ADDR (ucbp) = funcdesc->ptr;
  UCB_PR_GOT (ucbp) = funcdesc->got;
-- 
2.6.3