Re: Fw: [ppc-linux-nat]: set access flag for h/w watchpoint even if it is only read or write (fwd)

Re: Fw: [ppc-linux-nat]: set access flag for h/w watchpoint even if it is only read or write (fwd)

[Wu Zhou]

> This doesn't happen on x86, with this test program:
>
>     #include <stdio.h>
>
>     int main (void)
>     {
>       int var1 = 0, var2;
>       var1 = 10;
>       var2 = var1;
>
>       printf ("%d %d\n", var1, var2);
>       return 0;
>     }
>
> I think the reason it works for me is that on x86, there's no real
> support for read watchpoints, so we actually set a read-write
> watchpoint, and then the logic of watchpoint_check does TRT.
>
> What is the situation with this on a PPC?  What kinds of data
> breakpoints does it support, and what associated functionality can we
> use in the ptrace call or its PPC equivalent?

Embedded powerpc processors have optional hardware support (specilized  
registers) for watchpoints. If these registers are available, they  
will support read and write watchpoints at the same time. The  
situation is the same with POWER processors.

To insert read or write watchpoint, three new ptrace commands are  
added for ppc/ppc64 kernel: SET_DEBUGREG, GET_DEBUGREG, GET_SIGINFO.  
SET_DEBUGREG is used for setting the value of watchpoint register.  
GET_DEBUGREG is for getting the value of watchpoint register.  
GET_SIGINFO is for getting the siginfo struct when a trap is captured,  
the watched data address is in this struct.

>> My solution is to let gdb update the value stored in the watched
> variable,
>> so that it always get the fresh value for comparison.

> That will work, but the question is: is this the optimal solution for
> the PPC?  Maybe there's a better solution, one that doesn't slow down
> the normal case.

Yes. This might slow down read watchpoint, mainly when there are more  
write operation than read operation. But for write watchpoint, we can  
eliminate that slowdown by only setting write flag.

>> Another solution might be to change the verify logic of read watchpoint
>> hit in watchpoint_check.  Maybe we can just trust the underlying os will
>> only trigger in read hit?
>
> The question is, how to do that without breaking other platforms, like
> x86, which we cannot trust.  If you can come up with a design that
> accommodates both types of situations, I will be happy to review it.

I am not that familar with the situation in x86 platform. But I am  
willing to take some look into that to see if I can come out with any  
solution.

> Daniel, could you please point me to Ulrich's change, either in
> ChangeLogs or in the sources?  I cannot find it forf some reason.
>
>> Comparing all these solution, my solution is the most simple one.  I
> mean,
>> it makes little change to the current code.  To address the slowdown, we
>> can still use the original flags for write hit.
>>
>> What is your idea on this?
>
> I think the cleanest solution is for breakpoint.c to get the idea of
> the kind of support it can get from the target.  If the target
> implements real read watchpoints, it should propagate that knowledge
> to breakpoint.c, where it examines the watchpoints and decides which
> one to announce.

Yes. That is a good idea. Maybe a target vector can be added for that  
purpose. Every target can set that up when it initializes. What is  
more, the underlying os need to tell what kind of watchpoint it  
reports, maybe in the transfered-back siginfo struct. Then gdb can use  
that information to know if it is a read hit or write hit.

Although it is the very clean, but there is one problem in this  
method. The underlying os need to tell what the watchpoint hit is,  
read or write. Current ppc/ppc64 kernel don't do that. it just report  
the a hw watchpoint is hit and report the data address to gdb through  
siginfo struct.

So what about commit my code first, although there are some slowdown.  
And then we can follow the above method to resolve the problem in both  
kernel and gdb side?

> Please note that there are complications in this area: a user could
> have legitimately set several different watchpoints on the same
> address, each one with its own type (read, write, access) and its own
> set of conditions and/or commands.  Whatever new design we come up
> with, it has to support these complicated situations, because the user
> will expect GDB to announce only those watchpoints which meet the
> conditions and type constraints.  For example, what happens with your
> solution if I put both read and write watchpoints on the same
> variable?  The current code does TRT on x86 in that case.  As you see
> below, it correctly announces each one of the two watchpoints where
> expected (except for a slight off-by-one shift in line numbers):

My current code can handle this situation.

Regards
- Wu Zhou

Re: Fw: [ppc-linux-nat]: set access flag for h/w watchpoint even if it is only read or write (fwd)

[Daniel Jacobowitz]

On Tue, Jul 11, 2006 at 11:09:10AM -0400, Wu Zhou wrote:
> >I think the cleanest solution is for breakpoint.c to get the idea of
> >the kind of support it can get from the target.  If the target
> >implements real read watchpoints, it should propagate that knowledge
> >to breakpoint.c, where it examines the watchpoints and decides which
> >one to announce.
> 
> Yes. That is a good idea. Maybe a target vector can be added for that  
> purpose. Every target can set that up when it initializes. What is  
> more, the underlying os need to tell what kind of watchpoint it  
> reports, maybe in the transfered-back siginfo struct. Then gdb can use  
> that information to know if it is a read hit or write hit.
> 
> Although it is the very clean, but there is one problem in this  
> method. The underlying os need to tell what the watchpoint hit is,  
> read or write. Current ppc/ppc64 kernel don't do that. it just report  
> the a hw watchpoint is hit and report the data address to gdb through  
> siginfo struct.

There's two issues here, I think.  They solve different problems.

First, there's the question of targets which can't set true "read"
watchpoints, but can set "access" watchpoints.  Right now the x86
simply accepts a request to insert a read watchpoint, and generates an
access watchpoint instead.  How about if we changed it to refuse to
insert read watchpoints, changed breakpoint.c to attempt an access
watchpoint if inserting a read watchpoint fails, and then store in the
breakpoint which type was inserted?

In b->loc is probably a good place.  Right now there's no information
there about which kind of watchpoint was inserted.  We could add a
new field, and store hw_read/hw_access/hw_write in it when we insert
the breakpoint.

Would that fix this problem?

Second, there's the question of which sort of watchpoint we've hit. If
the target could tell us, say, as a return value from
target_stopped_data_address, then we could only check read watchpoints
when a read watchpoint triggers.  But I think this is a smaller issue;
at worst we might report that both a write and read watchpoint had
triggered when really only a write watchpoint had.  And there's corner
cases, like instructions which both read and write an address; they
should trigger both read and write watchpoints but I doubt any platform
gives us enough information to figure out that that's happened.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Fw: [ppc-linux-nat]: set access flag for h/w watchpoint even if it is only read or write (fwd)

[Eli Zaretskii]

> Date: Wed, 12 Jul 2006 13:31:25 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: eliz@gnu.org, gdb-patches@sourceware.org
> 
> First, there's the question of targets which can't set true "read"
> watchpoints, but can set "access" watchpoints.  Right now the x86
> simply accepts a request to insert a read watchpoint, and generates an
> access watchpoint instead.  How about if we changed it to refuse to
> insert read watchpoints, changed breakpoint.c to attempt an access
> watchpoint if inserting a read watchpoint fails, and then store in the
> breakpoint which type was inserted?

Sorry, I don't understand your plan.  How is it different from what
GDB does now (silently replace read with access watchpoint)?  And what
type will we store when the user says "rwatch"?

> Second, there's the question of which sort of watchpoint we've hit. If
> the target could tell us, say, as a return value from
> target_stopped_data_address, then we could only check read watchpoints
> when a read watchpoint triggers.  But I think this is a smaller issue;
> at worst we might report that both a write and read watchpoint had
> triggered when really only a write watchpoint had.

I think this is bad: if a user sets two different watchpoints at the
same address, she wants each watchpoint to fire when its specific
conditions are met.

My philosophy here is that watchpoints are a kind of silver bullet:
you use them when you have almost no idea who could touch the data in
question.  So watchpoints should perform as close to the spec as
possible, because the user is already in a very confused state;
confusing her even more with imprecise watch reports would be a bad
mantra, IMO.

> And there's corner
> cases, like instructions which both read and write an address; they
> should trigger both read and write watchpoints but I doubt any platform
> gives us enough information to figure out that that's happened.

In those cases, it is okay to show both watchpoints, or even just the
write one.

Re: Fw: [ppc-linux-nat]: set access flag for h/w watchpoint even if it is only read or write (fwd)

[Daniel Jacobowitz]

On Wed, Jul 12, 2006 at 09:15:48PM +0300, Eli Zaretskii wrote:
> > Date: Wed, 12 Jul 2006 13:31:25 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > Cc: eliz@gnu.org, gdb-patches@sourceware.org
> > 
> > First, there's the question of targets which can't set true "read"
> > watchpoints, but can set "access" watchpoints.  Right now the x86
> > simply accepts a request to insert a read watchpoint, and generates an
> > access watchpoint instead.  How about if we changed it to refuse to
> > insert read watchpoints, changed breakpoint.c to attempt an access
> > watchpoint if inserting a read watchpoint fails, and then store in the
> > breakpoint which type was inserted?
> 
> Sorry, I don't understand your plan.  How is it different from what
> GDB does now (silently replace read with access watchpoint)?  And what
> type will we store when the user says "rwatch"?

It's a layering change.

Right now the i386 target says "you asked me to insert a read
watchpoint.  OK!  I've done it" and sneakily inserts an access
watchpoint instead.  That means higher layers can't tell that it's
going to fire unexpectedly.

If the i386 target instead said "Sorry, I can't insert a read
watchpoint" and the higher layer said "well, how about this - can
you insert an access watchpoint?" then it would be able to handle and
skip the unexpected firing.

> > Second, there's the question of which sort of watchpoint we've hit. If
> > the target could tell us, say, as a return value from
> > target_stopped_data_address, then we could only check read watchpoints
> > when a read watchpoint triggers.  But I think this is a smaller issue;
> > at worst we might report that both a write and read watchpoint had
> > triggered when really only a write watchpoint had.
> 
> I think this is bad: if a user sets two different watchpoints at the
> same address, she wants each watchpoint to fire when its specific
> conditions are met.
> 
> My philosophy here is that watchpoints are a kind of silver bullet:
> you use them when you have almost no idea who could touch the data in
> question.  So watchpoints should perform as close to the spec as
> possible, because the user is already in a very confused state;
> confusing her even more with imprecise watch reports would be a bad
> mantra, IMO.

Yes, I know.  I'm just trying to be pragmatic.  The two items in my
mail are different problems and will require different solutions;
fixing one is better than fixing neither IMO, and this is the harder
one to fix.  It would be a shame to block a fix for one on a fix for
the other.

There's probably more than one target that can't tell us which sort of
watchpoint triggered.  In fact, I don't have first hand knowledge of
any target which could tell us which triggered.  You might be able to
distinguish if you used one set of watchpoint registers to set a write
watchpoint and another to set a read watchpoint - but that's awfully
wasteful of watchpoint registers.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Fw: [ppc-linux-nat]: set access flag for h/w watchpoint even if it is only read or write (fwd)

[Eli Zaretskii]

> Date: Wed, 12 Jul 2006 14:29:58 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Wu Zhou <woodzltc@cn.ibm.com>, gdb-patches@sourceware.org
> 
> Right now the i386 target says "you asked me to insert a read
> watchpoint.  OK!  I've done it" and sneakily inserts an access
> watchpoint instead.  That means higher layers can't tell that it's
> going to fire unexpectedly.
> 
> If the i386 target instead said "Sorry, I can't insert a read
> watchpoint" and the higher layer said "well, how about this - can
> you insert an access watchpoint?" then it would be able to handle and
> skip the unexpected firing.

I don't mind this change.  Thanks for explaining it.