Julian Seward wrote:
On Tuesday 18 November 2008, Tom Hughes wrote:
It's rubbish, and needs a complete ground up rework rather than band aid
patches. IIRC there are at least two different ideas internally of what
the current stack is and only one of those pays attention to the user
stack registration stuff.
I agree. I never really understood it, and from what I remember from
looking at it in the distant past, it's conceptually like swiss cheese
-- full of holes.
The relationship to stack unwinding should also be considered.
Absolutely, and that's part of the problem that the stack unwinder
doesn't take the user registered stacks into account IIRC.
Plus the initial thread stacks (to the extent we can determine them at
least) aren't registered with the stack manager, the the thread state
attempts to track the current stack but doesn't synchronise that at all
well with the stack manager.
At the moment the unwinder can segfault if a corrupted stack takes
it into a non-mapped area. This ought to be fixed, although perhaps
that's more a question of integration with the address space manager:
what the unwinder really needs is a very fast way to ask whether a
page is mapped or not; and simply stop if the page isn't mapped.
I wrote a patch to do just that once actually... I never committed it
though, possibly because I thought we should really have a better idea
of where the stack is first and then just use an address space check as
an emergency last ditch check.
The patch is attached anyway...
Tom
--
Tom Hughes ([EMAIL PROTECTED])
http://www.compton.nu/
Index: coregrind/pub_core_aspacemgr.h
===================================================================
--- coregrind/pub_core_aspacemgr.h (revision 7382)
+++ coregrind/pub_core_aspacemgr.h (working copy)
@@ -92,6 +92,9 @@
extern Bool VG_(am_is_valid_for_client_or_free_or_resvn)
( Addr start, SizeT len, UInt prot );
+/* Is the area [start .. start+len-1] readable? */
+ extern Bool VG_(am_is_readable)( Addr start, SizeT len );
+
/* Trivial fn: return the total amount of space in anonymous mappings,
both for V and the client. Is used for printing stats in
out-of-memory messages. */
Index: coregrind/m_aspacemgr/aspacemgr-linux.c
===================================================================
--- coregrind/m_aspacemgr/aspacemgr-linux.c (revision 7382)
+++ coregrind/m_aspacemgr/aspacemgr-linux.c (working copy)
@@ -1271,7 +1271,44 @@
return is_valid_for_client( start, len, prot, True/*free is OK*/ );
}
+/* Test if a piece of memory is readable. */
+Bool VG_(am_is_readable)( Addr start, SizeT len )
+{
+ Int i, iLo, iHi;
+ if (len == 0)
+ return True; /* somewhat dubious case */
+ if (start + len < start)
+ return False; /* reject wraparounds */
+
+ iLo = find_nsegment_idx(start);
+ aspacem_assert(start >= nsegments[iLo].start);
+
+ if (start+len-1 <= nsegments[iLo].end) {
+ /* This is a speedup hack which avoids calling find_nsegment_idx
+ a second time when possible. It is always correct to just
+ use the "else" clause below, but VG_(am_is_readable) is
+ called a lot by the stack unwinder, so avoiding pointless calls
+ to find_nsegment_idx, which can be expensive, is helpful. */
+ iHi = iLo;
+ } else {
+ iHi = find_nsegment_idx(start + len - 1);
+ }
+
+ for (i = iLo; i <= iHi; i++) {
+ if ( (nsegments[i].kind == SkFileC
+ || nsegments[i].kind == SkAnonC
+ || nsegments[i].kind == SkShmC)
+ && nsegments[i].hasR ) {
+ /* ok */
+ } else {
+ return False;
+ }
+ }
+ return True;
+}
+
+
/* Test if a piece of memory is addressable by valgrind with at least
PROT_NONE protection permissions by examining the underlying
segments. */
Index: coregrind/m_stacktrace.c
===================================================================
--- coregrind/m_stacktrace.c (revision 7382)
+++ coregrind/m_stacktrace.c (working copy)
@@ -148,7 +148,8 @@
fails, and is expensive. */
/* Deal with frames resulting from functions which begin "pushl%
ebp ; movl %esp, %ebp" which is the ABI-mandated preamble. */
- if (fp_min <= fp && fp <= fp_max) {
+ if (fp_min <= fp && fp <= fp_max &&
+ VG_(am_is_readable)(fp, fp + 2 * sizeof(UWord))) {
/* fp looks sane, so use it. */
ip = (((UWord*)fp)[1]);
sp = fp + sizeof(Addr) /*saved %ebp*/
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Valgrind-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/valgrind-users
