Long/address printing in non-native personalities

[Eugene S. <ev...@gm...>]

Hello.

It's been brought to my attention that one of strace tests (nsyscalls) is
failing when x32 ABI is used.

$ cat tests-mx32/nsyscalls.log
1c1
< syscall_546(0xbadc0ded, 0xbadc1ded, 0xbadc2ded, 0xbadc3ded, 0xbadc4ded, 0xbadc5ded) = -1 (errno 38)
---
> syscall_546(0xbadc0ded, 0xbadc1ded, 0xbadc2ded, 0xbadc3ded, 0xbadc4ded, 0xffffffffbadc5ded) = -1 (errno 38)
nsyscalls.test: failed test: ../strace -e trace=none ./nsyscalls output mismatch
FAIL nsyscalls.test (exit status: 1)

Upon investigation, it has been revealed that the problem is in the way strace
handles tracee's long integer variables, which, in turn, has been brought to
light by the quirk in the way argument passing code being generated by GCC.
More specifically, when a 6-parameter syscall is called (as nsyscalls tests
does), 7 parameters should be passed to libc's syscall() wrapper: syscall
number itself and its parameters. As a result, 7th argument should be passed
via stack in accordance with x32 ABI (
https://docs.google.com/viewer?a=v&pid=sites&srcid=ZGVmYXVsdGRvbWFpbnx4MzJhYml8Z3g6MzVkYzQwOWIwOGU4YzViYw
, page 22). Also, in conformance with x32 ABI, all arguments should be extended
to 8 bytes in size. Standard does not explicitly specify the way this should be
performed, and that's where GCC does something strange: it sign-extends
parameters passed via stack while zero-extends parameters passed via registers
(or, more precisely, doesn't care about higher bits at all in latter case, for
obvious reasons):

Dump of assembler code for function main:
0x00400370 <+0>: push %rbx
0x00400371 <+1>: mov $0xbadc4ded,%r9d
0x00400377 <+7>: mov $0xbadc3ded,%r8d
0x0040037d <+13>: mov $0xbadc2ded,%ecx
0x00400382 <+18>: mov $0xbadc1ded,%edx
0x00400387 <+23>: mov $0xbadc0ded,%esi
0x0040038c <+28>: sub $0x8,%esp
0x0040038f <+31>: mov $0x40000222,%edi
0x00400394 <+36>: xor %eax,%eax
=> 0x00400396 <+38>: pushq $0xffffffffbadc5ded
0x0040039b <+43>: callq 0x400350 <syscall@plt>

And this is what leads to test failure — strace does not care about tracee's
size of long when it prints syscall arguments (printargs() in syscall.c simply
calls tprintf("%s%#lx", i ? ", " : "", tcp->u_arg[i])). Moreover, it similarly
does not care about tracee's pointer size in places where it prints them (my
first attempt to fix this issue was by (incorrectly) utilising
printnum_ulong(), but it failed hilariously, since umoven_or_printaddr(), which
is apparently called before actual printing, leads to precisely the same
results since it simply calls tprintf("%#lx", addr)). As a result, I propose
the following change where all cases when tracee's long (or pointer, since they
have the same size in Linux ABIs AFAIK) should be printed are handled via
printaddr() and logic regarding dispatching current_wordsize is added to
printaddr() itself (maybe it also should be renamed to printlong() or something
more suitable).

I'm not really sure whether GCC's behaviour is correct, but it (at least)
doesn't contradict common sense (and x32 ABI spec) as far as I can see, since
higher 32 bits are not used anyway.


---
 syscall.c |    7 +++++--
 util.c    |   25 +++++++++++++------------
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/syscall.c b/syscall.c
index d71ead3..4d9d152 100644
--- a/syscall.c
+++ b/syscall.c
@@ -636,8 +636,11 @@ printargs(struct tcb *tcp)
 	if (entering(tcp)) {
 		int i;
 		int n = tcp->s_ent->nargs;
-		for (i = 0; i < n; i++)
-			tprintf("%s%#lx", i ? ", " : "", tcp->u_arg[i]);
+		for (i = 0; i < n; i++) {
+			if (i)
+				tprints(", ");
+			printaddr(tcp->u_arg[i]);
+		}
 	}
 	return 0;
 }
diff --git a/util.c b/util.c
index 9c8c978..ba0d2a6 100644
--- a/util.c
+++ b/util.c
@@ -398,10 +398,15 @@ printflags64(const struct xlat *xlat, uint64_t flags, const char *dflt)
 void
 printaddr(const long addr)
 {
-	if (!addr)
+	if (!addr) {
 		tprints("NULL");
-	else
-		tprintf("%#lx", addr);
+	} else {
+		if (current_wordsize sizeof(int)) {
+			tprintf("%#lx", addr);
+		} else {
+			tprintf("%#x", (unsigned int)addr);
+		}
+	}
 }
 
 #define DEF_PRINTNUM(name, type) \
@@ -761,7 +766,7 @@ printpathn(struct tcb *tcp, long addr, unsigned int n)
 	/* Fetch one byte more to find out whether path length n. */
 	nul_seen = umovestr(tcp, addr, n + 1, path);
 	if (nul_seen < 0)
-		tprintf("%#lx", addr);
+		printaddr(addr);
 	else {
 		path[n++] = '\0';
 		print_quoted_string(path, n, QUOTE_0_TERMINATED);
@@ -812,7 +817,7 @@ printstr(struct tcb *tcp, long addr, long len)
 		 * because string_quote may look one byte ahead.
 		 */
 		if (umovestr(tcp, addr, size + 1, str) < 0) {
-			tprintf("%#lx", addr);
+			printaddr(addr);
 			return;
 		}
 		style = QUOTE_0_TERMINATED;
@@ -821,7 +826,7 @@ printstr(struct tcb *tcp, long addr, long len)
 		if (size (unsigned long)len)
 			size = (unsigned long)len;
 		if (umoven(tcp, addr, size, str) < 0) {
-			tprintf("%#lx", addr);
+			printaddr(addr);
 			return;
 		}
 		style = 0;
@@ -1131,13 +1136,9 @@ int
 umoven_or_printaddr(struct tcb *tcp, const long addr, const unsigned int len,
 		    void *our_addr)
 {
-	if (!addr) {
-		tprints("NULL");
-		return -1;
-	}
-	if (!verbose(tcp) || (exiting(tcp) && syserror(tcp)) ||
+	if (!addr || !verbose(tcp) || (exiting(tcp) && syserror(tcp)) ||
 	    umoven(tcp, addr, len, our_addr) < 0) {
-		tprintf("%#lx", addr);
+		printaddr(addr);
 		return -1;
 	}
 	return 0;
--
1.7.10.4

--
Eugene "eSyr" Syromyatnikov
mailto:ev...@gm...
xmpp:eSyr@jabber.{ru|org}