[PATCH 0/4] systemtap: Some more powerpc patches

[PATCH 0/4] systemtap: Some more powerpc patches

[Anton Vorontsov]

Hi all,

Here are few more patches for systemtap/powerpc...

- Patches 1-3 are used to fix failing tests as reported by
  testsuite/systemtap.context/num_args (thanks to Jim Keniston for
  the hint);

- Fourth patch just improves backtrace output. Though, backtrace.stp
  test is still failing, I'm currently investigating this.

..I need set up a bit faster machine to run the whole testsuite
(just one test takes ages to complete on a diskless 300 MHz board ;-),
and when I set it up I'll post the results.

Thanks!

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

[PATCH 1/4] powerpc: Fix thinko in _stp_arg()

[Anton Vorontsov]

_stp_arg() has an almost unnoticeable thinko in the argnum handling,
which causes it to always return u_register("r10"):

'else (argnum == 8)' should actually be 'else if (argnum == 8)'.

Though, since we check for 'if (argnum < 1 || argnum > 8)' at the
beginning of _stp_arg(), let's make it just 'else'.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 tapset/powerpc/registers.stp |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tapset/powerpc/registers.stp b/tapset/powerpc/registers.stp
index c8713e5..7f66d36 100644
--- a/tapset/powerpc/registers.stp
+++ b/tapset/powerpc/registers.stp
@@ -146,7 +146,7 @@ function _stp_arg:long (argnum:long, sign_extend:long, truncate:long) {
 		val = u_register("r8")
 	else if (argnum == 7)
 		val = u_register("r9")
-	else (argnum == 8)
+	else
 		val = u_register("r10")
 
 	if (truncate) {
-- 
1.6.3.3

Re: [PATCH 1/4] powerpc: Fix thinko in _stp_arg()

[Ananth N Mavinakayanahalli]

On Fri, Dec 04, 2009 at 02:31:51AM +0300, Anton Vorontsov wrote:
> _stp_arg() has an almost unnoticeable thinko in the argnum handling,
> which causes it to always return u_register("r10"):
> 
> 'else (argnum == 8)' should actually be 'else if (argnum == 8)'.
> 
> Though, since we check for 'if (argnum < 1 || argnum > 8)' at the
> beginning of _stp_arg(), let's make it just 'else'.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

Ugh! What was I thinking?

Thanks for fixing this Anton.

Ananth

[PATCH 3/4] powerpc: Adjust registers.stp for ppc32

[Anton Vorontsov]

registers.stp is missing a few small bits when running on ppc32:

* Divide register offsets by 2;
* There is 'mq' register instead of 'softe';
* long_arg() should sign-extend the result on ppc32.

Suggested-by: Jim Keniston <jkenisto@us.ibm.com>
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 tapset/powerpc/registers.stp |   29 +++++++++++++++++++++--------
 1 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/tapset/powerpc/registers.stp b/tapset/powerpc/registers.stp
index 2403272..4e77c7a 100644
--- a/tapset/powerpc/registers.stp
+++ b/tapset/powerpc/registers.stp
@@ -1,5 +1,13 @@
 /* Dwarfless register access for powerpc */
 
+function is_powerpc32() %{ /* pure */
+#ifdef __powerpc64__
+        THIS->__retvalue = 0;
+#else
+        THIS->__retvalue = 1;
+#endif
+%}
+
 global _reg_offsets, _stp_regs_registered
 
 function _stp_register_regs() {
@@ -43,23 +51,24 @@ function _stp_register_regs() {
 	_reg_offsets["link"] = 288
 	_reg_offsets["xer"] = 296
 	_reg_offsets["ccr"] = 304
-	_reg_offsets["softe"] = 312
+	if (is_powerpc32())
+		_reg_offsets["mq"] = 312
+	else
+		_reg_offsets["softe"] = 312
 	_reg_offsets["trap"] = 320
 	_reg_offsets["dar"] = 328
 	_reg_offsets["dsisr"] = 336
 	_reg_offsets["result"] = 344
 
-	/*
-	 * If we ever need to support 32bit powerpc, we can
-	 * get to the register offsets by using just a
-	 * reg32_offset = _reg_offsets["reg"]/2
-	 * or somesuch
-	 */
 	_stp_regs_registered = 1
 }
 
 function probing_32bit_app() %{ /* pure */
+#ifdef __powerpc64__
         THIS->__retvalue = _stp_probing_32bit_app(CONTEXT->regs);
+#else
+        THIS->__retvalue = 1;
+#endif
 %}
 
 function _stp_get_register_by_offset:long (offset:long) %{ /* pure */
@@ -91,7 +100,11 @@ function _stp_register:long (name:string, sign_extend:long) {
 	}
 	if (!_stp_regs_registered)
 		_stp_register_regs()
+
 	offset = _reg_offsets[name]
+	if (is_powerpc32())
+		offset /= 2
+
 	if (offset == 0 && !(name in _reg_offsets)) {
 		error("Unknown register: " . name)
 		return 0
@@ -170,7 +183,7 @@ function uint_arg:long (argnum:long) {
 }
 
 function long_arg:long (argnum:long) {
-	return _stp_arg(argnum, 1, 0)
+	return _stp_arg(argnum, 1, is_powerpc32())
 }
 
 function ulong_arg:long (argnum:long) {
-- 
1.6.3.3

[PATCH 2/4] powerpc: Fix longlong args handling

[Anton Vorontsov]

When probing 32-bits apps on ppc64 or running native ppc32, int64
parts are stored in the big endian order, e.g. foo(1LL, 2LL, 3LL)'s
args turn into:

arg1: r3 = 0; r4 = 1
arg2: r5 = 0; r6 = 2
arg3: r7 = 0; r8 = 3

For example, to restore arg1 the current longlong_arg() will perform
(r4 << 32) | r3, which is obviously incorrect. And to restore arg2 it
will do (r5 << 32) | r4, which doesn't look right as well.

This patch fixes the issue.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 tapset/powerpc/registers.stp |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tapset/powerpc/registers.stp b/tapset/powerpc/registers.stp
index 7f66d36..2403272 100644
--- a/tapset/powerpc/registers.stp
+++ b/tapset/powerpc/registers.stp
@@ -179,8 +179,8 @@ function ulong_arg:long (argnum:long) {
 
 function longlong_arg:long (argnum:long) {
 	if (probing_32bit_app()) {
-		lowbits = _stp_arg(argnum, 0, 1)
-		highbits = _stp_arg(argnum+1, 0, 1)
+		lowbits = _stp_arg(argnum * 2, 0, 1)
+		highbits = _stp_arg(argnum * 2 - 1, 0, 1)
 		return ((highbits << 32) | lowbits)
 	} else
 		return _stp_arg(argnum, 0, 0)
-- 
1.6.3.3

[PATCH 4/4] powerpc: Improve backtrace output for ppc32

[Anton Vorontsov]

0x%016lx format causes a lot of unnecessary zero-padding on ppc32, so
let's use %p instead.

Before:

--- Exception: c00095c8 at 0xc02cff60 : _edata+0x1f60/0x2000 [kernel]
    LR =0xc02aef58 : per_cpu__runqueues+0x0/0x400 [kernel]
[0x00000000c02cfed0] [0x00000000c00113fc] 0xc00113fc : ret_from_except+0x0/0x14 [kernel] (unreliable)

After:

--- Exception: c00095c8 at 0xc02cff60 : _edata+0x1f60/0x2000 [kernel]
    LR =0xc02aef58 : per_cpu__runqueues+0x0/0x400 [kernel]
[0xc02cfed0] [0xc00113fc] 0xc00113fc : ret_from_except+0x0/0x14 [kernel] (unreliable)

p.s.
Note that the second and the third columns are dups, this is because
_stp_symbol_print(ip) also prints ip. Though, for now I wouldn't touch
this since I'm not sure if anybody depends on the current columns count
or "[]" around addresses.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 runtime/stack-ppc.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/runtime/stack-ppc.c b/runtime/stack-ppc.c
index df2db15..e3b3ede 100644
--- a/runtime/stack-ppc.c
+++ b/runtime/stack-ppc.c
@@ -24,14 +24,15 @@ static void __stp_stack_print (struct pt_regs *regs, int verbose, int levels,
 		ip = _sp[STACK_FRAME_LR_SAVE];
 		if (!firstframe || ip != lr) {
 			if (verbose) {
-				_stp_printf("[0x%016lx] [0x%016lx] ", sp, ip);
+				_stp_printf("[%p] [%p] ",
+					    (int64_t)sp, (int64_t)ip);
 				_stp_symbol_print(ip);
 				if (firstframe)
 					_stp_print(" (unreliable)");
 				_stp_print_char('\n');
 			}
 			else
-				_stp_printf("0x%016lx ", ip);
+				_stp_printf("%p ", (int64_t)ip);
 		}
 		firstframe = 0;
 		/*
@@ -52,8 +53,8 @@ static void __stp_stack_print (struct pt_regs *regs, int verbose, int levels,
 				firstframe = 1;
 			}
 			else {
-				_stp_printf("0x%016lx ",regs->nip);
-				_stp_printf("0x%016lx ",regs->link);
+				_stp_printf("%p ", (int64_t)regs->nip);
+				_stp_printf("%p ", (int64_t)regs->link);
 			}
 		}
 
-- 
1.6.3.3