This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch v8 17/24] record-btrace: provide xfer_partial target method
- From: Pedro Alves <palves at redhat dot com>
- To: "Metzger, Markus T" <markus dot t dot metzger at intel dot com>
- Cc: "jan dot kratochvil at redhat dot com" <jan dot kratochvil at redhat dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Tue, 17 Dec 2013 16:56:32 +0000
- Subject: Re: [patch v8 17/24] record-btrace: provide xfer_partial target method
- Authentication-results: sourceware.org; auth=none
- References: <1386839747-8860-1-git-send-email-markus dot t dot metzger at intel dot com> <1386839747-8860-18-git-send-email-markus dot t dot metzger at intel dot com> <52AB555A dot 3070301 at redhat dot com> <A78C989F6D9628469189715575E55B230AA38C53 at IRSMSX104 dot ger dot corp dot intel dot com> <52AF5188 dot 9040800 at redhat dot com> <A78C989F6D9628469189715575E55B230AA395A5 at IRSMSX104 dot ger dot corp dot intel dot com>
On 12/17/2013 11:57 AM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: Pedro Alves [mailto:palves@redhat.com]
>> Sent: Monday, December 16, 2013 8:16 PM
>
>
>>> I changed the return -1 to throw_error (...) and added a check for
>>> writebuf != NULL. Suddenly I got tons of errors when GDB can't insert
>>> breakpoints any longer for (reverse-)stepping.
>>
>> This is why precord keeps track of breakpoints itself too:
> [...]
>>> Also stepping gets broken.
>>
>> I can't immediately why that would be.
>
> Because we can't set temporary breakpoints.
>
>
>>> I now get an error when trying to access a variable with static storage
>>> duration or when trying to access memory directly via its address.
>>> It would be nice to also get an <unavailable> in those cases. This would
>>> require the respective layer to catch my exception.
>>
>> Please try returning TARGET_XFER_E_UNAVAILABLE instead.
>
> That is ignored just like the -1 I returned earlier. I nevertheless changed
> the default return to that since it is more descriptive.
Thanks. Hmm, yes, looks like raw_memory_xfer_partial carries on
looking at the target beneath, and then when that fails, we'll
lose TARGET_XFER_E_UNAVAILABLE, and return TARGET_XFER_E_IO/-1,
losing the better TARGET_XFER_E_UNAVAILABLE.
>>> To avoid those errors when trying to set breakpoints, I could try
>>> providing to_insert_breakpoint and to_remove_breakpoint methods
>>> and maintain my own breakpoints.
>>
>> Right.
>
> I have something to temporarily disable the xfer checks during
> to_insert_breakpoint and to_remove_breakpoint.
>
> Not sure whether this is considered too hacky or what else I'm missing.
It's hacky as the breakpoints in memory will never actually
trigger/execute. If you want to assume that the inferior's current
read only sections match exactly the read only sections the program
had when the trace was taken, I won't insist. The assumption will
fail across tracing e.g., dlopen/dlclose/mmap/unmmap, as breakpoints
will fail to insert on unmapped sections.
> My tests all pass. Any idea where else GDB would need to access
> target memory in order to function correctly?
Can't think of anything.
> Here's the patch. I omit a preparation patch to pass target_ops to
> to_insert_breakpoint and to_remove_breakpoint so that the request
> can be forwarded to the target beneath.
>
> diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
> index 00a056d..0536071 100644
> --- a/gdb/record-btrace.c
> +++ b/gdb/record-btrace.c
> @@ -42,6 +42,9 @@ static struct target_ops record_btrace_ops;
> /* A new thread observer enabling branch tracing for the new thread. */
> static struct observer *record_btrace_thread_observer;
>
> +/* Temporarily allow memory accesses. */
> +static int record_btrace_allow_memory_access;
> +
> /* Print a record-btrace debug message. Use do ... while (0) to avoid
> ambiguities when used in if statements. */
>
> @@ -805,7 +808,7 @@ record_btrace_xfer_partial (struct target_ops *ops, enum target_object object,
> struct target_ops *t;
>
> /* Filter out requests that don't make sense during replay. */
> - if (record_btrace_is_replaying ())
> + if (record_btrace_allow_memory_access == 0 && record_btrace_is_replaying ())
We use ! for boolean ints, so write:
if (!record_btrace_allow_memory_access && record_btrace_is_replaying ())
--
Pedro Alves