Fix for bug #78048.

Problem was that the malloc-replacing tools (memcheck, addrcheck, massif,
helgrind) would assert if a too-big malloc was attempted.  Now they return 0 to
the client.  I also cleaned up the code handling heap-block-metadata in Massif
and Addrcheck/Memcheck a little.

This exposed a nasty bug in VG_(client_alloc)() which wasn't checking if
find_map_space() was succeeding before attempting an mmap().  Before I added
the check, very big mallocs (eg 2GB) for Addrcheck were overwriting the client
space at address 0 and causing crashes.

Added a regtest to all the affected skins for this.


git-svn-id: svn://svn.valgrind.org/valgrind/trunk@2462
This commit is contained in:
Nicholas Nethercote 2004-07-10 14:56:28 +00:00
parent fa46b67475
commit 854d2ec10e
22 changed files with 177 additions and 110 deletions

View File

@ -9,4 +9,6 @@ EXTRA_DIST = $(noinst_SCRIPTS) \
$(addsuffix .stderr.exp,$(INSN_TESTS)) \
$(addsuffix .stdout.exp,$(INSN_TESTS)) \
$(addsuffix .vgtest,$(INSN_TESTS)) \
overlap.stderr.exp overlap.stdout.exp overlap.vgtest
overlap.stderr.exp overlap.stdout.exp overlap.vgtest \
toobig-allocs.stderr.exp toobig-allocs.vgtest

View File

@ -0,0 +1,9 @@
Attempting too-big malloc()...
Attempting too-big mmap()...
ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
malloc/free: in use at exit: 0 bytes in 0 blocks.
malloc/free: 1 allocs, 0 frees, 2145386496 bytes allocated.
For a detailed leak analysis, rerun with: --leak-check=yes
For counts of detected errors, rerun with: -v

View File

@ -0,0 +1 @@
prog: ../../tests/toobig-allocs

View File

@ -326,12 +326,18 @@ Superblock* newSuperblock ( Arena* a, Int cszW )
while ((cszW % VKI_WORDS_PER_PAGE) > 0) cszW++;
if (a->clientmem) {
// client allocation -- return 0 to client if it fails
sb = (Superblock *)
VG_(client_alloc)(0, cszW * sizeof(Word),
VKI_PROT_READ|VKI_PROT_WRITE|VKI_PROT_EXEC, 0);
} else
VKI_PROT_READ|VKI_PROT_WRITE|VKI_PROT_EXEC, 0);
if (NULL == sb) {
return 0;
}
} else {
// non-client allocation -- abort if it fails
sb = VG_(get_memory_from_mmap) ( cszW * sizeof(Word),
"newSuperblock" );
}
sb->n_payload_words = cszW - 2;
a->bytes_mmaped += cszW * sizeof(Word);
if (0)
@ -1023,7 +1029,12 @@ void* VG_(arena_malloc) ( ArenaId aid, Int req_pszB )
if (lno == VG_N_MALLOC_LISTS) {
req_bszW = pszW_to_bszW(a, req_pszW);
new_sb = newSuperblock(a, req_bszW);
vg_assert(new_sb != NULL);
if (NULL == new_sb) {
// Should only fail if for client, otherwise, should have aborted
// already.
vg_assert(VG_AR_CLIENT == aid);
return NULL;
}
new_sb->next = a->sblocks;
a->sblocks = new_sb;
b = &(new_sb->payload_words[0]);

View File

@ -676,6 +676,7 @@ Bool VG_(is_addressable)(Addr p, Int size)
/*--- manage allocation of memory on behalf of the client ---*/
/*--------------------------------------------------------------------*/
// Returns 0 on failure.
Addr VG_(client_alloc)(Addr addr, UInt len, UInt prot, UInt flags)
{
len = PGROUNDUP(len);
@ -683,6 +684,10 @@ Addr VG_(client_alloc)(Addr addr, UInt len, UInt prot, UInt flags)
if (!(flags & SF_FIXED))
addr = VG_(find_map_space)(addr, len, True);
// Don't do the mapping if we couldn't find space!
if (0 == addr)
return 0;
flags |= SF_CORE;
if (VG_(mmap)((void *)addr, len, prot,

View File

@ -1838,6 +1838,9 @@ void* alloc_and_new_mem ( Int size, UInt alignment, Bool is_zeroed )
if (size < 0) return NULL;
p = (Addr)VG_(cli_malloc)(alignment, size);
if (!p) {
return NULL;
}
if (is_zeroed) VG_(memset)((void*)p, 0, size);
add_HG_Chunk ( VG_(get_current_or_recent_tid)(), p, size );
eraser_new_mem_heap( p, size, is_zeroed );

View File

@ -11,7 +11,8 @@ EXTRA_DIST = $(noinst_SCRIPTS) \
$(addsuffix .vgtest,$(INSN_TESTS)) \
race.stderr.exp race.vgtest \
race2.stderr.exp race2.vgtest \
readshared.stderr.exp readshared.vgtest
readshared.stderr.exp readshared.vgtest \
toobig-allocs.stderr.exp toobig-allocs.vgtest
check_PROGRAMS = \
allok deadlock inherit race race2 readshared

View File

@ -0,0 +1,4 @@
Attempting too-big malloc()...
Attempting too-big mmap()...

View File

@ -0,0 +1 @@
prog: ../../tests/toobig-allocs

View File

@ -664,49 +664,42 @@ void remove_HP_Chunk(HP_Chunk* hc, HP_Chunk** prev_chunks_next_ptr)
static void hp_census(void);
static __inline__
void new_block_meta ( void* p, Int size, Bool custom_malloc )
void* new_block ( void* p, Int size, UInt align, Bool is_zeroed )
{
HP_Chunk* hc;
VGP_PUSHCC(VgpCliMalloc);
if (0 == size) n_zero_allocs++;
// Make new HP_Chunk node, add to malloclist
hc = VG_(malloc)(sizeof(HP_Chunk));
hc->size = size;
hc->data = (Addr)p;
if (clo_heap) {
hc->where = get_XCon( VG_(get_current_or_recent_tid)(), custom_malloc );
if (size != 0)
update_XCon(hc->where, size);
} else {
hc->where = NULL; // paranoia
}
add_HP_Chunk( hc );
hp_census(); // do a census!
VGP_POPCC(VgpCliMalloc);
}
static __inline__
void* new_block ( Int size, UInt align, Bool is_zeroed )
{
void* p;
Bool custom_alloc = (NULL == p);
if (size < 0) return NULL;
VGP_PUSHCC(VgpCliMalloc);
// Update statistics
n_allocs++;
if (0 == size) n_zero_allocs++;
p = VG_(cli_malloc)( align, size );
if (is_zeroed) VG_(memset)(p, 0, size);
new_block_meta(p, size, /*custom_malloc*/False);
// Allocate and zero if necessary
if (!p) {
p = VG_(cli_malloc)( align, size );
if (!p) {
VGP_POPCC(VgpCliMalloc);
return NULL;
}
if (is_zeroed) VG_(memset)(p, 0, size);
}
// Make new HP_Chunk node, add to malloclist
hc = VG_(malloc)(sizeof(HP_Chunk));
hc->size = size;
hc->data = (Addr)p;
hc->where = NULL; // paranoia
if (clo_heap) {
hc->where = get_XCon( VG_(get_current_or_recent_tid)(), custom_alloc );
if (0 != size)
update_XCon(hc->where, size);
}
add_HP_Chunk( hc );
// do a census!
hp_census();
VGP_POPCC(VgpCliMalloc);
return p;
@ -715,60 +708,59 @@ void* new_block ( Int size, UInt align, Bool is_zeroed )
static __inline__
void die_block ( void* p, Bool custom_free )
{
HP_Chunk* hc;
HP_Chunk** remove_handle;
HP_Chunk *hc, **remove_handle;
VGP_PUSHCC(VgpCliMalloc);
// Update statistics
n_frees++;
hc = get_HP_Chunk ( p, &remove_handle );
// Remove HP_Chunk from malloclist
hc = get_HP_Chunk( p, &remove_handle );
if (hc == NULL)
return; // must have been a bogus free(), or p==NULL
sk_assert(hc->data == (Addr)p);
remove_HP_Chunk(hc, remove_handle);
if (clo_heap && hc->size != 0)
update_XCon(hc->where, -hc->size);
// Actually free the heap block
VG_(free)( hc );
// Actually free the heap block, if necessary
if (!custom_free)
VG_(cli_free)( p );
// Remove HP_Chunk from malloclist, destroy
remove_HP_Chunk(hc, remove_handle);
// do a census!
hp_census();
hp_census(); // do a census!
VG_(free)( hc );
VGP_POPCC(VgpCliMalloc);
}
void* SK_(malloc) ( Int n )
{
return new_block( n, VG_(clo_alignment), /*is_zeroed*/False );
return new_block( NULL, n, VG_(clo_alignment), /*is_zeroed*/False );
}
void* SK_(__builtin_new) ( Int n )
{
return new_block( n, VG_(clo_alignment), /*is_zeroed*/False );
return new_block( NULL, n, VG_(clo_alignment), /*is_zeroed*/False );
}
void* SK_(__builtin_vec_new) ( Int n )
{
return new_block( n, VG_(clo_alignment), /*is_zeroed*/False );
return new_block( NULL, n, VG_(clo_alignment), /*is_zeroed*/False );
}
void* SK_(calloc) ( Int m, Int size )
{
return new_block( m*size, VG_(clo_alignment), /*is_zeroed*/True );
return new_block( NULL, m*size, VG_(clo_alignment), /*is_zeroed*/True );
}
void *SK_(memalign)( Int align, Int n )
{
return new_block( n, align, False );
return new_block( NULL, n, align, False );
}
void SK_(free) ( void* p )
@ -1133,10 +1125,12 @@ Bool SK_(handle_client_request) ( ThreadId tid, UInt* argv, UInt* ret )
{
switch (argv[0]) {
case VG_USERREQ__MALLOCLIKE_BLOCK: {
void* res;
void* p = (void*)argv[1];
UInt sizeB = argv[2];
*ret = 0;
new_block_meta( p, sizeB, /*custom_malloc*/True );
res = new_block( p, sizeB, /*align -- ignored*/0, /*is_zeroed*/False );
sk_assert(res == p);
return True;
}
case VG_USERREQ__FREELIKE_BLOCK: {

View File

@ -1,6 +1,7 @@
noinst_SCRIPTS = filter_stderr
EXTRA_DIST = $(noinst_SCRIPTS) \
toobig-allocs.stderr.exp toobig-allocs.vgtest \
true_html.stderr.exp true_html.vgtest \
true_text.stderr.exp true_text.vgtest

View File

@ -0,0 +1,8 @@
Attempting too-big malloc()...
Attempting too-big mmap()...
Total spacetime:
heap:
heap admin:
stack(s):

View File

@ -0,0 +1 @@
prog: ../../tests/toobig-allocs

View File

@ -132,8 +132,7 @@ MAC_Chunk* MAC_(first_matching_freed_MAC_Chunk) ( Bool (*p)(MAC_Chunk*, void*),
/* Allocate its shadow chunk, put it on the appropriate list. */
static
MAC_Chunk* add_MAC_Chunk ( Addr p, UInt size, MAC_AllocKind kind,
VgHashTable table)
void add_MAC_Chunk ( Addr p, UInt size, MAC_AllocKind kind, VgHashTable table)
{
MAC_Chunk* mc;
@ -153,8 +152,6 @@ MAC_Chunk* add_MAC_Chunk ( Addr p, UInt size, MAC_AllocKind kind,
}
VG_(HT_add_node)( table, (VgHashNode*)mc );
return mc;
}
/*------------------------------------------------------------*/
@ -163,18 +160,27 @@ MAC_Chunk* add_MAC_Chunk ( Addr p, UInt size, MAC_AllocKind kind,
/* Allocate memory and note change in memory available */
__inline__
MAC_Chunk* MAC_(new_block) ( Addr p, UInt size,
UInt rzB, Bool is_zeroed, MAC_AllocKind kind,
VgHashTable table)
void* MAC_(new_block) ( Addr p, UInt size, UInt align, UInt rzB,
Bool is_zeroed, MAC_AllocKind kind, VgHashTable table)
{
MAC_Chunk *mc;
VGP_PUSHCC(VgpCliMalloc);
cmalloc_n_mallocs ++;
cmalloc_bs_mallocd += size;
mc = add_MAC_Chunk( p, size, kind, table );
// Allocate and zero if necessary
if (p) {
sk_assert(MAC_AllocCustom == kind);
} else {
sk_assert(MAC_AllocCustom != kind);
p = (Addr)VG_(cli_malloc)( align, size );
if (!p) {
VGP_POPCC(VgpCliMalloc);
return NULL;
}
if (is_zeroed) VG_(memset)((void*)p, 0, size);
}
add_MAC_Chunk( p, size, kind, table );
MAC_(ban_mem_heap)( p-rzB, rzB );
MAC_(new_mem_heap)( p, size, is_zeroed );
@ -182,7 +188,7 @@ MAC_Chunk* MAC_(new_block) ( Addr p, UInt size,
VGP_POPCC(VgpCliMalloc);
return mc;
return (void*)p;
}
void* SK_(malloc) ( Int n )
@ -191,11 +197,9 @@ void* SK_(malloc) ( Int n )
VG_(message)(Vg_UserMsg, "Warning: silly arg (%d) to malloc()", n );
return NULL;
} else {
Addr p = (Addr)VG_(cli_malloc)( VG_(clo_alignment), n );
MAC_(new_block) ( p, n, VG_(vg_malloc_redzone_szB),
/*is_zeroed*/False, MAC_AllocMalloc,
MAC_(malloc_list));
return (void*)p;
return MAC_(new_block) ( 0, n, VG_(clo_alignment),
VG_(vg_malloc_redzone_szB), /*is_zeroed*/False, MAC_AllocMalloc,
MAC_(malloc_list));
}
}
@ -205,11 +209,9 @@ void* SK_(__builtin_new) ( Int n )
VG_(message)(Vg_UserMsg, "Warning: silly arg (%d) to __builtin_new()", n);
return NULL;
} else {
Addr p = (Addr)VG_(cli_malloc)( VG_(clo_alignment), n );
MAC_(new_block) ( p, n, VG_(vg_malloc_redzone_szB),
/*is_zeroed*/False, MAC_AllocNew,
MAC_(malloc_list));
return (void*)p;
return MAC_(new_block) ( 0, n, VG_(clo_alignment),
VG_(vg_malloc_redzone_szB), /*is_zeroed*/False, MAC_AllocNew,
MAC_(malloc_list));
}
}
@ -220,11 +222,9 @@ void* SK_(__builtin_vec_new) ( Int n )
"Warning: silly arg (%d) to __builtin_vec_new()", n );
return NULL;
} else {
Addr p = (Addr)VG_(cli_malloc)( VG_(clo_alignment), n );
MAC_(new_block) ( p, n, VG_(vg_malloc_redzone_szB),
/*is_zeroed*/False, MAC_AllocNewVec,
MAC_(malloc_list));
return (void*)p;
return MAC_(new_block) ( 0, n, VG_(clo_alignment),
VG_(vg_malloc_redzone_szB), /*is_zeroed*/False, MAC_AllocNewVec,
MAC_(malloc_list));
}
}
@ -234,32 +234,22 @@ void* SK_(memalign) ( Int align, Int n )
VG_(message)(Vg_UserMsg, "Warning: silly arg (%d) to memalign()", n);
return NULL;
} else {
Addr p = (Addr)VG_(cli_malloc)( align, n );
MAC_(new_block) ( p, n, VG_(vg_malloc_redzone_szB),
/*is_zeroed*/False, MAC_AllocMalloc,
MAC_(malloc_list));
return (void*)p;
return MAC_(new_block) ( 0, n, align,
VG_(vg_malloc_redzone_szB), /*is_zeroed*/False, MAC_AllocMalloc,
MAC_(malloc_list));
}
}
void* SK_(calloc) ( Int nmemb, Int size1 )
{
Int n, i;
n = nmemb * size1;
if (nmemb < 0 || size1 < 0) {
VG_(message)(Vg_UserMsg, "Warning: silly args (%d,%d) to calloc()",
nmemb, size1 );
return NULL;
} else {
Addr p = (Addr)VG_(cli_malloc)( VG_(clo_alignment), n );
MAC_(new_block) ( p, n, VG_(vg_malloc_redzone_szB),
/*is_zeroed*/True, MAC_AllocMalloc,
MAC_(malloc_list));
for (i = 0; i < n; i++)
((UChar*)p)[i] = 0;
return (void*)p;
return MAC_(new_block) ( 0, nmemb*size1, VG_(clo_alignment),
VG_(vg_malloc_redzone_szB), /*is_zeroed*/True, MAC_AllocMalloc,
MAC_(malloc_list));
}
}
@ -479,7 +469,6 @@ void MAC_(mempool_alloc)(Addr pool, Addr addr, UInt size)
{
MAC_Mempool* mp;
MAC_Mempool** prev_next;
MAC_Chunk* mc;
mp = (MAC_Mempool*)VG_(HT_get_node) ( MAC_(mempool_list), (UInt)pool,
(VgHashNode***)&prev_next );
@ -491,8 +480,8 @@ void MAC_(mempool_alloc)(Addr pool, Addr addr, UInt size)
return;
}
mc = MAC_(new_block)(addr, size, mp->rzB, mp->is_zeroed, MAC_AllocCustom,
mp->chunks);
MAC_(new_block)(addr, size, /*ignored*/0, mp->rzB, mp->is_zeroed,
MAC_AllocCustom, mp->chunks);
}
void MAC_(mempool_free)(Addr pool, Addr addr)

View File

@ -880,8 +880,8 @@ Bool MAC_(handle_common_client_requests)(ThreadId tid, UInt* arg, UInt* ret )
UInt rzB = arg[3];
Bool is_zeroed = (Bool)arg[4];
MAC_(new_block) ( p, sizeB, rzB, is_zeroed, MAC_AllocCustom,
MAC_(malloc_list) );
MAC_(new_block) ( p, sizeB, /*ignored*/0, rzB, is_zeroed,
MAC_AllocCustom, MAC_(malloc_list) );
return True;
}
case VG_USERREQ__FREELIKE_BLOCK: {

View File

@ -317,9 +317,9 @@ extern void MAC_(clear_MAC_Error) ( MAC_Error* err_extra );
extern Bool MAC_(shared_recognised_suppression) ( Char* name, Supp* su );
extern MAC_Chunk* MAC_(new_block) ( Addr p, UInt size, UInt rzB,
Bool is_zeroed, MAC_AllocKind kind,
VgHashTable table);
extern void* MAC_(new_block) ( Addr p, UInt size, UInt align, UInt rzB,
Bool is_zeroed, MAC_AllocKind kind,
VgHashTable table);
extern void MAC_(handle_free) ( Addr p, UInt rzB, MAC_AllocKind kind );
extern void MAC_(create_mempool)(Addr pool, UInt rzB, Bool is_zeroed);

View File

@ -64,6 +64,7 @@ EXTRA_DIST = $(noinst_SCRIPTS) \
supp2.stderr.exp supp2.vgtest \
supp.supp \
suppfree.stderr.exp suppfree.vgtest \
toobig-allocs.stderr.exp toobig-allocs.vgtest \
trivialleak.stderr.exp trivialleak.vgtest \
tronical.stderr.exp tronical.vgtest \
weirdioctl.stderr.exp weirdioctl.stdout.exp weirdioctl.vgtest \

View File

@ -0,0 +1,9 @@
Attempting too-big malloc()...
Attempting too-big mmap()...
ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
malloc/free: in use at exit: 0 bytes in 0 blocks.
malloc/free: 1 allocs, 0 frees, 2145386496 bytes allocated.
For a detailed leak analysis, rerun with: --leak-check=yes
For counts of detected errors, rerun with: -v

View File

@ -0,0 +1 @@
prog: ../../tests/toobig-allocs

View File

@ -2,4 +2,5 @@ Makefile.in
Makefile
cputest
vg_regtest
toobig-allocs
true

View File

@ -12,13 +12,15 @@ noinst_SCRIPTS = \
EXTRA_DIST = $(noinst_SCRIPTS)
check_PROGRAMS = \
true \
cputest
cputest \
toobig-allocs \
true
AM_CFLAGS = $(WERROR) -Winline -Wall -Wshadow -g
AM_CXXFLAGS = $(AM_CFLAGS)
# generic C ones
true_SOURCES = true.c
cputest_SOURCES = cputest.c
cputest_SOURCES = cputest.c
toobig_allocs_SOURCES = toobig-allocs.c
true_SOURCES = true.c

23
tests/toobig-allocs.c Normal file
View File

@ -0,0 +1,23 @@
#include <stdlib.h>
#include <sys/mman.h>
#include <stdio.h>
int main(void)
{
void *p;
int size = 2 * 1023 * 1024 * 1024; // just under 2^31 (2GB)
fprintf(stderr, "Attempting too-big malloc()...\n");
p = malloc(size); // way too big!
if (p)
fprintf(stderr, "huge malloc() succeeded??\n");
fprintf(stderr, "Attempting too-big mmap()...\n");
p = mmap( 0, size, PROT_READ|PROT_WRITE|PROT_EXEC,
MAP_PRIVATE|MAP_ANON, -1, 0 );
if (-1 != (int)p)
fprintf(stderr, "huge mmap() succeeded??\n");
return 0;
}