* [PATCH] gdb: Catch exceptions if the source file is not found
@ 2020-01-22 14:26 Shahab Vahedi
2020-01-22 16:24 ` Andrew Burgess
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Shahab Vahedi @ 2020-01-22 14:26 UTC (permalink / raw)
To: gdb-patches
Cc: Shahab Vahedi, Shahab Vahedi, Andrew Burgess, Tom Tromey,
Claudiu Zissulescu, Francois Bedard
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:
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] gdb: Catch exceptions if the source file is not found
2020-01-22 14:26 [PATCH] gdb: Catch exceptions if the source file is not found Shahab Vahedi
@ 2020-01-22 16:24 ` Andrew Burgess
2020-01-22 16:42 ` Shahab Vahedi
2020-01-24 17:05 ` [PATCH v2] " Shahab Vahedi
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Andrew Burgess @ 2020-01-22 16:24 UTC (permalink / raw)
To: Shahab Vahedi
Cc: gdb-patches, Shahab Vahedi, Tom Tromey, Claudiu Zissulescu,
Francois Bedard
* 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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] gdb: Catch exceptions if the source file is not found
2020-01-22 16:24 ` Andrew Burgess
@ 2020-01-22 16:42 ` Shahab Vahedi
2020-01-23 18:18 ` Tom Tromey
0 siblings, 1 reply; 13+ messages in thread
From: Shahab Vahedi @ 2020-01-22 16:42 UTC (permalink / raw)
To: Andrew Burgess
Cc: gdb-patches, Shahab Vahedi, Tom Tromey, Claudiu Zissulescu,
Francois Bedard
On Wed, Jan 22, 2020 at 03:49:01PM +0000, Andrew Burgess wrote:
>
> 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?
>
This is reproducible on an x86_64 host/target as well. These are
the necessary step that I have to automate by a test:
$ cat > file1.c <<EOF
extern int func2(int);
int main()
{
int a = 4;
return func2(a);
}
EOF
$ cat > file2.c <<EOF
int func2(int x)
{
x <<= 1;
return x+5;
}
EOF
$ gcc -g file1.c file2.c -o test
$ mv file1.c file1.c.moved
$ gdb -tui test.c
(gdb) start
(gdb) next
(gdb) step
at this point we should see func2 in source window.
It is just a matter of time for me to write my _first_ test.
If the changes are OK, I will submit the test in a separate
patch if you don't mind. I just don't want this fix fall
through the cracks as apparently it did once according to
Tom.
Cheers,
Shahab
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] gdb: Catch exceptions if the source file is not found
2020-01-22 16:42 ` Shahab Vahedi
@ 2020-01-23 18:18 ` Tom Tromey
2020-02-06 13:19 ` Shahab Vahedi
2020-02-06 15:26 ` Shahab Vahedi
0 siblings, 2 replies; 13+ messages in thread
From: Tom Tromey @ 2020-01-23 18:18 UTC (permalink / raw)
To: Shahab Vahedi
Cc: Andrew Burgess, gdb-patches, Shahab Vahedi, Tom Tromey,
Claudiu Zissulescu, Francois Bedard
>>>>> "Shahab" == Shahab Vahedi <shahab.vahedi@gmail.com> writes:
Shahab> It is just a matter of time for me to write my _first_ test.
Shahab> If the changes are OK, I will submit the test in a separate
Shahab> patch if you don't mind. I just don't want this fix fall
Shahab> through the cracks as apparently it did once according to
Shahab> Tom.
Is this needed for gdb 9? If so, then perhaps it can land on the branch
without a test. For trunk there's normally no particular rush to get
things in, and I think it would be good to have the test at the same
time.
Tom
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] gdb: Catch exceptions if the source file is not found
2020-01-22 14:26 [PATCH] gdb: Catch exceptions if the source file is not found Shahab Vahedi
2020-01-22 16:24 ` Andrew Burgess
@ 2020-01-24 17:05 ` Shahab Vahedi
2020-01-31 10:34 ` [PING] " Shahab Vahedi
` (2 more replies)
2020-02-06 13:07 ` [PATCH v3] " Shahab Vahedi
2020-02-06 15:25 ` [PATCH v4] " Shahab Vahedi
3 siblings, 3 replies; 13+ messages in thread
From: Shahab Vahedi @ 2020-01-24 17:05 UTC (permalink / raw)
To: gdb-patches
Cc: Shahab Vahedi, Shahab Vahedi, Andrew Burgess, Tom Tromey,
Claudiu Zissulescu, Francois Bedard
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.
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.
+
+# 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 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;
--
2.25.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PING] [PATCH v2] gdb: Catch exceptions if the source file is not found
2020-01-24 17:05 ` [PATCH v2] " Shahab Vahedi
@ 2020-01-31 10:34 ` Shahab Vahedi
2020-02-06 12:12 ` Andrew Burgess
2020-02-06 14:25 ` Andrew Burgess
2 siblings, 0 replies; 13+ messages in thread
From: Shahab Vahedi @ 2020-01-31 10:34 UTC (permalink / raw)
To: Shahab Vahedi, gdb-patches
Cc: Andrew Burgess, Tom Tromey, Claudiu Zissulescu, Francois Bedard
I have added the test in v2:
https://sourceware.org/ml/gdb-patches/2020-01/msg00815.html
as suggested here:
https://sourceware.org/ml/gdb-patches/2020-01/msg00678.html
Cheers,
Shahab
From gdb-patches-return-163676-listarch-gdb-patches=sources.redhat.com@sourceware.org Fri Jan 31 10:34:56 2020
Return-Path: <gdb-patches-return-163676-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Delivered-To: listarch-gdb-patches@sources.redhat.com
Received: (qmail 99826 invoked by alias); 31 Jan 2020 10:34:55 -0000
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
Delivered-To: mailing list gdb-patches@sourceware.org
Received: (qmail 99817 invoked by uid 89); 31 Jan 2020 10:34:55 -0000
Authentication-Results: sourceware.org; auth=none
X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.1 spammy=HX-Spam-Relays-External:15.20.2665.23, H*RU:15.20.2665.23
X-HELO: smtprelay-out1.synopsys.com
Received: from smtprelay-out1.synopsys.com (HELO smtprelay-out1.synopsys.com) (149.117.87.133) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 31 Jan 2020 10:34:54 +0000
Received: from mailhost.synopsys.com (badc-mailhost2.synopsys.com [10.192.0.18]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by smtprelay-out1.synopsys.com (Postfix) with ESMTPS id 8C432C056D; Fri, 31 Jan 2020 10:34:52 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=synopsys.com; s=mail; t\x1580466892; bh=JlOp23TCH46DPTY7FXumF25AEtO80p+9ejoDyPN4nn4=; h=From:To:CC:Subject:Date:References:In-Reply-To:From; b=SFG5xP6TRsOG8gZJKTWZ+VS/btimRLAPwYPa/dxP4u2YTm0AgtTGSSFwbrXIdEmgq +qzAx7F76LeIpmv6nkaCqfjyIMovTji0Z1su1M66StU6gX7Q8Scvf3Sj6ogMjrySKa VfCFydst7IpT7zRdHuUZobKeXDNlybbTsprhGGmnaEbzVNSJLUatpk2YaAU1EpYDiJ eaSHul+CNFEfpQV/p9SpovS/++xUrXyN8NglhDwa3xzrFnAyismTbo6NVNbJWr7glM VJKqhaMF1MKcA9rD60djwuO1Ne0I3GRBXY2C7uoEqVQhDqQns3FiVpwdFAH9oG8WDx PxrijPUg5A3Ag=Received: from US01WEHTC3.internal.synopsys.com (us01wehtc3.internal.synopsys.com [10.15.84.232]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mailhost.synopsys.com (Postfix) with ESMTPS id 45281A006A; Fri, 31 Jan 2020 10:34:47 +0000 (UTC)
Received: from US01HYBRID2.internal.synopsys.com (10.15.246.24) by US01WEHTC3.internal.synopsys.com (10.15.84.232) with Microsoft SMTP Server (TLS) id 14.3.408.0; Fri, 31 Jan 2020 02:34:36 -0800
Received: from NAM02-SN1-obe.outbound.protection.outlook.com (10.202.3.67) by mrs.synopsys.com (10.15.246.24) with Microsoft SMTP Server (TLS) id 14.3.408.0; Fri, 31 Jan 2020 02:34:36 -0800
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NHmYzCkUaiuI4//CXvCQnWkr+lFfhoN9gqa4GtVzEW1XOEO8WUaE2jzJZr3ZwPtc8uBlcOfR0GqgierX8PNa7DBUj2pW1VLoIi6UsNlTqOjEt78eJ0fYyOI7ByepI9ZYUn5VvdlPUakWfeRxg2PMbMesA25dM16PMiKCiYsTaYzxPHyo1gT1UEC8v4Dt5Vl9avxopaQxRdMA3Id+irKim6Y79jwpRQNNf5CCYSOI3DcLpvcapFwXEbbMjZKvYquCd2p/mg88ZLNFyxWRHKo5vKlHijooNeUVPziawhds7z+eP2ytWcFLLHYizlgPzzQEIbWkcX65rL1EzCpi4bwwSQ=ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=JlOp23TCH46DPTY7FXumF25AEtO80p+9ejoDyPN4nn4=; büaa72l4TnnTDNxD6GGe+4ftmo3yHmxPeXyXSQJKtAVB/CrSWxTWNdUgsb2XmbnDLyov7QIRCuoLSV8EBNwCgeUPrsH+NRcIluAtesVBb015HEVmI6NvfDuT2WtGeogtSw/MHLJ9Xeu0qTiLDYnprVGHxgDEvW5FJ4bqpSUdxG6/5xWKm/qugN8loPgFdBs0087SmX0gqaWD2D689ZbgsVVaVEXMWjtb4W8pXbIws0jlWaRicUQRp6j8dimdNhNKiYLrAo0uOVORJkbStF1Cj80GHWxXUKSIMpNp0zi51/OLaIfHEm7NqE9AVOpzdxdOPlYbBA1vPiG/L6ODuMCQiA=ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=synopsys.com; dmarc=pass action=none header.from=synopsys.com; dkim=pass header.d=synopsys.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=synopsys.onmicrosoft.com; s=selector2-synopsys-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=JlOp23TCH46DPTY7FXumF25AEtO80p+9ejoDyPN4nn4=; b=Tsnhy7Mlk+lcd6erlUknt7olXOSSfEhUy/02DIK87aE+uoBDC7kKuQqFqo44KlW9Q6vfVHewHrddQEAnuguSG4BkVx39Fh0+0LbJ+m1D/GP2nYs/3ue45vVLMRMZjt24sB1WdrnaFkdPHu2blhwgNjWN0Ytrp/ODUnPKsQe8IgwReceived: from CH2PR12MB3847.namprd12.prod.outlook.com (52.132.247.27) by CH2PR12MB4101.namprd12.prod.outlook.com (20.180.6.150) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2665.23; Fri, 31 Jan 2020 10:34:35 +0000
Received: from CH2PR12MB3847.namprd12.prod.outlook.com ([fe80::c820:fae4:7b47:1ecb]) by CH2PR12MB3847.namprd12.prod.outlook.com ([fe80::c820:fae4:7b47:1ecb%5]) with mapi id 15.20.2665.027; Fri, 31 Jan 2020 10:34:35 +0000
From: Shahab Vahedi <Shahab.Vahedi@synopsys.com>
To: Shahab Vahedi <shahab.vahedi@gmail.com>, "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
CC: Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com>, Francois Bedard <Francois.Bedard@synopsys.com>, Andrew Burgess <andrew.burgess@embecosm.com>
Subject: [PING] [PATCH] Do not print empty-group regs when printing general ones
Date: Fri, 31 Jan 2020 10:38:00 -0000
Message-ID: <CH2PR12MB3847E492663E7997CFAB1741A6070@CH2PR12MB3847.namprd12.prod.outlook.com>
References: <20200120155315.30333-1-shahab.vahedi@gmail.com>
In-Reply-To: <20200120155315.30333-1-shahab.vahedi@gmail.com>
authentication-results: spf=none (sender IP is ) smtp.mailfrom=shahab@synopsys.com;
x-ms-exchange-transport-forked: True
x-ms-oob-tlc-oobclassifiers: OLM:3044;
received-spf: None (protection.outlook.com: synopsys.com does not designate permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-ms-exchange-antispam-messagedata: XXXN2+2xMjQGhk54INErcX4EeRCvNn2H8mAKOAB/XFaKGfHK6u6EzgZtlKWN74o9ktGQppNnHuEHdPeBPnBO2w+TL/VULgHcG8aSx233wdB/DSdEfSo/+QfAjECXIcOAw6+T16dULWQzD+xq10lxaQ=Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: MTnkKmZJLwHnwDLfya+EUtFyyWlq6Sm9J4XvCYp7U/gBK4RPQ8ew4d8UlZE4NVC7UXRe3DiVIQ/ja2U70pYUow=X-SW-Source: 2020-01/txt/msg00990.txt.bz2
Content-length: 147
This patch was reviewed once (as OK):
https://sourceware.org/ml/gdb-patches/2020-01/msg00613.html
Could someone review/merge it?
Cheers,
Shahab
From gdb-patches-return-163677-listarch-gdb-patches=sources.redhat.com@sourceware.org Fri Jan 31 10:38:28 2020
Return-Path: <gdb-patches-return-163677-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Delivered-To: listarch-gdb-patches@sources.redhat.com
Received: (qmail 102224 invoked by alias); 31 Jan 2020 10:38:27 -0000
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
Delivered-To: mailing list gdb-patches@sourceware.org
Received: (qmail 102216 invoked by uid 89); 31 Jan 2020 10:38:27 -0000
Authentication-Results: sourceware.org; auth=none
X-Spam-SWARE-Status: No, score=-1.9 required=5.0 testsºYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=HX-Languages-Length:734, resulted, sk:tromey, sk:tromey@
X-HELO: rock.gnat.com
Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 31 Jan 2020 10:38:26 +0000
Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id A5D4E117919; Fri, 31 Jan 2020 05:38:24 -0500 (EST)
Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id frMVBUmZ5Egp; Fri, 31 Jan 2020 05:38:24 -0500 (EST)
Received: from murgatroyd (unknown [109.236.135.164]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPSA id 2EE50117918; Fri, 31 Jan 2020 05:38:24 -0500 (EST)
From: Tom Tromey <tromey@adacore.com>
To: Tom Tromey <tromey@adacore.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] Fix ravenscar-thread.c for multi-target
References: <20200123205909.30663-1-tromey@adacore.com>
Date: Fri, 31 Jan 2020 10:43:00 -0000
In-Reply-To: <20200123205909.30663-1-tromey@adacore.com> (Tom Tromey's message of "Thu, 23 Jan 2020 13:59:09 -0700")
Message-ID: <87pnezykpt.fsf@tromey.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-SW-Source: 2020-01/txt/msg00991.txt.bz2
Content-length: 686
>>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes:
Tom> ravenscar-thread.c needed a change to adapt to multi-target:
Tom> ravenscar_thread_target::mourn_inferior called the mourn_inferior
Tom> method on the target beneat -- but when the target beneath was the
Tom> remote target, this resulted in the ravenscar target being deleted.
Tom> Switching the order of the calls to unpush_target and the beneath's
Tom> mourn_inferior fixes this problem.
Tom> gdb/ChangeLog
Tom> 2020-01-23 Tom Tromey <tromey@adacore.com>
Tom> * ravenscar-thread.c (ravenscar_thread_target::mourn_inferior):
Tom> Call beneath target's mourn_inferior after unpushing.
I'm checking this in now.
Tom
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] gdb: Catch exceptions if the source file is not found
2020-01-24 17:05 ` [PATCH v2] " Shahab Vahedi
2020-01-31 10:34 ` [PING] " Shahab Vahedi
@ 2020-02-06 12:12 ` Andrew Burgess
2020-02-06 14:25 ` Andrew Burgess
2 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-02-06 12:12 UTC (permalink / raw)
To: Shahab Vahedi
Cc: gdb-patches, Shahab Vahedi, Tom Tromey, Claudiu Zissulescu,
Francois Bedard
* 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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3] gdb: Catch exceptions if the source file is not found
2020-01-22 14:26 [PATCH] gdb: Catch exceptions if the source file is not found Shahab Vahedi
2020-01-22 16:24 ` Andrew Burgess
2020-01-24 17:05 ` [PATCH v2] " Shahab Vahedi
@ 2020-02-06 13:07 ` Shahab Vahedi
2020-02-06 15:25 ` [PATCH v4] " Shahab Vahedi
3 siblings, 0 replies; 13+ messages in thread
From: Shahab Vahedi @ 2020-02-06 13:07 UTC (permalink / raw)
To: gdb-patches
Cc: Shahab Vahedi, Shahab Vahedi, Andrew Burgess, Tom Tromey,
Claudiu Zissulescu, Francois Bedard
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 [1].
The patch from Pedro [2] 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]
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
[2]
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=2f267673f0fdee9287e6d404ecd4f2d29da0d2f2
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.
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 71277ecc9b..9196e3a19e 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 0000000000..3027f3704b
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/tui-missing-src.exp
@@ -0,0 +1,100 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# 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 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 912eaa4544..3c7a8e1000 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;
--
2.25.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] gdb: Catch exceptions if the source file is not found
2020-01-23 18:18 ` Tom Tromey
@ 2020-02-06 13:19 ` Shahab Vahedi
2020-02-06 15:26 ` Shahab Vahedi
1 sibling, 0 replies; 13+ messages in thread
From: Shahab Vahedi @ 2020-02-06 13:19 UTC (permalink / raw)
To: Tom Tromey
Cc: Andrew Burgess, gdb-patches, Shahab Vahedi, Claudiu Zissulescu,
Francois Bedard
Hi Tom,
On Thu, Jan 23, 2020 at 11:14:16AM -0700, Tom Tromey wrote:
> >>>>> "Shahab" == Shahab Vahedi <shahab.vahedi@gmail.com> writes:
>
> Shahab> It is just a matter of time for me to write my _first_ test.
> Shahab> If the changes are OK, I will submit the test in a separate
> Shahab> patch if you don't mind. I just don't want this fix fall
> Shahab> through the cracks as apparently it did once according to
> Shahab> Tom.
>
> Is this needed for gdb 9? If so, then perhaps it can land on the branch
This issue does exist even in GDB 8.3. Nevertheless, I have included
the test into the latest patch [1]. That patch [1] has addressed the
latest remarks [2] from Andrew and should be fine.
A good way to check if the issue also happens for GDB 9 is by running
the test in a GDB 9 branch. I don't have one ready now, but if you
want I can give it a try.
Cheers,
Shahab
[1]
https://sourceware.org/ml/gdb-patches/2020-02/msg00127.html
[2]
https://sourceware.org/ml/gdb-patches/2020-02/msg00126.html
> without a test. For trunk there's normally no particular rush to get
> things in, and I think it would be good to have the test at the same
> time.
>
> Tom
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] gdb: Catch exceptions if the source file is not found
2020-01-24 17:05 ` [PATCH v2] " Shahab Vahedi
2020-01-31 10:34 ` [PING] " Shahab Vahedi
2020-02-06 12:12 ` Andrew Burgess
@ 2020-02-06 14:25 ` Andrew Burgess
2 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-02-06 14:25 UTC (permalink / raw)
To: Shahab Vahedi
Cc: gdb-patches, Shahab Vahedi, Tom Tromey, Claudiu Zissulescu,
Francois Bedard
* 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.
>
> 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.
> +
> +# 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 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"
I forgot to mention in my original review, we don't generally add
passes like this to GDB tests - all of the previous tests are enough.
If you've not pushed yet could you remove this please. If you have
already pushed then don't worry about it.
Thanks,
Andrew
> 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;
> --
> 2.25.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4] gdb: Catch exceptions if the source file is not found
2020-01-22 14:26 [PATCH] gdb: Catch exceptions if the source file is not found Shahab Vahedi
` (2 preceding siblings ...)
2020-02-06 13:07 ` [PATCH v3] " Shahab Vahedi
@ 2020-02-06 15:25 ` Shahab Vahedi
2020-02-06 16:42 ` Andrew Burgess
3 siblings, 1 reply; 13+ messages in thread
From: Shahab Vahedi @ 2020-02-06 15:25 UTC (permalink / raw)
To: gdb-patches
Cc: Shahab Vahedi, Shahab Vahedi, Andrew Burgess, Tom Tromey,
Claudiu Zissulescu, Francois Bedard
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 [1].
The patch from Pedro [2] 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]
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
[2]
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=2f267673f0fdee9287e6d404ecd4f2d29da0d2f2
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.
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 | 97 +++++++++++++++++++++++
gdb/tui/tui-source.c | 2 +-
3 files changed, 119 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 71277ecc9b..9196e3a19e 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 0000000000..2d9a851bec
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/tui-missing-src.exp
@@ -0,0 +1,97 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# 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 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"
diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
index 912eaa4544..3c7a8e1000 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;
--
2.25.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] gdb: Catch exceptions if the source file is not found
2020-01-23 18:18 ` Tom Tromey
2020-02-06 13:19 ` Shahab Vahedi
@ 2020-02-06 15:26 ` Shahab Vahedi
1 sibling, 0 replies; 13+ messages in thread
From: Shahab Vahedi @ 2020-02-06 15:26 UTC (permalink / raw)
To: Tom Tromey
Cc: Andrew Burgess, gdb-patches, Shahab Vahedi, Claudiu Zissulescu,
Francois Bedard
There is a newer patch (v4):
https://sourceware.org/ml/gdb-patches/2020-02/msg00134.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] gdb: Catch exceptions if the source file is not found
2020-02-06 15:25 ` [PATCH v4] " Shahab Vahedi
@ 2020-02-06 16:42 ` Andrew Burgess
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Burgess @ 2020-02-06 16:42 UTC (permalink / raw)
To: Shahab Vahedi
Cc: gdb-patches, Shahab Vahedi, Tom Tromey, Claudiu Zissulescu,
Francois Bedard
* Shahab Vahedi <shahab.vahedi@gmail.com> [2020-02-06 16:24:27 +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 [1].
>
> The patch from Pedro [2] 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]
> 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
>
> [2]
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=2f267673f0fdee9287e6d404ecd4f2d29da0d2f2
>
> 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.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.tui/tui-missing-src.exp: Add the "missing source
> file" test for the TUI.
LGTM. I see you have write access now, so feel free to go ahead and
push this.
Thanks,
Andrew
> ---
> gdb/source-cache.c | 39 ++++-----
> gdb/testsuite/gdb.tui/tui-missing-src.exp | 97 +++++++++++++++++++++++
> gdb/tui/tui-source.c | 2 +-
> 3 files changed, 119 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 71277ecc9b..9196e3a19e 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 0000000000..2d9a851bec
> --- /dev/null
> +++ b/gdb/testsuite/gdb.tui/tui-missing-src.exp
> @@ -0,0 +1,97 @@
> +# Copyright 2020 Free Software Foundation, Inc.
> +
> +# 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 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"
> diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
> index 912eaa4544..3c7a8e1000 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;
> --
> 2.25.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-02-06 16:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-22 14:26 [PATCH] gdb: Catch exceptions if the source file is not found Shahab Vahedi
2020-01-22 16:24 ` Andrew Burgess
2020-01-22 16:42 ` Shahab Vahedi
2020-01-23 18:18 ` Tom Tromey
2020-02-06 13:19 ` Shahab Vahedi
2020-02-06 15:26 ` Shahab Vahedi
2020-01-24 17:05 ` [PATCH v2] " Shahab Vahedi
2020-01-31 10:34 ` [PING] " Shahab Vahedi
2020-02-06 12:12 ` Andrew Burgess
2020-02-06 14:25 ` Andrew Burgess
2020-02-06 13:07 ` [PATCH v3] " Shahab Vahedi
2020-02-06 15:25 ` [PATCH v4] " Shahab Vahedi
2020-02-06 16:42 ` Andrew Burgess
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox