From: Philippe Waroquiers Date: Thu, 5 Apr 2012 22:44:36 +0000 (+0000) Subject: TCHAIN: remove caused_discard* argument to VG_(translate) X-Git-Tag: svn/VALGRIND_3_8_0~350^2~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=356c41be65f35e442ccd0918872c69148759cb07;p=thirdparty%2Fvalgrind.git TCHAIN: remove caused_discard* argument to VG_(translate) This is the followup to rev 12488. With this revision, translation chaining is not done if the translation with 'from address' is not existing anymore (discarded or erased). The assumption documented in 12488 comment has been checked by: * first reproduce a crash in Firefox when always setting caused discard to False * then upgrade to rev 12488 * with this upgrade, no crash anymore. => this verifies that the caused discard logic is properly replaced by revision 12488. So, the caused discard logic can be removed. git-svn-id: svn://svn.valgrind.org/valgrind/branches/TCHAIN@12492 --- diff --git a/coregrind/m_errormgr.c b/coregrind/m_errormgr.c index 788041b443..44976a804b 100644 --- a/coregrind/m_errormgr.c +++ b/coregrind/m_errormgr.c @@ -966,8 +966,7 @@ void VG_(show_all_errors) ( Int verbosity, Bool xml ) if ((i+1 == VG_(clo_dump_error))) { StackTrace ips = VG_(get_ExeContext_StackTrace)(p_min->where); - VG_(translate) ( NULL/*caused_discardP*/, - 0 /* dummy ThreadId; irrelevant due to debugging*/, + VG_(translate) ( 0 /* dummy ThreadId; irrelevant due to debugging*/, ips[0], /*debugging*/True, 0xFE/*verbosity*/, /*bbs_done*/0, /*allow redir?*/True); diff --git a/coregrind/m_gdbserver/server.c b/coregrind/m_gdbserver/server.c index 8e58589919..2736419c8c 100644 --- a/coregrind/m_gdbserver/server.c +++ b/coregrind/m_gdbserver/server.c @@ -310,8 +310,7 @@ int handle_gdb_valgrind_command (char* mon, OutputSink* sink_wanted_at_return) address = thumb_pc (address); # endif - VG_(translate) ( NULL/*caused_discardP*/, - 0 /* dummy ThreadId; irrelevant due to debugging*/, + VG_(translate) ( 0 /* dummy ThreadId; irrelevant due to debugging*/, address, /*debugging*/True, (Int) vex_verbosity, diff --git a/coregrind/m_main.c b/coregrind/m_main.c index 3bddb47926..057267bbc8 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -1373,8 +1373,7 @@ void show_BB_profile ( BBProfEntry tops[], UInt n_tops, ULong score_total ) score_here, buf_here, tops[r].addr, name ); VG_(printf)("\n"); VG_(discard_translations)(tops[r].addr, 1, "bb profile"); - VG_(translate)(NULL/*caused_discardP*/, - 0, tops[r].addr, True, VG_(clo_profile_flags), 0, True); + VG_(translate)(0, tops[r].addr, True, VG_(clo_profile_flags), 0, True); VG_(printf)("=-=-=-=-=-=-=-=-=-=-=-=-=-= end BB rank %d " "=-=-=-=-=-=-=-=-=-=-=-=-=-=\n\n", r); } diff --git a/coregrind/m_scheduler/scheduler.c b/coregrind/m_scheduler/scheduler.c index 6326224972..e52ab7f1aa 100644 --- a/coregrind/m_scheduler/scheduler.c +++ b/coregrind/m_scheduler/scheduler.c @@ -68,7 +68,6 @@ - move do_cacheflush out of m_transtab - more economical unchaining when nuking an entire sector - ditto w.r.t. cache flushes - - add comments about caused_discard to handle_chain_me() - verify case of 2 paths from A to B - check -- is IP_AT_SYSCALL still right? */ @@ -965,8 +964,7 @@ static void handle_tt_miss ( ThreadId tid ) ip, True/*upd_fast_cache*/ ); if (UNLIKELY(!found)) { /* Not found; we need to request a translation. */ - if (VG_(translate)( NULL/*caused_discardP*/, - tid, ip, /*debug*/False, 0/*not verbose*/, + if (VG_(translate)( tid, ip, /*debug*/False, 0/*not verbose*/, bbs_done, True/*allow redirection*/ )) { found = VG_(search_transtab)( NULL, NULL, NULL, ip, True ); @@ -989,14 +987,12 @@ void handle_chain_me ( ThreadId tid, void* place_to_chain, Bool toFastEP ) Addr ip = VG_(get_IP)(tid); UInt to_sNo = (UInt)-1; UInt to_tteNo = (UInt)-1; - Bool caused_discard = False; found = VG_(search_transtab)( NULL, &to_sNo, &to_tteNo, ip, False/*dont_upd_fast_cache*/ ); if (!found) { /* Not found; we need to request a translation. */ - if (VG_(translate)( &caused_discard, - tid, ip, /*debug*/False, 0/*not verbose*/, + if (VG_(translate)( tid, ip, /*debug*/False, 0/*not verbose*/, bbs_done, True/*allow redirection*/ )) { found = VG_(search_transtab)( NULL, &to_sNo, &to_tteNo, ip, False ); @@ -1017,9 +1013,8 @@ void handle_chain_me ( ThreadId tid, void* place_to_chain, Bool toFastEP ) /* So, finally we know where to patch through to. Do the patching and update the various admin tables that allow it to be undone in the case that the destination block gets deleted. */ - if (!caused_discard) - VG_(tt_tc_do_chaining)( place_to_chain, - to_sNo, to_tteNo, toFastEP ); + VG_(tt_tc_do_chaining)( place_to_chain, + to_sNo, to_tteNo, toFastEP ); } static void handle_syscall(ThreadId tid, UInt trc) @@ -1068,8 +1063,7 @@ void handle_noredir_jump ( /*OUT*/HWord* two_words, Bool found = VG_(search_unredir_transtab)( &hcode, ip ); if (!found) { /* Not found; we need to request a translation. */ - if (VG_(translate)( NULL/*caused_discardP*/, - tid, ip, /*debug*/False, 0/*not verbose*/, bbs_done, + if (VG_(translate)( tid, ip, /*debug*/False, 0/*not verbose*/, bbs_done, False/*NO REDIRECTION*/ )) { found = VG_(search_unredir_transtab)( &hcode, ip ); diff --git a/coregrind/m_translate.c b/coregrind/m_translate.c index 795c000e84..8c572fb307 100644 --- a/coregrind/m_translate.c +++ b/coregrind/m_translate.c @@ -1260,18 +1260,14 @@ typedef instead of the normal one. TID is the identity of the thread requesting this translation. - - *caused_discardP returns whether or not this translation resulting - in code being dumped from the main translation cache in order to - make space for the new translation. */ -Bool VG_(translate) ( /*OUT*/Bool* caused_discardP, - ThreadId tid, - Addr64 nraddr, - Bool debugging_translation, - Int debugging_verbosity, - ULong bbs_done, - Bool allow_redirection ) + +Bool VG_(translate) ( ThreadId tid, + Addr64 nraddr, + Bool debugging_translation, + Int debugging_verbosity, + ULong bbs_done, + Bool allow_redirection ) { Addr64 addr; T_Kind kind; @@ -1285,9 +1281,8 @@ Bool VG_(translate) ( /*OUT*/Bool* caused_discardP, VexTranslateResult tres; VgCallbackClosure closure; - if (caused_discardP) *caused_discardP = False; - /* Make sure Vex is initialised right. */ + static Bool vex_init_done = False; if (!vex_init_done) { @@ -1608,16 +1603,13 @@ Bool VG_(translate) ( /*OUT*/Bool* caused_discardP, // Note that we use nraddr (the non-redirected address), not // addr, which might have been changed by the redirection - Bool caused_discard - = VG_(add_to_transtab)( &vge, - nraddr, - (Addr)(&tmpbuf[0]), - tmpbuf_used, - tres.n_sc_extents > 0, - tres.offs_profInc, - vex_arch ); - if (caused_discardP) - *caused_discardP = caused_discard; + VG_(add_to_transtab)( &vge, + nraddr, + (Addr)(&tmpbuf[0]), + tmpbuf_used, + tres.n_sc_extents > 0, + tres.offs_profInc, + vex_arch ); } else { vg_assert(tres.offs_profInc == -1); /* -1 == unset */ VG_(add_to_unredir_transtab)( &vge, diff --git a/coregrind/m_transtab.c b/coregrind/m_transtab.c index 0a31fda2d2..a8e5eb512c 100644 --- a/coregrind/m_transtab.c +++ b/coregrind/m_transtab.c @@ -728,8 +728,6 @@ void VG_(tt_tc_do_chaining) ( void* from__patch_addr, vg_assert( (UChar*)host_code >= (UChar*)sectors[to_sNo].tc ); vg_assert( (UChar*)host_code <= (UChar*)sectors[to_sNo].tc_next + sizeof(ULong) - 1 ); - // stay sane -- the patch src is in some sector's code cache - vg_assert( is_in_the_main_TC(from__patch_addr) ); /* Find the TTEntry for the from__ code. This isn't simple since we only know the patch address, which is going to be somewhere @@ -1290,15 +1288,11 @@ static void invalidateFastCache ( void ) n_fast_flushes++; } -/* Returns True if the sector has been used before (hence, if we have - to eject existing code in it), False if it's never been used - before. */ -static Bool initialiseSector ( Int sno ) +static void initialiseSector ( Int sno ) { Int i; SysRes sres; Sector* sec; - Bool has_been_used_before = False; vg_assert(isValidSector(sno)); { Bool sane = sanity_check_sector_search_order(); @@ -1365,7 +1359,6 @@ static Bool initialiseSector ( Int sno ) /* Sector has been used before. Dump the old contents. */ VG_(debugLog)(1,"transtab", "recycle sector %d\n", sno); - has_been_used_before = True; vg_assert(sec->tt != NULL); vg_assert(sec->tc_next != NULL); n_dump_count += sec->tt_n_inuse; @@ -1434,8 +1427,6 @@ VG_(printf)("QQQ unlink-entire-sector: %d END\n", sno); { Bool sane = sanity_check_sector_search_order(); vg_assert(sane); } - - return has_been_used_before; } @@ -1444,11 +1435,8 @@ VG_(printf)("QQQ unlink-entire-sector: %d END\n", sno); pre: youngest_sector points to a valid (although possibly full) sector. - - Returns True if the call caused any existing translation(s) to get - thrown away in order to make space for this one. */ -Bool VG_(add_to_transtab)( VexGuestExtents* vge, +void VG_(add_to_transtab)( VexGuestExtents* vge, Addr64 entry, AddrH code, UInt code_len, @@ -1461,11 +1449,6 @@ Bool VG_(add_to_transtab)( VexGuestExtents* vge, UChar* srcP; UChar* dstP; - /* We need to tell the caller whether this call caused any code to - be thrown away due to the TC becoming full, and hence the oldest - Sector to be emptied out and recycled. */ - Bool caused_code_discarding = False; - vg_assert(init_done); vg_assert(vge->n_used >= 1 && vge->n_used <= 3); @@ -1485,10 +1468,8 @@ Bool VG_(add_to_transtab)( VexGuestExtents* vge, y = youngest_sector; vg_assert(isValidSector(y)); - if (sectors[y].tc == NULL) { - Bool used_before = initialiseSector(y); - vg_assert(!used_before); - } + if (sectors[y].tc == NULL) + initialiseSector(y); /* Try putting the translation in this sector. */ reqdQ = (code_len + 7) >> 3; @@ -1518,8 +1499,7 @@ Bool VG_(add_to_transtab)( VexGuestExtents* vge, if (youngest_sector >= N_SECTORS) youngest_sector = 0; y = youngest_sector; - caused_code_discarding = initialiseSector(y); - + initialiseSector(y); } /* Be sure ... */ @@ -1602,8 +1582,6 @@ Bool VG_(add_to_transtab)( VexGuestExtents* vge, /* Note the eclass numbers for this translation. */ upd_eclasses_after_add( §ors[y], i ); - - return caused_code_discarding; } diff --git a/coregrind/pub_core_translate.h b/coregrind/pub_core_translate.h index 3182f4f796..c6c24055d7 100644 --- a/coregrind/pub_core_translate.h +++ b/coregrind/pub_core_translate.h @@ -37,13 +37,12 @@ //-------------------------------------------------------------------- extern -Bool VG_(translate) ( /*OUT*/Bool* caused_discardP, - ThreadId tid, - Addr64 orig_addr, - Bool debugging_translation, - Int debugging_verbosity, - ULong bbs_done, - Bool allow_redirection ); +Bool VG_(translate) ( ThreadId tid, + Addr64 orig_addr, + Bool debugging_translation, + Int debugging_verbosity, + ULong bbs_done, + Bool allow_redirection ); extern void VG_(print_translation_stats) ( void ); diff --git a/coregrind/pub_core_transtab.h b/coregrind/pub_core_transtab.h index 52dc5a7eee..046bc21a95 100644 --- a/coregrind/pub_core_transtab.h +++ b/coregrind/pub_core_transtab.h @@ -56,7 +56,7 @@ extern __attribute__((aligned(16))) extern void VG_(init_tt_tc) ( void ); extern -Bool VG_(add_to_transtab)( VexGuestExtents* vge, +void VG_(add_to_transtab)( VexGuestExtents* vge, Addr64 entry, AddrH code, UInt code_len,