From: Philippe Waroquiers Date: Thu, 4 Apr 2013 21:10:22 +0000 (+0000) Subject: Solve false negative for various malloc replaced functions arguments X-Git-Tag: svn/VALGRIND_3_9_0~323 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a819df9d0a4ae49dd782804ac9ba7a73412fcbf3;p=thirdparty%2Fvalgrind.git Solve false negative for various malloc replaced functions arguments The replacement functions are running on the simulated CPU. The code on the simulated CPU does not necessarily use all arguments. E.g. args can be ignored and/or only given to a NON SIMD call. The definedness of such 'unused' arguments will not be verified by memcheck. A call to 'trigger_memcheck_error_if_undefined' allows memcheck to detect such errors for the otherwise unused args. Apart of allowing memcheck to detect an error, the function trigger_memcheck_error_if_undefined has no effect and has a minimal cost for other tools replacing malloc functions. (suggestion of the 'no operation check' from Julian). tested on f12/x86, debian6/amd64, f18/ppc64 Note that some Darwin specific code has been modified in coregrind/m_replace_malloc/vg_replace_malloc.c. (Some of) this code has not been compiled (no access to a Darwin system). The code changed is trivial, so there is some chance it will compile and even maybe work. Added a new test verifying that various malloc related functions undefined args are triggering an error in memcheck. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13361 --- diff --git a/coregrind/m_replacemalloc/vg_replace_malloc.c b/coregrind/m_replacemalloc/vg_replace_malloc.c index 9173e3c6c7..1a032ec760 100644 --- a/coregrind/m_replacemalloc/vg_replace_malloc.c +++ b/coregrind/m_replacemalloc/vg_replace_malloc.c @@ -174,6 +174,7 @@ static UWord umulHW ( UWord u, UWord v ) */ static struct vg_mallocfunc_info info; static int init_done; +#define DO_INIT if (UNLIKELY(!init_done)) init() /* Startup hook - called as init section */ __attribute__((constructor)) @@ -194,6 +195,22 @@ static void init(void); replacing. */ +/* The replacement functions are running on the simulated CPU. + The code on the simulated CPU does not necessarily use + all arguments. E.g. args can be ignored and/or only given + to a NON SIMD call. + The definedness of such 'unused' arguments will not be verified + by memcheck. + A call to 'trigger_memcheck_error_if_undefined' allows + memcheck to detect such errors for the otherwise unused args. + Apart of allowing memcheck to detect an error, the function + trigger_memcheck_error_if_undefined has no effect and + has a minimal cost for other tools replacing malloc functions. +*/ +static inline void trigger_memcheck_error_if_undefined ( ULong x ) +{ + if (x == 0) __asm__ __volatile__( "" ::: "memory" ); +} /*---------------------- malloc ----------------------*/ @@ -207,7 +224,8 @@ static void init(void); { \ void* v; \ \ - if (!init_done) init(); \ + DO_INIT; \ + trigger_memcheck_error_if_undefined((ULong)n ); \ MALLOC_TRACE(#fnname "(%llu)", (ULong)n ); \ \ v = (void*)VALGRIND_NON_SIMD_CALL1( info.tl_##vg_replacement, n ); \ @@ -222,7 +240,9 @@ static void init(void); { \ void* v; \ \ - if (!init_done) init(); \ + DO_INIT; \ + trigger_memcheck_error_if_undefined((ULong) zone); \ + trigger_memcheck_error_if_undefined((ULong) n); \ MALLOC_TRACE(#fnname "(%p, %llu)", zone, (ULong)n ); \ \ v = (void*)VALGRIND_NON_SIMD_CALL1( info.tl_##vg_replacement, n ); \ @@ -242,7 +262,8 @@ static void init(void); { \ void* v; \ \ - if (!init_done) init(); \ + DO_INIT; \ + trigger_memcheck_error_if_undefined((ULong) n); \ MALLOC_TRACE(#fnname "(%llu)", (ULong)n ); \ \ v = (void*)VALGRIND_NON_SIMD_CALL1( info.tl_##vg_replacement, n ); \ @@ -421,7 +442,8 @@ static void init(void); void VG_REPLACE_FUNCTION_EZU(10040,soname,fnname) (void *zone, void *p); \ void VG_REPLACE_FUNCTION_EZU(10040,soname,fnname) (void *zone, void *p) \ { \ - if (!init_done) init(); \ + DO_INIT; \ + trigger_memcheck_error_if_undefined((ULong) zone); \ MALLOC_TRACE(#fnname "(%p, %p)\n", zone, p ); \ if (p == NULL) \ return; \ @@ -433,7 +455,7 @@ static void init(void); void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p); \ void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p) \ { \ - if (!init_done) init(); \ + DO_INIT; \ MALLOC_TRACE(#fnname "(%p)\n", p ); \ if (p == NULL) \ return; \ @@ -554,7 +576,10 @@ static void init(void); { \ void* v; \ \ - if (!init_done) init(); \ + DO_INIT; \ + trigger_memcheck_error_if_undefined((ULong) zone); \ + trigger_memcheck_error_if_undefined((ULong) nmemb); \ + trigger_memcheck_error_if_undefined((ULong) size); \ MALLOC_TRACE("zone_calloc(%p, %llu,%llu)", zone, (ULong)nmemb, (ULong)size ); \ \ v = (void*)VALGRIND_NON_SIMD_CALL2( info.tl_calloc, nmemb, size ); \ @@ -571,7 +596,7 @@ static void init(void); { \ void* v; \ \ - if (!init_done) init(); \ + DO_INIT; \ MALLOC_TRACE("calloc(%llu,%llu)", (ULong)nmemb, (ULong)size ); \ \ /* Protect against overflow. See bug 24078. (that bug number is @@ -613,7 +638,7 @@ static void init(void); { \ void* v; \ \ - if (!init_done) init(); \ + DO_INIT; \ MALLOC_TRACE("zone_realloc(%p,%p,%llu)", zone, ptrV, (ULong)new_size ); \ \ if (ptrV == NULL) \ @@ -640,7 +665,7 @@ static void init(void); { \ void* v; \ \ - if (!init_done) init(); \ + DO_INIT; \ MALLOC_TRACE("realloc(%p,%llu)", ptrV, (ULong)new_size ); \ \ if (ptrV == NULL) \ @@ -682,7 +707,9 @@ static void init(void); { \ void* v; \ \ - if (!init_done) init(); \ + DO_INIT; \ + trigger_memcheck_error_if_undefined((ULong) zone); \ + trigger_memcheck_error_if_undefined((ULong) n); \ MALLOC_TRACE("zone_memalign(%p, al %llu, size %llu)", \ zone, (ULong)alignment, (ULong)n ); \ \ @@ -707,7 +734,8 @@ static void init(void); { \ void* v; \ \ - if (!init_done) init(); \ + DO_INIT; \ + trigger_memcheck_error_if_undefined((ULong) n); \ MALLOC_TRACE("memalign(al %llu, size %llu)", \ (ULong)alignment, (ULong)n ); \ \ @@ -760,6 +788,7 @@ static void init(void); static int pszB = 0; \ if (pszB == 0) \ pszB = my_getpagesize(); \ + trigger_memcheck_error_if_undefined((ULong) zone); \ return VG_REPLACE_FUNCTION_EZU(10110,VG_Z_LIBC_SONAME,memalign) \ ((SizeT)pszB, size); \ } @@ -788,6 +817,8 @@ static void init(void); { \ /* In glibc-2.2.4, 1 denotes a successful return value for \ mallopt */ \ + trigger_memcheck_error_if_undefined((ULong) cmd); \ + trigger_memcheck_error_if_undefined((ULong) value); \ return 1; \ } @@ -831,6 +862,7 @@ static void init(void); { \ /* 0 denotes that malloc_trim() either wasn't able \ to do anything, or was not implemented */ \ + trigger_memcheck_error_if_undefined((ULong) pad); \ return 0; \ } @@ -891,7 +923,7 @@ static void init(void); { \ SizeT pszB; \ \ - if (!init_done) init(); \ + DO_INIT; \ MALLOC_TRACE("malloc_usable_size(%p)", p ); \ if (NULL == p) \ return 0; \ @@ -981,7 +1013,7 @@ static void panic(const char *str) struct vg_mallinfo VG_REPLACE_FUNCTION_EZU(10200,soname,fnname) ( void ) \ { \ static struct vg_mallinfo mi; \ - if (!init_done) init(); \ + DO_INIT; \ MALLOC_TRACE("mallinfo()\n"); \ (void)VALGRIND_NON_SIMD_CALL1( info.mallinfo, &mi ); \ return mi; \ @@ -1005,7 +1037,9 @@ static size_t my_malloc_size ( void* zone, void* ptr ) { /* Implement "malloc_size" by handing the request through to the tool's .tl_usable_size method. */ - if (!init_done) init(); + DO_INIT; + trigger_memcheck_error_if_undefined((ULong) zone); + trigger_memcheck_error_if_undefined((ULong) ptr); size_t res = (size_t)VALGRIND_NON_SIMD_CALL1( info.tl_malloc_usable_size, ptr); return res; @@ -1065,6 +1099,7 @@ ZONE_FROM_PTR(SO_SYN_MALLOC, malloc_zone_from_ptr); int VG_REPLACE_FUNCTION_EZU(10230,soname,fnname)(void* zone); \ int VG_REPLACE_FUNCTION_EZU(10230,soname,fnname)(void* zone) \ { \ + trigger_memcheck_error_if_undefined((ULong) zone); \ return 1; \ } diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index 0cec5f1603..2a88cfc55e 100644 --- a/memcheck/tests/Makefile.am +++ b/memcheck/tests/Makefile.am @@ -214,6 +214,7 @@ EXTRA_DIST = \ test-plo-yes.stderr.exp-le64 test-plo-yes.stderr.exp-le32 \ test-plo-no.stderr.exp-s390x-mvc \ trivialleak.stderr.exp trivialleak.vgtest trivialleak.stderr.exp2 \ + undef_malloc_args.stderr.exp undef_malloc_args.vgtest \ unit_libcbase.stderr.exp unit_libcbase.vgtest \ unit_oset.stderr.exp unit_oset.stdout.exp unit_oset.vgtest \ varinfo1.vgtest varinfo1.stdout.exp varinfo1.stderr.exp varinfo1.stderr.exp-ppc64 \ @@ -301,6 +302,7 @@ check_PROGRAMS = \ supp_unknown supp1 supp2 suppfree \ test-plo \ trivialleak \ + undef_malloc_args \ unit_libcbase unit_oset \ varinfo1 varinfo2 varinfo3 varinfo4 \ varinfo5 varinfo5so.so varinfo6 \ diff --git a/memcheck/tests/undef_malloc_args.c b/memcheck/tests/undef_malloc_args.c new file mode 100644 index 0000000000..9c3adb0e71 --- /dev/null +++ b/memcheck/tests/undef_malloc_args.c @@ -0,0 +1,57 @@ +#include +#include +#include "../memcheck.h" +int main (int argc, char*argv[]) +{ + size_t def_size = 1<<20; + char *p; + char *new_p; + + if (argc > 10000) def_size = def_size * 2; + + { + size_t size = def_size; + VALGRIND_MAKE_MEM_UNDEFINED(&size, 1); + p = malloc(size); + } + + VALGRIND_MAKE_MEM_UNDEFINED(&p, 1); + new_p = realloc(p, def_size); + + VALGRIND_MAKE_MEM_UNDEFINED(&new_p, 1); + new_p = realloc(new_p, def_size); + + VALGRIND_MAKE_MEM_UNDEFINED(&new_p, 1); + free (new_p); + + { + size_t nmemb = 1; + VALGRIND_MAKE_MEM_UNDEFINED(&nmemb, 1); + new_p = calloc(nmemb, def_size); + free (new_p); + } + + { + size_t alignment = 1; + VALGRIND_MAKE_MEM_UNDEFINED(&alignment, 1); + new_p = memalign(alignment, def_size); + free(new_p); + } + + { + size_t nmemb = 16; + size_t size = def_size; + VALGRIND_MAKE_MEM_UNDEFINED(&size, 1); + new_p = memalign(nmemb, size); + free(new_p); + } + + { + size_t size = def_size; + VALGRIND_MAKE_MEM_UNDEFINED(&size, 1); + new_p = valloc(size); + free (new_p); + } + + return 0; +} diff --git a/memcheck/tests/undef_malloc_args.stderr.exp b/memcheck/tests/undef_malloc_args.stderr.exp new file mode 100644 index 0000000000..6e558f5f58 --- /dev/null +++ b/memcheck/tests/undef_malloc_args.stderr.exp @@ -0,0 +1,33 @@ +Conditional jump or move depends on uninitialised value(s) + at 0x........: malloc (vg_replace_malloc.c:...) + by 0x........: main (undef_malloc_args.c:15) + +Conditional jump or move depends on uninitialised value(s) + at 0x........: realloc (vg_replace_malloc.c:...) + by 0x........: main (undef_malloc_args.c:19) + +Conditional jump or move depends on uninitialised value(s) + at 0x........: realloc (vg_replace_malloc.c:...) + by 0x........: main (undef_malloc_args.c:22) + +Conditional jump or move depends on uninitialised value(s) + at 0x........: free (vg_replace_malloc.c:...) + by 0x........: main (undef_malloc_args.c:25) + +Conditional jump or move depends on uninitialised value(s) + at 0x........: calloc (vg_replace_malloc.c:...) + by 0x........: main (undef_malloc_args.c:30) + +Conditional jump or move depends on uninitialised value(s) + at 0x........: memalign (vg_replace_malloc.c:...) + by 0x........: main (undef_malloc_args.c:37) + +Conditional jump or move depends on uninitialised value(s) + at 0x........: memalign (vg_replace_malloc.c:...) + by 0x........: main (undef_malloc_args.c:45) + +Conditional jump or move depends on uninitialised value(s) + at 0x........: memalign (vg_replace_malloc.c:...) + by 0x........: valloc (vg_replace_malloc.c:...) + by 0x........: main (undef_malloc_args.c:52) + diff --git a/memcheck/tests/undef_malloc_args.vgtest b/memcheck/tests/undef_malloc_args.vgtest new file mode 100644 index 0000000000..9b411d98ec --- /dev/null +++ b/memcheck/tests/undef_malloc_args.vgtest @@ -0,0 +1,2 @@ +vgopts: --leak-check=yes -q +prog: undef_malloc_args