rpms
/
gdb
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix undefined behavior in find_and_open_source 

Resolves: RHEL-17631
This commit is contained in:
Keith Seitz 2023-12-06 10:04:18 -08:00
parent d63d041945
commit 0cda5771bf
8 changed files with 994 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
_gdb.spec.Patch.include
View File

@ -484,7 +484,7 @@ Patch116: libiberty-rhbz-2132600-prevent-buffer-overflow.patch
Patch117: gdb-rhel-7328-fix-fortran-28801.patch
# Backport "libiberty: Fix infinite recursion in rust demangler."
# (Nick Clifton, RHEL-4234)
# (Nick Clifton)
Patch118: libiberty-infinite-recursion-fix-1-of-3.patch
# Backport Add a recursion limit to the demangle_const function in the Rust demangler.
@ -492,6 +492,23 @@ Patch118: libiberty-infinite-recursion-fix-1-of-3.patch
Patch119: libiberty-infinite-recursion-fix-2-of-3.patch
# Backport Fix typo in recent code to add stack recursion limit to the Rust demangler.
# (Nick Clifton, RHEL-4234)
# (Nick Clifton)
Patch120: libiberty-infinite-recursion-fix-3-of-3.patch
# Backport "gdbsupport: make gdb_abspath return an std::string"
# (Simon Marchi)
Patch121: gdb-find_and_open_source-empty-string-ub-1of4.patch
# Backport "gdbsupport: add path_join function"
# (Simon Marchi)
Patch122: gdb-find_and_open_source-empty-string-ub-2of4.patch
# Backport "Fix undefined behaviour dereferencing empty string"
# (Magne Hov, RHEL-17631)
Patch123: gdb-find_and_open_source-empty-string-ub-3of4.patch
# Backport "gdbsupport: change path_join parameter to
# array_view<const char *>"
# (Simon Marchi)
Patch124: gdb-find_and_open_source-empty-string-ub-4of4.patch

4
_gdb.spec.patch.include
View File

@ -118,3 +118,7 @@
%patch118 -p1
%patch119 -p1
%patch120 -p1
%patch121 -p1
%patch122 -p1
%patch123 -p1
%patch124 -p1

4
_patch_order
View File

@ -118,3 +118,7 @@ gdb-rhel-7328-fix-fortran-28801.patch
libiberty-infinite-recursion-fix-1-of-3.patch
libiberty-infinite-recursion-fix-2-of-3.patch
libiberty-infinite-recursion-fix-3-of-3.patch
gdb-find_and_open_source-empty-string-ub-1of4.patch
gdb-find_and_open_source-empty-string-ub-2of4.patch
gdb-find_and_open_source-empty-string-ub-3of4.patch
gdb-find_and_open_source-empty-string-ub-4of4.patch

327
gdb-find_and_open_source-empty-string-ub-1of4.patch Normal file
View File

@ -0,0 +1,327 @@
From FEDORA_PATCHES Mon Sep 17 00:00:00 2001
From: Keith Seitz <keiths@redhat.com>
Date: Mon, 4 Dec 2023 11:04:22 -0800
Subject: gdb-find_and_open_source-empty-string-ub-1of4.patch
;; Backport "gdbsupport: make gdb_abspath return an std::string"
;; (Simon Marchi)
I'm trying to switch these functions to use std::string instead of char
arrays, as much as possible. Some callers benefit from it (can avoid
doing a copy of the result), while others suffer (have to make one more
copy).
Change-Id: Iced49b8ee2f189744c5072a3b217aab5af17a993
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -295,8 +295,8 @@ compile_file_command (const char *args, int from_tty)
error (_("You must provide a filename for this command."));
args = skip_spaces (args);
- gdb::unique_xmalloc_ptr<char> abspath = gdb_abspath (args);
- std::string buffer = string_printf ("#include \"%s\"\n", abspath.get ());
+ std::string abspath = gdb_abspath (args);
+ std::string buffer = string_printf ("#include \"%s\"\n", abspath.c_str ());
eval_compile_command (NULL, buffer.c_str (), scope, NULL);
}
diff --git a/gdb/completer.c b/gdb/completer.c
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -2036,7 +2036,7 @@ gdb_completion_word_break_characters_throw ()
rl_basic_quote_characters = NULL;
}
- return rl_completer_word_break_characters;
+ return (char *) rl_completer_word_break_characters;
}
char *
diff --git a/gdb/corelow.c b/gdb/corelow.c
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -447,7 +447,7 @@ core_target_open (const char *arg, int from_tty)
gdb::unique_xmalloc_ptr<char> filename (tilde_expand (arg));
if (!IS_ABSOLUTE_PATH (filename.get ()))
- filename = gdb_abspath (filename.get ());
+ filename = make_unique_xstrdup (gdb_abspath (filename.get ()).c_str ());
flags = O_BINARY | O_LARGEFILE;
if (write_files)
diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -291,7 +291,8 @@ set_index_cache_directory_command (const char *arg, int from_tty,
cmd_list_element *element)
{
/* Make sure the index cache directory is absolute and tilde-expanded. */
- gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (index_cache_directory));
+ gdb::unique_xmalloc_ptr<char> abs
+ = make_unique_xstrdup (gdb_abspath (index_cache_directory).c_str ());
xfree (index_cache_directory);
index_cache_directory = abs.release ();
global_index_cache.set_directory (index_cache_directory);
diff --git a/gdb/main.c b/gdb/main.c
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -135,12 +135,7 @@ set_gdb_data_directory (const char *new_datadir)
"../foo" and "../foo" doesn't exist then we'll record $(pwd)/../foo which
isn't canonical, but that's ok. */
if (!IS_ABSOLUTE_PATH (gdb_datadir.c_str ()))
- {
- gdb::unique_xmalloc_ptr<char> abs_datadir
- = gdb_abspath (gdb_datadir.c_str ());
-
- gdb_datadir = abs_datadir.get ();
- }
+ gdb_datadir = gdb_abspath (gdb_datadir.c_str ());
}
/* Relocate a file or directory. PROGNAME is the name by which gdb
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -341,7 +341,7 @@ objfile::objfile (bfd *abfd, const char *name, objfile_flags flags_)
objfile_alloc_data (this);
- gdb::unique_xmalloc_ptr<char> name_holder;
+ std::string name_holder;
if (name == NULL)
{
gdb_assert (abfd == NULL);
@@ -354,7 +354,7 @@ objfile::objfile (bfd *abfd, const char *name, objfile_flags flags_)
else
{
name_holder = gdb_abspath (name);
- expanded_name = name_holder.get ();
+ expanded_name = name_holder.c_str ();
}
original_name = obstack_strdup (&objfile_obstack, expanded_name);
diff --git a/gdb/source.c b/gdb/source.c
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -512,15 +512,15 @@ add_path (const char *dirname, char **which_path, int parse_separators)
for (const gdb::unique_xmalloc_ptr<char> &name_up : dir_vec)
{
- char *name = name_up.get ();
+ const char *name = name_up.get ();
char *p;
struct stat st;
- gdb::unique_xmalloc_ptr<char> new_name_holder;
+ std::string new_name_holder;
/* Spaces and tabs will have been removed by buildargv().
NAME is the start of the directory.
P is the '\0' following the end. */
- p = name + strlen (name);
+ p = name_up.get () + strlen (name);
while (!(IS_DIR_SEPARATOR (*name) && p <= name + 1) /* "/" */
#ifdef HAVE_DOS_BASED_FILE_SYSTEM
@@ -561,16 +561,18 @@ add_path (const char *dirname, char **which_path, int parse_separators)
}
if (name[0] == '~')
- new_name_holder.reset (tilde_expand (name));
+ new_name_holder
+ = gdb::unique_xmalloc_ptr<char[]> (tilde_expand (name)).get ();
#ifdef HAVE_DOS_BASED_FILE_SYSTEM
else if (IS_ABSOLUTE_PATH (name) && p == name + 2) /* "d:" => "d:." */
- new_name_holder.reset (concat (name, ".", (char *) NULL));
+ new_name_holder = std::string (name) + ".";
#endif
else if (!IS_ABSOLUTE_PATH (name) && name[0] != '$')
new_name_holder = gdb_abspath (name);
else
- new_name_holder.reset (savestring (name, p - name));
- name = new_name_holder.get ();
+ new_name_holder = std::string (name, p - name);
+
+ name = new_name_holder.c_str ();
/* Unless it's a variable, check existence. */
if (name[0] != '$')
@@ -915,7 +917,8 @@ openp (const char *path, openp_flags opts, const char *string,
else if ((opts & OPF_RETURN_REALPATH) != 0)
*filename_opened = gdb_realpath (filename);
else
- *filename_opened = gdb_abspath (filename);
+ *filename_opened
+ = make_unique_xstrdup (gdb_abspath (filename).c_str ());
}
errno = last_errno;
diff --git a/gdb/top.c b/gdb/top.c
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -2065,8 +2065,7 @@ init_history (void)
const char *fname = ".gdb_history";
#endif
- gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (fname));
- history_filename = temp.release ();
+ history_filename = xstrdup (gdb_abspath (fname).c_str ());
}
if (!history_filename_empty ())
@@ -2150,10 +2149,10 @@ set_history_filename (const char *args,
that was read. */
if (!history_filename_empty () && !IS_ABSOLUTE_PATH (history_filename))
{
- gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (history_filename));
+ std::string temp = gdb_abspath (history_filename);
xfree (history_filename);
- history_filename = temp.release ();
+ history_filename = xstrdup (temp.c_str ());
}
}
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -471,7 +471,7 @@ tfile_target_open (const char *arg, int from_tty)
gdb::unique_xmalloc_ptr<char> filename (tilde_expand (arg));
if (!IS_ABSOLUTE_PATH (filename.get ()))
- filename = gdb_abspath (filename.get ());
+ filename = make_unique_xstrdup (gdb_abspath (filename.get ()).c_str ());
flags = O_BINARY | O_LARGEFILE;
flags |= O_RDONLY;
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -93,31 +93,31 @@ bool non_stop;
static struct {
/* Set the PROGRAM_PATH. Here we adjust the path of the provided
binary if needed. */
- void set (gdb::unique_xmalloc_ptr<char> &&path)
+ void set (const char *path)
{
- m_path = std::move (path);
+ m_path = path;
/* Make sure we're using the absolute path of the inferior when
creating it. */
- if (!contains_dir_separator (m_path.get ()))
+ if (!contains_dir_separator (m_path.c_str ()))
{
int reg_file_errno;
/* Check if the file is in our CWD. If it is, then we prefix
its name with CURRENT_DIRECTORY. Otherwise, we leave the
name as-is because we'll try searching for it in $PATH. */
- if (is_regular_file (m_path.get (), &reg_file_errno))
- m_path = gdb_abspath (m_path.get ());
+ if (is_regular_file (m_path.c_str (), &reg_file_errno))
+ m_path = gdb_abspath (m_path.c_str ());
}
}
/* Return the PROGRAM_PATH. */
- char *get ()
- { return m_path.get (); }
+ const char *get ()
+ { return m_path.empty () ? nullptr : m_path.c_str (); }
private:
/* The program name, adjusted if needed. */
- gdb::unique_xmalloc_ptr<char> m_path;
+ std::string m_path;
} program_path;
static std::vector<char *> program_args;
static std::string wrapper_argv;
@@ -3054,7 +3054,7 @@ handle_v_run (char *own_buf)
}
}
else
- program_path.set (gdb::unique_xmalloc_ptr<char> (new_program_name));
+ program_path.set (new_program_name);
/* Free the old argv and install the new one. */
free_vector_argv (program_args);
@@ -3845,7 +3845,7 @@ captured_main (int argc, char *argv[])
int i, n;
n = argc - (next_arg - argv);
- program_path.set (make_unique_xstrdup (next_arg[0]));
+ program_path.set (next_arg[0]);
for (i = 1; i < n; i++)
program_args.push_back (xstrdup (next_arg[i]));
diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc
--- a/gdbsupport/pathstuff.cc
+++ b/gdbsupport/pathstuff.cc
@@ -126,23 +126,23 @@ gdb_realpath_keepfile (const char *filename)
/* See gdbsupport/pathstuff.h. */
-gdb::unique_xmalloc_ptr<char>
+std::string
gdb_abspath (const char *path)
{
gdb_assert (path != NULL && path[0] != '\0');
if (path[0] == '~')
- return gdb_tilde_expand_up (path);
+ return gdb_tilde_expand (path);
if (IS_ABSOLUTE_PATH (path) || current_directory == NULL)
- return make_unique_xstrdup (path);
+ return path;
/* Beware the // my son, the Emacs barfs, the botch that catch... */
- return gdb::unique_xmalloc_ptr<char>
- (concat (current_directory,
- IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
- ? "" : SLASH_STRING,
- path, (char *) NULL));
+ return string_printf
+ ("%s%s%s", current_directory,
+ (IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
+ ? "" : SLASH_STRING),
+ path);
}
/* See gdbsupport/pathstuff.h. */
@@ -225,8 +225,8 @@ get_standard_cache_dir ()
if (xdg_cache_home != NULL)
{
/* Make sure the path is absolute and tilde-expanded. */
- gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (xdg_cache_home));
- return string_printf ("%s/gdb", abs.get ());
+ std::string abs = gdb_abspath (xdg_cache_home);
+ return string_printf ("%s/gdb", abs.c_str ());
}
#endif
@@ -234,8 +234,8 @@ get_standard_cache_dir ()
if (home != NULL)
{
/* Make sure the path is absolute and tilde-expanded. */
- gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (home));
- return string_printf ("%s/" HOME_CACHE_DIR "/gdb", abs.get ());
+ std::string abs = gdb_abspath (home);
+ return string_printf ("%s/" HOME_CACHE_DIR "/gdb", abs.c_str ());
}
return {};
diff --git a/gdbsupport/pathstuff.h b/gdbsupport/pathstuff.h
--- a/gdbsupport/pathstuff.h
+++ b/gdbsupport/pathstuff.h
@@ -49,7 +49,7 @@ extern gdb::unique_xmalloc_ptr<char>
If CURRENT_DIRECTORY is NULL, this function returns a copy of
PATH. */
-extern gdb::unique_xmalloc_ptr<char> gdb_abspath (const char *path);
+extern std::string gdb_abspath (const char *path);
/* If the path in CHILD is a child of the path in PARENT, return a
pointer to the first component in the CHILD's pathname below the

475
gdb-find_and_open_source-empty-string-ub-2of4.patch Normal file
View File

@ -0,0 +1,475 @@
From FEDORA_PATCHES Mon Sep 17 00:00:00 2001
From: Keith Seitz <keiths@redhat.com>
Date: Mon, 4 Dec 2023 11:02:16 -0800
Subject: gdb-find_and_open_source-empty-string-ub-2of4.patch
;; Backport "gdbsupport: add path_join function"
;; (Simon Marchi)
In this review [1], Eli pointed out that we should be careful when
concatenating file names to avoid duplicated slashes. On Windows, a
double slash at the beginning of a file path has a special meaning. So
naively concatenating "/" and "foo/bar" would give "//foo/bar", which
would not give the desired results. We already have a few spots doing:
if (first_path ends with a slash)
path = first_path + second_path
else
path = first_path + slash + second_path
In general, I think it's nice to avoid superfluous slashes in file
paths, since they might end up visible to the user and look a bit
unprofessional.
Introduce the path_join function that can be used to join multiple path
components together (along with unit tests).
I initially wanted to make it possible to join two absolute paths, to
support the use case of prepending a sysroot path to a target file path,
or the prepending the debug-file-directory to a target file path. But
the code in solib_find_1 shows that it is more complex than this anyway
(for example, when the right hand side is a Windows path with a drive
letter). So I don't think we need to support that case in path_join.
That also keeps the implementation simpler.
Change a few spots to use path_join to show how it can be used. I
believe that all the spots I changed are guarded by some checks that
ensure the right hand side operand is not an absolute path.
Regression-tested on Ubuntu 18.04. Built-tested on Windows, and I also
ran the new unit-test there.
[1] https://sourceware.org/pipermail/gdb-patches/2022-April/187559.html
Change-Id: I0df889f7e3f644e045f42ff429277b732eb6c752
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -446,6 +446,7 @@ SELFTESTS_SRCS = \
unittests/observable-selftests.c \
unittests/optional-selftests.c \
unittests/parse-connection-spec-selftests.c \
+ unittests/path-join-selftests.c \
unittests/ptid-selftests.c \
unittests/main-thread-selftests.c \
unittests/mkdir-recursive-selftests.c \
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -19,6 +19,7 @@
#include "defs.h"
#include "buildsym-legacy.h"
#include "bfd.h"
+#include "gdbsupport/pathstuff.h"
#include "gdb_obstack.h"
#include "symtab.h"
#include "symfile.h"
@@ -512,27 +513,22 @@ buildsym_compunit::start_subfile (const char *name)
for (subfile = m_subfiles; subfile; subfile = subfile->next)
{
- char *subfile_name;
+ std::string subfile_name;
/* If NAME is an absolute path, and this subfile is not, then
attempt to create an absolute path to compare. */
if (IS_ABSOLUTE_PATH (name)
&& !IS_ABSOLUTE_PATH (subfile->name)
&& subfile_dirname != NULL)
- subfile_name = concat (subfile_dirname, SLASH_STRING,
- subfile->name, (char *) NULL);
+ subfile_name = path_join (subfile_dirname, subfile->name);
else
subfile_name = subfile->name;
- if (FILENAME_CMP (subfile_name, name) == 0)
+ if (FILENAME_CMP (subfile_name.c_str (), name) == 0)
{
m_current_subfile = subfile;
- if (subfile_name != subfile->name)
- xfree (subfile_name);
return;
}
- if (subfile_name != subfile->name)
- xfree (subfile_name);
}
/* This subfile is not known. Add an entry for it. */
diff --git a/gdb/dwarf2/line-header.c b/gdb/dwarf2/line-header.c
--- a/gdb/dwarf2/line-header.c
+++ b/gdb/dwarf2/line-header.c
@@ -24,6 +24,7 @@
#include "dwarf2/read.h"
#include "complaints.h"
#include "filenames.h"
+#include "gdbsupport/pathstuff.h"
void
line_header::add_include_dir (const char *include_dir)
@@ -73,9 +74,7 @@ line_header::file_file_name (int file) const
{
const char *dir = fe->include_dir (this);
if (dir != NULL)
- return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
- fe->name,
- (char *) NULL));
+ return make_unique_xstrdup (path_join (dir, fe->name).c_str ());
}
return make_unique_xstrdup (fe->name);
}
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -12782,14 +12782,13 @@ open_dwo_file (dwarf2_per_objfile *per_objfile,
if (comp_dir != NULL)
{
- gdb::unique_xmalloc_ptr<char> path_to_try
- (concat (comp_dir, SLASH_STRING, file_name, (char *) NULL));
+ std::string path_to_try = path_join (comp_dir, file_name);
/* NOTE: If comp_dir is a relative path, this will also try the
search path, which seems useful. */
- gdb_bfd_ref_ptr abfd (try_open_dwop_file (per_objfile, path_to_try.get (),
- 0 /*is_dwp*/,
- 1 /*search_cwd*/));
+ gdb_bfd_ref_ptr abfd (try_open_dwop_file
+ (per_objfile, path_to_try.c_str (), 0 /*is_dwp*/, 1 /*search_cwd*/));
+
if (abfd != NULL)
return abfd;
}
@@ -20537,7 +20536,7 @@ static const char *
psymtab_include_file_name (const struct line_header *lh, const file_entry &fe,
const dwarf2_psymtab *pst,
const char *comp_dir,
- gdb::unique_xmalloc_ptr<char> *name_holder)
+ std::string &name_holder)
{
const char *include_name = fe.name;
const char *include_name_to_compare = include_name;
@@ -20546,7 +20545,7 @@ psymtab_include_file_name (const struct line_header *lh, const file_entry &fe,
const char *dir_name = fe.include_dir (lh);
- gdb::unique_xmalloc_ptr<char> hold_compare;
+ std::string hold_compare;
if (!IS_ABSOLUTE_PATH (include_name)
&& (dir_name != NULL || comp_dir != NULL))
{
@@ -20573,26 +20572,23 @@ psymtab_include_file_name (const struct line_header *lh, const file_entry &fe,
if (dir_name != NULL)
{
- name_holder->reset (concat (dir_name, SLASH_STRING,
- include_name, (char *) NULL));
- include_name = name_holder->get ();
+ name_holder = path_join (dir_name, include_name);
+ include_name = name_holder.c_str ();
include_name_to_compare = include_name;
}
if (!IS_ABSOLUTE_PATH (include_name) && comp_dir != NULL)
{
- hold_compare.reset (concat (comp_dir, SLASH_STRING,
- include_name, (char *) NULL));
- include_name_to_compare = hold_compare.get ();
+ hold_compare = path_join (comp_dir, include_name);
+ include_name_to_compare = hold_compare.c_str ();
}
}
pst_filename = pst->filename;
- gdb::unique_xmalloc_ptr<char> copied_name;
+ std::string copied_name;
if (!IS_ABSOLUTE_PATH (pst_filename) && pst->dirname != NULL)
{
- copied_name.reset (concat (pst->dirname, SLASH_STRING,
- pst_filename, (char *) NULL));
- pst_filename = copied_name.get ();
+ copied_name = path_join (pst->dirname, pst_filename);
+ pst_filename = copied_name.c_str ();
}
file_is_pst = FILENAME_CMP (include_name_to_compare, pst_filename) == 0;
@@ -21303,10 +21299,10 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
for (auto &file_entry : lh->file_names ())
if (file_entry.included_p == 1)
{
- gdb::unique_xmalloc_ptr<char> name_holder;
+ std::string name_holder;
const char *include_name =
psymtab_include_file_name (lh, file_entry, pst,
- comp_dir, &name_holder);
+ comp_dir, name_holder);
if (include_name != NULL)
dwarf2_create_include_psymtab (include_name, pst, objfile);
}
@@ -21360,7 +21356,7 @@ static void
dwarf2_start_subfile (struct dwarf2_cu *cu, const char *filename,
const char *dirname)
{
- gdb::unique_xmalloc_ptr<char> copy;
+ std::string copy;
/* In order not to lose the line information directory,
we concatenate it to the filename when it makes sense.
@@ -21371,8 +21367,8 @@ dwarf2_start_subfile (struct dwarf2_cu *cu, const char *filename,
if (!IS_ABSOLUTE_PATH (filename) && dirname != NULL)
{
- copy.reset (concat (dirname, SLASH_STRING, filename, (char *) NULL));
- filename = copy.get ();
+ copy = path_join (dirname, filename);
+ filename = copy.c_str ();
}
cu->get_builder ()->start_subfile (filename);
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -18,6 +18,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "defs.h"
+#include "gdbsupport/pathstuff.h"
#include "gdb_obstack.h"
#include "splay-tree.h"
#include "filenames.h"
@@ -1071,5 +1072,5 @@ macro_source_fullname (struct macro_source_file *file)
if (comp_dir == NULL || IS_ABSOLUTE_PATH (file->filename))
return file->filename;
- return std::string (comp_dir) + SLASH_STRING + file->filename;
+ return path_join (comp_dir, file->filename);
}
diff --git a/gdb/testsuite/gdb.mi/mi-fullname-deleted.exp b/gdb/testsuite/gdb.mi/mi-fullname-deleted.exp
--- a/gdb/testsuite/gdb.mi/mi-fullname-deleted.exp
+++ b/gdb/testsuite/gdb.mi/mi-fullname-deleted.exp
@@ -36,6 +36,12 @@ if { [regsub {^(/[^/]+)/} $srcfileabs {\1subst/} srcfileabssubst] != 1
return -1
}
+# Generate a regular expression which to match $srcfileabs with
+# or without the doubled slash. This is used by the substituted
+# fullname test.
+set srcfileabssubst_regexp [string_to_regexp $srcfileabssubst]
+regsub {//} $srcfileabssubst_regexp {\0?} srcfileabssubst_regexp
+
set f [open $srcfileabs "w"]
puts $f "int main (void) { return 0; }"
close $f
@@ -54,7 +60,7 @@ mi_gdb_test "-interpreter-exec console \"set substitute-path ${initdir} ${initdi
mi_gdb_test "-file-list-exec-source-file" ".*\",fullname=\".*\".*" "fullname present"
-mi_gdb_test "-file-list-exec-source-file" ".*\",fullname=\"[string_to_regexp $srcfileabssubst]\".*" "substituted fullname"
+mi_gdb_test "-file-list-exec-source-file" ".*\",fullname=\"$srcfileabssubst_regexp\".*" "substituted fullname"
# Test compare_filenames_for_search does not falsely use absolute filename as
# a relative one.
diff --git a/gdb/unittests/path-join-selftests.c b/gdb/unittests/path-join-selftests.c
new file mode 100644
--- /dev/null
+++ b/gdb/unittests/path-join-selftests.c
@@ -0,0 +1,73 @@
+/* Self tests for path_join for GDB, the GNU debugger.
+
+ Copyright (C) 2022 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+#include "defs.h"
+#include "gdbsupport/pathstuff.h"
+#include "gdbsupport/selftest.h"
+
+namespace selftests {
+namespace path_join {
+
+template <typename ...Args>
+static void
+test_one (const char *expected, Args... paths)
+{
+ std::string actual = ::path_join (paths...);
+
+ SELF_CHECK (actual == expected);
+}
+
+/* Test path_join. */
+
+static void
+test ()
+{
+ test_one ("/foo/bar", "/foo", "bar");
+ test_one ("/bar", "/", "bar");
+ test_one ("foo/bar/", "foo", "bar", "");
+ test_one ("foo", "", "foo");
+ test_one ("foo/bar", "foo", "", "bar");
+ test_one ("foo/", "foo", "");
+ test_one ("foo/", "foo/", "");
+
+ test_one ("D:/foo/bar", "D:/foo", "bar");
+ test_one ("D:/foo/bar", "D:/foo/", "bar");
+
+#if defined(_WIN32)
+ /* The current implementation doesn't recognize backslashes as directory
+ separators on Unix-like systems, so only run these tests on Windows. If
+ we ever switch our implementation to use std::filesystem::path, they
+ should work anywhere, in theory. */
+ test_one ("D:\\foo/bar", "D:\\foo", "bar");
+ test_one ("D:\\foo\\bar", "D:\\foo\\", "bar");
+ test_one ("\\\\server\\dir\\file", "\\\\server\\dir\\", "file");
+ test_one ("\\\\server\\dir/file", "\\\\server\\dir", "file");
+#endif /* _WIN32 */
+}
+
+}
+}
+
+void _initialize_path_join_selftests ();
+void
+_initialize_path_join_selftests ()
+{
+ selftests::register_test ("path_join",
+ selftests::path_join::test);
+}
diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc
--- a/gdbsupport/pathstuff.cc
+++ b/gdbsupport/pathstuff.cc
@@ -87,7 +87,6 @@ gdb_realpath_keepfile (const char *filename)
{
const char *base_name = lbasename (filename);
char *dir_name;
- char *result;
/* Extract the basename of filename, and return immediately
a copy of filename if it does not contain any directory prefix. */
@@ -116,12 +115,7 @@ gdb_realpath_keepfile (const char *filename)
directory separator, avoid doubling it. */
gdb::unique_xmalloc_ptr<char> path_storage = gdb_realpath (dir_name);
const char *real_path = path_storage.get ();
- if (IS_DIR_SEPARATOR (real_path[strlen (real_path) - 1]))
- result = concat (real_path, base_name, (char *) NULL);
- else
- result = concat (real_path, SLASH_STRING, base_name, (char *) NULL);
-
- return gdb::unique_xmalloc_ptr<char> (result);
+ return make_unique_xstrdup (path_join (real_path, base_name).c_str ());
}
/* See gdbsupport/pathstuff.h. */
@@ -137,12 +131,7 @@ gdb_abspath (const char *path)
if (IS_ABSOLUTE_PATH (path) || current_directory == NULL)
return path;
- /* Beware the // my son, the Emacs barfs, the botch that catch... */
- return string_printf
- ("%s%s%s", current_directory,
- (IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
- ? "" : SLASH_STRING),
- path);
+ return path_join (current_directory, path);
}
/* See gdbsupport/pathstuff.h. */
@@ -197,6 +186,29 @@ child_path (const char *parent, const char *child)
/* See gdbsupport/pathstuff.h. */
+std::string
+path_join (gdb::array_view<const gdb::string_view> paths)
+{
+ std::string ret;
+
+ for (int i = 0; i < paths.size (); ++i)
+ {
+ const gdb::string_view path = paths[i];
+
+ if (i > 0)
+ gdb_assert (!IS_ABSOLUTE_PATH (path));
+
+ if (!ret.empty () && !IS_DIR_SEPARATOR (ret.back ()))
+ ret += '/';
+
+ ret.append (path.begin (), path.end ());
+ }
+
+ return ret;
+}
+
+/* See gdbsupport/pathstuff.h. */
+
bool
contains_dir_separator (const char *path)
{
@@ -226,7 +238,7 @@ get_standard_cache_dir ()
{
/* Make sure the path is absolute and tilde-expanded. */
std::string abs = gdb_abspath (xdg_cache_home);
- return string_printf ("%s/gdb", abs.c_str ());
+ return path_join (abs.c_str (), "gdb");
}
#endif
@@ -235,7 +247,7 @@ get_standard_cache_dir ()
{
/* Make sure the path is absolute and tilde-expanded. */
std::string abs = gdb_abspath (home);
- return string_printf ("%s/" HOME_CACHE_DIR "/gdb", abs.c_str ());
+ return path_join (abs.c_str (), HOME_CACHE_DIR, "gdb");
}
return {};
diff --git a/gdbsupport/pathstuff.h b/gdbsupport/pathstuff.h
--- a/gdbsupport/pathstuff.h
+++ b/gdbsupport/pathstuff.h
@@ -21,6 +21,7 @@
#define COMMON_PATHSTUFF_H
#include "gdbsupport/byte-vector.h"
+#include "gdbsupport/array-view.h"
/* Path utilities. */
@@ -57,6 +58,28 @@ extern std::string gdb_abspath (const char *path);
extern const char *child_path (const char *parent, const char *child);
+/* Join elements in PATHS into a single path.
+
+ The first element can be absolute or relative. All the others must be
+ relative. */
+
+extern std::string path_join (gdb::array_view<const gdb::string_view> paths);
+
+/* Same as the above, but accept paths as distinct parameters. */
+
+template<typename ...Args>
+std::string
+path_join (Args... paths)
+{
+ /* It doesn't make sense to join less than two paths. */
+ gdb_static_assert (sizeof... (Args) >= 2);
+
+ std::array<gdb::string_view, sizeof... (Args)> views
+ { gdb::string_view (paths)... };
+
+ return path_join (gdb::array_view<const gdb::string_view> (views));
+}
+
/* Return whether PATH contains a directory separator character. */
extern bool contains_dir_separator (const char *path);

47
gdb-find_and_open_source-empty-string-ub-3of4.patch Normal file
View File

@ -0,0 +1,47 @@
From FEDORA_PATCHES Mon Sep 17 00:00:00 2001
From: Keith Seitz <keiths@redhat.com>
Date: Tue, 5 Dec 2023 08:47:16 -0800
Subject: gdb-find_and_open_source-empty-string-ub-3of4.patch
;; Backport "Fix undefined behaviour dereferencing empty string"
;; (Magne Hov, RHEL-17631)
When a source file's dirname is solely made up of directory separators
we end up trying to dereference the last character of an empty string
with std::string::back, which results in undefined behaviour. A typical
use case where this can happen is when the root directory "/" is used as
a compilation directory.
With libstdc++.so.6.0.28 we get no out-of-bounds checks and the byte
preceding the storage of the empty string is returned. The character
value of this byte depends on heap implementation and usage, but when
this byte happens to hold the value of the directory separator character
we go on to call std::string::pop_back on the empty string which results
in an out_of_range exception which terminates GDB.
Fix this by using path_join. prepare_path_for_appending ensures that the
filename component is relative.
The testsuite has been run before and after the change and no
regressions were found.
diff --git a/gdb/source.c b/gdb/source.c
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1111,15 +1111,7 @@ find_and_open_source (const char *filename,
helpful if part of the compilation directory was removed,
e.g. using gcc's -fdebug-prefix-map, and we have added the missing
prefix to source_path. */
- std::string cdir_filename (dirname);
-
- /* Remove any trailing directory separators. */
- while (IS_DIR_SEPARATOR (cdir_filename.back ()))
- cdir_filename.pop_back ();
-
- /* Add our own directory separator. */
- cdir_filename.append (SLASH_STRING);
- cdir_filename.append (filename_start);
+ std::string cdir_filename = path_join (dirname, filename_start);
result = openp (path, OPF_SEARCH_IN_PATH | OPF_RETURN_REALPATH,
cdir_filename.c_str (), OPEN_MODE, fullname);

113
gdb-find_and_open_source-empty-string-ub-4of4.patch Normal file
View File

@ -0,0 +1,113 @@
From FEDORA_PATCHES Mon Sep 17 00:00:00 2001
From: Keith Seitz <keiths@redhat.com>
Date: Wed, 6 Dec 2023 07:19:49 -0800
Subject: gdb-find_and_open_source-empty-string-ub-4of4.patch
;; Backport "gdbsupport: change path_join parameter to
;; array_view<const char *>"
;; (Simon Marchi)
When a GDB built with -D_GLIBCXX_DEBUG=1 reads a binary with a single
character name, we hit this assertion failure:
$ ./gdb -q --data-directory=data-directory -nx ./x
/usr/include/c++/12.1.0/string_view:239: constexpr const std::basic_string_view<_CharT, _Traits>::value_type& std::basic_string_view<_CharT, _Traits>::operator[](size_type) const [with _CharT = char; _Traits = std::char_traits<char>; const_reference = const char&; size_type = long unsigned int]: Assertion '__pos < this->_M_len' failed.
The backtrace:
#3 0x00007ffff6c0f002 in std::__glibcxx_assert_fail (file=<optimized out>, line=<optimized out>, function=<optimized out>, condition=<optimized out>) at /usr/src/debug/gcc/libstdc++-v3/src/c++11/debug.cc:60
#4 0x000055555da8a864 in std::basic_string_view<char, std::char_traits<char> >::operator[] (this=0x7fffffffcc30, __pos=1) at /usr/include/c++/12.1.0/string_view:239
#5 0x00005555609dcb88 in path_join[abi:cxx11](gdb::array_view<std::basic_string_view<char, std::char_traits<char> > const>) (paths=...) at /home/simark/src/binutils-gdb/gdbsupport/pathstuff.cc:203
#6 0x000055555e0443f4 in path_join<char const*, char const*> () at /home/simark/src/binutils-gdb/gdb/../gdbsupport/pathstuff.h:84
#7 0x00005555609dc336 in gdb_realpath_keepfile[abi:cxx11](char const*) (filename=0x6060000a8d40 "/home/simark/build/binutils-gdb-one-target/gdb/./x") at /home/simark/src/binutils-gdb/gdbsupport/pathstuff.cc:122
#8 0x000055555ebd2794 in exec_file_attach (filename=0x7fffffffe0f9 "./x", from_tty=1) at /home/simark/src/binutils-gdb/gdb/exec.c:471
#9 0x000055555f2b3fb0 in catch_command_errors (command=0x55555ebd1ab6 <exec_file_attach(char const*, int)>, arg=0x7fffffffe0f9 "./x", from_tty=1, do_bp_actions=false) at /home/simark/src/binutils-gdb/gdb/main.c:513
#10 0x000055555f2b7e11 in captured_main_1 (context=0x7fffffffdb60) at /home/simark/src/binutils-gdb/gdb/main.c:1209
#11 0x000055555f2b9144 in captured_main (data=0x7fffffffdb60) at /home/simark/src/binutils-gdb/gdb/main.c:1319
#12 0x000055555f2b9226 in gdb_main (args=0x7fffffffdb60) at /home/simark/src/binutils-gdb/gdb/main.c:1344
#13 0x000055555d938c5e in main (argc=5, argv=0x7fffffffdcf8) at /home/simark/src/binutils-gdb/gdb/gdb.c:32
The problem is this line in path_join:
gdb_assert (strlen (path) == 0 || !IS_ABSOLUTE_PATH (path));
... where `path` is "x". IS_ABSOLUTE_PATH eventually calls
HAS_DRIVE_SPEC_1:
#define HAS_DRIVE_SPEC_1(dos_based, f) \
((f)[0] && ((f)[1] == ':') && (dos_based))
This macro accesses indices 0 and 1 of the input string. However, `f`
is a string_view of length 1, so it's incorrect to try to access index
1. We know that the string_view's underlying object is a null-terminated
string, so in practice there's no harm. But as far as the string_view
is concerned, index 1 is considered out of bounds.
This patch makes the easy fix, that is to change the path_join parameter
from a vector of to a vector of `const char *`. Another solution would
be to introduce a non-standard gdb::cstring_view class, which would be a
view over a null-terminated string. With that class, it would be
correct to access index 1, it would yield the NUL character. If there
is interest in having this class (it has been mentioned a few times in
the past) I can do it and use it here.
This was found by running tests such as gdb.ada/arrayidx.exp, which
produce 1-char long filenames, so adding a new test is not necessary.
Change-Id: Ia41a16c7243614636b18754fd98a41860756f7af
diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc
--- a/gdbsupport/pathstuff.cc
+++ b/gdbsupport/pathstuff.cc
@@ -187,21 +187,21 @@ child_path (const char *parent, const char *child)
/* See gdbsupport/pathstuff.h. */
std::string
-path_join (gdb::array_view<const gdb::string_view> paths)
+path_join (gdb::array_view<const char *> paths)
{
std::string ret;
for (int i = 0; i < paths.size (); ++i)
{
- const gdb::string_view path = paths[i];
+ const char *path = paths[i];
if (i > 0)
- gdb_assert (!IS_ABSOLUTE_PATH (path));
+ gdb_assert (strlen (path) == 0 || !IS_ABSOLUTE_PATH (path));
if (!ret.empty () && !IS_DIR_SEPARATOR (ret.back ()))
ret += '/';
- ret.append (path.begin (), path.end ());
+ ret.append (path);
}
return ret;
diff --git a/gdbsupport/pathstuff.h b/gdbsupport/pathstuff.h
--- a/gdbsupport/pathstuff.h
+++ b/gdbsupport/pathstuff.h
@@ -63,7 +63,7 @@ extern const char *child_path (const char *parent, const char *child);
The first element can be absolute or relative. All the others must be
relative. */
-extern std::string path_join (gdb::array_view<const gdb::string_view> paths);
+extern std::string path_join (gdb::array_view<const char *> paths);
/* Same as the above, but accept paths as distinct parameters. */
@@ -74,10 +74,10 @@ path_join (Args... paths)
/* It doesn't make sense to join less than two paths. */
gdb_static_assert (sizeof... (Args) >= 2);
- std::array<gdb::string_view, sizeof... (Args)> views
- { gdb::string_view (paths)... };
+ std::array<const char *, sizeof... (Args)> path_array
+ { paths... };
- return path_join (gdb::array_view<const gdb::string_view> (views));
+ return path_join (gdb::array_view<const char *> (path_array));
}
/* Return whether PATH contains a directory separator character. */

6
gdb.spec
View File

@ -37,7 +37,7 @@ Version: 10.2
# The release always contains a leading reserved number, start it at 1.
# `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing.
Release: 12%{?dist}
Release: 13%{?dist}
License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with exceptions and GPL+ and LGPLv2+ and LGPLv3+ and BSD and Public Domain and GFDL
# Do not provide URL for snapshots as the file lasts there only for 2 days.
@ -1158,6 +1158,10 @@ fi
%endif
%changelog
* Wed Dec 13 2023 Keith Seitz - 10.2-13.el9
- Backport patches for "Fix undefined behaviour dereferencing empty string"
(Magne Hov et al, RHEL-17631)
* Tue Oct 3 2023 Guinevere Larsen <blarsen@redhat.com> - 10.2-12.el9
- Backport "libiberty: Fix infinite recursion in rust demangler."
(Nick Clifton)