|
From: <sv...@va...> - 2008-05-29 08:52:44
|
Author: bart
Date: 2008-05-29 09:52:44 +0100 (Thu, 29 May 2008)
New Revision: 8142
Log:
Make sure the debug information is read before a tool is notified about an mmap() system call.
Modified:
trunk/coregrind/m_syswrap/syswrap-generic.c
Modified: trunk/coregrind/m_syswrap/syswrap-generic.c
===================================================================
--- trunk/coregrind/m_syswrap/syswrap-generic.c 2008-05-29 08:34:27 UTC (rev 8141)
+++ trunk/coregrind/m_syswrap/syswrap-generic.c 2008-05-29 08:52:44 UTC (rev 8142)
@@ -59,6 +59,15 @@
#include "priv_syswrap-generic.h"
+/* Local function declarations. */
+
+static
+void notify_aspacem_of_mmap(Addr a, SizeT len, UInt prot,
+ UInt flags, Int fd, Off64T offset);
+static
+void notify_tool_of_mmap(Addr a, SizeT len, UInt prot, Off64T offset);
+
+
/* Returns True iff address range is something the client can
plausibly mess with: all of it is either already belongs to the
client or is free or a reservation. */
@@ -147,8 +156,16 @@
ML_(notify_aspacem_and_tool_of_mmap) ( Addr a, SizeT len, UInt prot,
UInt flags, Int fd, Off64T offset )
{
- Bool rr, ww, xx, d;
+ notify_aspacem_of_mmap(a, len, prot, flags, fd, offset);
+ notify_tool_of_mmap(a, len, prot, offset);
+}
+static
+void notify_aspacem_of_mmap(Addr a, SizeT len, UInt prot,
+ UInt flags, Int fd, Off64T offset)
+{
+ Bool d;
+
/* 'a' is the return value from a real kernel mmap, hence: */
vg_assert(VG_IS_PAGE_ALIGNED(a));
/* whereas len is whatever the syscall supplied. So: */
@@ -156,15 +173,26 @@
d = VG_(am_notify_client_mmap)( a, len, prot, flags, fd, offset );
+ if (d)
+ VG_(discard_translations)( (Addr64)a, (ULong)len,
+ "ML_(notify_aspacem_of_mmap)" );
+}
+
+static
+void notify_tool_of_mmap(Addr a, SizeT len, UInt prot, Off64T offset)
+{
+ Bool rr, ww, xx;
+
+ /* 'a' is the return value from a real kernel mmap, hence: */
+ vg_assert(VG_IS_PAGE_ALIGNED(a));
+ /* whereas len is whatever the syscall supplied. So: */
+ len = VG_PGROUNDUP(len);
+
rr = toBool(prot & VKI_PROT_READ);
ww = toBool(prot & VKI_PROT_WRITE);
xx = toBool(prot & VKI_PROT_EXEC);
VG_TRACK( new_mem_mmap, a, len, rr, ww, xx );
-
- if (d)
- VG_(discard_translations)( (Addr64)a, (ULong)len,
- "ML_(notify_aspacem_and_tool_of_mmap)" );
}
/* Expand (or shrink) an existing mapping, potentially moving it at
@@ -1909,15 +1937,24 @@
}
if (!sres.isError) {
- /* Notify aspacem and the tool. */
- ML_(notify_aspacem_and_tool_of_mmap)(
+ /* Notify aspacem. */
+ notify_aspacem_of_mmap(
(Addr)sres.res, /* addr kernel actually assigned */
- arg2, arg3,
+ arg2, /* length */
+ arg3, /* prot */
arg4, /* the original flags value */
- arg5, arg6
+ arg5, /* fd */
+ arg6 /* offset */
);
/* Load symbols? */
VG_(di_notify_mmap)( (Addr)sres.res, False/*allow_SkFileV*/ );
+ /* Notify the tool. */
+ notify_tool_of_mmap(
+ (Addr)sres.res, /* addr kernel actually assigned */
+ arg2, /* length */
+ arg3, /* prot */
+ arg6 /* offset */
+ );
}
/* Stay sane */
|
|
From: Julian S. <js...@ac...> - 2008-05-29 08:56:46
|
On Thursday 29 May 2008 10:52, sv...@va... wrote:
> Author: bart
> Date: 2008-05-29 09:52:44 +0100 (Thu, 29 May 2008)
> New Revision: 8142
>
> Log:
> Make sure the debug information is read before a tool is notified about an
> mmap() system call.
What happens if the mmap fails? Do the symbols then get unloaded?
J
>
> Modified:
> trunk/coregrind/m_syswrap/syswrap-generic.c
>
>
> Modified: trunk/coregrind/m_syswrap/syswrap-generic.c
> ===================================================================
> --- trunk/coregrind/m_syswrap/syswrap-generic.c 2008-05-29 08:34:27 UTC
> (rev 8141) +++ trunk/coregrind/m_syswrap/syswrap-generic.c 2008-05-29
> 08:52:44 UTC (rev 8142) @@ -59,6 +59,15 @@
> #include "priv_syswrap-generic.h"
>
>
> +/* Local function declarations. */
> +
> +static
> +void notify_aspacem_of_mmap(Addr a, SizeT len, UInt prot,
> + UInt flags, Int fd, Off64T offset);
> +static
> +void notify_tool_of_mmap(Addr a, SizeT len, UInt prot, Off64T offset);
> +
> +
> /* Returns True iff address range is something the client can
> plausibly mess with: all of it is either already belongs to the
> client or is free or a reservation. */
> @@ -147,8 +156,16 @@
> ML_(notify_aspacem_and_tool_of_mmap) ( Addr a, SizeT len, UInt prot,
> UInt flags, Int fd, Off64T offset )
> {
> - Bool rr, ww, xx, d;
> + notify_aspacem_of_mmap(a, len, prot, flags, fd, offset);
> + notify_tool_of_mmap(a, len, prot, offset);
> +}
>
> +static
> +void notify_aspacem_of_mmap(Addr a, SizeT len, UInt prot,
> + UInt flags, Int fd, Off64T offset)
> +{
> + Bool d;
> +
> /* 'a' is the return value from a real kernel mmap, hence: */
> vg_assert(VG_IS_PAGE_ALIGNED(a));
> /* whereas len is whatever the syscall supplied. So: */
> @@ -156,15 +173,26 @@
>
> d = VG_(am_notify_client_mmap)( a, len, prot, flags, fd, offset );
>
> + if (d)
> + VG_(discard_translations)( (Addr64)a, (ULong)len,
> + "ML_(notify_aspacem_of_mmap)" );
> +}
> +
> +static
> +void notify_tool_of_mmap(Addr a, SizeT len, UInt prot, Off64T offset)
> +{
> + Bool rr, ww, xx;
> +
> + /* 'a' is the return value from a real kernel mmap, hence: */
> + vg_assert(VG_IS_PAGE_ALIGNED(a));
> + /* whereas len is whatever the syscall supplied. So: */
> + len = VG_PGROUNDUP(len);
> +
> rr = toBool(prot & VKI_PROT_READ);
> ww = toBool(prot & VKI_PROT_WRITE);
> xx = toBool(prot & VKI_PROT_EXEC);
>
> VG_TRACK( new_mem_mmap, a, len, rr, ww, xx );
> -
> - if (d)
> - VG_(discard_translations)( (Addr64)a, (ULong)len,
> - "ML_(notify_aspacem_and_tool_of_mmap)" );
> }
>
> /* Expand (or shrink) an existing mapping, potentially moving it at
> @@ -1909,15 +1937,24 @@
> }
>
> if (!sres.isError) {
> - /* Notify aspacem and the tool. */
> - ML_(notify_aspacem_and_tool_of_mmap)(
> + /* Notify aspacem. */
> + notify_aspacem_of_mmap(
> (Addr)sres.res, /* addr kernel actually assigned */
> - arg2, arg3,
> + arg2, /* length */
> + arg3, /* prot */
> arg4, /* the original flags value */
> - arg5, arg6
> + arg5, /* fd */
> + arg6 /* offset */
> );
> /* Load symbols? */
> VG_(di_notify_mmap)( (Addr)sres.res, False/*allow_SkFileV*/ );
> + /* Notify the tool. */
> + notify_tool_of_mmap(
> + (Addr)sres.res, /* addr kernel actually assigned */
> + arg2, /* length */
> + arg3, /* prot */
> + arg6 /* offset */
> + );
> }
>
> /* Stay sane */
>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
|
|
From: Bart V. A. <bar...@gm...> - 2008-05-29 09:31:10
|
On Thu, May 29, 2008 at 10:50 AM, Julian Seward <js...@ac...> wrote: > On Thursday 29 May 2008 10:52, sv...@va... wrote: >> Author: bart >> Date: 2008-05-29 09:52:44 +0100 (Thu, 29 May 2008) >> New Revision: 8142 >> >> Log: >> Make sure the debug information is read before a tool is notified about an >> mmap() system call. > > What happens if the mmap fails? Do the symbols then get unloaded? You are asking me about behavior of the Valgrind code that I did not implement and that I did not change. As far as I understand the core, when an mmap() call fails, symbols get discarded upon a subsequent call to mmap() that would use the same address range. Bart. |
|
From: Julian S. <js...@ac...> - 2008-05-29 09:08:16
|
On Thursday 29 May 2008 10:50, Julian Seward wrote:
> On Thursday 29 May 2008 10:52, sv...@va... wrote:
> > Author: bart
> > Date: 2008-05-29 09:52:44 +0100 (Thu, 29 May 2008)
> > New Revision: 8142
> >
> > Log:
> > Make sure the debug information is read before a tool is notified about
> > an mmap() system call.
>
> What happens if the mmap fails? Do the symbols then get unloaded?
Ah; I think I understand. This patch merely changes the order from
do the system call
if (syscall succeeded) {
notify the tool
load the debuginfo
}
to
do the system call
if (syscall succeeded) {
load the debuginfo
notify the tool
}
Is that correct?
J
|
|
From: Bart V. A. <bar...@gm...> - 2008-05-29 09:32:07
|
On Thu, May 29, 2008 at 11:02 AM, Julian Seward <js...@ac...> wrote:
> Ah; I think I understand. This patch merely changes the order from
>
> do the system call
> if (syscall succeeded) {
> notify the tool
> load the debuginfo
> }
>
> to
>
> do the system call
> if (syscall succeeded) {
> load the debuginfo
> notify the tool
> }
>
> Is that correct?
That was indeed my intention -- I hope I have implemented this correctly.
Bart.
|
|
From: Julian S. <js...@ac...> - 2008-05-29 09:35:51
|
> > do the system call
> > if (syscall succeeded) {
> > notify the tool
> > load the debuginfo
> > }
> >
> > to
> >
> > do the system call
> > if (syscall succeeded) {
> > load the debuginfo
> > notify the tool
> > }
> >
> > Is that correct?
>
> That was indeed my intention -- I hope I have implemented this correctly.
Sounds OK to me. Sorry for my half-baked previous message -- I replied
without thinking enough.
J
|