[PATCH] ARM: unwind: add unwind directives to bitops assembly macros

[PATCH] ARM: unwind: add unwind directives to bitops assembly macros

[Will Deacon]

The bitops functions (e.g. _test_and_set_bit) on ARM do not have unwind
annotations and therefore the kernel cannot backtrace out of them on a
fatal error (for example, NULL pointer dereference).

This patch annotates the bitops assembly macros with UNWIND annotations
so that we can produce a meaningful backtrace on error.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/lib/bitops.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/lib/bitops.h b/arch/arm/lib/bitops.h
index 10d868a..640ce58 100644
--- a/arch/arm/lib/bitops.h
+++ b/arch/arm/lib/bitops.h
@@ -1,5 +1,8 @@
+#include <asm/unwind.h>
+
 #if __LINUX_ARM_ARCH__ >= 6
 	.macro	bitop, instr
+UNWIND(	.fnstart	)
 	ands	ip, r1, #3
 	strneb	r1, [ip]		@ assert word-aligned
 	mov	r2, #1
@@ -13,9 +16,11 @@
 	cmp	r0, #0
 	bne	1b
 	bx	lr
+UNWIND(	.fnend		)
 	.endm
 
 	.macro	testop, instr, store
+UNWIND(	.fnstart	)
 	ands	ip, r1, #3
 	strneb	r1, [ip]		@ assert word-aligned
 	mov	r2, #1
@@ -34,9 +39,11 @@
 	cmp	r0, #0
 	movne	r0, #1
 2:	bx	lr
+UNWIND(	.fnend		)
 	.endm
 #else
 	.macro	bitop, instr
+UNWIND(	.fnstart	)
 	ands	ip, r1, #3
 	strneb	r1, [ip]		@ assert word-aligned
 	and	r2, r0, #31
@@ -49,6 +56,7 @@
 	str	r2, [r1, r0, lsl #2]
 	restore_irqs ip
 	mov	pc, lr
+UNWIND(	.fnend		)
 	.endm
 
 /**
@@ -60,6 +68,7 @@
  * to avoid dirtying the data cache.
  */
 	.macro	testop, instr, store
+UNWIND(	.fnstart	)
 	ands	ip, r1, #3
 	strneb	r1, [ip]		@ assert word-aligned
 	and	r3, r0, #31
@@ -73,5 +82,6 @@
 	moveq	r0, #0
 	restore_irqs ip
 	mov	pc, lr
+UNWIND(	.fnend		)
 	.endm
 #endif
-- 
1.7.4.1

[PATCH] ARM: unwind: add unwind directives to bitops assembly macros

[Dave Martin]

On Thu, Nov 17, 2011 at 06:04:56PM +0000, Will Deacon wrote:
> The bitops functions (e.g. _test_and_set_bit) on ARM do not have unwind
> annotations and therefore the kernel cannot backtrace out of them on a
> fatal error (for example, NULL pointer dereference).
> 
> This patch annotates the bitops assembly macros with UNWIND annotations
> so that we can produce a meaningful backtrace on error.

I take it these macros aren't likely ever to get used except in the
definition of complete functions (as in testsetbit.S etc.)?

One way to make the correct behaviour explicit would be to put the
function boilerplate into the macros instead:

#include <linux/linkage.h>
#include <asm/unwind.h>

	.macro bitop name, instr
ENTRY(\name )
UNWIND(	.fnstart	)
	@ stuff
	@ bx lr / mov pc,lr
UNWIND(	.fnend		)
ENDPROC(\name )
	.endm

That makes it harder to misuse the macros or to typo the ENDPROC()
directive, as well as allowing a few lines to be eliminated from the
other .S files.

Cheers
---Dave

> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/lib/bitops.h |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/lib/bitops.h b/arch/arm/lib/bitops.h
> index 10d868a..640ce58 100644
> --- a/arch/arm/lib/bitops.h
> +++ b/arch/arm/lib/bitops.h
> @@ -1,5 +1,8 @@
> +#include <asm/unwind.h>
> +
>  #if __LINUX_ARM_ARCH__ >= 6
>  	.macro	bitop, instr
> +UNWIND(	.fnstart	)
>  	ands	ip, r1, #3
>  	strneb	r1, [ip]		@ assert word-aligned
>  	mov	r2, #1
> @@ -13,9 +16,11 @@
>  	cmp	r0, #0
>  	bne	1b
>  	bx	lr
> +UNWIND(	.fnend		)
>  	.endm
>  
>  	.macro	testop, instr, store
> +UNWIND(	.fnstart	)
>  	ands	ip, r1, #3
>  	strneb	r1, [ip]		@ assert word-aligned
>  	mov	r2, #1
> @@ -34,9 +39,11 @@
>  	cmp	r0, #0
>  	movne	r0, #1
>  2:	bx	lr
> +UNWIND(	.fnend		)
>  	.endm
>  #else
>  	.macro	bitop, instr
> +UNWIND(	.fnstart	)
>  	ands	ip, r1, #3
>  	strneb	r1, [ip]		@ assert word-aligned
>  	and	r2, r0, #31
> @@ -49,6 +56,7 @@
>  	str	r2, [r1, r0, lsl #2]
>  	restore_irqs ip
>  	mov	pc, lr
> +UNWIND(	.fnend		)
>  	.endm
>  
>  /**
> @@ -60,6 +68,7 @@
>   * to avoid dirtying the data cache.
>   */
>  	.macro	testop, instr, store
> +UNWIND(	.fnstart	)
>  	ands	ip, r1, #3
>  	strneb	r1, [ip]		@ assert word-aligned
>  	and	r3, r0, #31
> @@ -73,5 +82,6 @@
>  	moveq	r0, #0
>  	restore_irqs ip
>  	mov	pc, lr
> +UNWIND(	.fnend		)
>  	.endm
>  #endif
> -- 
> 1.7.4.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] ARM: unwind: add unwind directives to bitops assembly macros

[Will Deacon]

On Thu, Nov 17, 2011 at 06:48:01PM +0000, Dave Martin wrote:
> On Thu, Nov 17, 2011 at 06:04:56PM +0000, Will Deacon wrote:
> > The bitops functions (e.g. _test_and_set_bit) on ARM do not have unwind
> > annotations and therefore the kernel cannot backtrace out of them on a
> > fatal error (for example, NULL pointer dereference).
> > 
> > This patch annotates the bitops assembly macros with UNWIND annotations
> > so that we can produce a meaningful backtrace on error.
> 
> I take it these macros aren't likely ever to get used except in the
> definition of complete functions (as in testsetbit.S etc.)?

Currently they're only used in that way and I don't see why that would
change.

> One way to make the correct behaviour explicit would be to put the
> function boilerplate into the macros instead:
> 
> #include <linux/linkage.h>
> #include <asm/unwind.h>
> 
> 	.macro bitop name, instr
> ENTRY(\name )
> UNWIND(	.fnstart	)
> 	@ stuff
> 	@ bx lr / mov pc,lr
> UNWIND(	.fnend		)
> ENDPROC(\name )
> 	.endm
> 
> That makes it harder to misuse the macros or to typo the ENDPROC()
> directive, as well as allowing a few lines to be eliminated from the
> other .S files.

That's not a bad idea, I'll re-roll the patch next week.

Will