x32 gcc sign extension bug (was: Long/address printing in non-native personalities)

[Dmitry V. Levin]

Hi,

On Fri, Jun 10, 2016 at 06:58:57PM +0300, Eugene Syromyatnikov wrote:
> 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 at plt>

Eugene, thanks for the analysis, this explains the issue.

GCC actually produces the following instructions:

        pushq   %rbx
        movl    $-1159967251, %r9d
        movl    $-1159971347, %r8d
        movl    $-1159975443, %ecx
        movl    $-1159979539, %edx
        movl    $-1159983635, %esi
        subl    $8, %esp
        movl    $1073742370, %edi
        xorl    %eax, %eax
        pushq   $-1159963155
        call    syscall

This is translated into the code you've cited.

> 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).

In strace, we are trying to display things from the kernel perspective rather
than from the userspace perspective.  When these points of view don't
match, we are trying to present the kernel's side of the story.

For example, when the kernel takes a pid argument, we print it as int.

With regards to address printing, the x86_64 kernel operates with
addresses as whole 64-bit words both in x86_64 and x32 modes, so explicit
cast to a 32-bit word won't be an appropriate solution.

Imagine an invocation of a 6-arg syscall with the last argument being an
address, e.g. pselect6.  Such syscall cannot be invoked properly on x32
using a syscall() wrapper because, as you've shown, the 32-bit address
will be sign-extended, producing a potentially wrong address that would
result to a failed syscall; I think strace would be less useful if it
stopped showing the 64-bit address actually passed to this syscall.

For this reason, I recommend x86_64 strace over native x32 strace for
tracing x32 processes because x32 strace doesn't show these high bits
(which are visible to the kernel) while x86_64 strace does show them.

Said that, I fully support your proposal of printing addresses consistently
using printaddr().  There are still about two dozens cases that should be
converted to use printaddr() instead of direct tprintf() calls.

printargs(), however, is used in different contexts, so changing it
to use printaddr() might result to NULL being printed instead of 0
in places where such a change is not quite appropriate.

> 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.

This behaviour is inconsistent, and I'd rather call it a bug that hurts:
one cannot reliably call 6-arg syscalls using a syscall() wrapper.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160611/744c82cc/attachment.bin>