Ensure VALGRIND_MALLOCLIKE_BLOCK protects the red zones.

* Redzones for custom alloc were not protected by VALGRIND_MALLOCLIKE_BLOCK.
  mc_main.c client request handling completed with protection
  of the redzones.
* custom_alloc.c test modified to test this case.
* mc_errors.c modified so as to first search for a malloc-ed block
  bracketting the error : for a custom allocator, a recently freed
  block can have just been re-allocated.
  In such a case, describing the address (e.g. in case of error)
  points to the block freed rather than to the block just allocated.
  If there is *also* a recently freed block bracketting the address,
  the block description is changed to indicate that.

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

diff --git a/NEWS b/NEWS
index 9e8d0c52486ed382ba81b03805a629171538dc76..f2a65c797b897382d9af6210837da87b3c4d2b89 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,10 @@ Release 3.8.0 (????)
   - Addition of GDB server monitor command 'who_points_at' that lists
     the locations pointing at a block.
 
+  - if a redzone size > 0 is given, VALGRIND_MALLOCLIKE_BLOCK now
+    will detect an invalid access of these redzones, by marking them
+    noaccess.
+
 * ==================== OTHER CHANGES ====================
 
 * The C++ demangler has been updated so as to work well with C++ 

diff --git a/memcheck/mc_errors.c b/memcheck/mc_errors.c
index 7c09804f2f59a7edd051fe0b42e2c748777b6958..5084f2853dae4d30605bafac744963cae1748abe 100644 (file)
--- a/memcheck/mc_errors.c
+++ b/memcheck/mc_errors.c
@@ -1115,30 +1115,41 @@ static void describe_addr ( Addr a, /*OUT*/AddrInfo* ai )
    if (mempool_block_maybe_describe( a, ai )) {
       return;
    }
-   /* -- Search for a recently freed block which might bracket it. -- */
-   mc = MC_(get_freed_block_bracketting)( a );
-   if (mc) {
-      ai->tag = Addr_Block;
-      ai->Addr.Block.block_kind = Block_Freed;
-      ai->Addr.Block.block_desc = "block";
-      ai->Addr.Block.block_szB  = mc->szB;
-      ai->Addr.Block.rwoffset   = (Word)a - (Word)mc->data;
-      ai->Addr.Block.lastchange = mc->where;
-      return;
-   }
+   /* Blocks allocated by memcheck malloc functions are either
+      on the recently freed list or on the malloc-ed list.
+      Custom blocks can be on both : a recently freed block might
+      have been just re-allocated.
+      So, first search the malloc-ed block, as the most recent
+      block is the probable cause of error.
+      We however detect and report that this is a recently re-allocated
+      block. */
    /* -- Search for a currently malloc'd block which might bracket it. -- */
    VG_(HT_ResetIter)(MC_(malloc_list));
    while ( (mc = VG_(HT_Next)(MC_(malloc_list))) ) {
       if (addr_is_in_MC_Chunk_default_REDZONE_SZB(mc, a)) {
          ai->tag = Addr_Block;
          ai->Addr.Block.block_kind = Block_Mallocd;
-         ai->Addr.Block.block_desc = "block";
+         if (MC_(get_freed_block_bracketting)( a ))
+            ai->Addr.Block.block_desc = "recently re-allocated block";
+         else
+            ai->Addr.Block.block_desc = "block";
          ai->Addr.Block.block_szB  = mc->szB;
          ai->Addr.Block.rwoffset   = (Word)a - (Word)mc->data;
          ai->Addr.Block.lastchange = mc->where;
          return;
       }
    }
+   /* -- Search for a recently freed block which might bracket it. -- */
+   mc = MC_(get_freed_block_bracketting)( a );
+   if (mc) {
+      ai->tag = Addr_Block;
+      ai->Addr.Block.block_kind = Block_Freed;
+      ai->Addr.Block.block_desc = "block";
+      ai->Addr.Block.block_szB  = mc->szB;
+      ai->Addr.Block.rwoffset   = (Word)a - (Word)mc->data;
+      ai->Addr.Block.lastchange = mc->where;
+      return;
+   }
    /* -- Perhaps the variable type/location data describes it? -- */
    ai->Addr.Variable.descr1
       = VG_(newXA)( VG_(malloc), "mc.da.descr1",

diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c
index 12f6f292edc81fb8c8462f93d94b1451693157a0..ccad3bc3514d5517964fc9f16e2c63b51ba9bf2f 100644 (file)
--- a/memcheck/mc_main.c
+++ b/memcheck/mc_main.c
@@ -5553,11 +5553,15 @@ static Bool mc_handle_client_request ( ThreadId tid, UWord* arg, UWord* ret )
       case VG_USERREQ__MALLOCLIKE_BLOCK: {
          Addr p         = (Addr)arg[1];
          SizeT sizeB    =       arg[2];
-         //UInt rzB       =       arg[3];    XXX: unused!
+         UInt rzB       =       arg[3];
          Bool is_zeroed = (Bool)arg[4];
 
          MC_(new_block) ( tid, p, sizeB, /*ignored*/0, is_zeroed, 
                           MC_AllocCustom, MC_(malloc_list) );
+         if (rzB > 0) {
+            MC_(make_mem_noaccess) ( p - rzB, rzB);
+            MC_(make_mem_noaccess) ( p + sizeB, rzB);
+         }
          return True;
       }
       case VG_USERREQ__RESIZEINPLACE_BLOCK: {

diff --git a/memcheck/tests/custom_alloc.c b/memcheck/tests/custom_alloc.c
index 2f91c6fe8741103eca5d9f93b0131f11b8bc8b6a..56cf94b2746e3840f48b3a7343c619321004a8a4 100644 (file)
--- a/memcheck/tests/custom_alloc.c
+++ b/memcheck/tests/custom_alloc.c
@@ -54,6 +54,44 @@ static void custom_free(void* p)
    VALGRIND_FREELIKE_BLOCK( p, RZ );
 }
 
+static void checkredzone(void)
+{
+   /* check that accessing the redzone of a MALLOCLIKE block
+      is detected  when the superblock was not marked as no access. */
+   char superblock[1 + RZ + 20 + RZ + 1];
+   char *p = 1 + RZ + superblock;
+   assert(RZ > 0);
+
+   // Indicate we have allocated p from our superblock:
+   VALGRIND_MALLOCLIKE_BLOCK( p, 20, RZ, /*is_zeroed*/1 );
+   p[0] = 0; 
+   p[-1] = p[0]; // error expected
+   p[-RZ] = p[0]; // error expected
+   p[-RZ-1] = p[0]; // no error expected
+   
+   p[19] = 0; 
+   p[19 + 1]  = p[0]; // error expected
+   p[19 + RZ] = p[0]; // error expected
+   p[19 + RZ + 1] = p[0]; // no error expected
+
+   VALGRIND_FREELIKE_BLOCK( p, RZ );
+
+   // Now, indicate we have re-allocated p from our superblock
+   // but with only a size 10.
+   VALGRIND_MALLOCLIKE_BLOCK( p, 10, RZ, /*is_zeroed*/1 );
+   p[0] = 0; 
+   p[-1] = p[0]; // error expected
+   p[-RZ] = p[0]; // error expected
+   p[-RZ-1] = p[0]; // no error expected
+   
+   p[9] = 0; 
+   p[9 + 1]  = p[0]; // error expected
+   p[9 + RZ] = p[0]; // error expected
+   p[9 + RZ + 1] = p[0]; // no error expected
+
+   VALGRIND_FREELIKE_BLOCK( p, RZ );
+
+}
 
 
 
@@ -104,16 +142,14 @@ int main(void)
 
    make_leak();
    x = array[0];        // use after free (ok without MALLOCLIKE/MAKE_MEM_NOACCESS)
-                        // (nb: initialised because is_zeroed==1 above)
-                        // unfortunately not identified as being in a free'd
-                        // block because the freeing of the block and shadow
-                        // chunk isn't postponed.
 
    // Bug 137073: passing 0 to MALLOCLIKE_BLOCK was causing an assertion
    // failure.  Test for this (and likewise for FREELIKE_BLOCK).
    VALGRIND_MALLOCLIKE_BLOCK(0,0,0,0);
    VALGRIND_FREELIKE_BLOCK(0,0);
-   
+
+   checkredzone();
+
    return x;
 
    // leak from make_leak()

diff --git a/memcheck/tests/custom_alloc.stderr.exp b/memcheck/tests/custom_alloc.stderr.exp
index bb55f8c0cfbbdaed3fc50c5d00355fb6f57783df..fcff3ec5f5ed6950b5105f3c8a61c80f8eb3083b 100644 (file)
--- a/memcheck/tests/custom_alloc.stderr.exp
+++ b/memcheck/tests/custom_alloc.stderr.exp
@@ -1,45 +1,108 @@
 Invalid write of size 4
-   at 0x........: main (custom_alloc.c:79)
+   at 0x........: main (custom_alloc.c:117)
  Address 0x........ is 0 bytes after a block of size 40 alloc'd
    at 0x........: custom_alloc (custom_alloc.c:47)
-   by 0x........: main (custom_alloc.c:76)
+   by 0x........: main (custom_alloc.c:114)
 
 Invalid write of size 4
-   at 0x........: main (custom_alloc.c:83)
+   at 0x........: main (custom_alloc.c:121)
  Address 0x........ is 0 bytes after a block of size 20 alloc'd
    at 0x........: custom_alloc (custom_alloc.c:47)
-   by 0x........: main (custom_alloc.c:76)
+   by 0x........: main (custom_alloc.c:114)
 
 Conditional jump or move depends on uninitialised value(s)
-   at 0x........: main (custom_alloc.c:90)
+   at 0x........: main (custom_alloc.c:128)
 
 Invalid write of size 4
-   at 0x........: main (custom_alloc.c:93)
+   at 0x........: main (custom_alloc.c:131)
  Address 0x........ is 0 bytes after a block of size 28 alloc'd
    at 0x........: custom_alloc (custom_alloc.c:47)
-   by 0x........: main (custom_alloc.c:76)
+   by 0x........: main (custom_alloc.c:114)
 
 Invalid free() / delete / delete[] / realloc()
-   at 0x........: main (custom_alloc.c:96)
+   at 0x........: main (custom_alloc.c:134)
  Address 0x........ is 4 bytes inside a block of size 28 alloc'd
    at 0x........: custom_alloc (custom_alloc.c:47)
-   by 0x........: main (custom_alloc.c:76)
+   by 0x........: main (custom_alloc.c:114)
 
 Invalid free() / delete / delete[] / realloc()
    at 0x........: custom_free (custom_alloc.c:54)
-   by 0x........: main (custom_alloc.c:100)
+   by 0x........: main (custom_alloc.c:138)
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
 Mismatched free() / delete / delete []
    at 0x........: custom_free (custom_alloc.c:54)
-   by 0x........: main (custom_alloc.c:103)
+   by 0x........: main (custom_alloc.c:141)
  Address 0x........ is 0 bytes inside a block of size 40 alloc'd
    at 0x........: malloc (vg_replace_malloc.c:...)
-   by 0x........: main (custom_alloc.c:102)
+   by 0x........: main (custom_alloc.c:140)
 
 Invalid read of size 4
-   at 0x........: main (custom_alloc.c:106)
+   at 0x........: main (custom_alloc.c:144)
  Address 0x........ is 0 bytes inside a block of size 28 free'd
    at 0x........: custom_free (custom_alloc.c:54)
-   by 0x........: main (custom_alloc.c:98)
+   by 0x........: main (custom_alloc.c:136)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:68)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 1 bytes before a block of size 20 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:66)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:69)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 8 bytes before a block of size 20 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:66)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:73)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 0 bytes after a block of size 20 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:66)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:74)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 7 bytes after a block of size 20 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:66)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:83)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 1 bytes before a recently re-allocated block of size 10 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:81)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:84)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 8 bytes before a recently re-allocated block of size 10 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:81)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:88)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 0 bytes after a recently re-allocated block of size 10 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:81)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:89)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 7 bytes after a recently re-allocated block of size 10 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:81)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:90)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 8 bytes after a recently re-allocated block of size 10 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:81)
+   by 0x........: main (custom_alloc.c:151)
 

diff --git a/memcheck/tests/custom_alloc.stderr.exp-s390x-mvc b/memcheck/tests/custom_alloc.stderr.exp-s390x-mvc
index 70dbd273552c3e91e7978fd9e723b72872c065db..6618a8946cdbf7ec29ad76b22f20c515b25bc82d 100644 (file)
--- a/memcheck/tests/custom_alloc.stderr.exp-s390x-mvc
+++ b/memcheck/tests/custom_alloc.stderr.exp-s390x-mvc
@@ -1,45 +1,108 @@
 Invalid write of size 4
-   at 0x........: main (custom_alloc.c:79)
+   at 0x........: main (custom_alloc.c:117)
  Address 0x........ is 0 bytes after a block of size 40 alloc'd
    at 0x........: custom_alloc (custom_alloc.c:47)
-   by 0x........: main (custom_alloc.c:76)
+   by 0x........: main (custom_alloc.c:114)
 
 Invalid write of size 4
-   at 0x........: main (custom_alloc.c:83)
+   at 0x........: main (custom_alloc.c:121)
  Address 0x........ is 0 bytes after a block of size 20 alloc'd
    at 0x........: custom_alloc (custom_alloc.c:47)
-   by 0x........: main (custom_alloc.c:76)
+   by 0x........: main (custom_alloc.c:114)
 
 Conditional jump or move depends on uninitialised value(s)
-   at 0x........: main (custom_alloc.c:90)
+   at 0x........: main (custom_alloc.c:128)
 
 Invalid write of size 4
-   at 0x........: main (custom_alloc.c:93)
+   at 0x........: main (custom_alloc.c:131)
  Address 0x........ is 0 bytes after a block of size 28 alloc'd
    at 0x........: custom_alloc (custom_alloc.c:47)
-   by 0x........: main (custom_alloc.c:76)
+   by 0x........: main (custom_alloc.c:114)
 
 Invalid free() / delete / delete[] / realloc()
-   at 0x........: main (custom_alloc.c:96)
+   at 0x........: main (custom_alloc.c:134)
  Address 0x........ is 4 bytes inside a block of size 28 alloc'd
    at 0x........: custom_alloc (custom_alloc.c:47)
-   by 0x........: main (custom_alloc.c:76)
+   by 0x........: main (custom_alloc.c:114)
 
 Invalid free() / delete / delete[] / realloc()
    at 0x........: custom_free (custom_alloc.c:54)
-   by 0x........: main (custom_alloc.c:100)
+   by 0x........: main (custom_alloc.c:138)
  Address 0x........ is not stack'd, malloc'd or (recently) free'd
 
 Mismatched free() / delete / delete []
    at 0x........: custom_free (custom_alloc.c:54)
-   by 0x........: main (custom_alloc.c:103)
+   by 0x........: main (custom_alloc.c:141)
  Address 0x........ is 0 bytes inside a block of size 40 alloc'd
    at 0x........: malloc (vg_replace_malloc.c:...)
-   by 0x........: main (custom_alloc.c:102)
+   by 0x........: main (custom_alloc.c:140)
 
 Invalid read of size 1
-   at 0x........: main (custom_alloc.c:106)
+   at 0x........: main (custom_alloc.c:144)
  Address 0x........ is 0 bytes inside a block of size 28 free'd
    at 0x........: custom_free (custom_alloc.c:54)
-   by 0x........: main (custom_alloc.c:98)
+   by 0x........: main (custom_alloc.c:136)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:68)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 1 bytes before a block of size 20 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:66)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:69)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 8 bytes before a block of size 20 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:66)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:73)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 0 bytes after a block of size 20 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:66)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:74)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 7 bytes after a block of size 20 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:66)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:83)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 1 bytes before a recently re-allocated block of size 10 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:81)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:84)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 8 bytes before a recently re-allocated block of size 10 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:81)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:88)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 0 bytes after a recently re-allocated block of size 10 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:81)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:89)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 7 bytes after a recently re-allocated block of size 10 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:81)
+   by 0x........: main (custom_alloc.c:151)
+
+Invalid write of size 1
+   at 0x........: checkredzone (custom_alloc.c:90)
+   by 0x........: main (custom_alloc.c:151)
+ Address 0x........ is 8 bytes after a recently re-allocated block of size 10 alloc'd
+   at 0x........: checkredzone (custom_alloc.c:81)
+   by 0x........: main (custom_alloc.c:151)