From: Julian Seward Date: Mon, 24 Oct 2011 05:59:54 +0000 (+0000) Subject: Change the behaviour of VALGRIND_CHECK_MEM_IS_DEFINED slightly, so X-Git-Tag: svn/VALGRIND_3_7_0~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=daa6c4607bbf5908e6aa80c71dc516668a606a6f;p=thirdparty%2Fvalgrind.git 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 --- diff --git a/memcheck/mc_main.c b/memcheck/mc_main.c index a69845fe05..ab6a15703d 100644 --- a/memcheck/mc_main.c +++ b/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; } diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index 52582e3e1a..327368c694 100644 --- a/memcheck/tests/Makefile.am +++ b/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 \ diff --git a/memcheck/tests/holey_buffer_too_small.c b/memcheck/tests/holey_buffer_too_small.c new file mode 100644 index 0000000000..ab420e999c --- /dev/null +++ b/memcheck/tests/holey_buffer_too_small.c @@ -0,0 +1,43 @@ + +#include +#include + +#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; +} diff --git a/memcheck/tests/holey_buffer_too_small.stderr.exp b/memcheck/tests/holey_buffer_too_small.stderr.exp new file mode 100644 index 0000000000..79e55e29b0 --- /dev/null +++ b/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) + diff --git a/memcheck/tests/holey_buffer_too_small.stdout.exp b/memcheck/tests/holey_buffer_too_small.stdout.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/memcheck/tests/holey_buffer_too_small.vgtest b/memcheck/tests/holey_buffer_too_small.vgtest new file mode 100644 index 0000000000..4dae58c7a3 --- /dev/null +++ b/memcheck/tests/holey_buffer_too_small.vgtest @@ -0,0 +1,2 @@ +prog: holey_buffer_too_small +vgopts: -q --track-origins=yes