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

Andrew Burgess
2020-01-22T15:49:07Z

* 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
>