This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] btrace: continue recording after connection loss.
- From: Pedro Alves <palves at redhat dot com>
- To: "Metzger, Markus T" <markus dot t dot metzger at intel dot com>, "Wiederhake, Tim" <tim dot wiederhake at intel dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Thu, 23 Jun 2016 15:10:04 +0100
- Subject: Re: [PATCH] btrace: continue recording after connection loss.
- Authentication-results: sourceware.org; auth=none
- References: <1465464748-24727-1-git-send-email-tim dot wiederhake at intel dot com> <A78C989F6D9628469189715575E55B23332EFBCB at IRSMSX104 dot ger dot corp dot intel dot com>
On 06/23/2016 02:40 PM, Metzger, Markus T wrote:
>> From: Wiederhake, Tim
>> With this patch, gdb checks for running records on connect, pushes the record
>> target if necessary and allows to resume the recording.
>
> For the record, this goes back to an issue pointed out by Pedro:
> https://sourceware.org/ml/gdb-patches/2015-11/msg00420.html.
>
Ah, thanks for fixing it!
/me runs a pair of extra eyes over the patch.
>
> +/* Read the current thread's btrace configuration from the target and
> + store it into CONF. Returns -1 on failure. */
> +
> +static int
> +remote_btrace_read_config (struct btrace_config *conf)
> +{
> + int ret = 0;
> + char *xml = target_read_stralloc (¤t_target,
> + TARGET_OBJECT_BTRACE_CONF, "");
> +
> + if (xml == NULL)
> + return -1;
> +
> + TRY
> + {
> + parse_xml_btrace_conf (conf, xml);
> + }
> + CATCH (err, RETURN_MASK_ERROR)
Since this doesn't catch QUITs (Ctrl-C) ...
> + {
> + if (err.message != NULL)
> + warning ("%s", err.message);
And also this warning can throw too (pagination -> Ctrl-C).
> + ret = -1;
> + }
> + END_CATCH
> +
> + xfree (xml);
... this xfree isn't guaranteed to be reached. You need to
use a cleanup to xfree xml instead.
> + return ret;
> +}
> + if (remote_btrace_read_config (&rs->btrace_config) != -1)
> + {
> + #if !defined (HAVE_LIBIPT)
We don't use this #if indentation style in gdb. Please put the #if
at column 0, and then indent the C if as if no #if was there.
> + if (rs->btrace_config.format == BTRACE_FORMAT_PT)
> +# Simulate connection loss by killing gdb.
> +set gdb_pid [exp_pid -i [board_info host fileid]]
> +exec kill "-KILL" ${gdb_pid}
This kills a random process in the build machine, when
remote-host testing. You need do use "remote_exec host"
instead of bare "exec".
Since "disconnect" is supposed to leave target as it was,
and now that we can recover the btrace state, I wonder about
making "disconnect" not stop the trace. (And with that, you
could test with the "disconnect" command, instead of
force-killing gdb.)
Thanks,
Pedro Alves