zenithsiz/ftmemsim-valgrind
1
0
Fork 0
mirror of https://github.com/Zenithsiz/ftmemsim-valgrind.git synced 2026-02-03 18:13:01 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

This commit subtly changes the meaning of the values obtained via the

Browse Source 
stack unwind mechanism (the function VG_(record_ExeContext) et al),
clears up some associated kludges, and makes suppression matching work
more reliably.

Prior to this commit, a stack snapshot contained, at [0], the IP of
the relevant thread, and at all positions [1] and above, the return
addresses for the open calls.

When showing a snapshot to the user (in VG_(apply_StackTrace)), and
searching the stack for stack blocks (in VG_(get_data_description)), 1
is subtracted from positions [1] and above, so as to move these return
addresses back to the last byte of the calling instruction.  This
subtraction is also done even in VG_(get_StackTrace_wrk) itself, in
order to make the stack unwinding work at all.

It turns out that suppression-vs-function-name matching requires the
same hack, and sometimes failed to match suppressions that should
match, because of this self-same problem.

So the commit changes the stack unwinder itself, so that entries [1]
and above point to the last byte of the call instruction, rather than
the return address.  The associated kludges in VG_(apply_StackTrace)
and VG_(get_StackTrace_wrk) are removed, and suppression matching is
observed to work in a case where it failed before.



git-svn-id: svn://svn.valgrind.org/valgrind/trunk@8818
This commit is contained in:
Julian Seward 2008-12-12 13:23:03 +00:00
parent 05e92e79d9
commit ba2ece03b8
4 changed files with 45 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
coregrind/m_debuginfo/debuginfo.c
View File

@ -2404,29 +2404,15 @@ Bool VG_(get_data_description)( /*OUT*/Char* dname1,
frames, and for each frame, consider the local variables. */
n_frames = VG_(get_StackTrace)( tid, ips, N_FRAMES,
sps, fps, 0/*first_ip_delta*/ );
/* Re ip_delta in the next loop: There's a subtlety in the meaning
of the IP values in a stack obtained from VG_(get_StackTrace).
The innermost value really is simply the thread's program
counter at the time the snapshot was taken. However, all the
other values are actually return addresses, and so point just
after the call instructions. Hence they notionally reflect not
what the program counters were at the time those calls were
made, but what they will be when those calls return. This can
be of significance should an address range happen to end at the
end of a call instruction -- we may ignore the range when in
fact it should be considered. Hence, back up the IPs by 1 for
all non-innermost IPs. Note that VG_(get_StackTrace_wrk) itself
has to use the same trick in order to use CFI data to unwind the
stack (as documented therein in comments). */
/* As a result of KLUDGE above, starting the loop at j = 0
duplicates examination of the top frame and so isn't necessary.
Oh well. */
vg_assert(n_frames >= 0 && n_frames <= N_FRAMES);
for (j = 0; j < n_frames; j++) {
Word ip_delta = j == 0 ? 0 : 1;
if (consider_vars_in_frame( dname1, dname2, n_dname,
data_addr,
ips[j] - ip_delta,
ips[j],
sps[j], fps[j], tid, j )) {
dname1[n_dname-1] = dname2[n_dname-1] = 0;
return True;
@ -2445,14 +2431,15 @@ Bool VG_(get_data_description)( /*OUT*/Char* dname1,
amd64), the variable's location list does claim it exists
starting at the first byte of the first instruction after the
call instruction. So, call consider_vars_in_frame a second
time, but this time don't subtract 1 from the IP. GDB
handles this example with no difficulty, which leads me to
believe that either (1) I misunderstood something, or (2) GDB
has an equivalent kludge. */
if (consider_vars_in_frame( dname1, dname2, n_dname,
data_addr,
ips[j],
sps[j], fps[j], tid, j )) {
time, but this time add 1 to the IP. GDB handles this
example with no difficulty, which leads me to believe that
either (1) I misunderstood something, or (2) GDB has an
equivalent kludge. */
if (j > 0 /* this is a non-innermost frame */
&& consider_vars_in_frame( dname1, dname2, n_dname,
data_addr,
ips[j] + 1,
sps[j], fps[j], tid, j )) {
dname1[n_dname-1] = dname2[n_dname-1] = 0;
return True;
}

42
coregrind/m_stacktrace.c
View File

@ -140,11 +140,6 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
* This most frequently happens at the end of a function when
* a tail call occurs and we wind up using the CFI info for the
* next function which is completely wrong.
*
* Note that VG_(get_data_description) (in m_debuginfo) has to take
* this same problem into account when unwinding the stack to
* examine local variable descriptions (as documented therein in
* comments).
*/
while (True) {
@ -170,10 +165,10 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
fp = (((UWord*)fp)[0]);
if (sps) sps[i] = sp;
if (fps) fps[i] = fp;
ips[i++] = ip;
ips[i++] = ip - 1; /* -1: refer to calling insn, not the RA */
if (debug)
VG_(printf)(" ipsF[%d]=0x%08lx\n", i-1, ips[i-1]);
ip = ip - 1;
ip = ip - 1; /* as per comment at the head of this loop */
continue;
}
@ -182,10 +177,10 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
if ( VG_(use_CF_info)( &ip, &sp, &fp, fp_min, fp_max ) ) {
if (sps) sps[i] = sp;
if (fps) fps[i] = fp;
ips[i++] = ip;
ips[i++] = ip - 1; /* -1: refer to calling insn, not the RA */
if (debug)
VG_(printf)(" ipsC[%d]=0x%08lx\n", i-1, ips[i-1]);
ip = ip - 1;
ip = ip - 1; /* as per comment at the head of this loop */
continue;
}
@ -218,11 +213,6 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
* This most frequently happens at the end of a function when
* a tail call occurs and we wind up using the CFI info for the
* next function which is completely wrong.
*
* Note that VG_(get_data_description) (in m_debuginfo) has to take
* this same problem into account when unwinding the stack to
* examine local variable descriptions (as documented therein in
* comments).
*/
while (True) {
@ -237,10 +227,10 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
if ( VG_(use_CF_info)( &ip, &sp, &fp, fp_min, fp_max ) ) {
if (sps) sps[i] = sp;
if (fps) fps[i] = fp;
ips[i++] = ip;
ips[i++] = ip - 1; /* -1: refer to calling insn, not the RA */
if (debug)
VG_(printf)(" ipsC[%d]=%#08lx\n", i-1, ips[i-1]);
ip = ip - 1;
ip = ip - 1; /* as per comment at the head of this loop */
continue;
}
@ -264,10 +254,10 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
fp = (((UWord*)fp)[0]);
if (sps) sps[i] = sp;
if (fps) fps[i] = fp;
ips[i++] = ip;
ips[i++] = ip - 1; /* -1: refer to calling insn, not the RA */
if (debug)
VG_(printf)(" ipsF[%d]=%#08lx\n", i-1, ips[i-1]);
ip = ip - 1;
ip = ip - 1; /* as per comment at the head of this loop */
continue;
}
@ -287,10 +277,13 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
ip = ((UWord*)sp)[0];
if (sps) sps[i] = sp;
if (fps) fps[i] = fp;
ips[i++] = ip;
ips[i++] = ip == 0
? 0 /* sp[0] == 0 ==> stuck at the bottom of a
thread stack */
: ip - 1; /* -1: refer to calling insn, not the RA */
if (debug)
VG_(printf)(" ipsH[%d]=%#08lx\n", i-1, ips[i-1]);
ip = ip - 1;
ip = ip - 1; /* as per comment at the head of this loop */
sp += 8;
continue;
}
@ -342,6 +335,8 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
{
# define M_VG_ERRTXT 1000
UChar buf_lr[M_VG_ERRTXT], buf_ip[M_VG_ERRTXT];
/* The following conditional looks grossly inefficient and
surely could be majorly improved, with not much effort. */
if (VG_(get_fnname_nodemangle) (lr, buf_lr, M_VG_ERRTXT))
if (VG_(get_fnname_nodemangle) (ip, buf_ip, M_VG_ERRTXT))
if (VG_(strncmp)(buf_lr, buf_ip, M_VG_ERRTXT))
@ -410,9 +405,12 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
fp = (((UWord*)fp)[0]);
if (sps) sps[i] = fp; /* NB. not sp */
if (fps) fps[i] = fp;
ips[i++] = ip;
ips[i++] = ip - 1; /* -1: refer to calling insn, not the RA */
if (debug)
VG_(printf)(" ipsF[%d]=%#08lx\n", i-1, ips[i-1]);
ip = ip - 1; /* ip is probably dead at this point, but
play safe, a la x86/amd64 above. See
extensive comments above. */
continue;
}
@ -543,8 +541,6 @@ void VG_(apply_StackTrace)( void(*action)(UInt n, Addr ip),
vg_assert(n_ips > 0);
do {
Addr ip = ips[i];
if (i > 0)
ip -= VG_MIN_INSTR_SZB; // point to calling line
// Stop after the first appearance of "main" or one of the other names
// (the appearance of which is a pretty good sign that we've gone past

3
include/pub_tool_execontext.h
View File

@ -54,6 +54,9 @@ typedef
// ThreadId should be passed in by the core. The initial IP value to
// use is adjusted by first_ip_delta before the stack is unwound.
// A safe value to pass is zero.
//
// See comments in pub_tool_stacktrace.h for precise definition of
// the meaning of the code addresses in the returned ExeContext.
extern
ExeContext* VG_(record_ExeContext) ( ThreadId tid, Word first_ip_delta );

12
include/pub_tool_stacktrace.h
View File

@ -41,6 +41,18 @@ typedef Addr* StackTrace;
// The initial IP value to use is adjusted by first_ip_delta before
// the stack is unwound. A safe value to pass is zero.
//
// The specific meaning of the returned addresses is:
//
// [0] is the IP of thread 'tid'
// [1] points to the last byte of the call instruction that called [0].
// [2] points to the last byte of the call instruction that called [1].
// etc etc
//
// Hence ips[0 .. return_value-1] should all point to currently
// 'active' (in the sense of a stack of unfinished function calls)
// instructions. [0] points to the start of an arbitrary instruction.#
// [1 ..] point to the last byte of a chain of call instructions.
//
// If sps and fps are non-NULL, the corresponding frame-pointer and
// stack-pointer values for each frame are stored there.