Patchwork mips: terminate the FDE before the return trampoline in makecontext

login
register
mail settings
Submitter Aurelien Jarno
Date March 9, 2016, 7:53 a.m.
Message ID <1457509993-2393-1-git-send-email-aurelien@aurel32.net>
Download mbox | patch
Permalink /patch/11285/
State New
Headers show

Comments

Aurelien Jarno - March 9, 2016, 7:53 a.m.
In makecontext the FDE needs to be terminated before the return
trampoline otherwise backtrace called within a context created by
makecontext yields infinite backtrace.

This bug has been present for a long time, stdlib/tst-makecontext did
not fail until recent commit e535ce25. Tested on mips-linux-gnu and
mips64el-linux-gnuabi64 and mips-linux-gnu, no regression.

This fixes stdlib/tst-makecontext on MIPS.

Changelog:
	[BZ #19792]
	* sysdeps/unix/sysv/linux/mips/makecontext.S (__makecontext):
	Terminate FDE before return label.
---
 ChangeLog                                  | 6 ++++++
 sysdeps/unix/sysv/linux/mips/makecontext.S | 7 +++++++
 2 files changed, 13 insertions(+)
Andreas Schwab - March 9, 2016, 8:18 a.m.
Aurelien Jarno <aurelien@aurel32.net> writes:

> 	[BZ #19792]
> 	* sysdeps/unix/sysv/linux/mips/makecontext.S (__makecontext):
> 	Terminate FDE before return label.

Ok.

Andreas.
Maciej W. Rozycki - March 22, 2016, 9:01 p.m.
On Wed, 9 Mar 2016, Aurelien Jarno wrote:

> diff --git a/sysdeps/unix/sysv/linux/mips/makecontext.S b/sysdeps/unix/sysv/linux/mips/makecontext.S
> index 66600c7..3196554 100644
> --- a/sysdeps/unix/sysv/linux/mips/makecontext.S
> +++ b/sysdeps/unix/sysv/linux/mips/makecontext.S
> @@ -153,6 +153,11 @@ NESTED (__makecontext, FRAMESZ, ra)
>  #endif
>  	jr	ra
>  
> +	/* We need to terminate the FDE to stop unwinding if backtrace was
> +	   called within a context created by makecontext.  */
> +	cfi_endproc
> +	nop
> +
>  99:

 What's this NOP needed for here?

  Maciej
Aurelien Jarno - March 23, 2016, 6:41 a.m.
On 2016-03-22 21:01, Maciej W. Rozycki wrote:
> On Wed, 9 Mar 2016, Aurelien Jarno wrote:
> 
> > diff --git a/sysdeps/unix/sysv/linux/mips/makecontext.S b/sysdeps/unix/sysv/linux/mips/makecontext.S
> > index 66600c7..3196554 100644
> > --- a/sysdeps/unix/sysv/linux/mips/makecontext.S
> > +++ b/sysdeps/unix/sysv/linux/mips/makecontext.S
> > @@ -153,6 +153,11 @@ NESTED (__makecontext, FRAMESZ, ra)
> >  #endif
> >  	jr	ra
> >  
> > +	/* We need to terminate the FDE to stop unwinding if backtrace was
> > +	   called within a context created by makecontext.  */
> > +	cfi_endproc
> > +	nop
> > +
> >  99:
> 
>  What's this NOP needed for here?

We have to separate these to blocks because the unwinder uses ra-1 to
find the FDE.
Maciej W. Rozycki - March 29, 2016, 1:09 p.m.
On Wed, 23 Mar 2016, Aurelien Jarno wrote:

> >  What's this NOP needed for here?
> 
> We have to separate these to blocks because the unwinder uses ra-1 to
> find the FDE.

 Ah indeed, I remember now, thanks!

 For the record: I found that bloody piece when trying to make GDB 
interpret DWARF records correctly for compressed code.  The choice to use 
PC-1 in the unwinder arbitrarily across all architectures broke DWARF 
record interpretation in the MIPS port of GDB as the calculation flips the 
ISA bit.  Since it's been like that from the beginning and consequently 
encoded across binaries out there handling GDB had to be adjusted to 
painfully recover the ISA bit from the symbol table as DWARF records are 
interpreted (while the DWARF records should have sufficed by themselves).

 If I had designed this part of the unwinder, I'd have made the adjustment 
target-specific and specifically PC-2 for the MIPS target, i.e. the 
minimum instruction size in the ISA (a rule which should actually work 
universally I believe).

 FWIW,

  Maciej

Patch

diff --git a/ChangeLog b/ChangeLog
index 73a49a0..4a2647d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@ 
+2016-03-09  Aurelien Jarno  <aurelien@aurel32.net>
+
+	[BZ #19792]
+	* sysdeps/unix/sysv/linux/mips/makecontext.S (__makecontext):
+	Terminate FDE before return label.
+
 2016-03-08  Roland McGrath  <roland@hack.frob.com>
 
 	* sysdeps/x86_64/tst-audit10.c: #include <cpu-features.h>.
diff --git a/sysdeps/unix/sysv/linux/mips/makecontext.S b/sysdeps/unix/sysv/linux/mips/makecontext.S
index 66600c7..3196554 100644
--- a/sysdeps/unix/sysv/linux/mips/makecontext.S
+++ b/sysdeps/unix/sysv/linux/mips/makecontext.S
@@ -153,6 +153,11 @@  NESTED (__makecontext, FRAMESZ, ra)
 #endif
 	jr	ra
 
+	/* We need to terminate the FDE to stop unwinding if backtrace was
+	   called within a context created by makecontext.  */
+	cfi_endproc
+	nop
+
 99:
 #ifdef __PIC__
 	move	gp, s1
@@ -186,6 +191,8 @@  NESTED (__makecontext, FRAMESZ, ra)
 1:
 	lb	zero, (zero)
 	b	1b
+
+	cfi_startproc
 PSEUDO_END (__makecontext)
 
 weak_alias (__makecontext, makecontext)