public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb/source: Fix open_source_file error handling
@ 2023-01-28  3:37 Aaron Merey
  2023-02-01 19:45 ` Keith Seitz
  2023-02-08 17:44 ` Tom Tromey
  0 siblings, 2 replies; 6+ messages in thread
From: Aaron Merey @ 2023-01-28  3:37 UTC (permalink / raw)
  To: gdb-patches; +Cc: simark, Aaron Merey

open_source_file relies on errno to communicate the reason for a missing
source file.

open_source_file also may call debuginfod_find_source.  It is possible
for debuginfod_find_source to set errno to a value unrelated to the
reason for a failed download.

This can result in bogus error messages being reported as the reason for
a missing source file.  The following error message should instead include 
"No such file or directory":

  Temporary breakpoint 1, 0x00005555556f4de0 in main ()
  (gdb) list
  Downloading source file /usr/src/debug/glibc-2.36-8.fc37.x86_64/elf/<built-in>
  1       /usr/src/debug/glibc-2.36-8.fc37.x86_64/elf/<built-in>: Directory not empty.

Fix this by having open_source_file return a negative errno if it fails
to open a source file.  Use this value to generate the error message
instead of errno. Also add new function throw_sys_errmsg for reporting
this value.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29999
---
 gdb/source-cache.c |  2 +-
 gdb/source.c       | 40 ++++++++++++++++++++++++++++++----------
 gdb/source.h       |  5 +++--
 gdb/utils.c        | 15 +++++++++++++++
 gdb/utils.h        |  2 ++
 5 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index b7410d46293..9759fabd45e 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -95,7 +95,7 @@ source_cache::get_plain_source_lines (struct symtab *s,
 {
   scoped_fd desc (open_source_file (s));
   if (desc.get () < 0)
-    perror_with_name (symtab_to_filename_for_display (s));
+    throw_sys_errmsg (symtab_to_filename_for_display (s), -desc.get ());
 
   struct stat st;
   if (fstat (desc.get (), &st) < 0)
diff --git a/gdb/source.c b/gdb/source.c
index be5af180c49..158b59fd613 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1069,7 +1069,7 @@ find_and_open_source (const char *filename,
      the attempt to read this source file failed.  GDB will then display
      the filename and line number instead.  */
   if (!source_open)
-    return scoped_fd (-1);
+    return scoped_fd (-ECANCELED);
 
   /* Quick way out if we already know its full name.  */
   if (*fullname)
@@ -1160,11 +1160,15 @@ find_and_open_source (const char *filename,
 			OPEN_MODE, fullname);
     }
 
+  /* If the file wasn't found, then openp will have set errno accordingly.  */
+  if (result < 0)
+    result = -errno;
+
   return scoped_fd (result);
 }
 
 /* Open a source file given a symtab S.  Returns a file descriptor or
-   negative number for error.  
+   negative errno for error.
    
    This function is a convenience function to find_and_open_source.  */
 
@@ -1172,7 +1176,7 @@ scoped_fd
 open_source_file (struct symtab *s)
 {
   if (!s)
-    return scoped_fd (-1);
+    return scoped_fd (-EINVAL);
 
   gdb::unique_xmalloc_ptr<char> fullname (s->fullname);
   s->fullname = NULL;
@@ -1200,10 +1204,21 @@ open_source_file (struct symtab *s)
 
 	  /* Query debuginfod for the source file.  */
 	  if (build_id != nullptr && !srcpath.empty ())
-	    fd = debuginfod_source_query (build_id->data,
-					  build_id->size,
-					  srcpath.c_str (),
-					  &fullname);
+	    {
+	      scoped_fd query_fd
+		= debuginfod_source_query (build_id->data,
+					   build_id->size,
+					   srcpath.c_str (),
+					   &fullname);
+
+	      /* Don't return a negative errno from debuginfod_source_query.
+		 It handles the reporting of its own errors.  */
+	      if (query_fd.get () >= 0)
+		{
+		  s->fullname = fullname.release ();
+		  return query_fd;
+		}
+	    }
 	}
     }
 
@@ -1306,6 +1321,7 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
 			 print_source_lines_flags flags)
 {
   bool noprint = false;
+  int errcode = ENOENT;
   int nlines = stopline - line;
   struct ui_out *uiout = current_uiout;
 
@@ -1336,7 +1352,10 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
 	  scoped_fd desc = open_source_file (s);
 	  last_source_error = desc.get () < 0;
 	  if (last_source_error)
-	    noprint = true;
+	    {
+	      noprint = true;
+	      errcode = -desc.get ();
+	    }
 	}
     }
   else
@@ -1354,7 +1373,7 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
 	  char *name = (char *) alloca (len);
 
 	  xsnprintf (name, len, "%d\t%s", line, filename);
-	  print_sys_errmsg (name, errno);
+	  print_sys_errmsg (name, errcode);
 	}
       else
 	{
@@ -1627,7 +1646,8 @@ search_command_helper (const char *regex, int from_tty, bool forward)
 
   scoped_fd desc (open_source_file (loc->symtab ()));
   if (desc.get () < 0)
-    perror_with_name (symtab_to_filename_for_display (loc->symtab ()));
+    throw_sys_errmsg (symtab_to_filename_for_display (loc->symtab ()),
+		      -desc.get ());
 
   int line = (forward
 	      ? last_line_listed + 1
diff --git a/gdb/source.h b/gdb/source.h
index 61d1bcd883b..dd6f58c579c 100644
--- a/gdb/source.h
+++ b/gdb/source.h
@@ -67,7 +67,8 @@ extern void init_source_path (void);
      The caller is responsible for freeing FULLNAME.
 
    On Failure
-     An invalid file descriptor is returned (the return value is negative).
+     An invalid file descriptor is returned.  The value of this file
+     descriptor is a negative errno indicating the reason for the failure.
      FULLNAME is set to NULL.  */
 extern scoped_fd find_and_open_source (const char *filename,
 				       const char *dirname,
@@ -81,7 +82,7 @@ extern gdb::unique_xmalloc_ptr<char> find_source_or_rewrite
      (const char *filename, const char *dirname);
 
 /* Open a source file given a symtab S.  Returns a file descriptor or
-   negative number for error.  */
+   negative errno indicating the reason for the failure.  */
 extern scoped_fd open_source_file (struct symtab *s);
 
 extern gdb::unique_xmalloc_ptr<char> rewrite_source_path (const char *path);
diff --git a/gdb/utils.c b/gdb/utils.c
index 95adbe58e4a..0a5c6a70531 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -670,6 +670,21 @@ print_sys_errmsg (const char *string, int errcode)
   gdb_printf (gdb_stderr, "%s: %s.\n", string, err);
 }
 
+/* Same as perror_with_name except that ERRCODE specifies the error
+   instead of errno.  */
+
+void
+throw_sys_errmsg (const char *string, int errcode)
+{
+  const char *err = safe_strerror (errcode);
+  std::string combined = std::string (string) + ": " + err;
+
+  bfd_set_error (bfd_error_no_error);
+  errno = 0;
+
+  throw_error (GENERIC_ERROR, _("%s."), combined.c_str ());
+}
+
 /* Control C eventually causes this to be called, at a convenient time.  */
 
 void
diff --git a/gdb/utils.h b/gdb/utils.h
index 7865812998e..54a1bc4e476 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -278,6 +278,8 @@ extern void fprintf_symbol (struct ui_file *, const char *,
 extern void perror_warning_with_name (const char *string);
 
 extern void print_sys_errmsg (const char *, int);
+
+extern void throw_sys_errmsg (const char *, int) ATTRIBUTE_NORETURN;
 \f
 /* Warnings and error messages.  */
 
-- 
2.39.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-02-08 17:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-28  3:37 [PATCH] gdb/source: Fix open_source_file error handling Aaron Merey
2023-02-01 19:45 ` Keith Seitz
2023-02-02  1:38   ` Aaron Merey
2023-02-08 17:53     ` Tom Tromey
2023-02-02 10:29   ` Bruno Larsen
2023-02-08 17:44 ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).