[PATCH v2] gdb: Catch exceptions if the source file is not found
Andrew Burgess
andrew.burgess@embecosm.com
Thu Feb 6 12:12:00 GMT 2020
* Shahab Vahedi <shahab.vahedi@gmail.com> [2020-01-24 17:45:36 +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.
>
> [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:
>
> * 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".
> * tui/tui-source.c (tui_source_window::set_contents):
> Simplify "nlines" calculation.
This can be merged with a couple of very minor nits highlighted below
addressed.
Thanks,
Andrew
>
> gdb/testsuite/ChangeLog:
>
> * gdb.tui/tui-missing-src.exp: Add the "missing source
> file" test for the TUI.
> ---
> gdb/source-cache.c | 39 +++++----
> gdb/testsuite/gdb.tui/tui-missing-src.exp | 100 ++++++++++++++++++++++
> gdb/tui/tui-source.c | 2 +-
> 3 files changed, 122 insertions(+), 19 deletions(-)
> create mode 100644 gdb/testsuite/gdb.tui/tui-missing-src.exp
>
> 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,
> diff --git a/gdb/testsuite/gdb.tui/tui-missing-src.exp b/gdb/testsuite/gdb.tui/tui-missing-src.exp
> new file mode 100644
> index 00000000000..27c4952bf85
> --- /dev/null
> +++ b/gdb/testsuite/gdb.tui/tui-missing-src.exp
> @@ -0,0 +1,100 @@
> +# Copyright 2019-2020 Free Software Foundation, Inc.
I think the test was first posted in 2020, in which case this you
should only say 2020 here.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# This test checks if gdb can handle missing source files gracefully.
> +# Testing steps are:
> +# 1. Have a main() in main.c that calls an external function f2().
> +# 2. Have f2() implemented in f2.c.
> +# 3. Build the two files into one executable.
> +# 4. Remove main.c.
> +# 5. Open the executable inside gdb while having gdb in source layout.
> +# No source is found for the moment.
> +# 6. After a little bit of playing, we enter f2() and now the source
> +# layout must show the contents of f2.c.
> +# 7. Going back to main() shall result in no contents again.
> +
> +load_lib "tuiterm.exp"
> +
> +standard_testfile
> +
> +set mainfile [standard_output_file main.c]
> +set f2file [standard_output_file f2.c]
> +set srcfiles [list $mainfile $f2file]
> +
> +# Step 1: Write the main.c file into the output directory.
> +# This file will be removed after compilation.
> +set fd [open "$mainfile" w]
> +puts $fd {
> +extern int f2(int);
> +int
> +main ()
> +{
> + int a = 4;
> + a = f2(a);
> + return a - a;
> +}
> +}
> +close $fd
> +
> +# Step 2: Write the f2.c file into the output directory.
> +set fd [open "$f2file" w]
> +puts $fd {
> +int
> +f2 (int x)
> +{
> + x <<= 1;
> + return x+5;
> +}
> +}
> +close $fd
> +
> +# Step 3: Compile the source files.
> +if { [gdb_compile "${srcfiles}" "${binfile}" \
> + executable {debug additional_flags=-O0}] != "" } {
> + untested "failed to compile"
> + return -1
> +}
> +
> +# Step 4: Remove the main.c file.
> +file delete $mainfile
> +
> +# Step 5: Load the executable into GDB.
> +# There shall be no source content.
> +Term::clean_restart 24 80 $testfile
> +if {![Term::enter_tui]} {
> + unsupported "TUI not supported"
> +}
> +# There must exist a source layout with the size 80x15 and
There's a trailing whitespace on this line. I only notice because
'git apply' complains.
> +# there should be nothing in it.
> +Term::check_box_contents "check source box is empty" \
> + 0 0 80 15 "No Source Available"
> +
> +# Step 6: Go to main and after one next, enter f2().
> +Term::command "set pagination off"
> +Term::command "start"
> +Term::command "next"
> +Term::command "step"
> +Term::check_contents "checking if inside f2 ()" "f2 \\(x=4\\)"
> +Term::check_box_contents "f2.c must be displayed in source window" \
> + 0 0 80 15 "return x\\+5"
> +
> +# Step 7: Back in main
> +Term::command "finish"
> +Term::check_box_contents "check source box is empty after return" \
> + 0 0 80 15 "No Source Available"
> +Term::check_contents "Back in main" "Value returned is .* 13"
> +
> +# Valhalla
> +pass "TUI can handle missing source files"
> diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
> index 912eaa45440..3c7a8e10008 100644
> --- a/gdb/tui/tui-source.c
> +++ b/gdb/tui/tui-source.c
> @@ -55,7 +55,7 @@ tui_source_window::set_contents (struct gdbarch *arch,
> line_width = width - TUI_EXECINFO_SIZE - 1;
> /* Take hilite (window border) into account, when
> calculating the number of lines. */
> - nlines = (line_no + (height - 2)) - line_no;
> + nlines = height - 2;
>
> std::string srclines;
> const std::vector<off_t> *offsets;
I'm OK with this being included in this patch, but generally something
unrelated like this should go into its own patch. But I don't suggest
splitting this out.
I would love to know what the original thinking behind this line was.
I'm sure there's a story there, but I suspect it's lost to time :)
Thanks,
Andrew
> --
> 2.25.0
>
More information about the Gdb-patches
mailing list