Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [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
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