Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for adding data watchpoints #1151

Open
RaamyPi opened this issue Oct 16, 2024 · 11 comments
Open

Support for adding data watchpoints #1151

RaamyPi opened this issue Oct 16, 2024 · 11 comments

Comments

@RaamyPi
Copy link

RaamyPi commented Oct 16, 2024

According to the latest debug spec (dated May 15, 2024), the select field in mcontrol and mcontrol6 provides an option to configure a watchpoint to break on address/data but openocd doesn't have any reference to this field.

@TommyMurphyTM1234
Copy link
Collaborator

@RaamyPi
Copy link
Author

RaamyPi commented Oct 16, 2024

Here is the function in discussion: https://github.com/riscv-collab/riscv-openocd/blob/riscv/src/target/riscv/riscv.c#L1514

As you can observe, there is no reference to the 'select' field.

@en-sc
Copy link
Collaborator

en-sc commented Oct 16, 2024

@RaamyPi, AFAIK neither GDB, nor OpenOCD provides means to set a watchpoint on data values (i.e. there is no "select" in OpenOCD's wp or GDB's watch/awatch.

If you wish for this feature to be supported (on the level of wp command) consider posting this question to mainline OpenOCD: https://openocd.org/pages/mailing-lists.html

In the meantime nothing prevents you from setting the watchpoint on data values manually, by reserving the trigger (riscv reserve_trigger command) and programming the trigger with accesses to tselect, tdata1, tdata2 etc.

@TommyMurphyTM1234
Copy link
Collaborator

@RaamyPi, AFAIK neither GDB, nor OpenOCD provides means to set a watchpoint on data values (i.e. there is no "select" in OpenOCD's wp or GDB's watch/awatch.

Hi @en-sc, I think you're correct about GDB:

but doesn't OpenOCD support this via the value argument?

Command: wp [address length [(r|w|a) [value [mask]]]]

With no parameters, lists all active watchpoints. Else sets a data watchpoint on data from address for length bytes. The watch point is an "access" watchpoint unless the r or w parameter is provided, defining it as respectively a read or write watchpoint. If a value is provided, that value is used when determining if the watchpoint should trigger. The value may be first be masked using mask to mark “don’t care” fields.

@en-sc
Copy link
Collaborator

en-sc commented Oct 16, 2024

@TommyMurphyTM1234, thanks! I've missed it.

Then it's a bit easier. RISC-V targets can provide support for OpenOCD watchpoints with value specified by chaining a watchpoint with select = address and a watchpoint with select = data.

@TommyMurphyTM1234
Copy link
Collaborator

FWIW this code seems to explicitly check for and reject watchpoints that specify a data value to match:

  • int riscv_add_watchpoint(struct target *target, struct watchpoint *watchpoint)
    {
    if (watchpoint->mask != WATCHPOINT_IGNORE_DATA_VALUE_MASK) {
    LOG_TARGET_ERROR(target, "Watchpoints on data values are not implemented");
    return ERROR_TARGET_RESOURCE_NOT_AVAILABLE;
    }

Then it's a bit easier. RISC-V targets can provide support for OpenOCD watchpoints with value specified by chaining a watchpoint with select = address and a watchpoint with select = data.

It's certainly feasible but I'm not sure about "easy"? At the moment the code seems to be predicated on using one trigger per watchpoint (and hardware breakpoint?) whereas this change would require additional trigger resource management and availability checking in order to deal with the situation where two triggers are required for a data watchpoint with a value match?

@en-sc
Copy link
Collaborator

en-sc commented Oct 16, 2024

At the moment the code seems to be predicated on using one trigger per watchpoint (and hardware breakpoint?)...

Not exactly. RISC-V OpenOCD already uses chained triggers, e.g. to set an unaligned watchpoint:

ret = try_setup_chained_match_triggers(target, trigger, ge_1, lt_2);

@TommyMurphyTM1234
Copy link
Collaborator

At the moment the code seems to be predicated on using one trigger per watchpoint (and hardware breakpoint?)...

Not exactly. RISC-V OpenOCD already uses chained triggers, e.g. to set an unaligned watchpoint:

ret = try_setup_chained_match_triggers(target, trigger, ge_1, lt_2);

Ah - OK - thanks. I wasn't aware of that. 👍

@RaamyPi
Copy link
Author

RaamyPi commented Oct 17, 2024

If you wish for this feature to be supported (on the level of wp command) consider posting this question to mainline OpenOCD: https://openocd.org/pages/mailing-lists.html

@en-sc thank you, I shall follow this up.

In the meantime nothing prevents you from setting the watchpoint on data values manually, by reserving the trigger (riscv reserve_trigger command) and programming the trigger with accesses to tselect, tdata1, tdata2 etc.

@en-sc you're right and I have already tried this as well but this is impractical as gdb is not aware of these artifical watchpoints.

@TommyMurphyTM1234
Copy link
Collaborator

@en-sc you're right and I have already tried this as well but this is impractical as gdb is not aware of these artifical watchpoints.

Even if/when OpenOCD is extended to support value option on data watchpoints this may still be the case given that GDB doesn't have such an option. Unless it will have some awareness of watchpoints set using something like monitor wp 0x80004000 w 0xdeadbeef? But I don't think that it will? If you want full control and awareness of the value option in GDB then GDB itself would probably also need to be extended?

@RaamyPi
Copy link
Author

RaamyPi commented Oct 17, 2024

Even if/when OpenOCD is extended to support value option on data watchpoints this may still be the case given that GDB doesn't have such an option. Unless it will have some awareness of watchpoints set using something like monitor wp 0x80004000 w 0xdeadbeef? But I don't think that it will? If you want full control and awareness of the value option in GDB then GDB itself would probably also need to be extended?

GDB definitely has to be extended as well, I wanted to take up the openocd side of things before that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants