|
From: <sv...@va...> - 2006-02-20 20:33:40
|
Author: njn
Date: 2006-02-20 20:33:27 +0000 (Mon, 20 Feb 2006)
New Revision: 5660
Log:
Fix a long-standing bug in the compressed V bits handling. When
copy_address_range_state() copies a partially defined byte, it failed to
also copy the corresponding entry in the sec-V-bits table. This later ca=
use
sec-V-bit table lookup failures. Big thanks to Julian for diagnosing thi=
s.
I added a regtest for this, and some warnings in comments. I couldn't th=
ink
of a way to make it impossible in the code without killing performance,
unfortunately.
Added:
branches/COMPVBITS/memcheck/tests/pdb-realloc.c
branches/COMPVBITS/memcheck/tests/pdb-realloc.stderr.exp
branches/COMPVBITS/memcheck/tests/pdb-realloc.vgtest
Modified:
branches/COMPVBITS/memcheck/mc_main.c
branches/COMPVBITS/memcheck/tests/Makefile.am
Modified: branches/COMPVBITS/memcheck/mc_main.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- branches/COMPVBITS/memcheck/mc_main.c 2006-02-19 14:05:28 UTC (rev 56=
59)
+++ branches/COMPVBITS/memcheck/mc_main.c 2006-02-20 20:33:27 UTC (rev 56=
60)
@@ -535,6 +535,10 @@
// clever things like combine the auxmap check (in
// get_secmap_{read,writ}able) with alignment checks.
=20
+// *** WARNING! ***
+// Any time this function is called, if it is possible that vabits2
+// is equal to VA_BITS2_OTHER, then the corresponding entry in the
+// sec-V-bits table must also be set!
static inline
void set_vabits2 ( Addr a, UChar vabits2 )
{
@@ -921,10 +925,14 @@
PROF_EVENT(150, "set_address_range_perms");
=20
/* Check the V+A bits make sense. */
- tl_assert(vabits16 =3D=3D VA_BITS16_NOACCESS ||
- vabits16 =3D=3D VA_BITS16_WRITABLE ||
- vabits16 =3D=3D VA_BITS16_READABLE);
+ tl_assert(VA_BITS16_NOACCESS =3D=3D vabits16 ||
+ VA_BITS16_WRITABLE =3D=3D vabits16 ||
+ VA_BITS16_READABLE =3D=3D vabits16);
=20
+ // This code should never write PDBs; ensure this. (See comment abo=
ve
+ // set_vabits2().)
+ tl_assert(VA_BITS2_OTHER !=3D vabits2);
+
if (lenT =3D=3D 0)
return;
=20
@@ -944,6 +952,9 @@
{
// Endianness doesn't matter here because all bytes are being set =
to
// the same value.
+ // Nb: We don't have to worry about updating the sec-V-bits table
+ // after these set_vabits2() calls because this code never writes
+ // VA_BITS2_OTHER values.
SizeT i;
for (i =3D 0; i < lenT; i++) {
set_vabits2(a + i, vabits2);
@@ -1159,6 +1170,7 @@
void MC_(copy_address_range_state) ( Addr src, Addr dst, SizeT len )
{
SizeT i, j;
+ UChar vabits2;
=20
DEBUG("MC_(copy_address_range_state)\n");
PROF_EVENT(50, "MC_(copy_address_range_state)");
@@ -1169,14 +1181,22 @@
if (src < dst) {
for (i =3D 0, j =3D len-1; i < len; i++, j--) {
PROF_EVENT(51, "MC_(copy_address_range_state)(loop)");
- set_vabits2( dst+j, get_vabits2( src+j ) );
+ vabits2 =3D get_vabits2( src+j );
+ set_vabits2( dst+j, vabits2 );
+ if (VA_BITS2_OTHER =3D=3D vabits2) {
+ set_sec_vbits8( dst+j, get_sec_vbits8( src+j ) );
+ }
}
}
=20
if (src > dst) {
for (i =3D 0; i < len; i++) {
- PROF_EVENT(51, "MC_(copy_address_range_state)(loop)");
- set_vabits2( dst+i, get_vabits2( src+i ) );
+ PROF_EVENT(52, "MC_(copy_address_range_state)(loop)");
+ vabits2 =3D get_vabits2( src+i );
+ set_vabits2( dst+i, vabits2 );
+ if (VA_BITS2_OTHER =3D=3D vabits2) {
+ set_sec_vbits8( dst+i, get_sec_vbits8( src+i ) );
+ }
}
}
}
Modified: branches/COMPVBITS/memcheck/tests/Makefile.am
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- branches/COMPVBITS/memcheck/tests/Makefile.am 2006-02-19 14:05:28 UTC=
(rev 5659)
+++ branches/COMPVBITS/memcheck/tests/Makefile.am 2006-02-20 20:33:27 UTC=
(rev 5660)
@@ -75,6 +75,7 @@
partiallydefinedeq.stdout.exp \
partial_load_ok.vgtest partial_load_ok.stderr.exp partial_load_ok.stder=
r.exp64 \
partial_load_dflt.vgtest partial_load_dflt.stderr.exp partial_load_dflt=
.stderr.exp64 \
+ pdb-realloc.stderr.exp pdb-realloc.vgtest \
pipe.stderr.exp pipe.vgtest \
pointer-trace.vgtest \
pointer-trace.stderr.exp pointer-trace.stderr.exp64 \
@@ -123,7 +124,7 @@
nanoleak new_nothrow \
null_socket oset_test overlap \
partiallydefinedeq \
- partial_load \
+ partial_load pdb-realloc \
pipe pointer-trace \
post-syscall \
realloc1 realloc2 realloc3 \
Added: branches/COMPVBITS/memcheck/tests/pdb-realloc.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- branches/COMPVBITS/memcheck/tests/pdb-realloc.c =
(rev 0)
+++ branches/COMPVBITS/memcheck/tests/pdb-realloc.c 2006-02-20 20:33:27 U=
TC (rev 5660)
@@ -0,0 +1,31 @@
+// This test-case exposes a bug that was present in the compressed V bit
+// handling for a while. The problem was that when
+// copy_address_range_state() copied a VA_BITS2_OTHER value, it failed t=
o
+// also copy the corresponding entry in the sec-V-bits table. Then late=
r on
+// when we searched for the sec-V-bits entry for the copied-to location,=
it
+// failed to find it:
+//
+// Memcheck: mc_main.c:766 (get_sec_vbits8): Assertion 'n' failed.
+// Memcheck: get_sec_vbits8: no node for address 0x4017440 (0x4017441)
+
+#include <stdlib.h>
+
+int main(void)
+{
+ int i, t;
+ char* x =3D malloc(1000);
+
+ // Write some PDBs (partially defined bytes)
+ for (i =3D 0; i < 1000; i++)
+ x[i] &=3D (i & 0xff);
+
+ // realloc them, invoking copy_address_range_state()
+ x =3D realloc(x, 10000);
+
+ // Read the PDBs -- this caused a sec-V-bits lookup failure.
+ for (i =3D 0; i < 1000; i++)
+ t +=3D x[i];
+ =20
+ return 0;
+}
+
Property changes on: branches/COMPVBITS/memcheck/tests/pdb-realloc.c
___________________________________________________________________
Name: svn:executable
+ *
Added: branches/COMPVBITS/memcheck/tests/pdb-realloc.stderr.exp
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
Added: branches/COMPVBITS/memcheck/tests/pdb-realloc.vgtest
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- branches/COMPVBITS/memcheck/tests/pdb-realloc.vgtest =
(rev 0)
+++ branches/COMPVBITS/memcheck/tests/pdb-realloc.vgtest 2006-02-20 20:33=
:27 UTC (rev 5660)
@@ -0,0 +1,2 @@
+prog: pdb-realloc
+vgopts: -q
|