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

Message ID 20180525080354.13295-14-christophe.lyon@st.com
State New
Headers show
Series
  • FDPIC ARM for ARM
Related show

Commit Message

Christophe Lyon May 25, 2018, 8:03 a.m.
2018-XX-XX  Christophe Lyon  <christophe.lyon@st.com>
	Mickaël Guêné <mickael.guene@st.com>

	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

-- 
2.6.3

Comments

Kyrill Tkachov June 8, 2018, 11:01 a.m. | #1
Hi Christophe,

Similar comments to patch 11/21

On 25/05/18 09:03, Christophe Lyon wrote:
> 2018-XX-XX  Christophe Lyon <christophe.lyon@st.com>

>         Mickaël Guêné <mickael.guene@st.com>

>

>         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_UCONTEXT    0x80

> @@ -228,7 +231,7 @@ __gnu_personality_sigframe_fdpic (_Unwind_State state,

>      _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP, _UVRSD_UINT32, &sp);

>      _Unwind_VRS_Get (context, _UVRSC_CORE, R_PC, _UVRSD_UINT32, &pc);

>

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

>
Christophe Lyon June 11, 2018, 6:56 a.m. | #2
On 8 June 2018 at 13:01, Kyrill  Tkachov <kyrylo.tkachov@foss.arm.com> wrote:
> Hi Christophe,

>

> Similar comments to patch 11/21

>

>

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

>>

>> 2018-XX-XX  Christophe Lyon <christophe.lyon@st.com>

>>         Mickaël Guêné <mickael.guene@st.com>

>>

>>         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_UCONTEXT    0x80

>> @@ -228,7 +231,7 @@ __gnu_personality_sigframe_fdpic (_Unwind_State state,

>>      _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP, _UVRSD_UINT32, &sp);

>>      _Unwind_VRS_Get (context, _UVRSC_CORE, R_PC, _UVRSD_UINT32, &pc);

>>

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

>> +             && 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

>>

>

Patch

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_UCONTEXT	0x80
@@ -228,7 +231,7 @@  __gnu_personality_sigframe_fdpic (_Unwind_State state,
     _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP, _UVRSD_UINT32, &sp);
     _Unwind_VRS_Get (context, _UVRSC_CORE, R_PC, _UVRSD_UINT32, &pc);
 
-    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;