Re: [PATCH] gdb: Catch exceptions if the source file is not found

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Shahab Vahedi <shahab.vahedi@gmail.com> [2020-01-22 15:08:18 +0100]:

> From: Shahab Vahedi <shahab@synopsys.com>
> 
> The source_cache::ensure method may throw an exception through
> the invocation of source_cache::get_plain_source_lines. This
> happens when the source file is not found. The expected behaviour
> of "ensure" is only returning "true" or "false" according to the
> documentation in the header file.
> 
> So far, if gdb is in source layout and a file is missing, you see
> some outputs like below:
> 
>  ,---------------------------------------------.
>  | test.c file is loaded in the source window. |
>  |                                             |
>  | int main()                                  |
>  | ...                                         |
>  |---------------------------------------------|
>  | Remote debugging using :1234                |
>  | __start () at /path/to/crt0.S:141           |
>  | /path/to/crt0.S: No such file or directory. |
>  | (gdb) p/x $pc                               |
>  | $1 = 0x124                                  |
>  | (gdb) n                                     |
>  | /path/to/crt0.S: No such file or directory. |
>  | (gdb) p/x $pc                               |
>  | $2 = 0x128                                  |
>  | (gdb) [pressing arrow-down key]             |
>  | (gdb) terminate called after throwing an    |
>  |       instance of 'gdb_exception_error'     |
>  `---------------------------------------------'
> Other issues have been encountered as well [2].
> 
> The patch from Pedro [1] which is about preventing exceptions
> from crossing the "readline" mitigates the situation by not
> causing gdb crash, but still there are lots of errors printed:
> 
>  ,---------------------------------------------.
>  | test.c file is loaded in the source window. |
>  |                                             |
>  | int main()                                  |
>  | ...                                         |
>  |---------------------------------------------|
>  | Remote debugging using :1234                |
>  | __start () at /path/to/crt0.S:141           |
>  | /path/to/crt0.S: No such file or directory. |
>  | (gdb) [pressing arrow-down key]             |
>  | /path/to/crt0.S: No such file or directory. |
>  | (gdb) [pressing arrow-down key]             |
>  | /path/to/crt0.S: No such file or directory. |
>  | (gdb) [pressing arrow-up key]               |
>  | /path/to/crt0.S: No such file or directory. |
>  `---------------------------------------------'
> 
> With the changes of this patch, the behavior is like:
>  ,---------------------------------------------.
>  | initially, source window is empty because   |
>  | crt0.S is not found and according to the    |
>  | program counter that is the piece of code   |
>  | being executed.                             |
>  |                                             |
>  | later, when we break at main (see commands  |
>  | below), this window will be filled with the |
>  | the contents of test.c file.                |
>  |---------------------------------------------|
>  | Remote debugging using :1234                |
>  | __start () at /path/to/crt0.S:141           |
>  | (gdb) p/x $pc                               |
>  | $1 = 0x124                                  |
>  | (gdb) n                                     |
>  | (gdb) p/x $pc                               |
>  | $2 = 0x128                                  |
>  | (gdb) b main                                |
>  | Breakpoint 1 at 0x334: file test.c, line 8. |
>  | (gdb) cont                                  |
>  | Continuing.                                 |
>  | Breakpoint 1, main () at hello.c:8          |
>  | (gdb) n                                     |
>  | (gdb)                                       |
>  `---------------------------------------------'
> 
> There is no crash and the error message is completely
> gone. Maybe it is good practice that the error is
> shown inside the source window.
> 
> I tested this change against gdb.base/list-missing-source.exp
> and there was no regression.

Would it be possible to create a version of this test (or similar)
within gdb.tui/ so this fix would be protected in the future?

Thanks,
Andrew



> 
> [1]
> https://sourceware.org/ml/gdb-patches/2020-01/msg00440.html
> 
> [2]
> It has also been observed in the past that the register
> values are not transferred from qemu's gdb stub, see:
> https://github.com/foss-for-synopsys-dwc-arc-processors/toolchain/issues/226
> 
> gdb/ChangeLog:
> 2020-01-22  Shahab Vahedi  <shahab@synopsys.com>
> 
> 	* source-cache.c (source_cache::ensure): Surround
> 	get_plain_source_lines with a try/catch.
> 	(source_cache::get_line_charpos): Get rid of try/catch
> 	and only check for the return value of "ensure".
> ---
>  gdb/source-cache.c | 39 +++++++++++++++++++++------------------
>  1 file changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/gdb/source-cache.c b/gdb/source-cache.c
> index 71277ecc9b3..9196e3a19e3 100644
> --- a/gdb/source-cache.c
> +++ b/gdb/source-cache.c
> @@ -176,7 +176,16 @@ source_cache::ensure (struct symtab *s)
>  	}
>      }
>  
> -  std::string contents = get_plain_source_lines (s, fullname);
> +  std::string contents;
> +  try
> +    {
> +      contents = get_plain_source_lines (s, fullname);
> +    }
> +  catch (const gdb_exception_error &e)
> +    {
> +      /* If 's' is not found, an exception is thrown.  */
> +      return false;
> +    }
>  
>    if (source_styling && gdb_stdout->can_emit_style_escape ())
>      {
> @@ -241,26 +250,20 @@ bool
>  source_cache::get_line_charpos (struct symtab *s,
>  				const std::vector<off_t> **offsets)
>  {
> -  try
> -    {
> -      std::string fullname = symtab_to_fullname (s);
> -
> -      auto iter = m_offset_cache.find (fullname);
> -      if (iter == m_offset_cache.end ())
> -	{
> -	  ensure (s);
> -	  iter = m_offset_cache.find (fullname);
> -	  /* cache_source_text ensured this was entered.  */
> -	  gdb_assert (iter != m_offset_cache.end ());
> -	}
> +  std::string fullname = symtab_to_fullname (s);
>  
> -      *offsets = &iter->second;
> -      return true;
> -    }
> -  catch (const gdb_exception_error &e)
> +  auto iter = m_offset_cache.find (fullname);
> +  if (iter == m_offset_cache.end ())
>      {
> -      return false;
> +      if (!ensure (s))
> +	return false;
> +      iter = m_offset_cache.find (fullname);
> +      /* cache_source_text ensured this was entered.  */
> +      gdb_assert (iter != m_offset_cache.end ());
>      }
> +
> +  *offsets = &iter->second;
> +  return true;
>  }
>  
>  /* A helper function that extracts the desired source lines from TEXT,
> -- 
> 2.25.0
>