Re: [PATCH v2 1/2] Debugging without a binary (regression)

[Pedro Alves <palves@redhat.com>]

On 04/12/2016 08:24 PM, Luis Machado wrote:
> When we attempt to debug a process using GDBserver in standard remote mode
> without a symbol file on GDB's end, we may run into an issue where GDB cuts
> the connection attempt short due to an error. The error is caused by not
> being able to open a symbol file, like so:
> 
> --
> 
> (gdb) set sysroot
> (gdb) tar rem :2345
> Remote debugging using :2345
> /proc/23769/exe: Permission denied.
> (gdb) i r
> The program has no registers now.
> (gdb)
> 
> It should've been like this:
> 
> (gdb) set sysroot
> (gdb) tar rem :2345
> Remote debugging using :2345
> 0xf7ddb2d0 in ?? ()

The warning output is missing here, right?

> (gdb) i r
> eax            0x0  0
> ecx            0x0  0
> edx            0x0  0
> ebx            0x0  0
> esp            0xffffdfa0 0xffffdfa0
> ebp            0x0  0x0
> esi            0x0  0
> edi            0x0  0
> eip            0xf7ddb2d0 0xf7ddb2d0
> eflags         0x200  [ IF ]
> cs             0x33 51
> ss             0x2b 43
> ds             0x0  0
> es             0x0  0
> fs             0x0  0
> gs             0x0  0
> (gdb)
> 

>  void
> @@ -142,6 +161,7 @@ exec_file_locate_attach (int pid, int from_tty)
>  {
>    char *exec_file, *full_exec_path = NULL;
>    struct cleanup *old_chain;
> +  struct gdb_exception prev_err;
>  
>    /* Do nothing if we already have an executable filename.  */
>    exec_file = (char *) get_exec_file (0);
> @@ -182,9 +202,46 @@ exec_file_locate_attach (int pid, int from_tty)
>  
>    old_chain = make_cleanup (xfree, full_exec_path);
>  
> -  exec_file_attach (full_exec_path, from_tty);
> -  symbol_file_add_main (full_exec_path, from_tty);
> +  /* exec_file_attach and symbol_file_add_main may throw an error if the file
> +     cannot be opened either locally or remotely.
> +
> +     This happens for example, when the file is first found in the local
> +     sysroot (above), and then disappears (a TOCTOU race), or when it doesn't
> +     exist in the target filesystem, or when the file does exist, but
> +     is not readable.
> +
> +     Even without a symbol file, the remote-based debugging session should
> +     continue normally instead of ending abruptly.  Hence we catch thrown
> +     errors/exceptions in the following code.  */
> +  TRY
> +    {
> +      exec_file_attach (full_exec_path, from_tty);
> +    }
> +  CATCH (err, RETURN_MASK_ERROR)
> +    {
> +      if (err.message != NULL)
> +	warning ("%s", err.message);
> +
> +      prev_err = err;
> +
> +      /* Save message so it doesn't get trashed by the catch problem

"the catch problem below" should be either the "the caught
problem below", or "the catch below", I think.

> +	 below.  */
> +      prev_err.message = xstrdup (err.message);
> +    }
> +  END_CATCH
> +
> +  TRY
> +    {
> +      symbol_file_add_main (full_exec_path, from_tty);
> +    }
> +  CATCH (err, RETURN_MASK_ERROR)
> +    {
> +      if (!exception_print_same (prev_err, err))
> +	warning ("%s", err.message);
> +    }
> +  END_CATCH
>  
> +  xfree ((void *) prev_err.message);

This will reference an uninitialized prev_err.message if the
first TRY doesn't throw.  Initialize it with:

 struct gdb_exception prev_err = exception_none;


Also, it will leak prev_err.message on Ctrl-C/QUIT, which is
not caught by RETURN_MASK_ERROR.  prev_err.message needs to be
freed with a cleanup.  Say:

  struct gdb_exception prev_err = exception_none;

  old_chain = make_cleanup (xfree, full_exec_path);

  make_cleanup (free_current_contents, &prev_err.message);

  TRY
    {
       ...
    }
  CATCH (err, RETURN_MASK_ERROR)
    {
      if (err.message != NULL)
	warning ("%s", err.message);

      prev_err = err;
      prev_err.message = xstrdup (err.message);
    }
   ...

  do_cleanups (old_chain);

Thanks,
Pedro Alves