From a545bc15cc65b93d338809708caf23803bbb64cb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 3 Aug 2004 23:14:00 +0000 Subject: [PATCH] Tweaked sanity-checking: made function naming more consistent, removed unnecessarily global functions from vg_include.h, etc. Also tweaked printing of malloc stats. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@2562 --- coregrind/vg_from_ucode.c | 11 ++------ coregrind/vg_include.h | 12 ++++----- coregrind/vg_main.c | 13 ++++----- coregrind/vg_malloc2.c | 55 +++++++++++++++++++-------------------- coregrind/vg_proxylwp.c | 2 +- coregrind/vg_scheduler.c | 2 +- coregrind/vg_to_ucode.c | 14 ++-------- coregrind/vg_translate.c | 42 ++++++++++++++++++++++++++---- memcheck/mc_translate.c | 4 +-- 9 files changed, 84 insertions(+), 71 deletions(-) diff --git a/coregrind/vg_from_ucode.c b/coregrind/vg_from_ucode.c index 8dfb4f9bbf..fbb2a7c68c 100644 --- a/coregrind/vg_from_ucode.c +++ b/coregrind/vg_from_ucode.c @@ -4492,17 +4492,10 @@ UChar* VG_(emit_code) ( UCodeBlock* cb, /* for each uinstr ... */ for (i = 0; i < cb->used; i++) { UInstr* u = &cb->instrs[i]; + VG_(sanity_check_UInstr)( i, u ); if (cb->instrs[i].opcode != NOP) { - - /* Check on the sanity of this insn. */ - Bool sane = VG_(saneUInstr)( False, False, u ); - if (!sane) { - VG_(printf)("\ninsane instruction\n"); - VG_(up_UInstr)( i, u ); - } - vg_assert(sane); emitUInstr( cb, i, regs_live_before, - &sselive, &orig_eip, &curr_eip ); + &sselive, &orig_eip, &curr_eip ); } regs_live_before = u->regs_live_after; } diff --git a/coregrind/vg_include.h b/coregrind/vg_include.h index f79f3d1e5c..5b00ca6e28 100644 --- a/coregrind/vg_include.h +++ b/coregrind/vg_include.h @@ -379,9 +379,10 @@ extern void* VG_(arena_malloc_aligned) ( ArenaId aid, Int req_alignB, extern Int VG_(arena_payload_szB) ( ArenaId aid, void* payload ); -extern void VG_(mallocSanityCheckAll) ( void ); +extern void VG_(sanity_check_malloc_all) ( void ); extern void VG_(print_all_arena_stats) ( void ); + extern Bool VG_(is_empty_arena) ( ArenaId aid ); /* --------------------------------------------------------------------- @@ -1129,10 +1130,7 @@ struct _UCodeBlock { extern void VG_(translate) ( ThreadId tid, Addr orig_addr, Bool debugging ); -extern Bool VG_(saneUInstr) ( Bool beforeRA, Bool beforeLiveness, - UInstr* u ); -extern void VG_(saneUCodeBlock) ( UCodeBlock* cb ); -extern Bool VG_(saneUCodeBlockCalls) ( UCodeBlock* cb ); +extern void VG_(sanity_check_UInstr) ( UInt n, UInstr* u ); extern void VG_(print_reg_alloc_stats) ( void ); @@ -1229,7 +1227,7 @@ extern Bool VG_(have_ssestate); extern Bool VG_(logging_to_filedes); /* Sanity checks which may be done at any time. The scheduler decides when. */ -extern void VG_(do_sanity_checks) ( Bool force_expensive ); +extern void VG_(sanity_check_general) ( Bool force_expensive ); /* Address space */ extern Addr VG_(client_base); /* client address space limits */ @@ -1402,7 +1400,7 @@ extern Int VG_(proxy_resfd) ( void ); // FD something can select on to know // a syscall finished /* Sanity-check the whole proxy-LWP machinery */ -void VG_(proxy_sanity)(void); +void VG_(sanity_check_proxy)(void); /* Send a signal from a thread's proxy to the thread. This longjmps back into the proxy's main loop, so it doesn't return. */ diff --git a/coregrind/vg_main.c b/coregrind/vg_main.c index 7c8f50e62f..5ef5ea357d 100644 --- a/coregrind/vg_main.c +++ b/coregrind/vg_main.c @@ -254,12 +254,13 @@ static void print_all_stats ( void ) VG_(print_UInstr_histogram)(); // Memory stats - if (0) { + if (VG_(clo_verbosity) > 2) { VG_(message)(Vg_DebugMsg, ""); VG_(message)(Vg_DebugMsg, "------ Valgrind's internal memory use stats follow ------" ); - VG_(mallocSanityCheckAll)(); + VG_(sanity_check_malloc_all)(); VG_(print_all_arena_stats)(); + VG_(message)(Vg_DebugMsg, ""); VG_(message)(Vg_DebugMsg, "------ Valgrind's ExeContext management stats follow ------" ); VG_(print_ExeContext_stats)(); @@ -2577,7 +2578,7 @@ static void build_segment_map_callback /* A fast sanity check -- suitable for calling circa once per millisecond. */ -void VG_(do_sanity_checks) ( Bool force_expensive ) +void VG_(sanity_check_general) ( Bool force_expensive ) { VGP_PUSHCC(VgpCoreCheapSanity); @@ -2607,7 +2608,7 @@ void VG_(do_sanity_checks) ( Bool force_expensive ) VGP_PUSHCC(VgpCoreExpensiveSanity); sanity_slow_count++; - VG_(proxy_sanity)(); + VG_(sanity_check_proxy)(); # if 0 { void zzzmemscan(void); zzzmemscan(); } @@ -2633,7 +2634,7 @@ void VG_(do_sanity_checks) ( Bool force_expensive ) in the client's code can cause this to fail, so we don't do this check unless specially asked for. And because it's potentially very expensive. */ - VG_(mallocSanityCheckAll)(); + VG_(sanity_check_malloc_all)(); VGP_POPCC(VgpCoreExpensiveSanity); } VGP_POPCC(VgpCoreCheapSanity); @@ -3039,7 +3040,7 @@ int main(int argc, char **argv) SK_(fini)( exitcode ); - VG_(do_sanity_checks)( True /*include expensive checks*/ ); + VG_(sanity_check_general)( True /*include expensive checks*/ ); if (VG_(clo_verbosity) > 1) print_all_stats(); diff --git a/coregrind/vg_malloc2.c b/coregrind/vg_malloc2.c index 9999295fca..34dd228e5b 100644 --- a/coregrind/vg_malloc2.c +++ b/coregrind/vg_malloc2.c @@ -224,15 +224,15 @@ void arena_init ( Arena* a, Char* name, /* Print vital stats for an arena. */ -void VG_(show_all_arena_stats) ( void ) +void VG_(print_all_arena_stats) ( void ) { Int i; for (i = 0; i < VG_N_ARENAS; i++) { VG_(message)(Vg_DebugMsg, - "Arena `%s': %7d max useful, %7d mmap'd, %7d current useful", + "AR %8s: %8d mmap'd, %8d/%8d max/curr", vg_arena[i].name, - vg_arena[i].bytes_on_loan_max, vg_arena[i].bytes_mmaped, + vg_arena[i].bytes_on_loan_max, vg_arena[i].bytes_on_loan ); } @@ -293,7 +293,7 @@ void ensure_mm_init ( void ) init_done = True; # ifdef DEBUG_MALLOC - VG_(mallocSanityCheckAll)(); + VG_(sanity_check_malloc_all)(); # endif } @@ -807,7 +807,7 @@ void ppSuperblocks ( Arena* a ) /* Sanity check both the superblocks and the chains. */ -static void mallocSanityCheckArena ( ArenaId aid ) +static void sanity_check_malloc_arena ( ArenaId aid ) { Int i, superblockctr, b_bszW, b_pszW, blockctr_sb, blockctr_li; Int blockctr_sb_free, listno, list_min_pszW, list_max_pszW; @@ -818,12 +818,11 @@ static void mallocSanityCheckArena ( ArenaId aid ) UInt arena_bytes_on_loan; Arena* a; -# define BOMB VG_(core_panic)("mallocSanityCheckArena") +# define BOMB VG_(core_panic)("sanity_check_malloc_arena") a = arenaId_to_ArenaP(aid); - /* First, traverse all the superblocks, inspecting the chunks in - each. */ + /* First, traverse all the superblocks, inspecting the chunks in each. */ superblockctr = blockctr_sb = blockctr_sb_free = 0; arena_bytes_on_loan = 0; sb = a->sblocks; @@ -837,14 +836,14 @@ static void mallocSanityCheckArena ( ArenaId aid ) b = &sb->payload_words[i]; b_bszW = get_bszW_lo(b); if (!blockSane(a, b)) { - VG_(printf)("mallocSanityCheckArena: sb %p, block %d (bszW %d): " + VG_(printf)("sanity_check_malloc_arena: sb %p, block %d (bszW %d): " " BAD\n", sb, i, b_bszW ); BOMB; } thisFree = !is_inuse_bszW(b_bszW); if (thisFree && lastWasFree) { - VG_(printf)("mallocSanityCheckArena: sb %p, block %d (bszW %d): " + VG_(printf)("sanity_check_malloc_arena: sb %p, block %d (bszW %d): " "UNMERGED FREES\n", sb, i, b_bszW ); BOMB; @@ -856,7 +855,7 @@ static void mallocSanityCheckArena ( ArenaId aid ) i += mk_plain_bszW(b_bszW); } if (i > sb->n_payload_words) { - VG_(printf)( "mallocSanityCheckArena: sb %p: last block " + VG_(printf)( "sanity_check_malloc_arena: sb %p: last block " "overshoots end\n", sb); BOMB; } @@ -865,7 +864,7 @@ static void mallocSanityCheckArena ( ArenaId aid ) if (arena_bytes_on_loan != a->bytes_on_loan) { VG_(printf)( - "mallocSanityCheckArena: a->bytes_on_loan %d, " + "sanity_check_malloc_arena: a->bytes_on_loan %d, " "arena_bytes_on_loan %d: " "MISMATCH\n", a->bytes_on_loan, arena_bytes_on_loan); ppSuperblocks(a); @@ -885,7 +884,7 @@ static void mallocSanityCheckArena ( ArenaId aid ) b_prev = b; b = get_next_p(b); if (get_prev_p(b) != b_prev) { - VG_(printf)( "mallocSanityCheckArena: list %d at %p: " + VG_(printf)( "sanity_check_malloc_arena: list %d at %p: " "BAD LINKAGE\n", listno, b ); BOMB; @@ -893,7 +892,7 @@ static void mallocSanityCheckArena ( ArenaId aid ) b_pszW = bszW_to_pszW(a, mk_plain_bszW(get_bszW_lo(b))); if (b_pszW < list_min_pszW || b_pszW > list_max_pszW) { VG_(printf)( - "mallocSanityCheckArena: list %d at %p: " + "sanity_check_malloc_arena: list %d at %p: " "WRONG CHAIN SIZE %d (%d, %d)\n", listno, b, b_pszW, list_min_pszW, list_max_pszW ); BOMB; @@ -905,30 +904,30 @@ static void mallocSanityCheckArena ( ArenaId aid ) if (blockctr_sb_free != blockctr_li) { VG_(printf)( - "mallocSanityCheckArena: BLOCK COUNT MISMATCH " + "sanity_check_malloc_arena: BLOCK COUNT MISMATCH " "(via sbs %d, via lists %d)\n", blockctr_sb_free, blockctr_li ); ppSuperblocks(a); BOMB; } - VG_(message)(Vg_DebugMsg, - "mSC [%8s]: %2d sbs, %5d tot bs, %4d/%-4d free bs, " - "%2d lists, %7d mmap, %7d loan", - a->name, - superblockctr, - blockctr_sb, blockctr_sb_free, blockctr_li, - VG_N_MALLOC_LISTS, - a->bytes_mmaped, a->bytes_on_loan); + if (VG_(clo_verbosity) > 2) + VG_(message)(Vg_DebugMsg, + "AR %8s: %2d sbs, %5d bs, %2d/%-2d free bs, " + "%7d mmap, %7d loan", + a->name, + superblockctr, + blockctr_sb, blockctr_sb_free, blockctr_li, + a->bytes_mmaped, a->bytes_on_loan); # undef BOMB } -void VG_(mallocSanityCheckAll) ( void ) +void VG_(sanity_check_malloc_all) ( void ) { Int i; for (i = 0; i < VG_N_ARENAS; i++) - mallocSanityCheckArena ( i ); + sanity_check_malloc_arena ( i ); } @@ -1076,7 +1075,7 @@ void* VG_(arena_malloc) ( ArenaId aid, Int req_pszB ) a->bytes_on_loan_max = a->bytes_on_loan; # ifdef DEBUG_MALLOC - mallocSanityCheckArena(aid); + sanity_check_malloc_arena(aid); # endif VGP_POPCC(VgpMalloc); @@ -1164,7 +1163,7 @@ void VG_(arena_free) ( ArenaId aid, void* ptr ) } # ifdef DEBUG_MALLOC - mallocSanityCheckArena(aid); + sanity_check_malloc_arena(aid); # endif VGP_POPCC(VgpMalloc); @@ -1305,7 +1304,7 @@ void* VG_(arena_malloc_aligned) ( ArenaId aid, Int req_alignB, Int req_pszB ) a->bytes_on_loan_max = a->bytes_on_loan; # ifdef DEBUG_MALLOC - mallocSanityCheckArena(aid); + sanity_check_malloc_arena(aid); # endif VGP_POPCC(VgpMalloc); diff --git a/coregrind/vg_proxylwp.c b/coregrind/vg_proxylwp.c index 3d7e88d01f..831469b069 100644 --- a/coregrind/vg_proxylwp.c +++ b/coregrind/vg_proxylwp.c @@ -1314,7 +1314,7 @@ Int VG_(sys_issue)(int tid) } /* Relatively expensive sanity tests for the syscall machinery */ -void VG_(proxy_sanity)(void) +void VG_(sanity_check_proxy)(void) { Int tid; Bool sane = True; diff --git a/coregrind/vg_scheduler.c b/coregrind/vg_scheduler.c index 3449ebf6e8..0847c5d119 100644 --- a/coregrind/vg_scheduler.c +++ b/coregrind/vg_scheduler.c @@ -865,7 +865,7 @@ VgSchedReturnCode VG_(scheduler) ( Int* exitcode ) Be paranoid. Always a good idea. */ stage1: scheduler_sanity(); - VG_(do_sanity_checks)( False ); + VG_(sanity_check_general)( False ); /* ======================= Phase 1 of 3 ======================= Handle I/O completions and signals. This may change the diff --git a/coregrind/vg_to_ucode.c b/coregrind/vg_to_ucode.c index f05e3f32af..f0c34adea2 100644 --- a/coregrind/vg_to_ucode.c +++ b/coregrind/vg_to_ucode.c @@ -7322,12 +7322,9 @@ static Addr disInstr ( UCodeBlock* cb, Addr eip, Bool* isEnd ) /* All decode successes end up here. */ DIP("\n"); for (; first_uinstr < cb->used; first_uinstr++) { - Bool sane = VG_(saneUInstr)(True, True, &cb->instrs[first_uinstr]); - if (!sane) - VG_(up_UInstr)(first_uinstr, &cb->instrs[first_uinstr]); - else if (VG_(print_codegen)) + VG_(sanity_check_UInstr)( first_uinstr, &cb->instrs[first_uinstr] ); + if (VG_(print_codegen)) VG_(pp_UInstr)(first_uinstr, &cb->instrs[first_uinstr]); - vg_assert(sane); } return eip; } @@ -7341,7 +7338,6 @@ Int VG_(disBB) ( UCodeBlock* cb, Addr eip0 ) { Addr eip = eip0; Bool isEnd = False; - Bool block_sane; Int delta = 0; DIP("Original x86 code to UCode:\n\n"); @@ -7400,12 +7396,6 @@ Int VG_(disBB) ( UCodeBlock* cb, Addr eip0 ) /* Patch instruction size into final JMP. */ LAST_UINSTR(cb).extra4b = delta; - block_sane = VG_(saneUCodeBlockCalls)(cb); - if (!block_sane) { - VG_(pp_UCodeBlock)(cb, "block failing sanity check"); - vg_assert(block_sane); - } - return eip - eip0; } diff --git a/coregrind/vg_translate.c b/coregrind/vg_translate.c index c2efa4f037..6c9e8a1b1f 100644 --- a/coregrind/vg_translate.c +++ b/coregrind/vg_translate.c @@ -366,6 +366,11 @@ UInstr* VG_(get_last_instr) ( UCodeBlock* cb ) /*--- Sanity checking uinstrs. ---*/ /*------------------------------------------------------------*/ +// Global variables that indicate where we are in the translation of a basic +// block, and affect exactly how UInstrs are sanity-checked. +static Bool beforeRA = True; +static Bool beforeLiveness = True; + /* This seems as good a place as any to record some important stuff about ucode semantics. @@ -438,7 +443,7 @@ UInstr* VG_(get_last_instr) ( UCodeBlock* cb ) The size field should be 0 for insns for which it is meaningless, ie those which do not directly move/operate on data. */ -Bool VG_(saneUInstr) ( Bool beforeRA, Bool beforeLiveness, UInstr* u ) +static Bool is_sane_UInstr ( UInstr* u ) { # define LIT0 (u->lit32 == 0) # define LIT8 (((u->lit32) & 0xFFFFFF00) == 0) @@ -703,12 +708,23 @@ Bool VG_(saneUInstr) ( Bool beforeRA, Bool beforeLiveness, UInstr* u ) # undef XOTHER } -void VG_(saneUCodeBlock) ( UCodeBlock* cb ) +void VG_(sanity_check_UInstr)( UInt n, UInstr* u ) +{ + Bool sane = is_sane_UInstr(u); + if (!sane) { + VG_(printf)("\nInsane instruction:\n"); + VG_(pp_UInstr)(n, u); + VG_(up_UInstr)(n, u); + vg_assert(sane); + } +} + +static void sanity_check_UCodeBlock ( UCodeBlock* cb ) { Int i; for (i = 0; i < cb->used; i++) { - Bool sane = VG_(saneUInstr)(True, True, &cb->instrs[i]); + Bool sane = is_sane_UInstr(&cb->instrs[i]); if (!sane) { VG_(printf)("Instruction failed sanity check:\n"); VG_(up_UInstr)(i, &cb->instrs[i]); @@ -718,7 +734,7 @@ void VG_(saneUCodeBlock) ( UCodeBlock* cb ) } /* Sanity checks to do with CALLMs in UCodeBlocks. */ -Bool VG_(saneUCodeBlockCalls) ( UCodeBlock* cb ) +static Bool is_sane_UCodeBlockCalls ( UCodeBlock* cb ) { Int callm = 0; Int callm_s = 0; @@ -799,6 +815,13 @@ Bool VG_(saneUCodeBlockCalls) ( UCodeBlock* cb ) goto find_next_CALLM; } +static void sanity_check_UCodeBlockCalls( UCodeBlock* cb ) +{ + if ( ! is_sane_UCodeBlockCalls( cb ) ) { + VG_(pp_UCodeBlock)(cb, "block failing calls sanity check"); + VG_(core_panic)("bad block"); + } +} /*------------------------------------------------------------*/ /*--- Printing uinstrs. ---*/ @@ -2418,6 +2441,9 @@ void VG_(translate) ( ThreadId tid, Addr orig_addr, VGP_PUSHCC(VgpTranslate); + beforeRA = True; + beforeLiveness = True; + for (i = 0; i < VG_MAX_JUMPS; i++) jumps[i] = (UShort)-1; @@ -2485,6 +2511,10 @@ void VG_(translate) ( ThreadId tid, Addr orig_addr, VG_(print_codegen) = DECIDE_IF_PRINTING_CODEGEN_FOR_PHASE(1); VGP_PUSHCC(VgpToUCode); orig_size = VG_(disBB) ( cb, orig_addr ); + sanity_check_UCodeBlock ( cb ); + // Only sanity-check calls now because tools might remove the + // CALLM_[ES] pairs. + sanity_check_UCodeBlockCalls ( cb ); VGP_POPCC(VgpToUCode); /* Try and improve the code a bit. */ @@ -2502,7 +2532,7 @@ void VG_(translate) ( ThreadId tid, Addr orig_addr, cb = SK_(instrument) ( cb, orig_addr ); if (VG_(print_codegen)) VG_(pp_UCodeBlock) ( cb, "Instrumented UCode:" ); - VG_(saneUCodeBlock)( cb ); + sanity_check_UCodeBlock( cb ); VGP_POPCC(VgpInstrument); /* Add %ESP-update hooks if the tool requires them */ @@ -2517,12 +2547,14 @@ void VG_(translate) ( ThreadId tid, Addr orig_addr, VG_(print_codegen) = DECIDE_IF_PRINTING_CODEGEN_FOR_PHASE(4); VGP_PUSHCC(VgpRegAlloc); cb = vg_do_register_allocation ( cb ); + beforeRA = False; VGP_POPCC(VgpRegAlloc); /* Do post reg-alloc %e[acd]x liveness analysis (too boring to print * anything; results can be seen when emitting final code). */ VGP_PUSHCC(VgpLiveness); vg_realreg_liveness_analysis ( cb ); + beforeLiveness = False; VGP_POPCC(VgpLiveness); /* Emit final code */ diff --git a/memcheck/mc_translate.c b/memcheck/mc_translate.c index f089ae6559..af01e6d4fb 100644 --- a/memcheck/mc_translate.c +++ b/memcheck/mc_translate.c @@ -36,8 +36,8 @@ ------------------------------------------------------------------ */ /* Compare this with the restrictions on core instructions in - vg_translate.c:VG_(saneUInstr)(). Everything general said there applies - here too. + vg_translate.c:is_sane_UInstr(). Everything general said there + applies here too. */ Bool SK_(sane_XUInstr)(Bool beforeRA, Bool beforeLiveness, UInstr* u) { -- 2.47.2