Optimize sys_old_mmap

* mem.c (sys_old_mmap): For Ia64 and 32-bit personality of x86-64,
copy narrow parameters from userspace by single umove, not by six
separate ones; then assign them to long u_arg[i]. For SH[64],
avoid copying of tcp->u_arg.
(sys_mmap): Add FIXME comment - SH64 and i386 seem to be handled
differently for no apparent reason.
* test/mmap_offset_decode.c: New test program, illustrates FIXME.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>

diff --git a/mem.c b/mem.c
index de9b6bbe38a152fb5490751a5996eea812150067..7ae8b5cc3e602c8c44d89298318e47a6fdbec73d 100644 (file)
--- a/mem.c
+++ b/mem.c
@@ -265,46 +265,44 @@ print_mmap(struct tcb *tcp, long *u_arg, long long offset)
 #ifdef LINUX
 int sys_old_mmap(struct tcb *tcp)
 {
-       long u_arg[6];
-
 #if defined(IA64)
-       int i, v;
        /*
-        *  IA64 processes never call this routine, they only use the
-        *  new `sys_mmap' interface.  This code converts the integer
-        *  arguments that the IA32 process pushed onto the stack into
-        *  longs.
+        * IA64 processes never call this routine, they only use the
+        * new `sys_mmap' interface.
+        * For IA32 processes, this code converts the integer arguments
+        * that they pushed onto the stack, into longs.
         *
-        *  Note that addresses with bit 31 set will be sign extended.
-        *  Fortunately, those addresses are not currently being generated
-        *  for IA32 processes so it's not a problem.
+        * Note that addresses with bit 31 set will be sign extended.
+        * Fortunately, those addresses are not currently being generated
+        * for IA32 processes so it's not a problem.
         */
+       int i;
+       long u_arg[6];
+       int narrow_arg[6];
+       if (umoven(tcp, tcp->u_arg[0], sizeof(narrow_arg), (char *) narrow_arg) == -1)
+               return 0;
        for (i = 0; i < 6; i++)
-               if (umove(tcp, tcp->u_arg[0] + (i * sizeof(int)), &v) == -1)
-                       return 0;
-               else
-                       u_arg[i] = v;
+               u_arg[i] = narrow_arg[i];
 #elif defined(SH) || defined(SH64)
        /* SH has always passed the args in registers */
-       int i;
-       for (i = 0; i < 6; i++)
-               u_arg[i] = tcp->u_arg[i];
+       long *u_arg = tcp->u_arg;
 #else
+       long u_arg[6];
 # if defined(X86_64)
        if (current_personality == 1) {
                int i;
-               for (i = 0; i < 6; ++i) {
-                       unsigned int val;
-                       if (umove(tcp, tcp->u_arg[0] + i * 4, &val) == -1)
-                               return 0;
-                       u_arg[i] = val;
-               }
+               unsigned narrow_arg[6];
+               if (umoven(tcp, tcp->u_arg[0], sizeof(narrow_arg), (char *) narrow_arg) == -1)
+                       return 0;
+               for (i = 0; i < 6; ++i)
+                       u_arg[i] = narrow_arg[i];
        }
        else
 # endif
-       if (umoven(tcp, tcp->u_arg[0], sizeof u_arg, (char *) u_arg) == -1)
+       if (umoven(tcp, tcp->u_arg[0], sizeof(u_arg), (char *) u_arg) == -1)
                return 0;
-#endif /* !IA64 && !SH[64] */
+#endif /* other architectures */
+
        return print_mmap(tcp, u_arg, u_arg[5]);
 }
 #endif /* LINUX */
@@ -314,6 +312,11 @@ sys_mmap(struct tcb *tcp)
 {
        long long offset = tcp->u_arg[5];
 
+       /* FIXME: why only SH64? i386 mmap2 syscall ends up
+        * in this function, but does not convert offset
+        * from pages to bytes. See test/mmap_offset_decode.c
+        * Why SH64 and i386 are handled differently?
+        */
 #if defined(LINUX) && defined(SH64)
        /*
         * Old mmap differs from new mmap in specifying the

diff --git a/test/mmap_offset_decode.c b/test/mmap_offset_decode.c
new file mode 100644 (file)
index 0000000..875ea9c
--- /dev/null
+++ b/test/mmap_offset_decode.c
@@ -0,0 +1,31 @@
+/* Should strace show byte or page offsets in mmap syscalls
+ * which take page offset parameters?
+ *
+ * At the time of writing, sys_mmap() converts page to byte offsets,
+ * but only for SH64! But this routine is used on i386 too - by mmap2 syscall,
+ * which uses page offsets too. As it stands now, SH64 and i386 are inconsistent.
+ *
+ * sys_old_mmap() is used for old mmap syscall, which uses byte offset -
+ * should be ok.
+ * sys_mmap64() is currently buggy (should print bogus offset, but I can't
+ * test it right now. What arch/bitness invokes sys_mmap64?)
+ *
+ * This program is intended for testing what strace actually shows. Usage:
+ * $ gcc test/mmap_offset_decode.c -o mmap_offset_decode -static
+ * $ strace ./mmap_offset_decode
+ *
+ * As of today (2011-08), on i386 strace prints page offset.
+ */
+
+#define _LARGEFILE_SOURCE
+#define _LARGEFILE64_SOURCE
+#define _FILE_OFFSET_BITS 64
+#include <sys/mman.h>
+#include <errno.h>
+int main()
+{
+       /* 0x1000 is meant to be page size multiplier */
+       mmap(0, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1,
+                       0x7fff0000LL * 0x1000);
+       return errno != 0;
+}