Re: [PATCH] btrace: continue recording after connection loss.

[Pedro Alves <palves at redhat 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 (&current_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