From: Aaron Merey <amerey@redhat.com>
To: gdb-patches@sourceware.org
Cc: simark@simark.ca, Aaron Merey <amerey@redhat.com>
Subject: [PATCH] gdb/source: Fix open_source_file error handling
Date: Fri, 27 Jan 2023 22:37:48 -0500 [thread overview]
Message-ID: <20230128033748.2839360-1-amerey@redhat.com> (raw)
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
next reply other threads:[~2023-01-28 3:38 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-28 3:37 Aaron Merey [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230128033748.2839360-1-amerey@redhat.com \
--to=amerey@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=simark@simark.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).