read target register to decide breakpoint size

read target register to decide breakpoint size

[Tim Newsome]

I'm still working on RISC-V support for gdb. Any given RISC-V core may
support a compressed instruction set (2 bytes per instruction as
opposed to 4). There are corresponding 2-byte and 4-byte breakpoint
instructions. On cores that support the compressed instruction set it
is safe to just always use the 2-byte version, and there is a register
I can read to tell me whether the compressed instruction set is
supported. What I would like to do is read (and cache) that register
when breakpoint size is determined. That seems more robust than making
a decision based on ELF info, which may not reflect what is actually
being executed.

Is that a good idea? Are there examples of operations that read target
registers to complete?

Thank you,
Tim

Re: read target register to decide breakpoint size

[Ofir Cohen]

Hi,
I've been messing with that code lately,
and I recall gdb is checking if the instruction is a breakpoint instruction
in validate_inserted_breakpoint() function:

if (err || memcmp (buf, bp_opcode (bp), bp_size (bp)) != 0)

Also, you should check out breakpoint_from_pc callback function (IIRC
implemented in target_ops), from [1]:
"breakpoint_from_pc. Returns the breakpoint instruction to be used
when the PC is at a
particular location in memory. For architectures with variable length
instructions, the
choice of breakpoint instruction may depend on the length of the
instruction at the
program counter. Returns the instruction sequence and its length.
The default value is NULL (undefined). This function should always be
defined if GDB is
to support breakpointing for this architecture."

So IOW gdb is (somewhat) flexible when it comes to determining
instruction breakpoints.

You should probably initialize the breakpoint size (2/4) at target
init stage, cache it within your module and return at runtime the
(correct) cached size.

Guys on the mailing list can probably elabore more on this, but hey
this is a place to start :-).

Good luck.

Regards,
Ofir Cohen

[1] http://www.embecosm.com/appnotes/ean3/embecosm-howto-gdb-porting-ean3-issue-2.pdf

On 19 November 2016 at 01:44, Tim Newsome <tim@sifive.com> wrote:
> I'm still working on RISC-V support for gdb. Any given RISC-V core may
> support a compressed instruction set (2 bytes per instruction as
> opposed to 4). There are corresponding 2-byte and 4-byte breakpoint
> instructions. On cores that support the compressed instruction set it
> is safe to just always use the 2-byte version, and there is a register
> I can read to tell me whether the compressed instruction set is
> supported. What I would like to do is read (and cache) that register
> when breakpoint size is determined. That seems more robust than making
> a decision based on ELF info, which may not reflect what is actually
> being executed.
>
> Is that a good idea? Are there examples of operations that read target
> registers to complete?
>
> Thank you,
> Tim

Re: read target register to decide breakpoint size

[Antoine Tremblay]

Tim Newsome writes:

> I'm still working on RISC-V support for gdb. Any given RISC-V core may
> support a compressed instruction set (2 bytes per instruction as
> opposed to 4). There are corresponding 2-byte and 4-byte breakpoint
> instructions. On cores that support the compressed instruction set it
> is safe to just always use the 2-byte version, and there is a register
> I can read to tell me whether the compressed instruction set is
> supported. What I would like to do is read (and cache) that register
> when breakpoint size is determined. That seems more robust than making
> a decision based on ELF info, which may not reflect what is actually
> being executed.
>
> Is that a good idea? Are there examples of operations that read target
> registers to complete?

Yes actually you can check how ARM does it, it has the same kind of
problem with 3 breakpoints you can set for thumb, thumb2 and arm
instruction sets.

See arm-tdep.c:arm_sw_breakpoint_from_kind and
arm_breakpoint_kind_from_current_state

This is called in breakpoint.c:breakpoint_kind and it can use a register
to make the decision from the current state of that register.

So possibly just implementing the sw_breakpoint_from_kind and
breakpoint_kind_from_current state would be ok your you.

Regards,
Antoine Tremblay

Re: read target register to decide breakpoint size

[Tim Newsome]

Thanks, Antoine! That's exactly what I was looking for.

Tim

On Mon, Nov 21, 2016 at 8:36 AM, Antoine Tremblay
<antoine.tremblay@ericsson.com> wrote:
>
>
> Tim Newsome writes:
>
> > I'm still working on RISC-V support for gdb. Any given RISC-V core may
> > support a compressed instruction set (2 bytes per instruction as
> > opposed to 4). There are corresponding 2-byte and 4-byte breakpoint
> > instructions. On cores that support the compressed instruction set it
> > is safe to just always use the 2-byte version, and there is a register
> > I can read to tell me whether the compressed instruction set is
> > supported. What I would like to do is read (and cache) that register
> > when breakpoint size is determined. That seems more robust than making
> > a decision based on ELF info, which may not reflect what is actually
> > being executed.
> >
> > Is that a good idea? Are there examples of operations that read target
> > registers to complete?
>
> Yes actually you can check how ARM does it, it has the same kind of
> problem with 3 breakpoints you can set for thumb, thumb2 and arm
> instruction sets.
>
> See arm-tdep.c:arm_sw_breakpoint_from_kind and
> arm_breakpoint_kind_from_current_state
>
> This is called in breakpoint.c:breakpoint_kind and it can use a register
> to make the decision from the current state of that register.
>
> So possibly just implementing the sw_breakpoint_from_kind and
> breakpoint_kind_from_current state would be ok your you.
>
> Regards,
> Antoine Tremblay

Re: read target register to decide breakpoint size

[Tim Newsome]

I finally got around to implementing this, but gdb still insists on
calling breakpoint_kind_from_pc, where I don't have access to a
regcache object (I think). Eg.
```
#0  riscv_breakpoint_kind_from_pc (gdbarch=0xcc8a90,
pcptr=0x7fffffffde68) at riscv-tdep.c:184
#1  0x0000000000541301 in default_breakpoint_from_pc
(gdbarch=0xcc8a90, pcptr=<optimized out>, lenptr=0x7fffffffde64)
    at arch-utils.c:847
#2  0x00000000004b40b3 in program_breakpoint_here_p
(gdbarch=<optimized out>, address=2147550320) at breakpoint.c:9113
#3  0x00000000004b424c in bp_loc_is_permanent (loc=0xdd57b0) at
breakpoint.c:9156
#4  add_location_to_breakpoint (b=b@entry=0xda9aa0,
sal=sal@entry=0x7fffffffdf70) at breakpoint.c:9093
#5  0x00000000004b5169 in init_raw_breakpoint (ops=0xb4d200
<bkpt_breakpoint_ops>, bptype=bp_breakpoint, sal=...,
    gdbarch=0xcc8a90, b=0xda9aa0) at breakpoint.c:7586
#6  init_breakpoint_sal (b=b@entry=0xda9aa0,
gdbarch=gdbarch@entry=0xcc8a90, location=location@entry=0xda8f30,
    filter=filter@entry=0x0, cond_string=0x0, extra_string=0x0,
type=bp_breakpoint, disposition=disp_donttouch,
    thread=-1, task=0, ignore_count=0, ops=0xb4d200
<bkpt_breakpoint_ops>, from_tty=1, enabled=1, flags=0,
    display_canonical=0, internal=<optimized out>, sals=...) at
breakpoint.c:9300
#7  0x00000000004bb490 in create_breakpoint_sal (display_canonical=0,
flags=0, internal=0, enabled=1, from_tty=1,
    ops=0xb4d200 <bkpt_breakpoint_ops>, ignore_count=0, task=0,
thread=-1, disposition=disp_donttouch,
    type=bp_breakpoint, extra_string=0x0, cond_string=0x0, filter=0x0,
location=0xda8f30, gdbarch=0xcc8a90, sals=...)
    at breakpoint.c:9436
#8  create_breakpoints_sal (gdbarch=0xcc8a90,
canonical=0x7fffffffe170, cond_string=0x0, extra_string=0x0,
    type=bp_breakpoint, disposition=disp_donttouch, thread=-1, task=0,
ignore_count=0,
    ops=0xb4d200 <bkpt_breakpoint_ops>, from_tty=1, enabled=1,
internal=0, flags=0) at breakpoint.c:9490
#9  0x00000000004bbbfc in create_breakpoint (gdbarch=0xcc8a90,
location=location@entry=0xda3af0,
    cond_string=cond_string@entry=0x0, thread=-1, thread@entry=0,
extra_string=<optimized out>,
    extra_string@entry=0xda3ad6 "", parse_extra=parse_extra@entry=1,
tempflag=0, type_wanted=bp_breakpoint,
    ignore_count=0, pending_break_support=AUTO_BOOLEAN_AUTO,
ops=0xb4d200 <bkpt_breakpoint_ops>, from_tty=1, enabled=1,
    internal=0, flags=0) at breakpoint.c:9912
#10 0x00000000004bc099 in break_command_1 (arg=0xda3ad6 "",
flag=<optimized out>, from_tty=1) at breakpoint.c:10020
...
```

Is there some way I can check the misa register of my target from
default_breakpoint_from_pc, or have gdb not call
default_breakpoint_from_pc and use breakpoint_kind_from_current_state
instead?

Thank you,
Tim

On Mon, Nov 21, 2016 at 10:00 AM, Tim Newsome <tim@sifive.com> wrote:
> Thanks, Antoine! That's exactly what I was looking for.
>
> Tim
>
> On Mon, Nov 21, 2016 at 8:36 AM, Antoine Tremblay
> <antoine.tremblay@ericsson.com> wrote:
>>
>>
>> Tim Newsome writes:
>>
>> > I'm still working on RISC-V support for gdb. Any given RISC-V core may
>> > support a compressed instruction set (2 bytes per instruction as
>> > opposed to 4). There are corresponding 2-byte and 4-byte breakpoint
>> > instructions. On cores that support the compressed instruction set it
>> > is safe to just always use the 2-byte version, and there is a register
>> > I can read to tell me whether the compressed instruction set is
>> > supported. What I would like to do is read (and cache) that register
>> > when breakpoint size is determined. That seems more robust than making
>> > a decision based on ELF info, which may not reflect what is actually
>> > being executed.
>> >
>> > Is that a good idea? Are there examples of operations that read target
>> > registers to complete?
>>
>> Yes actually you can check how ARM does it, it has the same kind of
>> problem with 3 breakpoints you can set for thumb, thumb2 and arm
>> instruction sets.
>>
>> See arm-tdep.c:arm_sw_breakpoint_from_kind and
>> arm_breakpoint_kind_from_current_state
>>
>> This is called in breakpoint.c:breakpoint_kind and it can use a register
>> to make the decision from the current state of that register.
>>
>> So possibly just implementing the sw_breakpoint_from_kind and
>> breakpoint_kind_from_current state would be ok your you.
>>
>> Regards,
>> Antoine Tremblay

Re: read target register to decide breakpoint size

[Tim Newsome]

Actually, this seems to work inside breakpoint_kind_from_pc():
```
    struct frame_info *frame = get_current_frame ();
    uint32_t misa = get_frame_register_unsigned (frame, RISCV_CSR_MISA_REGNUM);
```

Is that kosher? If so, is there any reason for me to implement
breakpoint_kind_from_current_state?

Thanks,
Tim

On Tue, Dec 13, 2016 at 12:58 PM, Tim Newsome <tim@sifive.com> wrote:
> I finally got around to implementing this, but gdb still insists on
> calling breakpoint_kind_from_pc, where I don't have access to a
> regcache object (I think). Eg.
> ```
> #0  riscv_breakpoint_kind_from_pc (gdbarch=0xcc8a90,
> pcptr=0x7fffffffde68) at riscv-tdep.c:184
> #1  0x0000000000541301 in default_breakpoint_from_pc
> (gdbarch=0xcc8a90, pcptr=<optimized out>, lenptr=0x7fffffffde64)
>     at arch-utils.c:847
> #2  0x00000000004b40b3 in program_breakpoint_here_p
> (gdbarch=<optimized out>, address=2147550320) at breakpoint.c:9113
> #3  0x00000000004b424c in bp_loc_is_permanent (loc=0xdd57b0) at
> breakpoint.c:9156
> #4  add_location_to_breakpoint (b=b@entry=0xda9aa0,
> sal=sal@entry=0x7fffffffdf70) at breakpoint.c:9093
> #5  0x00000000004b5169 in init_raw_breakpoint (ops=0xb4d200
> <bkpt_breakpoint_ops>, bptype=bp_breakpoint, sal=...,
>     gdbarch=0xcc8a90, b=0xda9aa0) at breakpoint.c:7586
> #6  init_breakpoint_sal (b=b@entry=0xda9aa0,
> gdbarch=gdbarch@entry=0xcc8a90, location=location@entry=0xda8f30,
>     filter=filter@entry=0x0, cond_string=0x0, extra_string=0x0,
> type=bp_breakpoint, disposition=disp_donttouch,
>     thread=-1, task=0, ignore_count=0, ops=0xb4d200
> <bkpt_breakpoint_ops>, from_tty=1, enabled=1, flags=0,
>     display_canonical=0, internal=<optimized out>, sals=...) at
> breakpoint.c:9300
> #7  0x00000000004bb490 in create_breakpoint_sal (display_canonical=0,
> flags=0, internal=0, enabled=1, from_tty=1,
>     ops=0xb4d200 <bkpt_breakpoint_ops>, ignore_count=0, task=0,
> thread=-1, disposition=disp_donttouch,
>     type=bp_breakpoint, extra_string=0x0, cond_string=0x0, filter=0x0,
> location=0xda8f30, gdbarch=0xcc8a90, sals=...)
>     at breakpoint.c:9436
> #8  create_breakpoints_sal (gdbarch=0xcc8a90,
> canonical=0x7fffffffe170, cond_string=0x0, extra_string=0x0,
>     type=bp_breakpoint, disposition=disp_donttouch, thread=-1, task=0,
> ignore_count=0,
>     ops=0xb4d200 <bkpt_breakpoint_ops>, from_tty=1, enabled=1,
> internal=0, flags=0) at breakpoint.c:9490
> #9  0x00000000004bbbfc in create_breakpoint (gdbarch=0xcc8a90,
> location=location@entry=0xda3af0,
>     cond_string=cond_string@entry=0x0, thread=-1, thread@entry=0,
> extra_string=<optimized out>,
>     extra_string@entry=0xda3ad6 "", parse_extra=parse_extra@entry=1,
> tempflag=0, type_wanted=bp_breakpoint,
>     ignore_count=0, pending_break_support=AUTO_BOOLEAN_AUTO,
> ops=0xb4d200 <bkpt_breakpoint_ops>, from_tty=1, enabled=1,
>     internal=0, flags=0) at breakpoint.c:9912
> #10 0x00000000004bc099 in break_command_1 (arg=0xda3ad6 "",
> flag=<optimized out>, from_tty=1) at breakpoint.c:10020
> ...
> ```
>
> Is there some way I can check the misa register of my target from
> default_breakpoint_from_pc, or have gdb not call
> default_breakpoint_from_pc and use breakpoint_kind_from_current_state
> instead?
>
> Thank you,
> Tim
>
> On Mon, Nov 21, 2016 at 10:00 AM, Tim Newsome <tim@sifive.com> wrote:
>> Thanks, Antoine! That's exactly what I was looking for.
>>
>> Tim
>>
>> On Mon, Nov 21, 2016 at 8:36 AM, Antoine Tremblay
>> <antoine.tremblay@ericsson.com> wrote:
>>>
>>>
>>> Tim Newsome writes:
>>>
>>> > I'm still working on RISC-V support for gdb. Any given RISC-V core may
>>> > support a compressed instruction set (2 bytes per instruction as
>>> > opposed to 4). There are corresponding 2-byte and 4-byte breakpoint
>>> > instructions. On cores that support the compressed instruction set it
>>> > is safe to just always use the 2-byte version, and there is a register
>>> > I can read to tell me whether the compressed instruction set is
>>> > supported. What I would like to do is read (and cache) that register
>>> > when breakpoint size is determined. That seems more robust than making
>>> > a decision based on ELF info, which may not reflect what is actually
>>> > being executed.
>>> >
>>> > Is that a good idea? Are there examples of operations that read target
>>> > registers to complete?
>>>
>>> Yes actually you can check how ARM does it, it has the same kind of
>>> problem with 3 breakpoints you can set for thumb, thumb2 and arm
>>> instruction sets.
>>>
>>> See arm-tdep.c:arm_sw_breakpoint_from_kind and
>>> arm_breakpoint_kind_from_current_state
>>>
>>> This is called in breakpoint.c:breakpoint_kind and it can use a register
>>> to make the decision from the current state of that register.
>>>
>>> So possibly just implementing the sw_breakpoint_from_kind and
>>> breakpoint_kind_from_current state would be ok your you.
>>>
>>> Regards,
>>> Antoine Tremblay

Re: read target register to decide breakpoint size

[Yao Qi]

On 16-12-13 13:30:02, Tim Newsome wrote:
> Actually, this seems to work inside breakpoint_kind_from_pc():
> ```
>     struct frame_info *frame = get_current_frame ();
>     uint32_t misa = get_frame_register_unsigned (frame, RISCV_CSR_MISA_REGNUM);
> ```
> 
> Is that kosher? If so, is there any reason for me to implement
> breakpoint_kind_from_current_state?

I'd like not to do so.  Can't you decode the instruction to see whether
it is compressed or uncompressed?  I also think it is a good idea to
make a decision based on ELF info, as you mentioned in the first email.

-- 
Yao (齐尧)

Re: read target register to decide breakpoint size

[Antoine Tremblay]

Yao Qi writes:

> On 16-12-13 13:30:02, Tim Newsome wrote:
>> Actually, this seems to work inside breakpoint_kind_from_pc():
>> ```
>>     struct frame_info *frame = get_current_frame ();
>>     uint32_t misa = get_frame_register_unsigned (frame, RISCV_CSR_MISA_REGNUM);
>> ```
>> 
>> Is that kosher? If so, is there any reason for me to implement
>> breakpoint_kind_from_current_state?
>
> I'd like not to do so.  Can't you decode the instruction to see whether
> it is compressed or uncompressed?  I also think it is a good idea to
> make a decision based on ELF info, as you mentioned in the first email.

Correct me if I'm wrong but reading the RISC-V arch manual, it seems
like the MISA register information is static.

So unlike arm where the ISA might change, on RISC-V it is static for the
life of the program ? 

If that's true, and that you can't decode the instruction, it doesn't
seem that bad to use the register in kind_from_pc ?

Yao, was your concern that this would be non-static, or there is another
reason ?

However thinking about the ELF info, could you elaborate on why it may
not reflect what is actually being executed ? And how this is a problem
?

A note about _from_current_state too this is used while single_stepping
since GDB knows it's about to set a breakpoint from a known state to
the next location, this differs from from_pc which is used to set a pc from
any state to any location.

So in your case it's really from_pc that you want.

Thanks,
Antoine

Re: read target register to decide breakpoint size

[Tim Newsome]

On Wed, Dec 14, 2016 at 4:32 AM, Antoine Tremblay
<antoine.tremblay@ericsson.com> wrote:
>
> Yao Qi writes:
>
>> On 16-12-13 13:30:02, Tim Newsome wrote:
>>> Actually, this seems to work inside breakpoint_kind_from_pc():
>>> ```
>>>     struct frame_info *frame = get_current_frame ();
>>>     uint32_t misa = get_frame_register_unsigned (frame, RISCV_CSR_MISA_REGNUM);
>>> ```
>>>
>>> Is that kosher? If so, is there any reason for me to implement
>>> breakpoint_kind_from_current_state?
>>
>> I'd like not to do so.  Can't you decode the instruction to see whether
>> it is compressed or uncompressed?  I also think it is a good idea to
>> make a decision based on ELF info, as you mentioned in the first email.
>
> Correct me if I'm wrong but reading the RISC-V arch manual, it seems
> like the MISA register information is static.
>
> So unlike arm where the ISA might change, on RISC-V it is static for the
> life of the program ?

That's right. MISA doesn't ever change.

> If that's true, and that you can't decode the instruction, it doesn't
> seem that bad to use the register in kind_from_pc ?

I could take a stab at decoding the instruction, but that seems a lot
more painful.
1. It's more code to write.
2. RISC-V encourages people adding custom instructions, which gdb
shouldn't have to know about.
3. Decoding the instruction might force a memory read, which is slow.
I only need to read MISA once.

> Yao, was your concern that this would be non-static, or there is another
> reason ?
>
> However thinking about the ELF info, could you elaborate on why it may
> not reflect what is actually being executed ? And how this is a problem
> ?

In the simplest case, I can connect to my target using OpenOCD, and
not tell gdb what file is being executed. It should still be possible
to debug at the assembly level in that case.

> A note about _from_current_state too this is used while single_stepping
> since GDB knows it's about to set a breakpoint from a known state to
> the next location, this differs from from_pc which is used to set a pc from
> any state to any location.
>
> So in your case it's really from_pc that you want.

I see. So that's a case for ARM where it could look at cpsr.

Thank you, sounds like reading MISA from kind_from_pc should work out
fine then (unless Yao has an objection we didn't consider).

Tim

Re: read target register to decide breakpoint size

[Yao Qi]

On 16-12-14 09:01:53, Tim Newsome wrote:
> 
> Thank you, sounds like reading MISA from kind_from_pc should work out
> fine then (unless Yao has an objection we didn't consider).

You can't set breakpoint without the live inferior in this way.

./gdb a.out
(gdb) b main

-- 
Yao (齐尧)

Re: read target register to decide breakpoint size

[Antoine Tremblay]

Yao Qi writes:

> On 16-12-14 09:01:53, Tim Newsome wrote:
>> 
>> Thank you, sounds like reading MISA from kind_from_pc should work out
>> fine then (unless Yao has an objection we didn't consider).
>
> You can't set breakpoint without the live inferior in this way.
>
> ./gdb a.out
> (gdb) b main

I wonder if GDB could fallback to ELF if the register can't be read ?

Not sure how that would be done...