[1/2] Set bp_tgt->reqstd_address and bp_tgt->placed_size in record_full_insert_breakpoint

Message ID 1459759138-1494-2-git-send-email-yao.qi@linaro.org
State New, archived
Headers

Commit Message

Yao Qi April 4, 2016, 8:38 a.m. UTC
  I notice that bp_tgt won't be fully initialized if to_insert_breakpoint
isn't called in record_full_insert_breakpoint, and bp_tgt->reqstd_address
is zero, so an entry is added to record_full_breakpoints, but its address
is zero, which is wrong.  This patch is to call gdbarch_breakpoint_from_pc
in the else branch to set bp_tgt->reqstd_address and bp_tgt->placed_size.

gdb:

2016-04-04  Yao Qi  <yao.qi@linaro.org>

	* record-full.c (record_full_insert_breakpoint): Set
	bp_tgt->reqstd_address and bp_tgt->placed_size.
---
 gdb/record-full.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
  

Comments

Luis Machado April 5, 2016, 8:51 p.m. UTC | #1
On 04/04/2016 03:38 AM, Yao Qi wrote:
> I notice that bp_tgt won't be fully initialized if to_insert_breakpoint
> isn't called in record_full_insert_breakpoint, and bp_tgt->reqstd_address
> is zero, so an entry is added to record_full_breakpoints, but its address
> is zero, which is wrong.  This patch is to call gdbarch_breakpoint_from_pc
> in the else branch to set bp_tgt->reqstd_address and bp_tgt->placed_size.
>
> gdb:
>
> 2016-04-04  Yao Qi  <yao.qi@linaro.org>
>
> 	* record-full.c (record_full_insert_breakpoint): Set
> 	bp_tgt->reqstd_address and bp_tgt->placed_size.
> ---
>   gdb/record-full.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/gdb/record-full.c b/gdb/record-full.c
> index f6023bf..066a8e7 100644
> --- a/gdb/record-full.c
> +++ b/gdb/record-full.c
> @@ -1670,6 +1670,16 @@ record_full_insert_breakpoint (struct target_ops *ops,
>
>         in_target_beneath = 1;
>       }
> +  else
> +    {
> +      CORE_ADDR addr = bp_tgt->reqstd_address;

Do we really need to get this initialized?

> +      int bplen;
> +
> +      gdbarch_breakpoint_from_pc (gdbarch, &addr, &bplen);
> +
> +      bp_tgt->placed_address = addr;
> +      bp_tgt->placed_size = bplen;
> +    }
>
>     bp = XNEW (struct record_full_breakpoint);
>     bp->addr = bp_tgt->placed_address;
>

Otherwise looks good.
  
Yao Qi April 6, 2016, 8:36 a.m. UTC | #2
Luis Machado <lgustavo@codesourcery.com> writes:

>> +      CORE_ADDR addr = bp_tgt->reqstd_address;
>
> Do we really need to get this initialized?
>

Yes, 'addr' is passed to gdbarch_breakpoint_from_pc below.

>> +      int bplen;
>> +
>> +      gdbarch_breakpoint_from_pc (gdbarch, &addr, &bplen);
>> +
>> +      bp_tgt->placed_address = addr;
>> +      bp_tgt->placed_size = bplen;
  
Luis Machado April 6, 2016, 12:23 p.m. UTC | #3
On 04/06/2016 03:36 AM, Yao Qi wrote:
> Luis Machado <lgustavo@codesourcery.com> writes:
>
>>> +      CORE_ADDR addr = bp_tgt->reqstd_address;
>>
>> Do we really need to get this initialized?
>>
>
> Yes, 'addr' is passed to gdbarch_breakpoint_from_pc below.
>

Ah, indeed. For a moment i thought addr was initialized within 
gdbarch_breakpoint_from_pc.

>>> +      int bplen;
>>> +
>>> +      gdbarch_breakpoint_from_pc (gdbarch, &addr, &bplen);
>>> +
>>> +      bp_tgt->placed_address = addr;
>>> +      bp_tgt->placed_size = bplen;
>
  

Patch

diff --git a/gdb/record-full.c b/gdb/record-full.c
index f6023bf..066a8e7 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1670,6 +1670,16 @@  record_full_insert_breakpoint (struct target_ops *ops,
 
       in_target_beneath = 1;
     }
+  else
+    {
+      CORE_ADDR addr = bp_tgt->reqstd_address;
+      int bplen;
+
+      gdbarch_breakpoint_from_pc (gdbarch, &addr, &bplen);
+
+      bp_tgt->placed_address = addr;
+      bp_tgt->placed_size = bplen;
+    }
 
   bp = XNEW (struct record_full_breakpoint);
   bp->addr = bp_tgt->placed_address;