Patchwork pwrite64 error because of argument position

login
register
mail settings
Submitter Alexander Graf
Date Oct. 1, 2012, 11:52 a.m.
Message ID <88B3430D-F2EC-4E94-8D92-A9CA0FC1E1D1@suse.de>
Download mbox | patch
Permalink /patch/188275/
State New
Headers show

Comments

Alexander Graf - Oct. 1, 2012, 11:52 a.m.
On 30.09.2012, at 20:50, Alex Barcelo wrote:

> This error may be a PPC specific problem, but I don't have another
> environment where I can test it (i386 doesn't seem to use pwrite64),
> so I ask for a bit of help/check.
> 
> I am in a 32bit linux testing the qemu-ppc.
> 
> My test program:
> 
> // -------------------------------
> #include <unistd.h>
> #include <stdio.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> 
> int main (void) {
>    int fd, len;
> 
>    char* asd = "This is a random test";
>    char asd2[20];
> 
>    fd = open ("./test.pwrite", O_RDWR | O_CREAT);
> 
>    printf ( "fd: %d\n", fd);
>    pwrite ( fd, asd, 15, 0 );
>    pwrite ( fd, asd+5, 6, 10);
>    pwrite ( fd, asd, 4, 30);
> 
>    len = pread ( fd, asd2, 5, 5);
>    asd2[len]='\0';	
>    printf ( "Read %d bytes: %s", len, asd2);
> 
>    // This is to force two int arguments for 64bit
>    //len = pread ( fd, asd2, -1, -1);
>    close(fd);
>    return 0;
> }
> // -------------------------------
> 
> Then I do
> $ powerpc-linux-gnu-gcc -g --static -o pwrite-alien pwrite-test.c
> 
> and when I launch a qemu-ppc to test, it should be (on the file)
> This is a is a rThis
> 
> instead I get:
> This rs a randorThis
> 
> and if I print some debug information inside pwrite64 and pread64 I see:
> syscall: pwrite	arg_i: 3 268909324 15 0 0 0 0 0
> syscall: pwrite	arg_i: 3 268909329 6 0 0 10 0 0
> syscall: pwrite	arg_i: 3 268909324 4 0 0 30 0 0
> syscall: pread	arg_i: 3 1082133156 5 0 0 5 0 0
> (those are arg1, arg2, arg3, arg4, arg5, arg6 and the unused arg7 and arg8)
> 
> As can be seen, arg4 is not used, and the line
> ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
> should be, in my case
> ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg5, arg6)));
> 
> With this changes, the qemu "Works For Me (TM)".
> 
> So, anybody can confirm it or (if it is indeed my problem) can give me
> some pointers? I will not post this as a patch until I understand the
> problem... and first step is making sure that it really is a qemu
> problem. And not my toolchain or something random.

Running the above code on a real ppc machine (compiled with -m32, running a ppc64 kernel) I get:

00000000  54 68 69 73 20 69 73 20  61 20 69 73 20 61 20 72  |This is a is a r|
00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 54 68  |..............Th|
00000020  69 73                                             |is|

as output. Is this the expected result? Running it in qemu-ppc, I get:

00000000  54 68 69 73 20 72 73 20  61 20 72 61 6e 64 6f     |This rs a rando|
0000000f

So yes, something looks broken here.

According to arch/powerpc/kernel/sys_ppc32.c, the ppc32 ABI carries 64bit parameters on odd/even pairs:

/* 
 * long long munging:
 * The 32 bit ABI passes long longs in an odd even register pair.
 */

which is almost the same as what ARM or MIPS do and which explains why you see arg5/arg6 (r7/r8) used instead of arg4/arg5 (r6/r7).

Could you please try the below patch?


Alex
Alex Barcelo - Oct. 1, 2012, 4:57 p.m.
ok, thank you very much. Now I understand the problem... and I see the
correct way of mending it.

Your patch works for me.

>On Mon, Oct 1, 2012 at 1:52 PM, Alexander Graf <agraf@suse.de> wrote:
>
> On 30.09.2012, at 20:50, Alex Barcelo wrote:
>
>> This error may be a PPC specific problem, but I don't have another
>> environment where I can test it (i386 doesn't seem to use pwrite64),
>> so I ask for a bit of help/check.
>>
>> I am in a 32bit linux testing the qemu-ppc.
>>
>> My test program:
>>
>> // -------------------------------
>> #include <unistd.h>
>> #include <stdio.h>
>> #include <sys/types.h>
>> #include <sys/stat.h>
>> #include <fcntl.h>
>>
>> int main (void) {
>>    int fd, len;
>>
>>    char* asd = "This is a random test";
>>    char asd2[20];
>>
>>    fd = open ("./test.pwrite", O_RDWR | O_CREAT);
>>
>>    printf ( "fd: %d\n", fd);
>>    pwrite ( fd, asd, 15, 0 );
>>    pwrite ( fd, asd+5, 6, 10);
>>    pwrite ( fd, asd, 4, 30);
>>
>>    len = pread ( fd, asd2, 5, 5);
>>    asd2[len]='\0';
>>    printf ( "Read %d bytes: %s", len, asd2);
>>
>>    // This is to force two int arguments for 64bit
>>    //len = pread ( fd, asd2, -1, -1);
>>    close(fd);
>>    return 0;
>> }
>> // -------------------------------
>>
>> Then I do
>> $ powerpc-linux-gnu-gcc -g --static -o pwrite-alien pwrite-test.c
>>
>> and when I launch a qemu-ppc to test, it should be (on the file)
>> This is a is a rThis
>>
>> instead I get:
>> This rs a randorThis
>>
>> and if I print some debug information inside pwrite64 and pread64 I see:
>> syscall: pwrite       arg_i: 3 268909324 15 0 0 0 0 0
>> syscall: pwrite       arg_i: 3 268909329 6 0 0 10 0 0
>> syscall: pwrite       arg_i: 3 268909324 4 0 0 30 0 0
>> syscall: pread        arg_i: 3 1082133156 5 0 0 5 0 0
>> (those are arg1, arg2, arg3, arg4, arg5, arg6 and the unused arg7 and arg8)
>>
>> As can be seen, arg4 is not used, and the line
>> ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
>> should be, in my case
>> ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg5, arg6)));
>>
>> With this changes, the qemu "Works For Me (TM)".
>>
>> So, anybody can confirm it or (if it is indeed my problem) can give me
>> some pointers? I will not post this as a patch until I understand the
>> problem... and first step is making sure that it really is a qemu
>> problem. And not my toolchain or something random.
>
> Running the above code on a real ppc machine (compiled with -m32, running a ppc64 kernel) I get:
>
> 00000000  54 68 69 73 20 69 73 20  61 20 69 73 20 61 20 72  |This is a is a r|
> 00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 54 68  |..............Th|
> 00000020  69 73                                             |is|
>
> as output. Is this the expected result? Running it in qemu-ppc, I get:
>
> 00000000  54 68 69 73 20 72 73 20  61 20 72 61 6e 64 6f     |This rs a rando|
> 0000000f
>
> So yes, something looks broken here.
>
> According to arch/powerpc/kernel/sys_ppc32.c, the ppc32 ABI carries 64bit parameters on odd/even pairs:
>
> /*
>  * long long munging:
>  * The 32 bit ABI passes long longs in an odd even register pair.
>  */
>
> which is almost the same as what ARM or MIPS do and which explains why you see arg5/arg6 (r7/r8) used instead of arg4/arg5 (r6/r7).
>
> Could you please try the below patch?
>
>
> Alex
>
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1a38169..e03b3a8 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -587,12 +587,16 @@ extern int setfsgid(int);
>  extern int setgroups(int, gid_t *);
>
>  /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
> -#ifdef TARGET_ARM
> +#ifdef TARGET_ARM
>  static inline int regpairs_aligned(void *cpu_env) {
>      return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
>  }
>  #elif defined(TARGET_MIPS)
>  static inline int regpairs_aligned(void *cpu_env) { return 1; }
> +#elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
> +/* PPC32 expects 64bit parameters to be passed on odd/even pairs of registers
> +   which is the same as ARM/MIPS, because we start with r3 as arg1. */
> +static inline int regpairs_aligned(void *cpu_env) { return 1; }
>  #else
>  static inline int regpairs_aligned(void *cpu_env) { return 0; }
>  #endif
> @@ -7419,12 +7423,20 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
>  #endif
>  #ifdef TARGET_NR_pread64
>      case TARGET_NR_pread64:
> +        if (regpairs_aligned(cpu_env)) {
> +            arg4 = arg5;
> +            arg5 = arg6;
> +        }
>          if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
>              goto efault;
>          ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5)));
>          unlock_user(p, arg2, ret);
>          break;
>      case TARGET_NR_pwrite64:
> +        if (regpairs_aligned(cpu_env)) {
> +            arg4 = arg5;
> +            arg5 = arg6;
> +        }
>          if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
>              goto efault;
>          ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
Alexander Graf - Oct. 1, 2012, 4:59 p.m.
On 01.10.2012, at 18:57, Alex Barcelo wrote:

> ok, thank you very much. Now I understand the problem... and I see the
> correct way of mending it.
> 
> Your patch works for me.

Awesome. Could you please put your Tested-by tag below the patches I sent to the ML earlier to day then? :)


Alex

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 1a38169..e03b3a8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -587,12 +587,16 @@  extern int setfsgid(int);
 extern int setgroups(int, gid_t *);
 
 /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
-#ifdef TARGET_ARM 
+#ifdef TARGET_ARM
 static inline int regpairs_aligned(void *cpu_env) {
     return ((((CPUARMState *)cpu_env)->eabi) == 1) ;
 }
 #elif defined(TARGET_MIPS)
 static inline int regpairs_aligned(void *cpu_env) { return 1; }
+#elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
+/* PPC32 expects 64bit parameters to be passed on odd/even pairs of registers
+   which is the same as ARM/MIPS, because we start with r3 as arg1. */
+static inline int regpairs_aligned(void *cpu_env) { return 1; }
 #else
 static inline int regpairs_aligned(void *cpu_env) { return 0; }
 #endif
@@ -7419,12 +7423,20 @@  abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #endif
 #ifdef TARGET_NR_pread64
     case TARGET_NR_pread64:
+        if (regpairs_aligned(cpu_env)) {
+            arg4 = arg5;
+            arg5 = arg6;
+        }
         if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
             goto efault;
         ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5)));
         unlock_user(p, arg2, ret);
         break;
     case TARGET_NR_pwrite64:
+        if (regpairs_aligned(cpu_env)) {
+            arg4 = arg5;
+            arg5 = arg6;
+        }
         if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1)))
             goto efault;
         ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));