Change the behaviour of VALGRIND_CHECK_MEM_IS_DEFINED slightly, so

that if the range is partially non-addressable and it contains
undefined data, both errors are reported.



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

memcheck/mc_main.c

@@ -3553,6 +3553,65 @@ static MC_ReadResult is_mem_defined ( Addr a, SizeT len,
 }
 
 
+/* Like is_mem_defined but doesn't give up at the first uninitialised
+   byte -- the entire range is always checked.  This is important for
+   detecting errors in the case where a checked range strays into
+   invalid memory, but that fact is not detected by the ordinary
+   is_mem_defined(), because of an undefined section that precedes the
+   out of range section, possibly as a result of an alignment hole in
+   the checked data.  This version always checks the entire range and
+   can report both a definedness and an accessbility error, if
+   necessary. */
+static void is_mem_defined_comprehensive (
+               Addr a, SizeT len,
+               /*OUT*/Bool* errorV,    /* is there a definedness err? */
+               /*OUT*/Addr* bad_addrV, /* if so where? */
+               /*OUT*/UInt* otagV,     /* and what's its otag? */
+               /*OUT*/Bool* errorA,    /* is there an addressability err? */
+               /*OUT*/Addr* bad_addrA  /* if so where? */
+            )
+{
+   SizeT i;
+   UWord vabits2;
+   Bool  already_saw_errV = False;
+
+   PROF_EVENT(64, "is_mem_defined"); // fixme
+   DEBUG("is_mem_defined_comprehensive\n");
+
+   tl_assert(!(*errorV || *errorA));
+
+   for (i = 0; i < len; i++) {
+      PROF_EVENT(65, "is_mem_defined(loop)"); // fixme
+      vabits2 = get_vabits2(a);
+      switch (vabits2) {
+         case VA_BITS2_DEFINED: 
+            a++; 
+            break;
+         case VA_BITS2_UNDEFINED:
+         case VA_BITS2_PARTDEFINED:
+            if (!already_saw_errV) {
+               *errorV    = True;
+               *bad_addrV = a;
+               if (MC_(clo_mc_level) == 3) {
+                  *otagV = MC_(helperc_b_load1)( a );
+               } else {
+                  *otagV = 0;
+               }
+               already_saw_errV = True;
+            }
+            a++; /* keep going */
+            break;
+         case VA_BITS2_NOACCESS:
+            *errorA    = True;
+            *bad_addrA = a;
+            return; /* give up now. */
+         default:
+            tl_assert(0);
+      }
+   }
+}
+
+
 /* Check a zero-terminated ascii string.  Tricky -- don't want to
    examine the actual bytes, to find the end, until we're sure it is
    safe to do so. */
@@ -5198,14 +5257,34 @@ static Bool mc_handle_client_request ( ThreadId tid, UWord* arg, UWord* ret )
          break;
 
       case VG_USERREQ__CHECK_MEM_IS_DEFINED: {
-         MC_ReadResult res;
-         UInt otag = 0;
-         res = is_mem_defined ( arg[1], arg[2], &bad_addr, &otag );
-         if (MC_AddrErr == res)
-            MC_(record_user_error) ( tid, bad_addr, /*isAddrErr*/True, 0 );
-         else if (MC_ValueErr == res)
-            MC_(record_user_error) ( tid, bad_addr, /*isAddrErr*/False, otag );
-         *ret = ( res==MC_Ok ? (UWord)NULL : bad_addr );
+         Bool errorV    = False;
+         Addr bad_addrV = 0;
+         UInt otagV     = 0;
+         Bool errorA    = False;
+         Addr bad_addrA = 0;
+         is_mem_defined_comprehensive( 
+            arg[1], arg[2],
+            &errorV, &bad_addrV, &otagV, &errorA, &bad_addrA
+         );
+         if (errorV) {
+            MC_(record_user_error) ( tid, bad_addrV,
+                                     /*isAddrErr*/False, otagV );
+         }
+         if (errorA) {
+            MC_(record_user_error) ( tid, bad_addrA,
+                                     /*isAddrErr*/True, 0 );
+         }
+         /* Return the lower of the two erring addresses, if any. */
+         *ret = 0;
+         if (errorV && !errorA) {
+            *ret = bad_addrV;
+         }
+         if (!errorV && errorA) {
+            *ret = bad_addrA;
+         }
+         if (errorV && errorA) {
+            *ret = bad_addrV < bad_addrA ? bad_addrV : bad_addrA;
+         }
          break;
       }
 

memcheck/tests/Makefile.am

@@ -87,6 +87,8 @@ EXTRA_DIST = \
 	file_locking.stderr.exp file_locking.vgtest \
 	fprw.stderr.exp fprw.vgtest \
 	fwrite.stderr.exp fwrite.vgtest fwrite.stderr.exp-kfail \
+	holey_buffer_too_small.vgtest holey_buffer_too_small.stdout.exp \
+	holey_buffer_too_small.stderr.exp \
 	inits.stderr.exp inits.vgtest \
 	inline.stderr.exp inline.stdout.exp inline.vgtest \
 	leak-0.vgtest leak-0.stderr.exp \
@@ -228,6 +230,7 @@ check_PROGRAMS = \
 	err_disable1 err_disable2 err_disable3 err_disable4 \
 	file_locking \
 	fprw fwrite inits inline \
+	holey_buffer_too_small \
 	leak-0 \
 	leak-cases \
 	leak-cycle \

memcheck/tests/holey_buffer_too_small.c

@@ -0,0 +1,43 @@
+
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "../memcheck.h"
+
+/* This test checks that VALGRIND_CHECK_MEM_IS_DEFINED correctly
+   reports two errors when presented with a buffer which contains both
+   undefined data and some out of range component(s), and the
+   undefined data appears before the out of range components.  Should
+   report 5 errors in total: the first test should report 2, the rest
+   1 each. */
+
+int main ( void )
+{
+   char* a;
+
+   fprintf(stderr, "\n---- part defined, address error at end ----\n\n");
+   a = malloc(8);
+   a[0] = a[1] = a[2] = a[3] = a[6] = a[7] = 'x';
+   VALGRIND_CHECK_MEM_IS_DEFINED(a, 9);
+   free(a);
+
+   fprintf(stderr, "\n---- part defined, address error at start ----\n\n");
+   a = malloc(8);
+   a[0] = a[1] = a[2] = a[3] = a[6] = a[7] = 'x';
+   VALGRIND_CHECK_MEM_IS_DEFINED(a-1, 9);
+   free(a);
+
+   fprintf(stderr, "\n---- fully defined, address error at end ----\n\n");
+   a = malloc(8);
+   a[0] = a[1] = a[2] = a[3] = a[4] = a[5] = a[6] = a[7] = 'x';
+   VALGRIND_CHECK_MEM_IS_DEFINED(a, 9);
+   free(a);
+
+   fprintf(stderr, "\n---- fully defined, address error at start ----\n\n");
+   a = malloc(8);
+   a[0] = a[1] = a[2] = a[3] = a[4] = a[5] = a[6] = a[7] = 'x';
+   VALGRIND_CHECK_MEM_IS_DEFINED(a-1, 9);
+   free(a);
+
+   return 0;
+}

memcheck/tests/holey_buffer_too_small.stderr.exp

@@ -0,0 +1,45 @@
+
+---- part defined, address error at end ----
+
+Uninitialised byte(s) found during client check request
+   at 0x........: main (holey_buffer_too_small.c:21)
+ Address 0x........ is 4 bytes inside a block of size 8 alloc'd
+   at 0x........: malloc (vg_replace_malloc.c:...)
+   by 0x........: main (holey_buffer_too_small.c:19)
+ Uninitialised value was created by a heap allocation
+   at 0x........: malloc (vg_replace_malloc.c:...)
+   by 0x........: main (holey_buffer_too_small.c:19)
+
+Unaddressable byte(s) found during client check request
+   at 0x........: main (holey_buffer_too_small.c:21)
+ Address 0x........ is 0 bytes after a block of size 8 alloc'd
+   at 0x........: malloc (vg_replace_malloc.c:...)
+   by 0x........: main (holey_buffer_too_small.c:19)
+
+
+---- part defined, address error at start ----
+
+Unaddressable byte(s) found during client check request
+   at 0x........: main (holey_buffer_too_small.c:27)
+ Address 0x........ is 1 bytes before a block of size 8 alloc'd
+   at 0x........: malloc (vg_replace_malloc.c:...)
+   by 0x........: main (holey_buffer_too_small.c:25)
+
+
+---- fully defined, address error at end ----
+
+Unaddressable byte(s) found during client check request
+   at 0x........: main (holey_buffer_too_small.c:33)
+ Address 0x........ is 0 bytes after a block of size 8 alloc'd
+   at 0x........: malloc (vg_replace_malloc.c:...)
+   by 0x........: main (holey_buffer_too_small.c:31)
+
+
+---- fully defined, address error at start ----
+
+Unaddressable byte(s) found during client check request
+   at 0x........: main (holey_buffer_too_small.c:39)
+ Address 0x........ is 1 bytes before a block of size 8 alloc'd
+   at 0x........: malloc (vg_replace_malloc.c:...)
+   by 0x........: main (holey_buffer_too_small.c:37)
+

memcheck/tests/holey_buffer_too_small.vgtest

@@ -0,0 +1,2 @@
+prog: holey_buffer_too_small
+vgopts: -q --track-origins=yes