Make the checking of poll() more accurate.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@2938

diff --git a/coregrind/vg_syscalls.c b/coregrind/vg_syscalls.c
index 4279f391590985b2974a38fcd9567f95dee8787e..b694fade1fcbef534eaf7f7b7f887fa1ce0d24e3 100644 (file)
--- a/coregrind/vg_syscalls.c
+++ b/coregrind/vg_syscalls.c
@@ -4339,24 +4339,32 @@ PRE(poll)
        short events;     -- requested events
        short revents;    -- returned events
       };
-      int poll(struct pollfd *ufds, unsigned int nfds, 
-      int timeout) 
+      int poll(struct pollfd *ufds, unsigned int nfds, int timeout) 
    */
+   UInt i;
+   struct vki_pollfd* ufds = (struct vki_pollfd *)arg1;
    MAYBE_PRINTF("poll ( %p, %d, %d )\n",arg1,arg2,arg3);
-   /* In fact some parts of this struct should be readable too.
-      This should be fixed properly. */
-   SYSCALL_TRACK( pre_mem_write, tid, "poll(ufds)", 
-                 arg1, arg2 * sizeof(struct vki_pollfd) );
+
+   for (i = 0; i < arg2; i++) {
+      // 'fd' and 'events' field are inputs;  'revents' is output.
+      // XXX: this is x86 specific -- the pollfd struct varies across
+      // different architectures.
+      SYSCALL_TRACK( pre_mem_read, tid, "poll(ufds)",
+                     (Addr)(&ufds[i]), sizeof(int) + sizeof(short) );
+      SYSCALL_TRACK( pre_mem_write, tid, "poll(ufds)", 
+                     (Addr)(&ufds[i].revents), sizeof(short) );
+   }
 }
 
 POST(poll)
 {
    if (res > 0) {
       UInt i;
-      struct vki_pollfd * arr = (struct vki_pollfd *)arg1;
+      struct vki_pollfd* ufds = (struct vki_pollfd *)arg1;
+      // XXX: again, this is x86-specific
       for (i = 0; i < arg2; i++)
-        VG_TRACK( post_mem_write, (Addr)(&arr[i].revents), 
-                  sizeof(Short) );
+        VG_TRACK( post_mem_write, (Addr)(&ufds[i].revents), 
+                  sizeof(short) );
    }
 }
 

diff --git a/memcheck/tests/.cvsignore b/memcheck/tests/.cvsignore
index 8cc030cdb2660e83c62b285f01b8c07f70d9b922..7ef3ece758c8623d919f1920e4fa67d8842fa11f 100644 (file)
--- a/memcheck/tests/.cvsignore
+++ b/memcheck/tests/.cvsignore
@@ -5,6 +5,7 @@ badfree
 badjump
 badjump2
 badloop
+badpoll
 badrw
 brk
 brk2

diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am
index 14aa31bccc3a9b49288a70a575b90c83af01c6e2..c5041ccb89740d0cae76166891686e1aa6ed48a5 100644 (file)
--- a/memcheck/tests/Makefile.am
+++ b/memcheck/tests/Makefile.am
@@ -11,6 +11,7 @@ EXTRA_DIST = $(noinst_SCRIPTS) \
        badjump.stderr.exp badjump.vgtest \
        badjump2.stderr.exp badjump2.vgtest \
        badloop.stderr.exp badloop.vgtest \
+       badpoll.stderr.exp badpoll.vgtest \
        badrw.stderr.exp badrw.vgtest \
        brk.stderr.exp brk.vgtest \
        brk2.stderr.exp brk2.vgtest \
@@ -69,7 +70,7 @@ EXTRA_DIST = $(noinst_SCRIPTS) \
 
 check_PROGRAMS = \
        badaddrvalue badfree badjump badjump2 \
-       badloop badrw brk brk2 buflen_check \
+       badloop badpoll badrw brk brk2 buflen_check \
        clientperm custom_alloc \
        doublefree error_counts errs1 exitprog execve execve2 \
        fprw fwrite hello inits inline \
@@ -94,6 +95,7 @@ badfree_SOURCES       = badfree.c
 badjump_SOURCES        = badjump.c
 badjump2_SOURCES       = badjump2.c
 badloop_SOURCES        = badloop.c
+badpoll_SOURCES                = badpoll.c
 badrw_SOURCES          = badrw.c
 brk_SOURCES            = brk.c
 brk2_SOURCES           = brk2.c

diff --git a/memcheck/tests/badpoll.c b/memcheck/tests/badpoll.c
new file mode 100644 (file)
index 0000000..61b8d31
--- /dev/null
+++ b/memcheck/tests/badpoll.c
@@ -0,0 +1,25 @@
+#include <assert.h>
+#include <stdlib.h>
+#include <sys/poll.h>
+
+// At one point, poll()'s checking was not done accurately.  This test
+// exposes this -- previously Memcheck only found one error, now if finds
+// two.
+
+int main(void)
+{
+   // Under-allocate by one byte so we get an addressability error.
+   struct pollfd* ufds = malloc(2 * sizeof(struct pollfd) - 1);
+   assert(ufds);
+
+   ufds[0].fd = 0;
+   ufds[0].events = 0;
+   //ufds[1].fd = 0;    // leave undefined so we get another error.
+   ufds[1].events = 0;
+
+   // Previously, the bounds-error due to the under-allocation was detected,
+   // but not the undefined value error due to ufds[1].fd not being defined.
+   poll(ufds, 2, 200);
+
+   return 0;
+}

diff --git a/memcheck/tests/badpoll.stderr.exp b/memcheck/tests/badpoll.stderr.exp
new file mode 100644 (file)
index 0000000..5193a31
--- /dev/null
+++ b/memcheck/tests/badpoll.stderr.exp
@@ -0,0 +1,13 @@
+Syscall param poll(ufds) contains uninitialised or unaddressable byte(s)
+   at 0x........: poll (in /...libc...)
+   by 0x........: main (badpoll.c:22)
+ Address 0x........ is 8 bytes inside a block of size 15 alloc'd
+   at 0x........: malloc (vg_replace_malloc.c:...)
+   by 0x........: main (badpoll.c:12)
+
+Syscall param poll(ufds) contains unaddressable byte(s)
+   at 0x........: poll (in /...libc...)
+   by 0x........: main (badpoll.c:22)
+ Address 0x........ is 0 bytes after a block of size 15 alloc'd
+   at 0x........: malloc (vg_replace_malloc.c:...)
+   by 0x........: main (badpoll.c:12)

diff --git a/memcheck/tests/badpoll.vgtest b/memcheck/tests/badpoll.vgtest
new file mode 100644 (file)
index 0000000..ac49e7f
--- /dev/null
+++ b/memcheck/tests/badpoll.vgtest
@@ -0,0 +1,2 @@
+prog: badpoll
+vgopts: -q