diff mbox

tile: Fix BZ #18508 (makecontext yield infinite backtrace)

Message ID 1437495189-3076-1-git-send-email-cmetcalf@ezchip.com
State New
Headers show

Commit Message

Chris Metcalf July 21, 2015, 4:13 p.m. UTC 
It turns out tile suffered from the same problem as S390.  However,
disabling CFI information for the __startcontext on tile was not
sufficient to fix the problem; I think the backtracer will just
blindly try to follow the link register (lr) in that case.

Instead, the change adds a cfi_undefined directive for "lr"
and then arranges to call __startcontext directly when the new
context starts, rather than just synthesizing a return to it.
In addition to being a bit easier now to understand the control
flow, this also allows the cfi_undefined directive to be placed in
a way that causes it to be in force at the address that the "lr"
from the called function points to.
---
 ChangeLog                                  | 7 +++++++
 sysdeps/unix/sysv/linux/tile/makecontext.c | 8 ++++----
 sysdeps/unix/sysv/linux/tile/setcontext.S  | 2 ++
 3 files changed, 13 insertions(+), 4 deletions(-)

Comments

Chris Metcalf July 21, 2015, 6:13 p.m. UTC | #1 
Sorry, forgot to mark this patch as [COMMITTED].

This is the last change I have for the 2.22 release.

On 07/21/2015 12:13 PM, Chris Metcalf wrote:
> It turns out tile suffered from the same problem as S390.  However,
> disabling CFI information for the __startcontext on tile was not
> sufficient to fix the problem; I think the backtracer will just
> blindly try to follow the link register (lr) in that case.
>
> Instead, the change adds a cfi_undefined directive for "lr"
> and then arranges to call __startcontext directly when the new
> context starts, rather than just synthesizing a return to it.
> In addition to being a bit easier now to understand the control
> flow, this also allows the cfi_undefined directive to be placed in
> a way that causes it to be in force at the address that the "lr"
> from the called function points to.
> ---
>   ChangeLog                                  | 7 +++++++
>   sysdeps/unix/sysv/linux/tile/makecontext.c | 8 ++++----
>   sysdeps/unix/sysv/linux/tile/setcontext.S  | 2 ++
>   3 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 490c0acf962f..d855f769e19e 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,10 @@
> +2015-07-21  Chris Metcalf  <cmetcalf@ezchip.com>
> +
> +	* sysdeps/unix/sysv/linux/tile/makecontext.c (__makecontext): Call
> +	__startcontext to initialize the new context.
> +	* sysdeps/unix/sysv/linux/tile/setcontext.S (__startcontext): Set
> +	up CFI directive to forbid further backtracing.
> +
>   2015-07-21  Marko Myllynen  <myllynen@redhat.com>
>   
>   	* charmaps/ANSI_X3.110-1983: Remove obsolete repertoire map
> diff --git a/sysdeps/unix/sysv/linux/tile/makecontext.c b/sysdeps/unix/sysv/linux/tile/makecontext.c
> index b14b8d5cc91f..c77d00567296 100644
> --- a/sysdeps/unix/sysv/linux/tile/makecontext.c
> +++ b/sysdeps/unix/sysv/linux/tile/makecontext.c
> @@ -52,14 +52,14 @@ __makecontext (ucontext_t *ucp, void (*func) (void), int argc, ...)
>       }
>     va_end (ap);
>   
> -  /* Pass (*func) to __startcontext in pc.  */
> -  ucp->uc_mcontext.pc = (long) func;
> +  /* Start in the trampoline.  */
> +  ucp->uc_mcontext.pc = (long) __startcontext;
>   
>     /* Set stack pointer.  */
>     ucp->uc_mcontext.sp = (long) sp;
>   
> -  /* Set the return address to trampoline.  */
> -  ucp->uc_mcontext.lr = (long) __startcontext;
> +  /* Pass FUNC to __startcontext in r31.  */
> +  ucp->uc_mcontext.gregs[31] = (long) func;
>   
>     /* Pass ucp->uc_link to __startcontext in r30.  */
>     ucp->uc_mcontext.gregs[30] = (long) ucp->uc_link;
> diff --git a/sysdeps/unix/sysv/linux/tile/setcontext.S b/sysdeps/unix/sysv/linux/tile/setcontext.S
> index ee9edbefe1bb..02d6a1578c75 100644
> --- a/sysdeps/unix/sysv/linux/tile/setcontext.S
> +++ b/sysdeps/unix/sysv/linux/tile/setcontext.S
> @@ -190,7 +190,9 @@ END (__setcontext)
>   weak_alias (__setcontext, setcontext)
>   
>   ENTRY (__startcontext)
> +	cfi_undefined (lr)
>   	FEEDBACK_ENTER(__startcontext)
> +	jalr r31
>   	BEQZ r30, 1f
>   	{
>   	 move r0, r30
diff mbox

Patch

diff --git a/ChangeLog b/ChangeLog
index 490c0acf962f..d855f769e19e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@ 
+2015-07-21  Chris Metcalf  <cmetcalf@ezchip.com>
+
+	* sysdeps/unix/sysv/linux/tile/makecontext.c (__makecontext): Call
+	__startcontext to initialize the new context.
+	* sysdeps/unix/sysv/linux/tile/setcontext.S (__startcontext): Set
+	up CFI directive to forbid further backtracing.
+
 2015-07-21  Marko Myllynen  <myllynen@redhat.com>
 
 	* charmaps/ANSI_X3.110-1983: Remove obsolete repertoire map
diff --git a/sysdeps/unix/sysv/linux/tile/makecontext.c b/sysdeps/unix/sysv/linux/tile/makecontext.c
index b14b8d5cc91f..c77d00567296 100644
--- a/sysdeps/unix/sysv/linux/tile/makecontext.c
+++ b/sysdeps/unix/sysv/linux/tile/makecontext.c
@@ -52,14 +52,14 @@  __makecontext (ucontext_t *ucp, void (*func) (void), int argc, ...)
     }
   va_end (ap);
 
-  /* Pass (*func) to __startcontext in pc.  */
-  ucp->uc_mcontext.pc = (long) func;
+  /* Start in the trampoline.  */
+  ucp->uc_mcontext.pc = (long) __startcontext;
 
   /* Set stack pointer.  */
   ucp->uc_mcontext.sp = (long) sp;
 
-  /* Set the return address to trampoline.  */
-  ucp->uc_mcontext.lr = (long) __startcontext;
+  /* Pass FUNC to __startcontext in r31.  */
+  ucp->uc_mcontext.gregs[31] = (long) func;
 
   /* Pass ucp->uc_link to __startcontext in r30.  */
   ucp->uc_mcontext.gregs[30] = (long) ucp->uc_link;
diff --git a/sysdeps/unix/sysv/linux/tile/setcontext.S b/sysdeps/unix/sysv/linux/tile/setcontext.S
index ee9edbefe1bb..02d6a1578c75 100644
--- a/sysdeps/unix/sysv/linux/tile/setcontext.S
+++ b/sysdeps/unix/sysv/linux/tile/setcontext.S
@@ -190,7 +190,9 @@  END (__setcontext)
 weak_alias (__setcontext, setcontext)
 
 ENTRY (__startcontext)
+	cfi_undefined (lr)
 	FEEDBACK_ENTER(__startcontext)
+	jalr r31
 	BEQZ r30, 1f
 	{
 	 move r0, r30