diff mbox

[v2,3/7] Implement breakpoint_from_kind for supported architectures in GDBServer.

Message ID 1444063455-31558-4-git-send-email-antoine.tremblay@ericsson.com
State New, archived
Headers

Commit Message

Antoine Tremblay Oct. 5, 2015, 4:44 p.m. UTC 
  This patch implements the breakpoint_from_kind linux_target_ops for
architectures that support software breakpoints, namely : x86 and aarch64.

No regressions, tested on Ubuntu 14.04 x86.
With gdbserver-{native,extended}.

Compilation was also tested on aarch64.
---
 gdb/gdbserver/linux-aarch64-low.c | 9 +++++++++
 gdb/gdbserver/linux-x86-low.c     | 9 +++++++++
 2 files changed, 18 insertions(+)

Comments

Yao Qi Oct. 15, 2015, 9:19 a.m. UTC | #1 
Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

There is no changelog entry.

> +/* Implementation of linux_target_ops method "breakpoint_from_kind".  */
> +
> +static const unsigned char *
> +aarch64_breakpoint_from_kind (int *kind)
> +{
> +    return (const unsigned char *) &aarch64_breakpoint;

Indentation looks odd, and do we really need the cast?

Note that this function is correct because we restrict the usage of Z0
packet.  Z0 packet is only used with non-extended protocol and inferior
is 64bit.  See aarch64_supports_z_point_type.  Once we remove the
restriction, we need to update this function to return different
breakpoint instructions (aarch64, arm, thumb, and thumb2) according to
*KIND and other information.

Otherwise, patch is OK to me.
Antoine Tremblay Oct. 15, 2015, 10:57 a.m. UTC | #2 
On 10/15/2015 05:19 AM, Yao Qi wrote:
> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
> There is no changelog entry.
>
Wow sorry adding it.

>> +/* Implementation of linux_target_ops method "breakpoint_from_kind".  */
>> +
>> +static const unsigned char *
>> +aarch64_breakpoint_from_kind (int *kind)
>> +{
>> +    return (const unsigned char *) &aarch64_breakpoint;
>
> Indentation looks odd, and do we really need the cast?
>

Fixed.

> Note that this function is correct because we restrict the usage of Z0
> packet.  Z0 packet is only used with non-extended protocol and inferior
> is 64bit.  See aarch64_supports_z_point_type.  Once we remove the
> restriction, we need to update this function to return different
> breakpoint instructions (aarch64, arm, thumb, and thumb2) according to
> *KIND and other information.

Yes indeed.

>
> Otherwise, patch is OK to me.
>
Antoine Tremblay Oct. 15, 2015, 5:13 p.m. UTC | #3 
On 10/15/2015 06:57 AM, Antoine Tremblay wrote:
>
>
> On 10/15/2015 05:19 AM, Yao Qi wrote:
>> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>>
>> There is no changelog entry.
>>
> Wow sorry adding it.
>
>>> +/* Implementation of linux_target_ops method
>>> "breakpoint_from_kind".  */
>>> +
>>> +static const unsigned char *
>>> +aarch64_breakpoint_from_kind (int *kind)
>>> +{
>>> +    return (const unsigned char *) &aarch64_breakpoint;
>>
>> Indentation looks odd, and do we really need the cast?
>>
>
> Fixed.

Note this was changed to gdb_byte the cast is removed and 
aarch64_breakpoint is returned not &aarch64_breakpoint...

>
>> Note that this function is correct because we restrict the usage of Z0
>> packet.  Z0 packet is only used with non-extended protocol and inferior
>> is 64bit.  See aarch64_supports_z_point_type.  Once we remove the
>> restriction, we need to update this function to return different
>> breakpoint instructions (aarch64, arm, thumb, and thumb2) according to
>> *KIND and other information.
>
> Yes indeed.
>
>>
>> Otherwise, patch is OK to me.
>>
diff mbox

Patch

diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 1e7a0ff..46b79a5 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -3238,6 +3238,14 @@  aarch64_supports_range_stepping (void)
   return 1;
 }
 
+/* Implementation of linux_target_ops method "breakpoint_from_kind".  */
+
+static const unsigned char *
+aarch64_breakpoint_from_kind (int *kind)
+{
+    return (const unsigned char *) &aarch64_breakpoint;
+}
+
 struct linux_target_ops the_low_target =
 {
   aarch64_arch_setup,
@@ -3270,6 +3278,7 @@  struct linux_target_ops the_low_target =
   aarch64_emit_ops,
   aarch64_get_min_fast_tracepoint_insn_len,
   aarch64_supports_range_stepping,
+  aarch64_breakpoint_from_kind,
 };
 
 void
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 41803c4..e64b498 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -3258,6 +3258,14 @@  x86_supports_range_stepping (void)
   return 1;
 }
 
+/* Implementation of linux_target_ops method "breakpoint_from_kind".  */
+
+static const unsigned char *
+x86_breakpoint_from_kind (int *kind)
+{
+  return x86_breakpoint;
+}
+
 /* This is initialized assuming an amd64 target.
    x86_arch_setup will correct it for i386 or amd64 targets.  */
 
@@ -3297,6 +3305,7 @@  struct linux_target_ops the_low_target =
   x86_emit_ops,
   x86_get_min_fast_tracepoint_insn_len,
   x86_supports_range_stepping,
+  x86_breakpoint_from_kind,
 };
 
 void