From: Philippe Waroquiers Date: Sun, 13 Jan 2013 13:59:17 +0000 (+0000) Subject: Avoid to record execontext used for origin tracking when --trac-origins=no X-Git-Tag: svn/VALGRIND_3_9_0~456 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8817a79c9d7f8034c190986baed12faac08e39e3;p=thirdparty%2Fvalgrind.git Avoid to record execontext used for origin tracking when --trac-origins=no All calls to VG_(unknown_SP_update) were recording an execontext of one IP, useful only for track origin. This patch implements splits VG_(unknown_SP_update) in two different functions VG_(unknown_SP_update_w_ECU) (doing origin tracking) and VG_(unknown_SP_update) (not doing origin tracking). git-svn-id: svn://svn.valgrind.org/valgrind/trunk@13225 --- diff --git a/coregrind/m_scheduler/scheduler.c b/coregrind/m_scheduler/scheduler.c index b3941f2f60..248dc35951 100644 --- a/coregrind/m_scheduler/scheduler.c +++ b/coregrind/m_scheduler/scheduler.c @@ -285,7 +285,10 @@ void VG_(acquire_BigLock)(ThreadId tid, const HChar* who) VG_(running_tid) = tid; { Addr gsp = VG_(get_SP)(tid); - VG_(unknown_SP_update)(gsp, gsp, 0/*unknown origin*/); + if (NULL != VG_(tdict).track_new_mem_stack_w_ECU) + VG_(unknown_SP_update_w_ECU)(gsp, gsp, 0/*unknown origin*/); + else + VG_(unknown_SP_update)(gsp, gsp); } if (VG_(clo_trace_sched)) { diff --git a/coregrind/m_stacks.c b/coregrind/m_stacks.c index 000aaa94c1..60c31ac9eb 100644 --- a/coregrind/m_stacks.c +++ b/coregrind/m_stacks.c @@ -268,59 +268,97 @@ void VG_(stack_limits)(Addr SP, Addr *start, Addr *end ) } } -/* This function gets called if new_mem_stack and/or die_mem_stack are + +/* complaints_stack_switch reports that SP has changed by more than some + threshold amount (by default, 2MB). We take this to mean that the + application is switching to a new stack, for whatever reason. + + JRS 20021001: following discussions with John Regehr, if a stack + switch happens, it seems best not to mess at all with memory + permissions. Seems to work well with Netscape 4.X. Really the + only remaining difficulty is knowing exactly when a stack switch is + happening. */ +__attribute__((noinline)) +static void complaints_stack_switch (Addr old_SP, Addr new_SP) +{ + static Int complaints = 3; + if (VG_(clo_verbosity) > 0 && complaints > 0 && !VG_(clo_xml)) { + Word delta = (Word)new_SP - (Word)old_SP; + complaints--; + VG_(message)(Vg_UserMsg, + "Warning: client switching stacks? " + "SP change: 0x%lx --> 0x%lx\n", old_SP, new_SP); + VG_(message)(Vg_UserMsg, + " to suppress, use: --max-stackframe=%ld " + "or greater\n", + (delta < 0 ? -delta : delta)); + if (complaints == 0) + VG_(message)(Vg_UserMsg, + " further instances of this message " + "will not be shown.\n"); + } +} + +/* The functions VG_(unknown_SP_update) and VG_(unknown_SP_update_w_ECU) + get called if new_mem_stack and/or die_mem_stack are tracked by the tool, and one of the specialised cases (eg. new_mem_stack_4) isn't used in preference. -*/ -VG_REGPARM(3) -void VG_(unknown_SP_update)( Addr old_SP, Addr new_SP, UInt ecu ) -{ - static Int moans = 3; - Word delta = (Word)new_SP - (Word)old_SP; - - /* Check if the stack pointer is still in the same stack as before. */ - if (current_stack == NULL || - new_SP < current_stack->start || new_SP > current_stack->end) { - Stack* new_stack = find_stack_by_addr(new_SP); - if (new_stack - && (current_stack == NULL || new_stack->id != current_stack->id)) { - /* The stack pointer is now in another stack. Update the current - stack information and return without doing anything else. */ - current_stack = new_stack; - return; - } + + These functions are performance critical, so are built with macros. */ + +// preamble + check if stack has switched. +#define IF_STACK_SWITCH_SET_current_task_AND_RETURN \ + Word delta = (Word)new_SP - (Word)old_SP; \ + \ + /* Check if the stack pointer is still in the same stack as before. */ \ + if (UNLIKELY(current_stack == NULL || \ + new_SP < current_stack->start || new_SP > current_stack->end)) { \ + Stack* new_stack = find_stack_by_addr(new_SP); \ + if (new_stack \ + && (current_stack == NULL || new_stack->id != current_stack->id)) { \ + /* The stack pointer is now in another stack. Update the current */ \ + /* stack information and return without doing anything else. */ \ + current_stack = new_stack; \ + return; \ + } \ } - if (delta < -VG_(clo_max_stackframe) || VG_(clo_max_stackframe) < delta) { - /* SP has changed by more than some threshold amount (by - default, 2MB). We take this to mean that the application is - switching to a new stack, for whatever reason. - - JRS 20021001: following discussions with John Regehr, if a stack - switch happens, it seems best not to mess at all with memory - permissions. Seems to work well with Netscape 4.X. Really the - only remaining difficulty is knowing exactly when a stack switch is - happening. */ - if (VG_(clo_verbosity) > 0 && moans > 0 && !VG_(clo_xml)) { - moans--; - VG_(message)(Vg_UserMsg, - "Warning: client switching stacks? " - "SP change: 0x%lx --> 0x%lx\n", old_SP, new_SP); - VG_(message)(Vg_UserMsg, - " to suppress, use: --max-stackframe=%ld or greater\n", - (delta < 0 ? -delta : delta)); - if (moans == 0) - VG_(message)(Vg_UserMsg, - " further instances of this message " - "will not be shown.\n"); - } - } else if (delta < 0) { +#define IF_BIG_DELTA_complaints_AND_RETURN \ + if (UNLIKELY(delta < -VG_(clo_max_stackframe) \ + || VG_(clo_max_stackframe) < delta)) { \ + complaints_stack_switch(old_SP, new_SP); \ + return; \ + } + +#define IF_SMALLER_STACK_die_mem_stack_AND_RETURN \ + if (delta > 0) { \ + VG_TRACK( die_mem_stack, old_SP, delta ); \ + return; \ + } + + +VG_REGPARM(3) +void VG_(unknown_SP_update_w_ECU)( Addr old_SP, Addr new_SP, UInt ecu ) { + IF_STACK_SWITCH_SET_current_task_AND_RETURN; + IF_BIG_DELTA_complaints_AND_RETURN; + IF_SMALLER_STACK_die_mem_stack_AND_RETURN; + if (delta < 0) { // IF_BIGGER_STACK VG_TRACK( new_mem_stack_w_ECU, new_SP, -delta, ecu ); - VG_TRACK( new_mem_stack, new_SP, -delta ); + return; + } + // SAME_STACK. nothing to do. +} - } else if (delta > 0) { - VG_TRACK( die_mem_stack, old_SP, delta ); +VG_REGPARM(2) +void VG_(unknown_SP_update)( Addr old_SP, Addr new_SP ) { + IF_STACK_SWITCH_SET_current_task_AND_RETURN; + IF_BIG_DELTA_complaints_AND_RETURN; + IF_SMALLER_STACK_die_mem_stack_AND_RETURN; + if (delta < 0) { // IF_BIGGER_STACK + VG_TRACK( new_mem_stack, new_SP, -delta ); + return; } + // SAME_STACK. nothing to do. } /*--------------------------------------------------------------------*/ diff --git a/coregrind/m_translate.c b/coregrind/m_translate.c index fbc5c8c7ee..6f8eeef415 100644 --- a/coregrind/m_translate.c +++ b/coregrind/m_translate.c @@ -45,7 +45,7 @@ #include "pub_core_redir.h" // VG_(redir_do_lookup) #include "pub_core_signals.h" // VG_(synth_fault_{perms,mapping} -#include "pub_core_stacks.h" // VG_(unknown_SP_update)() +#include "pub_core_stacks.h" // VG_(unknown_SP_update*)() #include "pub_core_tooliface.h" // VG_(tdict) #include "pub_core_translate.h" @@ -524,16 +524,25 @@ IRSB* vg_SP_update_pass ( void* closureV, /* Now we know what the old value of SP is. But knowing the new value is a bit tricky if there is a partial write. */ if (first_Put == first_SP && last_Put == last_SP) { - /* The common case, an exact write to SP. So st->Ist.Put.data - does hold the new value; simple. */ + /* The common case, an exact write to SP. So st->Ist.Put.data + does hold the new value; simple. */ vg_assert(curr_IP_known); - dcall = unsafeIRDirty_0_N( - 3/*regparms*/, - "VG_(unknown_SP_update)", - VG_(fnptr_to_fnentry)( &VG_(unknown_SP_update) ), - mkIRExprVec_3( IRExpr_RdTmp(old_SP), st->Ist.Put.data, - mk_ecu_Expr(curr_IP) ) - ); + if (NULL != VG_(tdict).track_new_mem_stack_w_ECU) + dcall = unsafeIRDirty_0_N( + 3/*regparms*/, + "VG_(unknown_SP_update_w_ECU)", + VG_(fnptr_to_fnentry)( &VG_(unknown_SP_update_w_ECU) ), + mkIRExprVec_3( IRExpr_RdTmp(old_SP), st->Ist.Put.data, + mk_ecu_Expr(curr_IP) ) + ); + else + dcall = unsafeIRDirty_0_N( + 2/*regparms*/, + "VG_(unknown_SP_update)", + VG_(fnptr_to_fnentry)( &VG_(unknown_SP_update) ), + mkIRExprVec_2( IRExpr_RdTmp(old_SP), st->Ist.Put.data ) + ); + addStmtToIRSB( bb, IRStmt_Dirty(dcall) ); /* don't forget the original assignment */ addStmtToIRSB( bb, st ); @@ -562,14 +571,23 @@ IRSB* vg_SP_update_pass ( void* closureV, addStmtToIRSB( bb, IRStmt_Put(offset_SP, IRExpr_RdTmp(old_SP) )); /* 4 */ vg_assert(curr_IP_known); - dcall = unsafeIRDirty_0_N( - 3/*regparms*/, - "VG_(unknown_SP_update)", - VG_(fnptr_to_fnentry)( &VG_(unknown_SP_update) ), - mkIRExprVec_3( IRExpr_RdTmp(old_SP), - IRExpr_RdTmp(new_SP), - mk_ecu_Expr(curr_IP) ) - ); + if (NULL != VG_(tdict).track_new_mem_stack_w_ECU) + dcall = unsafeIRDirty_0_N( + 3/*regparms*/, + "VG_(unknown_SP_update_w_ECU)", + VG_(fnptr_to_fnentry)( &VG_(unknown_SP_update_w_ECU) ), + mkIRExprVec_3( IRExpr_RdTmp(old_SP), + IRExpr_RdTmp(new_SP), + mk_ecu_Expr(curr_IP) ) + ); + else + dcall = unsafeIRDirty_0_N( + 2/*regparms*/, + "VG_(unknown_SP_update)", + VG_(fnptr_to_fnentry)( &VG_(unknown_SP_update) ), + mkIRExprVec_2( IRExpr_RdTmp(old_SP), + IRExpr_RdTmp(new_SP) ) + ); addStmtToIRSB( bb, IRStmt_Dirty(dcall) ); /* 5 */ addStmtToIRSB( bb, IRStmt_Put(offset_SP, IRExpr_RdTmp(new_SP) )); diff --git a/coregrind/pub_core_stacks.h b/coregrind/pub_core_stacks.h index eefd8a5f45..0a05b5031d 100644 --- a/coregrind/pub_core_stacks.h +++ b/coregrind/pub_core_stacks.h @@ -42,7 +42,10 @@ extern void VG_(change_stack) ( UWord id, Addr start, Addr end ); extern void VG_(stack_limits) ( Addr SP, Addr *start, Addr *end ); extern VG_REGPARM(3) - void VG_(unknown_SP_update) ( Addr old_SP, Addr new_SP, UInt otag ); + void VG_(unknown_SP_update_w_ECU) + ( Addr old_SP, Addr new_SP, UInt ecu ); +extern VG_REGPARM(2) + void VG_(unknown_SP_update) ( Addr old_SP, Addr new_SP ); #endif // __PUB_CORE_STACKS_H