Make the allocator's access functions slightly stricter in their

checking of the lo/hi size fields.  If we are corrupting the metadata,
this should make it more likely that we get an assertion failure rather
than an outright crash.

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

diff --git a/coregrind/m_mallocfree.c b/coregrind/m_mallocfree.c
index ae739193cf9dced464bb40fed5934d090fc6750a..9a4a6dd23e121be45cd5010a2c188a454cd9af8e 100644 (file)
--- a/coregrind/m_mallocfree.c
+++ b/coregrind/m_mallocfree.c
@@ -170,48 +170,50 @@ SizeT mk_plain_bszB ( SizeT bszB )
    return bszB & (~SIZE_T_0x1);
 }
 
-// Set get the lower size field of a block.
+//---------------------------------------------------------------------------
+
+// Get a block's size as stored, ie with the in-use/free attribute.
 static __inline__
-SizeT get_bszB_lo ( Block* b )
+SizeT get_bszB_as_is ( Block* b )
 {
-   return *(SizeT*)&b[0];
+   UByte* b2     = (UByte*)b;
+   SizeT bszB_lo = *(SizeT*)&b2[0];
+   SizeT bszB_hi = *(SizeT*)&b2[mk_plain_bszB(bszB_lo) - sizeof(SizeT)];
+   vg_assert2(bszB_lo == bszB_hi, 
+      "Heap block lo/hi size mismatch: lo = %llu, hi = %llu.\n"
+      "Probably caused by overrunning/underrunning a heap block's bounds\n");
+   return bszB_lo;
 }
 
-// Does this block have the in-use attribute?
+// Get a block's plain size, ie. remove the in-use/free attribute.
 static __inline__
-Bool is_inuse_block ( Block* b )
+SizeT get_bszB ( Block* b )
 {
-   SizeT bszB = get_bszB_lo(b);
-   vg_assert(bszB != 0);
-   return (0 != (bszB & SIZE_T_0x1)) ? False : True;
+   return mk_plain_bszB(get_bszB_as_is(b));
 }
 
-// Get the address of the last byte in a block
+// Set the size fields of a block.  bszB may have the in-use/free attribute.
 static __inline__
-UByte* last_byte ( Block* b )
+void set_bszB ( Block* b, SizeT bszB )
 {
    UByte* b2 = (UByte*)b;
-   return &b2[mk_plain_bszB(get_bszB_lo(b)) - 1];
+   *(SizeT*)&b2[0]                                   = bszB;
+   *(SizeT*)&b2[mk_plain_bszB(bszB) - sizeof(SizeT)] = bszB;
 }
 
-// Get the upper size field of a block.
-static __inline__
-SizeT get_bszB_hi ( Block* b )
-{
-   UByte* lb = last_byte(b);
-   return *(SizeT*)&lb[-sizeof(SizeT) + 1];
-}
+//---------------------------------------------------------------------------
 
-// Set the size fields of a block.
+// Does this block have the in-use attribute?
 static __inline__
-void set_bszB ( Block* b, SizeT bszB )
+Bool is_inuse_block ( Block* b )
 {
-   UByte* lb;
-   *(SizeT*)&b[0] = bszB;     // Set lo bszB;  must precede last_byte() call
-   lb = last_byte(b);
-   *(SizeT*)&lb[-sizeof(SizeT) + 1] = bszB;  // Set hi bszB
+   SizeT bszB = get_bszB_as_is(b);
+   vg_assert(bszB != 0);
+   return (0 != (bszB & SIZE_T_0x1)) ? False : True;
 }
 
+//---------------------------------------------------------------------------
+
 // Return the lower, upper and total overhead in bytes for a block.
 // These are determined purely by which arena the block lives in.
 static __inline__
@@ -230,6 +232,8 @@ SizeT overhead_szB ( Arena* a )
    return overhead_szB_lo(a) + overhead_szB_hi(a);
 }
 
+//---------------------------------------------------------------------------
+
 // Return the minimum bszB for a block in this arena.  Can have zero-length
 // payloads, so it's the size of the admin bytes.
 static __inline__
@@ -238,6 +242,8 @@ SizeT min_useful_bszB ( Arena* a )
    return overhead_szB(a);
 }
 
+//---------------------------------------------------------------------------
+
 // Convert payload size <--> block size (both in bytes).
 static __inline__
 SizeT pszB_to_bszB ( Arena* a, SizeT pszB )
@@ -251,22 +257,7 @@ SizeT bszB_to_pszB ( Arena* a, SizeT bszB )
    return bszB - overhead_szB(a);
 }
 
-// Get a block's size as stored, ie with the in-use/free attribute.
-static __inline__
-SizeT get_bszB_as_is ( Block* b )
-{
-   SizeT bszB_lo = get_bszB_lo(b);
-   SizeT bszB_hi = get_bszB_hi(b);
-   vg_assert(bszB_lo == bszB_hi);
-   return bszB_lo;
-}
-
-// Get a block's plain size, ie. remove the in-use/free attribute.
-static __inline__
-SizeT get_bszB ( Block* b )
-{
-   return mk_plain_bszB(get_bszB_as_is(b));
-}
+//---------------------------------------------------------------------------
 
 // Get a block's payload size.
 static __inline__
@@ -275,7 +266,9 @@ SizeT get_pszB ( Arena* a, Block* b )
    return bszB_to_pszB(a, get_bszB(b));
 }
 
-// Given the addr of a block, return the addr of its payload.
+//---------------------------------------------------------------------------
+
+// Given the addr of a block, return the addr of its payload, and vice versa.
 static __inline__
 UByte* get_block_payload ( Arena* a, Block* b )
 {
@@ -289,6 +282,7 @@ Block* get_payload_block ( Arena* a, UByte* payload )
    return (Block*)&payload[ -overhead_szB_lo(a) ];
 }
 
+//---------------------------------------------------------------------------
 
 // Set and get the next and previous link fields of a block.
 static __inline__
@@ -300,8 +294,8 @@ void set_prev_b ( Block* b, Block* prev_p )
 static __inline__
 void set_next_b ( Block* b, Block* next_p )
 {
-   UByte* lb = last_byte(b);
-   *(Block**)&lb[-sizeof(SizeT) - sizeof(void*) + 1] = next_p;
+   UByte* b2 = (UByte*)b;
+   *(Block**)&b2[get_bszB(b) - sizeof(SizeT) - sizeof(void*)] = next_p;
 }
 static __inline__
 Block* get_prev_b ( Block* b )
@@ -312,10 +306,11 @@ Block* get_prev_b ( Block* b )
 static __inline__
 Block* get_next_b ( Block* b )
 { 
-   UByte* lb = last_byte(b);
-   return *(Block**)&lb[-sizeof(SizeT) - sizeof(void*) + 1];
+   UByte* b2 = (UByte*)b;
+   return *(Block**)&b2[get_bszB(b) - sizeof(SizeT) - sizeof(void*)];
 }
 
+//---------------------------------------------------------------------------
 
 // Get the block immediately preceding this one in the Superblock.
 static __inline__
@@ -326,6 +321,8 @@ Block* get_predecessor_block ( Block* b )
    return (Block*)&b2[-bszB];
 }
 
+//---------------------------------------------------------------------------
+
 // Read and write the lower and upper red-zone bytes of a block.
 static __inline__
 void set_rz_lo_byte ( Arena* a, Block* b, UInt rz_byteno, UByte v )
@@ -336,8 +333,8 @@ void set_rz_lo_byte ( Arena* a, Block* b, UInt rz_byteno, UByte v )
 static __inline__
 void set_rz_hi_byte ( Arena* a, Block* b, UInt rz_byteno, UByte v )
 {
-   UByte* lb = last_byte(b);
-   lb[-sizeof(SizeT) - rz_byteno] = v;
+   UByte* b2 = (UByte*)b;
+   b2[get_bszB(b) - sizeof(SizeT) - rz_byteno - 1] = v;
 }
 static __inline__
 UByte get_rz_lo_byte ( Arena* a, Block* b, UInt rz_byteno )
@@ -348,8 +345,8 @@ UByte get_rz_lo_byte ( Arena* a, Block* b, UInt rz_byteno )
 static __inline__
 UByte get_rz_hi_byte ( Arena* a, Block* b, UInt rz_byteno )
 {
-   UByte* lb = last_byte(b);
-   return lb[-sizeof(SizeT) - rz_byteno];
+   UByte* b2 = (UByte*)b;
+   return b2[get_bszB(b) - sizeof(SizeT) - rz_byteno - 1];
 }
 
 
@@ -633,8 +630,8 @@ Bool blockSane ( Arena* a, Block* b )
 {
 #  define BLEAT(str) VG_(printf)("blockSane: fail -- %s\n",str)
    UInt i;
-   if (get_bszB_lo(b) != get_bszB_hi(b))
-      {BLEAT("sizes");return False;}
+   // The lo and hi size fields will be checked (indirectly) by the call
+   // to get_rz_hi_byte().
    if (!a->clientmem && is_inuse_block(b)) {
       for (i = 0; i < a->rz_szB; i++) {
          if (get_rz_lo_byte(a, b, i) !=