|
From: Ashley P. <as...@qu...> - 2006-01-18 19:02:56
|
Hi,
I'm trying to instrument my code with the correct VALGRIND_[MAKE|
CHECK]_[READ|WRITE]ABLE macros and want to check with you guys that I'm
using the correct ones.
As a bit of background my library interacts with the network hardware
directly in user-space so the normal system calls don't intercept data
transfers over the network, I'm trying to make up for this by
instrumenting the code to instruct valgrind properly.
The functions I want to instrument vary greatly in semantics but in the
end all boil down to asynchronous reads and writes, that is you start a
DMA, keep a handle to it and come back later to wait for it to finish.
The macros I've defined are as follows, hopefully the names are fairly
self explanatory...
#define V_START_GET(TARGET,LEN) \
do { \
VALGRIND_CHECK_WRITABLE(TARGET,LEN); \
VALGRIND_MAKE_WRITABLE(TARGET,LEN); \
} while (0)
#define V_END_GET(TARGET,LEN) \
do { \
VALGRIND_MAKE_READABLE(TARGET,LEN); \
} while (0)
#define V_START_PUT(TARGET,LEN) \
do { \
VALGRIND_CHECK_READABLE(TARGET,LEN); \
} while (0)
Now my questions are mainly about the asynchronous get, it checks that
the buffer it's been given is valid (VALGRIND_CHECK_WRITABLE) and
temporarily marks it as addressable but undefined whilst the data is
arriving. My question is what does VALGRIND_MAKE_WRITABLE do if the
buffer isn't addressable before it's called. I don't want it to mark a
buffer as addressable when it isn't, only to mark it as undefined while
the network transfer is underway. Am I doing the right thing here?
Another request I have is can error reports from client requests include
a byte counter in the output, in the case of buffer over-runs it would
really help, in this case it was reading eight bytes from a malloced
buffer of four although it's impossible to see that from the output.
==26004== Uninitialised byte(s) found during client check request
==26004== at 0x40641F3: elan_hbcast (common/group_hbcast.c:820)
==26004== by 0x8048661: main
(in /opt/ashley/auto-build/quadrics/qstests/a.out)
==26004== Address 0x5ABDFC0 is 0 bytes inside a block of size 4 alloc'd
==26004== at 0x400446D: malloc (vg_replace_malloc.c:149)
==26004== by 0x804863E: main
(in /opt/ashley/auto-build/quadrics/qstests/a.out)
Ashley,
|
|
From: Nicholas N. <nj...@cs...> - 2006-01-18 23:29:53
|
On Wed, 18 Jan 2006, Ashley Pittman wrote:
> I'm trying to instrument my code with the correct VALGRIND_[MAKE|
> CHECK]_[READ|WRITE]ABLE macros and want to check with you guys that I'm
> using the correct ones.
>
> [snip]
>
> #define V_START_GET(TARGET,LEN) \
> do { \
> VALGRIND_CHECK_WRITABLE(TARGET,LEN); \
> VALGRIND_MAKE_WRITABLE(TARGET,LEN); \
> } while (0)
You don't need the MAKE_WRITABLE here, AFAICT.
> #define V_END_GET(TARGET,LEN) \
> do { \
> VALGRIND_MAKE_READABLE(TARGET,LEN); \
> } while (0)
>
> #define V_START_PUT(TARGET,LEN) \
> do { \
> VALGRIND_CHECK_READABLE(TARGET,LEN); \
> } while (0)
>
> Now my questions are mainly about the asynchronous get, it checks that
> the buffer it's been given is valid (VALGRIND_CHECK_WRITABLE) and
> temporarily marks it as addressable but undefined whilst the data is
> arriving. My question is what does VALGRIND_MAKE_WRITABLE do if the
> buffer isn't addressable before it's called. I don't want it to mark a
> buffer as addressable when it isn't, only to mark it as undefined while
> the network transfer is underway. Am I doing the right thing here?
There is no way to mark memory as undefined without also forcing it to be
considered addressable, which sounds like is what you want. But just
removing the MKAE_WRITABLE like I said above might be good enough?
Nick
|
|
From: Ashley P. <as...@qu...> - 2006-01-20 10:32:56
|
On Wed, 2006-01-18 at 17:29 -0600, Nicholas Nethercote wrote:
> > Now my questions are mainly about the asynchronous get, it checks that
> > the buffer it's been given is valid (VALGRIND_CHECK_WRITABLE) and
> > temporarily marks it as addressable but undefined whilst the data is
> > arriving. My question is what does VALGRIND_MAKE_WRITABLE do if the
> > buffer isn't addressable before it's called. I don't want it to mark a
> > buffer as addressable when it isn't, only to mark it as undefined while
> > the network transfer is underway. Am I doing the right thing here?
>
> There is no way to mark memory as undefined without also forcing it to be
> considered addressable, which sounds like is what you want. But just
> removing the MKAE_WRITABLE like I said above might be good enough?
I've removed the the MAKE_WRITEABLE, it's now correct for most cases but
won't catch this example:
{
int i = 4;
static int j = 5;
ELAN_EVENT *e = elan_get(elan_base->state,&j,&i,sizeof(int),0);
printf("i is %d\n",i);
elan_wait(e,ELAN_POLL_EVENT);
printf("i is %d\n",i);
}
The prototype for elan_get is this, it starts a simple network read:
extern ELAN_EVENT *elan_get (ELAN_STATE *state, void *source, void
*target, size_t len, uint32_t srcvp);
In this code the result of first printf is undefined and the second one
will print 5. I was hoping Valgrind could be made to flag an error on
the first printf.
Ashley,
|
|
From: Julian S. <js...@ac...> - 2006-01-21 02:58:01
|
Ashley
> #define V_START_GET(TARGET,LEN) \
> do { \
> VALGRIND_CHECK_WRITABLE(TARGET,LEN); \
> VALGRIND_MAKE_WRITABLE(TARGET,LEN); \
> } while (0)
>
> #define V_END_GET(TARGET,LEN) \
> do { \
> VALGRIND_MAKE_READABLE(TARGET,LEN); \
> } while (0)
>
> #define V_START_PUT(TARGET,LEN) \
> do { \
> VALGRIND_CHECK_READABLE(TARGET,LEN); \
> } while (0)
>
> Now my questions are mainly about the asynchronous get, it checks that
> the buffer it's been given is valid (VALGRIND_CHECK_WRITABLE) and
> temporarily marks it as addressable but undefined whilst the data is
> arriving. My question is what does VALGRIND_MAKE_WRITABLE do if the
> buffer isn't addressable before it's called. I don't want it to mark a
> buffer as addressable when it isn't, only to mark it as undefined while
> the network transfer is underway. Am I doing the right thing here?
My impression is that what you need to do is:
- at the initial request (V_START_GET), first check the recv
buffer is addressible (VALGRIND_CHECK_WRITABLE), and then
force it to be unaddressible so V will yelp if anyone pokes
there (VALGRIND_MAKE_NOACCESS)
- when the request completes and you know how much data really
arrived, do VALGRIND_MAKE_READABLE. This marks the area as
both addressible and containing defined data.
Does that make sense?
> Another request I have is can error reports from client requests include
> a byte counter in the output, in the case of buffer over-runs it would
> really help, in this case it was reading eight bytes from a malloced
> buffer of four although it's impossible to see that from the output.
>
> ==26004== Uninitialised byte(s) found during client check request
> ==26004== at 0x40641F3: elan_hbcast (common/group_hbcast.c:820)
> ==26004== by 0x8048661: main
> (in /opt/ashley/auto-build/quadrics/qstests/a.out)
> ==26004== Address 0x5ABDFC0 is 0 bytes inside a block of size 4 alloc'd
> ==26004== at 0x400446D: malloc (vg_replace_malloc.c:149)
> ==26004== by 0x804863E: main
> (in /opt/ashley/auto-build/quadrics/qstests/a.out)
The problem here is there are two possible sources of error and V can
only report one or the other. Let . mean a byte which is unaddressible
and X mean one which is addressible but is undefined. Your buffer
looks like this
.......XXXX......
^
a
Now you do VALGRIND_CHECK_READABLE(a,8). Should it complain that
a[0 .. 3] are undefined, or that a[4 .. 7] are unaddressible? Both
are legitimate complaints. I think the reason for the confusion is
that it's reporting the former and you're expecting the latter.
If those four bytes were defined then it should indeed complain
about a[4..7] being unaddressible.
Could be I'm way off base, but I _think_ that's the behaviour.
J
|
|
From: Ashley P. <as...@qu...> - 2006-01-23 13:48:36
|
On Sat, 2006-01-21 at 02:57 +0000, Julian Seward wrote: > My impression is that what you need to do is: > > - at the initial request (V_START_GET), first check the recv > buffer is addressible (VALGRIND_CHECK_WRITABLE), and then > force it to be unaddressible so V will yelp if anyone pokes > there (VALGRIND_MAKE_NOACCESS) > > - when the request completes and you know how much data really > arrived, do VALGRIND_MAKE_READABLE. This marks the area as > both addressible and containing defined data. > > Does that make sense? This sounds almost exactly what I want, I was thinking that the data should be addressable but undefined between the get() and the wait() however unaddressable would do. I'll try some experiments with it. > The problem here is there are two possible sources of error and V can > only report one or the other. Let . mean a byte which is unaddressible > and X mean one which is addressible but is undefined. Your buffer > looks like this > > .......XXXX...... > ^ > a > > Now you do VALGRIND_CHECK_READABLE(a,8). Should it complain that > a[0 .. 3] are undefined, or that a[4 .. 7] are unaddressible? Both > are legitimate complaints. I think the reason for the confusion is > that it's reporting the former and you're expecting the latter. > > If those four bytes were defined then it should indeed complain > about a[4..7] being unaddressible. > > Could be I'm way off base, but I _think_ that's the behaviour. I'm not to concerned about which is complains about in this case, in fact that side of it isn't something I'd considered, what I'd like it to say is something like the following: "...found during client check request of size 8" For memory reads/writes the size is implicit in the operation and reported as such but with client checks it's harder to tell currently. Ashley, |
|
From: Julian S. <js...@ac...> - 2006-01-21 03:19:42
|
> {
> int i = 4;
> static int j = 5;
> ELAN_EVENT *e = elan_get(elan_base->state,&j,&i,sizeof(int),0);
> printf("i is %d\n",i);
> elan_wait(e,ELAN_POLL_EVENT);
> printf("i is %d\n",i);
> }
>
> The prototype for elan_get is this, it starts a simple network read:
> extern ELAN_EVENT *elan_get (ELAN_STATE *state, void *source, void
> *target, size_t len, uint32_t srcvp);
>
> In this code the result of first printf is undefined and the second one
> will print 5. I was hoping Valgrind could be made to flag an error on
> the first printf.
I think you're hoping for a bit much. You assigned a value to i
at the start so it's defined.
But if I read this correctly, elan_get just remembers &i (presumably
in ELAN_EVENT which is some kind of handle) and writes the actual
amount of received data there in elan_wait. So why don't you
start with no value in i - then V will consider it undefined until
elan_wait writes it.
> {
> int i;
> static int j = 5;
> [...]
J
|
|
From: Ashley P. <as...@qu...> - 2006-01-23 14:11:28
|
On Sat, 2006-01-21 at 03:19 +0000, Julian Seward wrote:
> > {
> > int i = 4;
> > static int j = 5;
> > ELAN_EVENT *e = elan_get(elan_base->state,&j,&i,sizeof(int),0);
> > printf("i is %d\n",i);
> > elan_wait(e,ELAN_POLL_EVENT);
> > printf("i is %d\n",i);
> > }
> >
> > The prototype for elan_get is this, it starts a simple network read:
> > extern ELAN_EVENT *elan_get (ELAN_STATE *state, void *source, void
> > *target, size_t len, uint32_t srcvp);
> >
> > In this code the result of first printf is undefined and the second one
> > will print 5. I was hoping Valgrind could be made to flag an error on
> > the first printf.
>
> I think you're hoping for a bit much. You assigned a value to i
> at the start so it's defined.
>
> But if I read this correctly, elan_get just remembers &i (presumably
> in ELAN_EVENT which is some kind of handle) and writes the actual
> amount of received data there in elan_wait. So why don't you
> start with no value in i - then V will consider it undefined until
> elan_wait writes it.
That's almost right, ELAN_EVENT is a opaque handle and the elan_wait()
call blocks until the data is in &i. In practice &i isn't written to by
either the get() or the wait() call, the data is written by a DMA
running on a remote machine so valgrind will never observe the CPU
setting it. This is why I want to put VALGRIND_MAKE_READABLE() calls in
elan_wait().
The result of the first printf is undefined and will actually vary
between hardware platforms and even different runs of the same program
on the same machine. You're right in that I could just not assign i at
startup however I'm trying to provide an example of a programming bug
that I would like to be able to detect using valgrind, hence my desire
to be able to mark &i it as undefined in elan_get() preferably without
changing the addressability of it.
Ashley,
|