* [PATCH, v4] PR 20569, segv in follow_exec
@ 2016-10-25 17:12 Luis Machado
2016-10-25 18:04 ` Pedro Alves
0 siblings, 1 reply; 13+ messages in thread
From: Luis Machado @ 2016-10-25 17:12 UTC (permalink / raw)
To: gdb-patches, palves; +Cc: sandra
Adjusted a few more bits.
I ended up using the ugly explicit cast for an enum that is being used as a
bitfield. I'm guessing we'll want to convert this to the macro-fied
template-fied version that is more appropriate to C++?
If so then i can come up with a follow up patch that does the conversion. I
didn't want to disrupt v4 with more unrelated changes.
I have regression-tested v4 on Ubuntu 16.04 x86-64 to make sure the new changes
to try_open_exec_file were sane.
How does this version look?
Changes in v4:
* Adjusted source files based on overall comments.
* Renamed maybe_open_exec_file to try_open_exec_file as suggested.
* Simplified try_maybe_open_exec_file by exporting symbol_file_add_main_1
and calling it from within the function.
* Consolidated a couple flags into a single argument to try_open_exec_file.
* Adjusted testcase based on comments.
Changes in v3:
Follows the updated patch with the comments addressed and a few more things
adjusted. I can't say i like the name "maybe_open_exec_file", but that was
the best that came to mind on a Monday. :-)
How does it look now?
* Addressed overal review comments (explicit NULL check, misleading variable
name and trailing whitespaces).
* Separated bug fixes for exception_print_same and NULL dereferences. They
were submitted as a couple separate (obvious?) patches.
* Moved duplicate code from exec_file_locate_attach and follow_exec to a
shared function maybe_open_exec_file that gets called from both places.
* Addressed comments for the testcase, cleaned it up and made it expect a
different output message.
Changes in v2:
* Changed exec_file_target cleanup to a exec_file_host cleanup inside
exec.c:exec_file_locate_attach. It was a typo that caused a crash.
* Added the gdb.base/exec-invalid-sysroot.exp testcase.
As I noted in PR20569, several exec-related tests cause GDB to segv when sysroot translation fails on the executable pathname reported by gdbserver. The immediate cause of the segv is that follow_exec is passing a NULL argument (the result of exec_file_find) to strlen, but as I looked at the code in more detail it seemed like follow_exec simply isn't prepared for the case where sysroot translation fails to locate the new executable, and there is no obvious recovery strategy.
I thought I could copy logic from the other caller of exec_file_find, exec_file_locate_attach, but I think it's doing the wrong thing there as well. Plus, from reading the code I found other bugs in both callers of exec_file_find due to confusion between host and target pathnames.
The attached patch attempts to fix all the bugs. In terms of the testcases that were formerly segv'ing, GDB now prints a warning but continues execution of the new program, so that the tests now mostly FAIL instead. You could argue the FAILs are due to a legitimate problem with the test environment setting up the sysroot translation incorrectly, but I'm not sure continuing execution rather than leaving the target stopped is the most user-friendly fallback behavior, either. E.g. the GDB manual suggests that you can set a breakpoint on main and GDB will stop on main of the newly exec'ed program, too, but it can't do that if it can't find the symbol information, and there's no way for the user to specify the executable file to GDB explicitly before it resumes execution. But, seemed better not to complicate this already-too-complicated patch further by trying to address that in the first cut.
The following testcases will make GDB crash whenever an invalid sysroot is provided, causing GDB to be unabled to find a valid path to the symbol file.
gdb.base/catch-syscall.exp
gdb.base/execl-update-breakpoints.exp
gdb.base/foll-exec-mode.exp
gdb.base/foll-exec.exp
gdb.base/foll-vfork.exp
gdb.base/pie-execl.exp
gdb.multi/bkpt-multi-exec.exp
gdb.python/py-finish-breakpoint.exp
gdb.threads/execl.exp
gdb.threads/non-ldr-exc-1.exp
gdb.threads/non-ldr-exc-2.exp
gdb.threads/non-ldr-exc-3.exp
gdb.threads/non-ldr-exc-4.exp
gdb.threads/thread-execl.exp
gdb/ChangeLog:
2016-10-25 Sandra Loosemore <sandra@codesourcery.com>
Luis Machado <lgustavo@codesourcery.com>
PR gdb/20569
* exceptions.c (exception_print_same): Moved here from exec.c.
* exceptions.h (exception_print_same): Declare.
* exec.h (try_open_exec_file): New declaration.
Include symfile.h.
* exec.c (exception_print_same): Moved to exceptions.c.
(try_open_exec_file): New function.
(exec_file_locate_attach): Rename exec_file and full_exec_path
variables to avoid confusion between target and host pathnames.
Move pathname processing logic to exec_file_find. Do not return
early if pathname lookup fails;
Call try_open_exec_file.
* infrun.c (follow_exec): Split and rename execd_pathname variable
to avoid confusion between target and host pathnames. Replace
brokenpathname copy with cleanup to free malloc'ed string. Warn
if pathname lookup fails. Pass target pathname to
target_follow_exec, not hostpathname.
Call try_open_exec_file.
* solib.c (exec_file_find): Incorporate fallback logic for relative
pathnames formerly in exec_file_locate_attach.
* symfile.c (symbol_file_add_main_1): Make non-static.
Move declaration to ...
* symfile.h (symbol_file_add_main_1): ... here and make extern.
gdb/testsuite/ChangeLog:
2016-10-25 Luis Machado <lgustavo@codesourcery.com>
* gdb.base/exec-invalid-sysroot.exp: New file.
---
gdb/ChangeLog | 27 +++++
gdb/exceptions.c | 19 ++++
gdb/exceptions.h | 3 +
gdb/exec.c | 131 +++++++++++-------------
gdb/exec.h | 9 ++
gdb/infrun.c | 44 ++++----
gdb/solib.c | 32 ++++--
gdb/symfile.c | 4 +-
gdb/symfile.h | 2 +
gdb/testsuite/ChangeLog | 4 +
gdb/testsuite/gdb.base/exec-invalid-sysroot.exp | 70 +++++++++++++
11 files changed, 238 insertions(+), 107 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 43175ff..c9cf146 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,30 @@
+2016-10-25 Sandra Loosemore <sandra@codesourcery.com>
+ Luis Machado <lgustavo@codesourcery.com>
+
+ PR gdb/20569
+ * exceptions.c (exception_print_same): Moved here from exec.c.
+ * exceptions.h (exception_print_same): Declare.
+ * exec.h (try_open_exec_file): New declaration.
+ Include symfile.h.
+ * exec.c (exception_print_same): Moved to exceptions.c.
+ (try_open_exec_file): New function.
+ (exec_file_locate_attach): Rename exec_file and full_exec_path
+ variables to avoid confusion between target and host pathnames.
+ Move pathname processing logic to exec_file_find. Do not return
+ early if pathname lookup fails;
+ Call try_open_exec_file.
+ * infrun.c (follow_exec): Split and rename execd_pathname variable
+ to avoid confusion between target and host pathnames. Replace
+ brokenpathname copy with cleanup to free malloc'ed string. Warn
+ if pathname lookup fails. Pass target pathname to
+ target_follow_exec, not hostpathname.
+ Call try_open_exec_file.
+ * solib.c (exec_file_find): Incorporate fallback logic for relative
+ pathnames formerly in exec_file_locate_attach.
+ * symfile.c (symbol_file_add_main_1): Make non-static.
+ Move declaration to ...
+ * symfile.h (symbol_file_add_main_1): ... here and make extern.
+
2016-10-24 Luis Machado <lgustavo@codesourcery.com>
* exec.c (exec_file_locate_attach): Prevent NULL pointer dereference
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index 9a10f66..0e269de 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -256,3 +256,22 @@ catch_errors (catch_errors_ftype *func, void *func_args, char *errstring,
return 0;
return val;
}
+
+/* Returns non-zero if exceptions E1 and E2 are equal. Returns zero
+ otherwise. */
+
+int
+exception_print_same (struct gdb_exception e1, struct gdb_exception e2)
+{
+ const char *msg1 = e1.message;
+ const char *msg2 = e2.message;
+
+ if (msg1 == NULL)
+ msg1 = "";
+ if (msg2 == NULL)
+ msg2 = "";
+
+ return (e1.reason == e2.reason
+ && e1.error == e2.error
+ && strcmp (msg1, msg2) == 0);
+}
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index 5711c1a..a2d39d7 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -88,4 +88,7 @@ extern int catch_exceptions_with_msg (struct ui_out *uiout,
typedef int (catch_errors_ftype) (void *);
extern int catch_errors (catch_errors_ftype *, void *, char *, return_mask);
+/* Compare two exception objects for print equality. */
+extern int exception_print_same (struct gdb_exception e1,
+ struct gdb_exception e2);
#endif
diff --git a/gdb/exec.c b/gdb/exec.c
index 6e2a296..09c456e 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -136,73 +136,16 @@ exec_file_clear (int from_tty)
printf_unfiltered (_("No executable file now.\n"));
}
-/* Returns non-zero if exceptions E1 and E2 are equal. Returns zero
- otherwise. */
-
-static int
-exception_print_same (struct gdb_exception e1, struct gdb_exception e2)
-{
- const char *msg1 = e1.message;
- const char *msg2 = e2.message;
-
- if (msg1 == NULL)
- msg1 = "";
- if (msg2 == NULL)
- msg2 = "";
-
- return (e1.reason == e2.reason
- && e1.error == e2.error
- && strcmp (msg1, msg2) == 0);
-}
-
-/* See gdbcore.h. */
+/* See exec.h. */
void
-exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
+try_open_exec_file (const char *exec_file_host, struct inferior *inf,
+ enum symfile_add_flags flags, int from_tty)
{
- char *exec_file, *full_exec_path = NULL;
struct cleanup *old_chain;
struct gdb_exception prev_err = exception_none;
- /* Do nothing if we already have an executable filename. */
- exec_file = (char *) get_exec_file (0);
- if (exec_file != NULL)
- return;
-
- /* Try to determine a filename from the process itself. */
- exec_file = target_pid_to_exec_file (pid);
- if (exec_file == NULL)
- {
- warning (_("No executable has been specified and target does not "
- "support\n"
- "determining executable automatically. "
- "Try using the \"file\" command."));
- return;
- }
-
- /* If gdb_sysroot is not empty and the discovered filename
- is absolute then prefix the filename with gdb_sysroot. */
- if (*gdb_sysroot != '\0' && IS_ABSOLUTE_PATH (exec_file))
- {
- full_exec_path = exec_file_find (exec_file, NULL);
- if (full_exec_path == NULL)
- return;
- }
- else
- {
- /* It's possible we don't have a full path, but rather just a
- filename. Some targets, such as HP-UX, don't provide the
- full path, sigh.
-
- Attempt to qualify the filename against the source path.
- (If that fails, we'll just fall back on the original
- filename. Not much more we can do...) */
- if (!source_full_path_of (exec_file, &full_exec_path))
- full_exec_path = xstrdup (exec_file);
- }
-
- old_chain = make_cleanup (xfree, full_exec_path);
- make_cleanup (free_current_contents, &prev_err.message);
+ old_chain = make_cleanup (free_current_contents, &prev_err.message);
/* exec_file_attach and symbol_file_add_main may throw an error if the file
cannot be opened either locally or remotely.
@@ -217,7 +160,9 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
errors/exceptions in the following code. */
TRY
{
- exec_file_attach (full_exec_path, from_tty);
+ /* We must do this step even if exec_file_host is NULL, so that
+ exec_file_attach will clear state. */
+ exec_file_attach (exec_file_host, from_tty);
}
CATCH (err, RETURN_MASK_ERROR)
{
@@ -232,20 +177,64 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
}
END_CATCH
- TRY
+ if (exec_file_host != NULL)
{
- if (defer_bp_reset)
- current_inferior ()->symfile_flags |= SYMFILE_DEFER_BP_RESET;
- symbol_file_add_main (full_exec_path, from_tty);
+ /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
+ (Position Independent Executable) main symbol file will get applied by
+ solib_create_inferior_hook below. breakpoint_re_set would fail to insert
+ the breakpoints with the zero displacement. */
+ TRY
+ {
+ symbol_file_add_main_1 (exec_file_host, from_tty, flags);
+
+ if ((inf->symfile_flags & SYMFILE_NO_READ) == 0)
+ set_initial_language ();
+ }
+ CATCH (err, RETURN_MASK_ERROR)
+ {
+ if (!exception_print_same (prev_err, err))
+ warning ("%s", err.message);
+ }
+ END_CATCH
+
+ inf->symfile_flags &= ~SYMFILE_DEFER_BP_RESET;
}
- CATCH (err, RETURN_MASK_ERROR)
+
+ do_cleanups (old_chain);
+}
+
+/* See gdbcore.h. */
+
+void
+exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
+{
+ char *exec_file_target, *exec_file_host;
+ struct cleanup *old_chain;
+ symfile_add_flags flags = SYMFILE_MAINLINE;
+
+ /* Do nothing if we already have an executable filename. */
+ if (get_exec_file (0) != NULL)
+ return;
+
+ /* Try to determine a filename from the process itself. */
+ exec_file_target = target_pid_to_exec_file (pid);
+ if (exec_file_target == NULL)
{
- if (!exception_print_same (prev_err, err))
- warning ("%s", err.message);
+ warning (_("No executable has been specified and target does not "
+ "support\n"
+ "determining executable automatically. "
+ "Try using the \"file\" command."));
+ return;
}
- END_CATCH
- current_inferior ()->symfile_flags &= ~SYMFILE_DEFER_BP_RESET;
+ exec_file_host = exec_file_find (exec_file_target, NULL);
+ old_chain = make_cleanup (xfree, exec_file_host);
+
+ if (defer_bp_reset)
+ flags = (enum symfile_add_flags) (flags | SYMFILE_DEFER_BP_RESET);
+
+ /* Attempt to open the exec file. */
+ try_open_exec_file (exec_file_host, current_inferior (), flags, from_tty);
do_cleanups (old_chain);
}
diff --git a/gdb/exec.h b/gdb/exec.h
index 4044cb7..151ac62 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -23,6 +23,7 @@
#include "target.h"
#include "progspace.h"
#include "memrange.h"
+#include "symfile.h"
struct target_section;
struct target_ops;
@@ -113,4 +114,12 @@ extern void print_section_info (struct target_section_table *table,
extern void exec_close (void);
+/* Helper function that attempts to open the symbol file at EXEC_FILE_HOST.
+ If successful, it proceeds to add the symbol file as the main symbol file.
+
+ FLAGS is passed on to the function adding the symbol file as well as
+ FROM_TTY. */
+extern void try_open_exec_file (const char *exec_file_host,
+ struct inferior *inf,
+ enum symfile_add_flags flags, int from_tty);
#endif
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 00bba16..d198dc8 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1077,15 +1077,17 @@ show_follow_exec_mode_string (struct ui_file *file, int from_tty,
fprintf_filtered (file, _("Follow exec mode is \"%s\".\n"), value);
}
-/* EXECD_PATHNAME is assumed to be non-NULL. */
+/* EXEC_FILE_TARGET is assumed to be non-NULL. */
static void
-follow_exec (ptid_t ptid, char *execd_pathname)
+follow_exec (ptid_t ptid, char *exec_file_target)
{
struct thread_info *th, *tmp;
struct inferior *inf = current_inferior ();
int pid = ptid_get_pid (ptid);
ptid_t process_ptid;
+ char *exec_file_host;
+ struct cleanup *old_chain;
/* This is an exec event that we actually wish to pay attention to.
Refresh our symbol table to the newly exec'd program, remove any
@@ -1155,7 +1157,7 @@ follow_exec (ptid_t ptid, char *execd_pathname)
process_ptid = pid_to_ptid (pid);
printf_unfiltered (_("%s is executing new program: %s\n"),
target_pid_to_str (process_ptid),
- execd_pathname);
+ exec_file_target);
/* We've followed the inferior through an exec. Therefore, the
inferior has essentially been killed & reborn. */
@@ -1164,14 +1166,17 @@ follow_exec (ptid_t ptid, char *execd_pathname)
breakpoint_init_inferior (inf_execd);
- if (*gdb_sysroot != '\0')
- {
- char *name = exec_file_find (execd_pathname, NULL);
+ exec_file_host = exec_file_find (exec_file_target, NULL);
+ old_chain = make_cleanup (xfree, exec_file_host);
- execd_pathname = (char *) alloca (strlen (name) + 1);
- strcpy (execd_pathname, name);
- xfree (name);
- }
+ /* If we were unable to map the executable target pathname onto a host
+ pathname, tell the user that. Otherwise GDB's subsequent behavior
+ is confusing. Maybe it would even be better to stop at this point
+ so that the user can specify a file manually before continuing. */
+ if (exec_file_host == NULL)
+ warning (_("Could not load symbols for executable %s.\n"
+ "Do you need \"set sysroot\"?"),
+ exec_file_target);
/* Reset the shared library package. This ensures that we get a
shlib event when the child reaches "_start", at which point the
@@ -1193,7 +1198,7 @@ follow_exec (ptid_t ptid, char *execd_pathname)
inf = add_inferior_with_spaces ();
inf->pid = pid;
- target_follow_exec (inf, execd_pathname);
+ target_follow_exec (inf, exec_file_target);
set_current_inferior (inf);
set_current_program_space (inf->pspace);
@@ -1212,21 +1217,10 @@ follow_exec (ptid_t ptid, char *execd_pathname)
gdb_assert (current_program_space == inf->pspace);
- /* That a.out is now the one to use. */
- exec_file_attach (execd_pathname, 0);
+ /* Attempt to open the exec file. */
+ try_open_exec_file (exec_file_host, inf, (enum symfile_add_flags) 0, 0);
- /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
- (Position Independent Executable) main symbol file will get applied by
- solib_create_inferior_hook below. breakpoint_re_set would fail to insert
- the breakpoints with the zero displacement. */
-
- symbol_file_add (execd_pathname,
- (inf->symfile_flags
- | SYMFILE_MAINLINE | SYMFILE_DEFER_BP_RESET),
- NULL, 0);
-
- if ((inf->symfile_flags & SYMFILE_NO_READ) == 0)
- set_initial_language ();
+ do_cleanups (old_chain);
/* If the target can specify a description, read it. Must do this
after flipping to the new executable (because the target supplied
diff --git a/gdb/solib.c b/gdb/solib.c
index b8c2b42..d49dd99 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -380,21 +380,22 @@ solib_find_1 (char *in_pathname, int *fd, int is_solib)
/* Return the full pathname of the main executable, or NULL if not
found. The returned pathname is malloc'ed and must be freed by
the caller. If FD is non-NULL, *FD is set to either -1 or an open
- file handle for the main executable.
-
- The search algorithm used is described in solib_find_1's comment
- above. */
+ file handle for the main executable. */
char *
exec_file_find (char *in_pathname, int *fd)
{
- char *result = solib_find_1 (in_pathname, fd, 0);
+ char *result;
+ const char *fskind = effective_target_file_system_kind ();
+
+ if (in_pathname == NULL)
+ return NULL;
- if (result == NULL)
+ if (*gdb_sysroot != '\0' && IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname))
{
- const char *fskind = effective_target_file_system_kind ();
+ result = solib_find_1 (in_pathname, fd, 0);
- if (fskind == file_system_kind_dos_based)
+ if (result == NULL && fskind == file_system_kind_dos_based)
{
char *new_pathname;
@@ -405,6 +406,21 @@ exec_file_find (char *in_pathname, int *fd)
result = solib_find_1 (new_pathname, fd, 0);
}
}
+ else
+ {
+ /* It's possible we don't have a full path, but rather just a
+ filename. Some targets, such as HP-UX, don't provide the
+ full path, sigh.
+
+ Attempt to qualify the filename against the source path.
+ (If that fails, we'll just fall back on the original
+ filename. Not much more we can do...) */
+
+ if (!source_full_path_of (in_pathname, &result))
+ result = xstrdup (in_pathname);
+ if (fd != NULL)
+ *fd = -1;
+ }
return result;
}
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 7eb6cdc..210badb 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -85,8 +85,6 @@ int readnow_symbol_files; /* Read full symbols immediately. */
static void load_command (char *, int);
-static void symbol_file_add_main_1 (const char *args, int from_tty, int flags);
-
static void add_symbol_file_command (char *, int);
static const struct sym_fns *find_sym_fns (bfd *);
@@ -1306,7 +1304,7 @@ symbol_file_add_main (const char *args, int from_tty)
symbol_file_add_main_1 (args, from_tty, 0);
}
-static void
+void
symbol_file_add_main_1 (const char *args, int from_tty, int flags)
{
const int add_flags = (current_inferior ()->symfile_flags
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 32902cb..cc54530 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -475,6 +475,8 @@ extern struct objfile *symbol_file_add_from_bfd (bfd *, const char *, int,
extern void symbol_file_add_separate (bfd *, const char *, int,
struct objfile *);
+extern void symbol_file_add_main_1 (const char *args, int from_tty, int flags);
+
extern char *find_separate_debug_file_by_debuglink (struct objfile *);
/* Create a new section_addr_info, with room for NUM_SECTIONS. */
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 460e6b9..d0a2bc9 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2016-10-25 Luis Machado <lgustavo@codesourcery.com>
+
+ * gdb.base/exec-invalid-sysroot.exp: New file.
+
2016-10-24 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.base/morestack.exp: Try to build it using -fuse-ld=gold first.
diff --git a/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp b/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
new file mode 100644
index 0000000..a42de0b
--- /dev/null
+++ b/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
@@ -0,0 +1,70 @@
+# Copyright 1997-2016 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 exercises PR20569. GDB would crash when attempting to follow
+# an exec call when it could not resolve the path to the symbol file.
+# This was the case when an invalid sysroot is provided.
+
+standard_testfile foll-exec.c
+
+global binfile
+set binfile [standard_output_file "foll-exec"]
+set testfile2 "execd-prog"
+set srcfile2 ${testfile2}.c
+set binfile2 [standard_output_file ${testfile2}]
+
+set compile_options debug
+
+# build the first test case
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable $compile_options] != "" } {
+ untested "could not compile test program ${binfile2}"
+ return -1
+}
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $compile_options] != "" } {
+ untested "could not compile test program ${binfile}"
+ return -1
+}
+
+proc do_exec_sysroot_test {} {
+ global binfile srcfile srcfile2 testfile testfile2
+ global gdb_prompt
+
+ gdb_test_no_output "set sysroot /a/path/that/does/not/exist"
+
+ # Start the program running, and stop at main.
+ #
+ if ![runto_main] then {
+ fail "Couldn't run ${testfile}"
+ return
+ }
+
+ # Verify that the system supports "catch exec".
+ gdb_test "catch exec" "Catchpoint \[0-9\]* \\(exec\\)" "insert exec catchpoint"
+ set test "continue to first exec catchpoint"
+ gdb_test_multiple "continue" $test {
+ -re ".*Your system does not support this type\r\nof catchpoint.*$gdb_prompt $" {
+ unsupported $test
+ return
+ }
+ -re ".*Could not load symbols for executable.*$gdb_prompt $" {
+ pass $test
+ }
+ }
+}
+
+# Start with a fresh gdb
+clean_restart $binfile
+do_exec_sysroot_test
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH, v4] PR 20569, segv in follow_exec
2016-10-25 17:12 [PATCH, v4] PR 20569, segv in follow_exec Luis Machado
@ 2016-10-25 18:04 ` Pedro Alves
2016-10-25 18:16 ` Luis Machado
0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2016-10-25 18:04 UTC (permalink / raw)
To: Luis Machado, gdb-patches; +Cc: sandra
On 10/25/2016 06:12 PM, Luis Machado wrote:
> Adjusted a few more bits.
>
> I ended up using the ugly explicit cast for an enum that is being used as a
> bitfield. I'm guessing we'll want to convert this to the macro-fied
> template-fied version that is more appropriate to C++?
Yup. I actually have a patch for that in one of my C++ branches
somewhere.
>
> If so then i can come up with a follow up patch that does the conversion. I
> didn't want to disrupt v4 with more unrelated changes.
Use 'int' in this patch like the rest of the functions that pass
around this enum, and remove the casts. Then when the enum
is made a proper enum-flags with DEF_ENUM_FLAGS_TYPE, you won't
have to remove the casts then, and all functions that need adjustment
will get the same adjustment at the same time. No need for a partial
transition.
>
> void
> -exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
> +try_open_exec_file (const char *exec_file_host, struct inferior *inf,
> + enum symfile_add_flags flags, int from_tty)
Use int.
> {
> - char *exec_file, *full_exec_path = NULL;
> struct cleanup *old_chain;
> struct gdb_exception prev_err = exception_none;
>
> - /* Do nothing if we already have an executable filename. */
> - exec_file = (char *) get_exec_file (0);
> - if (exec_file != NULL)
> - return;
> -
> - /* Try to determine a filename from the process itself. */
> - exec_file = target_pid_to_exec_file (pid);
> - if (exec_file == NULL)
> - {
> - warning (_("No executable has been specified and target does not "
> - "support\n"
> - "determining executable automatically. "
> - "Try using the \"file\" command."));
> - return;
> - }
> -
> - /* If gdb_sysroot is not empty and the discovered filename
> - is absolute then prefix the filename with gdb_sysroot. */
> - if (*gdb_sysroot != '\0' && IS_ABSOLUTE_PATH (exec_file))
> - {
> - full_exec_path = exec_file_find (exec_file, NULL);
> - if (full_exec_path == NULL)
> - return;
> - }
> - else
> - {
> - /* It's possible we don't have a full path, but rather just a
> - filename. Some targets, such as HP-UX, don't provide the
> - full path, sigh.
> -
> - Attempt to qualify the filename against the source path.
> - (If that fails, we'll just fall back on the original
> - filename. Not much more we can do...) */
> - if (!source_full_path_of (exec_file, &full_exec_path))
> - full_exec_path = xstrdup (exec_file);
> - }
> -
> - old_chain = make_cleanup (xfree, full_exec_path);
> - make_cleanup (free_current_contents, &prev_err.message);
> + old_chain = make_cleanup (free_current_contents, &prev_err.message);
>
> /* exec_file_attach and symbol_file_add_main may throw an error if the file
> cannot be opened either locally or remotely.
> @@ -217,7 +160,9 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
> errors/exceptions in the following code. */
> TRY
> {
> - exec_file_attach (full_exec_path, from_tty);
> + /* We must do this step even if exec_file_host is NULL, so that
> + exec_file_attach will clear state. */
> + exec_file_attach (exec_file_host, from_tty);
> }
> CATCH (err, RETURN_MASK_ERROR)
> {
> @@ -232,20 +177,64 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
> }
> END_CATCH
>
> - TRY
> + if (exec_file_host != NULL)
> {
> - if (defer_bp_reset)
> - current_inferior ()->symfile_flags |= SYMFILE_DEFER_BP_RESET;
> - symbol_file_add_main (full_exec_path, from_tty);
> + /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
> + (Position Independent Executable) main symbol file will get applied by
> + solib_create_inferior_hook below. breakpoint_re_set would fail to insert
> + the breakpoints with the zero displacement. */
> + TRY
I don't understand how it makes sense to this comment _here_.
Move this to where the flag is first passed down. Note how there's
no "solib_create_inferior_hook below" at all here. This function
just passes down the "flags" argument unmodified.
> + {
> + symbol_file_add_main_1 (exec_file_host, from_tty, flags);
> +
> + if ((inf->symfile_flags & SYMFILE_NO_READ) == 0)
> + set_initial_language ();
This is already done by symbol_file_add_main_1..
> + }
> + CATCH (err, RETURN_MASK_ERROR)
> + {
> + if (!exception_print_same (prev_err, err))
> + warning ("%s", err.message);
> + }
> + END_CATCH
> +
> + inf->symfile_flags &= ~SYMFILE_DEFER_BP_RESET;
Before the patch, inf->symfile_flags is enabled/disabled
around the body of the code. But now you pass down
SYMFILE_DEFER_BP_RESET in 'flags'. I think we should
be able to remove this?
> }
> - CATCH (err, RETURN_MASK_ERROR)
> +
> + do_cleanups (old_chain);
> +}
> +
> +/* See gdbcore.h. */
> +
> +void
> +exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
> +{
> + char *exec_file_target, *exec_file_host;
> + struct cleanup *old_chain;
> + symfile_add_flags flags = SYMFILE_MAINLINE;
Use int for now. Start with 0. try_open_exec_file
always adds the SYMFILE_MAINLINE for you.
> +
> + /* Do nothing if we already have an executable filename. */
> + if (get_exec_file (0) != NULL)
> + return;
> +
> + /* Try to determine a filename from the process itself. */
> + exec_file_target = target_pid_to_exec_file (pid);
> + if (exec_file_target == NULL)
> {
> - if (!exception_print_same (prev_err, err))
> - warning ("%s", err.message);
> + warning (_("No executable has been specified and target does not "
> + "support\n"
> + "determining executable automatically. "
> + "Try using the \"file\" command."));
> + return;
> }
> - END_CATCH
> - current_inferior ()->symfile_flags &= ~SYMFILE_DEFER_BP_RESET;
>
> + exec_file_host = exec_file_find (exec_file_target, NULL);
> + old_chain = make_cleanup (xfree, exec_file_host);
> +
> + if (defer_bp_reset)
> + flags = (enum symfile_add_flags) (flags | SYMFILE_DEFER_BP_RESET);
Now with int, you can write:
flags |= SYMFILE_DEFER_BP_RESET;
> gdb_assert (current_program_space == inf->pspace);
>
> - /* That a.out is now the one to use. */
> - exec_file_attach (execd_pathname, 0);
> + /* Attempt to open the exec file. */
> + try_open_exec_file (exec_file_host, inf, (enum symfile_add_flags) 0, 0);
Hmm, doesn't thi lose the SYMFILE_DEFER_BP_RESET described ...
>
> - /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
> - (Position Independent Executable) main symbol file will get applied by
> - solib_create_inferior_hook below. breakpoint_re_set would fail to insert
> - the breakpoints with the zero displacement. */
> -
> - symbol_file_add (execd_pathname,
> - (inf->symfile_flags
> - | SYMFILE_MAINLINE | SYMFILE_DEFER_BP_RESET),
> - NULL, 0);
... here, in the code that you're replacing?
Seems to be that you should leave the comment here, and do:
/* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
(Position Independent Executable) main symbol file will get applied by
solib_create_inferior_hook below. breakpoint_re_set would fail to insert
the breakpoints with the zero displacement. */
try_open_exec_file (exec_file_host, inf, SYMFILE_DEFER_BP_RESET, 0);
Here is where it makes sense to talk about the solib_create_inferior_hook
call below, because there's actually such a call below.
> -
> - if ((inf->symfile_flags & SYMFILE_NO_READ) == 0)
> - set_initial_language ();
> + do_cleanups (old_chain);
>
> /* If the target can specify a description, read it. Must do this
> after flipping to the new executable (because the target supplied
BTW, symbol_file_add_main_1 doesn't actually add "flags" to
its add_flags local, so it seems like we're actually losing
the passed down SYMFILE_DEFER_BP_RESET? Or is there some confusion
regarding symfile_add_flags vs objfile flags here? Oh looks
like there is...
I think I'll go dig up my enum-flags patch, and that will I think
help clear this up a lot.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH, v4] PR 20569, segv in follow_exec
2016-10-25 18:04 ` Pedro Alves
@ 2016-10-25 18:16 ` Luis Machado
2016-10-25 18:20 ` Pedro Alves
0 siblings, 1 reply; 13+ messages in thread
From: Luis Machado @ 2016-10-25 18:16 UTC (permalink / raw)
To: Pedro Alves, gdb-patches; +Cc: sandra
On 10/25/2016 01:04 PM, Pedro Alves wrote:
> On 10/25/2016 06:12 PM, Luis Machado wrote:
>> Adjusted a few more bits.
>>
>> I ended up using the ugly explicit cast for an enum that is being used as a
>> bitfield. I'm guessing we'll want to convert this to the macro-fied
>> template-fied version that is more appropriate to C++?
>
> Yup. I actually have a patch for that in one of my C++ branches
> somewhere.
>
>>
>> If so then i can come up with a follow up patch that does the conversion. I
>> didn't want to disrupt v4 with more unrelated changes.
>
> Use 'int' in this patch like the rest of the functions that pass
> around this enum, and remove the casts. Then when the enum
> is made a proper enum-flags with DEF_ENUM_FLAGS_TYPE, you won't
> have to remove the casts then, and all functions that need adjustment
> will get the same adjustment at the same time. No need for a partial
> transition.
>
>>
>
>> void
>> -exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
>> +try_open_exec_file (const char *exec_file_host, struct inferior *inf,
>> + enum symfile_add_flags flags, int from_tty)
>
> Use int.
>
>> {
>> - char *exec_file, *full_exec_path = NULL;
>> struct cleanup *old_chain;
>> struct gdb_exception prev_err = exception_none;
>>
>> - /* Do nothing if we already have an executable filename. */
>> - exec_file = (char *) get_exec_file (0);
>> - if (exec_file != NULL)
>> - return;
>> -
>> - /* Try to determine a filename from the process itself. */
>> - exec_file = target_pid_to_exec_file (pid);
>> - if (exec_file == NULL)
>> - {
>> - warning (_("No executable has been specified and target does not "
>> - "support\n"
>> - "determining executable automatically. "
>> - "Try using the \"file\" command."));
>> - return;
>> - }
>> -
>> - /* If gdb_sysroot is not empty and the discovered filename
>> - is absolute then prefix the filename with gdb_sysroot. */
>> - if (*gdb_sysroot != '\0' && IS_ABSOLUTE_PATH (exec_file))
>> - {
>> - full_exec_path = exec_file_find (exec_file, NULL);
>> - if (full_exec_path == NULL)
>> - return;
>> - }
>> - else
>> - {
>> - /* It's possible we don't have a full path, but rather just a
>> - filename. Some targets, such as HP-UX, don't provide the
>> - full path, sigh.
>> -
>> - Attempt to qualify the filename against the source path.
>> - (If that fails, we'll just fall back on the original
>> - filename. Not much more we can do...) */
>> - if (!source_full_path_of (exec_file, &full_exec_path))
>> - full_exec_path = xstrdup (exec_file);
>> - }
>> -
>> - old_chain = make_cleanup (xfree, full_exec_path);
>> - make_cleanup (free_current_contents, &prev_err.message);
>> + old_chain = make_cleanup (free_current_contents, &prev_err.message);
>>
>> /* exec_file_attach and symbol_file_add_main may throw an error if the file
>> cannot be opened either locally or remotely.
>> @@ -217,7 +160,9 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
>> errors/exceptions in the following code. */
>> TRY
>> {
>> - exec_file_attach (full_exec_path, from_tty);
>> + /* We must do this step even if exec_file_host is NULL, so that
>> + exec_file_attach will clear state. */
>> + exec_file_attach (exec_file_host, from_tty);
>> }
>> CATCH (err, RETURN_MASK_ERROR)
>> {
>> @@ -232,20 +177,64 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
>> }
>> END_CATCH
>>
>> - TRY
>> + if (exec_file_host != NULL)
>> {
>> - if (defer_bp_reset)
>> - current_inferior ()->symfile_flags |= SYMFILE_DEFER_BP_RESET;
>> - symbol_file_add_main (full_exec_path, from_tty);
>> + /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
>> + (Position Independent Executable) main symbol file will get applied by
>> + solib_create_inferior_hook below. breakpoint_re_set would fail to insert
>> + the breakpoints with the zero displacement. */
>> + TRY
>
> I don't understand how it makes sense to this comment _here_.
> Move this to where the flag is first passed down. Note how there's
> no "solib_create_inferior_hook below" at all here. This function
> just passes down the "flags" argument unmodified.
>
>> + {
>> + symbol_file_add_main_1 (exec_file_host, from_tty, flags);
>> +
>> + if ((inf->symfile_flags & SYMFILE_NO_READ) == 0)
>> + set_initial_language ();
>
> This is already done by symbol_file_add_main_1..
>
>> + }
>> + CATCH (err, RETURN_MASK_ERROR)
>> + {
>> + if (!exception_print_same (prev_err, err))
>> + warning ("%s", err.message);
>> + }
>> + END_CATCH
>> +
>> + inf->symfile_flags &= ~SYMFILE_DEFER_BP_RESET;
>
> Before the patch, inf->symfile_flags is enabled/disabled
> around the body of the code. But now you pass down
> SYMFILE_DEFER_BP_RESET in 'flags'. I think we should
> be able to remove this?
>
>> }
>> - CATCH (err, RETURN_MASK_ERROR)
>> +
>> + do_cleanups (old_chain);
>> +}
>> +
>> +/* See gdbcore.h. */
>> +
>> +void
>> +exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
>> +{
>> + char *exec_file_target, *exec_file_host;
>> + struct cleanup *old_chain;
>> + symfile_add_flags flags = SYMFILE_MAINLINE;
>
> Use int for now. Start with 0. try_open_exec_file
> always adds the SYMFILE_MAINLINE for you.
>
>> +
>> + /* Do nothing if we already have an executable filename. */
>> + if (get_exec_file (0) != NULL)
>> + return;
>> +
>> + /* Try to determine a filename from the process itself. */
>> + exec_file_target = target_pid_to_exec_file (pid);
>> + if (exec_file_target == NULL)
>> {
>> - if (!exception_print_same (prev_err, err))
>> - warning ("%s", err.message);
>> + warning (_("No executable has been specified and target does not "
>> + "support\n"
>> + "determining executable automatically. "
>> + "Try using the \"file\" command."));
>> + return;
>> }
>> - END_CATCH
>> - current_inferior ()->symfile_flags &= ~SYMFILE_DEFER_BP_RESET;
>>
>> + exec_file_host = exec_file_find (exec_file_target, NULL);
>> + old_chain = make_cleanup (xfree, exec_file_host);
>> +
>> + if (defer_bp_reset)
>> + flags = (enum symfile_add_flags) (flags | SYMFILE_DEFER_BP_RESET);
>
> Now with int, you can write:
>
> flags |= SYMFILE_DEFER_BP_RESET;
>
>> gdb_assert (current_program_space == inf->pspace);
>>
>> - /* That a.out is now the one to use. */
>> - exec_file_attach (execd_pathname, 0);
>> + /* Attempt to open the exec file. */
>> + try_open_exec_file (exec_file_host, inf, (enum symfile_add_flags) 0, 0);
>
> Hmm, doesn't thi lose the SYMFILE_DEFER_BP_RESET described ...
>
>>
>> - /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
>> - (Position Independent Executable) main symbol file will get applied by
>> - solib_create_inferior_hook below. breakpoint_re_set would fail to insert
>> - the breakpoints with the zero displacement. */
>> -
>> - symbol_file_add (execd_pathname,
>> - (inf->symfile_flags
>> - | SYMFILE_MAINLINE | SYMFILE_DEFER_BP_RESET),
>> - NULL, 0);
>
> ... here, in the code that you're replacing?
>
> Seems to be that you should leave the comment here, and do:
>
> /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
> (Position Independent Executable) main symbol file will get applied by
> solib_create_inferior_hook below. breakpoint_re_set would fail to insert
> the breakpoints with the zero displacement. */
> try_open_exec_file (exec_file_host, inf, SYMFILE_DEFER_BP_RESET, 0);
>
> Here is where it makes sense to talk about the solib_create_inferior_hook
> call below, because there's actually such a call below.
>
>> -
>> - if ((inf->symfile_flags & SYMFILE_NO_READ) == 0)
>> - set_initial_language ();
>> + do_cleanups (old_chain);
>>
>> /* If the target can specify a description, read it. Must do this
>> after flipping to the new executable (because the target supplied
>
> BTW, symbol_file_add_main_1 doesn't actually add "flags" to
> its add_flags local, so it seems like we're actually losing
> the passed down SYMFILE_DEFER_BP_RESET? Or is there some confusion
> regarding symfile_add_flags vs objfile flags here? Oh looks
> like there is...
>
> I think I'll go dig up my enum-flags patch, and that will I think
> help clear this up a lot.
Before i go ahead and adjust this even more, what's your plan and ETA
for the above? This is disturbing more code as we try to consolidade
slightly different functions into a single one in order to make things a
bit more clean. But i'm afraid this is besides the point of the original
patch itself?
I just want to understand what's the end goal, because the scope seems
to be changing slightly with each iteration. :-)
Thanks,
Luis
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH, v4] PR 20569, segv in follow_exec
2016-10-25 18:16 ` Luis Machado
@ 2016-10-25 18:20 ` Pedro Alves
2016-10-25 18:31 ` Luis Machado
0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2016-10-25 18:20 UTC (permalink / raw)
To: Luis Machado, gdb-patches; +Cc: sandra
On 10/25/2016 07:15 PM, Luis Machado wrote:
> Before i go ahead and adjust this even more, what's your plan and ETA
> for the above?
I'll try to post this today.
> This is disturbing more code as we try to consolidade
> slightly different functions into a single one in order to make things a
> bit more clean. But i'm afraid this is besides the point of the original
> patch itself?
>
> I just want to understand what's the end goal, because the scope seems
> to be changing slightly with each iteration. :-)
No, the scope has not changed at all. Your original version duplicated
a large chunk of code, and then the attempt to refactor things did it
incorrectly. Still the same scope, but the patch as is, is buggy.
Between accepted duplicated code, and fixing the patch, I take the latter.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH, v4] PR 20569, segv in follow_exec
2016-10-25 18:20 ` Pedro Alves
@ 2016-10-25 18:31 ` Luis Machado
2016-10-25 19:41 ` Luis Machado
0 siblings, 1 reply; 13+ messages in thread
From: Luis Machado @ 2016-10-25 18:31 UTC (permalink / raw)
To: Pedro Alves, gdb-patches; +Cc: sandra
On 10/25/2016 01:20 PM, Pedro Alves wrote:
> On 10/25/2016 07:15 PM, Luis Machado wrote:
>
>> Before i go ahead and adjust this even more, what's your plan and ETA
>> for the above?
>
> I'll try to post this today.
>
Sounds good.
>> This is disturbing more code as we try to consolidade
>> slightly different functions into a single one in order to make things a
>> bit more clean. But i'm afraid this is besides the point of the original
>> patch itself?
>>
>> I just want to understand what's the end goal, because the scope seems
>> to be changing slightly with each iteration. :-)
>
> No, the scope has not changed at all. Your original version duplicated
> a large chunk of code, and then the attempt to refactor things did it
> incorrectly. Still the same scope, but the patch as is, is buggy.
>
> Between accepted duplicated code, and fixing the patch, I take the latter.
Fine by me.
Thanks,
Luis
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH, v4] PR 20569, segv in follow_exec
2016-10-25 18:31 ` Luis Machado
@ 2016-10-25 19:41 ` Luis Machado
2016-10-25 19:46 ` Pedro Alves
0 siblings, 1 reply; 13+ messages in thread
From: Luis Machado @ 2016-10-25 19:41 UTC (permalink / raw)
To: Pedro Alves, gdb-patches; +Cc: sandra
On 10/25/2016 01:30 PM, Luis Machado wrote:
> On 10/25/2016 01:20 PM, Pedro Alves wrote:
>> On 10/25/2016 07:15 PM, Luis Machado wrote:
>>
>>> Before i go ahead and adjust this even more, what's your plan and ETA
>>> for the above?
>>
>> I'll try to post this today.
>>
>
> Sounds good.
>
>>> This is disturbing more code as we try to consolidade
>>> slightly different functions into a single one in order to make things a
>>> bit more clean. But i'm afraid this is besides the point of the original
>>> patch itself?
>>>
>>> I just want to understand what's the end goal, because the scope seems
>>> to be changing slightly with each iteration. :-)
>>
>> No, the scope has not changed at all. Your original version duplicated
>> a large chunk of code, and then the attempt to refactor things did it
>> incorrectly. Still the same scope, but the patch as is, is buggy.
>>
>> Between accepted duplicated code, and fixing the patch, I take the
>> latter.
>
> Fine by me.
>
> Thanks,
> Luis
Going back to the flags problem, maybe adding "add_flags" as argument to
symbol_file_add_main_1, just like symbol_file_add, and appending those
flags to the existing flag variable there would make things work?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH, v4] PR 20569, segv in follow_exec
2016-10-25 19:41 ` Luis Machado
@ 2016-10-25 19:46 ` Pedro Alves
2016-10-25 19:50 ` Luis Machado
0 siblings, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2016-10-25 19:46 UTC (permalink / raw)
To: Luis Machado, gdb-patches; +Cc: sandra
On 10/25/2016 08:41 PM, Luis Machado wrote:
> Going back to the flags problem, maybe adding "add_flags" as argument to
> symbol_file_add_main_1, just like symbol_file_add, and appending those
> flags to the existing flag variable there would make things work?
I've now cleaned up that patch, and simply rebasing yours on top
shows what I meant:
src/gdb/exec.c:184:59: error: could not convert ‘flags’ from
‘symfile_add_flags {aka enum_flags<symfile_add_flag>}’ to
‘objfile_flags {aka enum_flags<objfile_flag>}’
symbol_file_add_main_1 (exec_file_host, from_tty, flags);
^
Makefile:1138: recipe for target 'exec.o' failed
I.e., the existing flags parameter to symbol_file_add_main_1
is _not_ a symfile_add_flags. We can't just use it as is.
So yes, I think so. I was just doing that here, this minute.
But I was replacing the from_tty parameter symbol_file_add_main
with a symfile_add_flags instead. Let me do that and send
both patches.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH, v4] PR 20569, segv in follow_exec
2016-10-25 19:46 ` Pedro Alves
@ 2016-10-25 19:50 ` Luis Machado
2016-10-25 23:40 ` [PATCH v5 2/2] " Pedro Alves
2016-10-25 23:40 ` [PATCH v5 1/2] Make symfile_add_flags and objfile->flags strongly typed Pedro Alves
0 siblings, 2 replies; 13+ messages in thread
From: Luis Machado @ 2016-10-25 19:50 UTC (permalink / raw)
To: Pedro Alves, gdb-patches; +Cc: sandra
On 10/25/2016 02:46 PM, Pedro Alves wrote:
> On 10/25/2016 08:41 PM, Luis Machado wrote:
>
>> Going back to the flags problem, maybe adding "add_flags" as argument to
>> symbol_file_add_main_1, just like symbol_file_add, and appending those
>> flags to the existing flag variable there would make things work?
>
> I've now cleaned up that patch, and simply rebasing yours on top
> shows what I meant:
>
> src/gdb/exec.c:184:59: error: could not convert ‘flags’ from
> ‘symfile_add_flags {aka enum_flags<symfile_add_flag>}’ to
> ‘objfile_flags {aka enum_flags<objfile_flag>}’
> symbol_file_add_main_1 (exec_file_host, from_tty, flags);
> ^
Ah yes. Confusing names in those functions.
> Makefile:1138: recipe for target 'exec.o' failed
>
> I.e., the existing flags parameter to symbol_file_add_main_1
> is _not_ a symfile_add_flags. We can't just use it as is.
>
> So yes, I think so. I was just doing that here, this minute.
> But I was replacing the from_tty parameter symbol_file_add_main
> with a symfile_add_flags instead. Let me do that and send
> both patches.
>
Oki!
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 1/2] Make symfile_add_flags and objfile->flags strongly typed
2016-10-25 19:50 ` Luis Machado
2016-10-25 23:40 ` [PATCH v5 2/2] " Pedro Alves
@ 2016-10-25 23:40 ` Pedro Alves
1 sibling, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2016-10-25 23:40 UTC (permalink / raw)
To: gdb-patches; +Cc: Luis Machado, Sandra Loosemore
This makes these flag types be "enum flag" types. The benefit is
making use of C++'s stronger typing -- mixing the flags types by
mistake errors at compile time.
This caught one old bug in symbol_file_add_main_1 already, fixed by
this patch as well:
@@ -1318,7 +1326,7 @@ symbol_file_add_main_1 (const char *args, int from_tty, int flags)
what is frameless. */
reinit_frame_cache ();
- if ((flags & SYMFILE_NO_READ) == 0)
+ if ((add_flags & SYMFILE_NO_READ) == 0)
set_initial_language ();
}
Above, "flags" are objfile flags, not symfile_add_flags. So that was
actually checking for "flag & OBJF_PSYMTABS_READ", which has the same
value as SYMFILE_NO_READ...
I moved the flags definitions to separate files to break circular
dependencies.
Built with --enable-targets=all and tested on x86-64 Fedora 23.
gdb/ChangeLog:
2016-10-25 Pedro Alves <palves@redhat.com>
* coffread.c (coff_symfile_read): Use symfile_add_flags.
* dbxread.c (dbx_symfile_read): Ditto.
* elfread.c (elf_symfile_read): Ditto.
* inferior.h: Include symfile-add-flags.h.
(struct inferior) <symfile_flags>: Now symfile_add_flags.
* machoread.c (macho_add_oso_symfile, macho_symfile_read_all_oso)
(macho_symfile_read, mipscoff_symfile_read): Use
symfile_add_flags.
* objfile-flags.h: New file.
* objfiles.c (allocate_objfile): Use objfile_flags.
* objfiles.h: Include objfile-flags.h.
(struct objfile) <flags>: Now an objfile_flags.
(OBJF_REORDERED, OBJF_SHARED, OBJF_READNOW, OBJF_USERLOADED)
(OBJF_PSYMTABS_READ, OBJF_MAINLINE, OBJF_NOT_FILENAME): Delete.
Converted to an enum-flags in objfile-flags.h.
(allocate_objfile): Use objfile_flags.
* python/py-objfile.c (objfpy_add_separate_debug_file): Remove
unnecessary local.
* solib.c (solib_read_symbols, solib_add)
(reload_shared_libraries_1): Use symfile_add_flags.
* solib.h: Include "symfile-add-flags.h".
(solib_read_symbols): Use symfile_add_flags.
* symfile-add-flags.h: New file.
* symfile-debug.c (debug_sym_read): Use symfile_add_flags.
* symfile-mem.c (symbol_file_add_from_memory): Use
symfile_add_flags.
* symfile.c (read_symbols, syms_from_objfile_1)
(syms_from_objfile, finish_new_objfile): Use symfile_add_flags.
(symbol_file_add_with_addrs): Use symfile_add_flags and
objfile_flags.
(symbol_file_add_separate): Use symfile_add_flags.
(symbol_file_add_from_bfd, symbol_file_add): Use symfile_add_flags
and objfile_flags.
(symbol_file_add_main_1): : Use objfile_flags. Fix add_flags vs
flags confusion.
(symbol_file_command): Use objfile_flags.
(add_symbol_file_command): Use symfile_add_flags and
objfile_flags.
(clear_symtab_users): Use symfile_add_flags.
* symfile.h: Include "symfile-add-flags.h" and "objfile-flags.h".
(struct sym_fns) <sym_read>: Use symfile_add_flags.
(clear_symtab_users): Use symfile_add_flags.
(enum symfile_add_flags): Delete, moved to symfile-add-flags.h and
converted to enum-flags.
(symbol_file_add, symbol_file_add_from_bfd)
(symbol_file_add_separate): Use symfile_add_flags.
* xcoffread.c (xcoff_initial_scan): Use symfile_add_flags.
---
gdb/coffread.c | 2 +-
gdb/dbxread.c | 4 +--
gdb/elfread.c | 2 +-
gdb/inferior.h | 7 ++---
gdb/machoread.c | 7 ++---
gdb/mipsread.c | 2 +-
gdb/objfile-flags.h | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
gdb/objfiles.c | 2 +-
gdb/objfiles.h | 54 ++++---------------------------------
gdb/python/py-objfile.c | 3 +--
gdb/solib.c | 18 ++++++++-----
gdb/solib.h | 4 ++-
gdb/symfile-add-flags.h | 48 +++++++++++++++++++++++++++++++++
gdb/symfile-debug.c | 4 +--
gdb/symfile-mem.c | 7 +++--
gdb/symfile.c | 61 +++++++++++++++++++++++++-----------------
gdb/symfile.h | 36 +++++++------------------
gdb/xcoffread.c | 4 +--
18 files changed, 207 insertions(+), 129 deletions(-)
create mode 100644 gdb/objfile-flags.h
create mode 100644 gdb/symfile-add-flags.h
diff --git a/gdb/coffread.c b/gdb/coffread.c
index 3125fb1..501e901 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -559,7 +559,7 @@ static bfd *symfile_bfd;
/* Read a symbol file, after initialization by coff_symfile_init. */
static void
-coff_symfile_read (struct objfile *objfile, int symfile_flags)
+coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
{
struct coff_symfile_info *info;
bfd *abfd = objfile->obfd;
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index 9e6df69..2230de8 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -283,7 +283,7 @@ static void dbx_symfile_init (struct objfile *);
static void dbx_new_init (struct objfile *);
-static void dbx_symfile_read (struct objfile *, int);
+static void dbx_symfile_read (struct objfile *, symfile_add_flags);
static void dbx_symfile_finish (struct objfile *);
@@ -520,7 +520,7 @@ record_minimal_symbol (minimal_symbol_reader &reader,
hung off the objfile structure. */
static void
-dbx_symfile_read (struct objfile *objfile, int symfile_flags)
+dbx_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
{
bfd *sym_bfd;
int val;
diff --git a/gdb/elfread.c b/gdb/elfread.c
index f5aa55e..7349c83 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -1181,7 +1181,7 @@ elf_read_minimal_symbols (struct objfile *objfile, int symfile_flags,
capability even for files compiled without -g. */
static void
-elf_symfile_read (struct objfile *objfile, int symfile_flags)
+elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
{
bfd *abfd = objfile->obfd;
struct elfinfo ei;
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 54c6f65..1d9b22f 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -43,6 +43,8 @@ struct target_desc_info;
#include "progspace.h"
#include "registry.h"
+#include "symfile-add-flags.h"
+
struct infcall_suspend_state;
struct infcall_control_state;
@@ -388,9 +390,8 @@ struct inferior
LONGEST exit_code;
/* Default flags to pass to the symbol reading functions. These are
- used whenever a new objfile is created. The valid values come
- from enum symfile_add_flags. */
- int symfile_flags;
+ used whenever a new objfile is created. */
+ symfile_add_flags symfile_flags;
/* Info about an inferior's target description (if it's fetched; the
user supplied description's filename, if any; etc.). */
diff --git a/gdb/machoread.c b/gdb/machoread.c
index 7889b00..00f25a4 100644
--- a/gdb/machoread.c
+++ b/gdb/machoread.c
@@ -430,7 +430,8 @@ macho_resolve_oso_sym_with_minsym (struct objfile *main_objfile, asymbol *sym)
static void
macho_add_oso_symfile (oso_el *oso, bfd *abfd, const char *name,
- struct objfile *main_objfile, int symfile_flags)
+ struct objfile *main_objfile,
+ symfile_add_flags symfile_flags)
{
int storage;
int i;
@@ -632,7 +633,7 @@ macho_add_oso_symfile (oso_el *oso, bfd *abfd, const char *name,
static void
macho_symfile_read_all_oso (VEC (oso_el) **oso_vector_ptr,
struct objfile *main_objfile,
- int symfile_flags)
+ symfile_add_flags symfile_flags)
{
int ix;
VEC (oso_el) *vec = *oso_vector_ptr;
@@ -826,7 +827,7 @@ macho_check_dsym (struct objfile *objfile, char **filenamep)
}
static void
-macho_symfile_read (struct objfile *objfile, int symfile_flags)
+macho_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
{
bfd *abfd = objfile->obfd;
long storage_needed;
diff --git a/gdb/mipsread.c b/gdb/mipsread.c
index 39af0d3..d9cf3bd 100644
--- a/gdb/mipsread.c
+++ b/gdb/mipsread.c
@@ -67,7 +67,7 @@ mipscoff_symfile_init (struct objfile *objfile)
/* Read a symbol file from a file. */
static void
-mipscoff_symfile_read (struct objfile *objfile, int symfile_flags)
+mipscoff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
{
bfd *abfd = objfile->obfd;
diff --git a/gdb/objfile-flags.h b/gdb/objfile-flags.h
new file mode 100644
index 0000000..da03918
--- /dev/null
+++ b/gdb/objfile-flags.h
@@ -0,0 +1,71 @@
+/* Definition of objfile flags.
+
+ Copyright (C) 1992-2016 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/>. */
+
+#if !defined (OBJFILE_FLAGS_H)
+#define OBJFILE_FLAGS_H
+
+#include "common/enum-flags.h"
+
+/* Defines for the objfile flags field. Defined in a separate file to
+ break circular header dependencies. */
+
+enum objfile_flag
+ {
+ /* When an object file has its functions reordered (currently
+ Irix-5.2 shared libraries exhibit this behaviour), we will need
+ an expensive algorithm to locate a partial symtab or symtab via
+ an address. To avoid this penalty for normal object files, we
+ use this flag, whose setting is determined upon symbol table
+ read in. */
+ OBJF_REORDERED = 1 << 0, /* Functions are reordered */
+
+ /* Distinguish between an objfile for a shared library and a
+ "vanilla" objfile. This may come from a target's
+ implementation of the solib interface, from add-symbol-file, or
+ any other mechanism that loads dynamic objects. */
+ OBJF_SHARED = 1 << 1, /* From a shared library */
+
+ /* User requested that this objfile be read in it's entirety. */
+ OBJF_READNOW = 1 << 2, /* Immediate full read */
+
+ /* This objfile was created because the user explicitly caused it
+ (e.g., used the add-symbol-file command). This bit offers a
+ way for run_command to remove old objfile entries which are no
+ longer valid (i.e., are associated with an old inferior), but
+ to preserve ones that the user explicitly loaded via the
+ add-symbol-file command. */
+ OBJF_USERLOADED = 1 << 3, /* User loaded */
+
+ /* Set if we have tried to read partial symtabs for this objfile.
+ This is used to allow lazy reading of partial symtabs. */
+ OBJF_PSYMTABS_READ = 1 << 4,
+
+ /* Set if this is the main symbol file (as opposed to symbol file
+ for dynamically loaded code). */
+ OBJF_MAINLINE = 1 << 5,
+
+ /* ORIGINAL_NAME and OBFD->FILENAME correspond to text description
+ unrelated to filesystem names. It can be for example
+ "<image in memory>". */
+ OBJF_NOT_FILENAME = 1 << 6,
+ };
+
+DEF_ENUM_FLAGS_TYPE (enum objfile_flag, objfile_flags);
+
+#endif /* !defined (OBJFILE_FLAGS_H) */
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 6e66a8e..4dc6a0a 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -366,7 +366,7 @@ build_objfile_section_table (struct objfile *objfile)
simply copied through to the new objfile flags member. */
struct objfile *
-allocate_objfile (bfd *abfd, const char *name, int flags)
+allocate_objfile (bfd *abfd, const char *name, objfile_flags flags)
{
struct objfile *objfile;
char *expanded_name;
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 87a974c..dbf38e3 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -22,6 +22,7 @@
#include "hashtab.h"
#include "gdb_obstack.h" /* For obstack internals. */
+#include "objfile-flags.h"
#include "symfile.h" /* For struct psymbol_allocation_list. */
#include "progspace.h"
#include "registry.h"
@@ -288,10 +289,9 @@ struct objfile
CORE_ADDR addr_low;
- /* Some flag bits for this objfile.
- The values are defined by OBJF_*. */
+ /* Some flag bits for this objfile. */
- unsigned short flags;
+ objfile_flags flags;
/* The program space associated with this objfile. */
@@ -444,54 +444,10 @@ struct objfile
htab_t static_links;
};
-/* Defines for the objfile flag word. */
-
-/* When an object file has its functions reordered (currently Irix-5.2
- shared libraries exhibit this behaviour), we will need an expensive
- algorithm to locate a partial symtab or symtab via an address.
- To avoid this penalty for normal object files, we use this flag,
- whose setting is determined upon symbol table read in. */
-
-#define OBJF_REORDERED (1 << 0) /* Functions are reordered */
-
-/* Distinguish between an objfile for a shared library and a "vanilla"
- objfile. This may come from a target's implementation of the solib
- interface, from add-symbol-file, or any other mechanism that loads
- dynamic objects. */
-
-#define OBJF_SHARED (1 << 1) /* From a shared library */
-
-/* User requested that this objfile be read in it's entirety. */
-
-#define OBJF_READNOW (1 << 2) /* Immediate full read */
-
-/* This objfile was created because the user explicitly caused it
- (e.g., used the add-symbol-file command). This bit offers a way
- for run_command to remove old objfile entries which are no longer
- valid (i.e., are associated with an old inferior), but to preserve
- ones that the user explicitly loaded via the add-symbol-file
- command. */
-
-#define OBJF_USERLOADED (1 << 3) /* User loaded */
-
-/* Set if we have tried to read partial symtabs for this objfile.
- This is used to allow lazy reading of partial symtabs. */
-
-#define OBJF_PSYMTABS_READ (1 << 4)
-
-/* Set if this is the main symbol file
- (as opposed to symbol file for dynamically loaded code). */
-
-#define OBJF_MAINLINE (1 << 5)
-
-/* ORIGINAL_NAME and OBFD->FILENAME correspond to text description unrelated to
- filesystem names. It can be for example "<image in memory>". */
-
-#define OBJF_NOT_FILENAME (1 << 6)
-
/* Declarations for functions defined in objfiles.c */
-extern struct objfile *allocate_objfile (bfd *, const char *name, int);
+extern struct objfile *allocate_objfile (bfd *, const char *name,
+ objfile_flags);
extern struct gdbarch *get_objfile_arch (const struct objfile *);
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index 82df4b2..1972fb5 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -437,7 +437,6 @@ objfpy_add_separate_debug_file (PyObject *self, PyObject *args, PyObject *kw)
static char *keywords[] = { "file_name", NULL };
objfile_object *obj = (objfile_object *) self;
const char *file_name;
- int symfile_flags = 0;
OBJFPY_REQUIRE_VALID (obj);
@@ -448,7 +447,7 @@ objfpy_add_separate_debug_file (PyObject *self, PyObject *args, PyObject *kw)
{
bfd *abfd = symfile_bfd_open (file_name);
- symbol_file_add_separate (abfd, file_name, symfile_flags, obj->objfile);
+ symbol_file_add_separate (abfd, file_name, 0, obj->objfile);
}
CATCH (except, RETURN_MASK_ALL)
{
diff --git a/gdb/solib.c b/gdb/solib.c
index b8c2b42..5067191 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -669,7 +669,7 @@ master_so_list (void)
loaded. */
int
-solib_read_symbols (struct so_list *so, int flags)
+solib_read_symbols (struct so_list *so, symfile_add_flags flags)
{
if (so->symbols_loaded)
{
@@ -999,8 +999,10 @@ solib_add (const char *pattern, int from_tty,
{
int any_matches = 0;
int loaded_any_symbols = 0;
- const int flags =
- SYMFILE_DEFER_BP_RESET | (from_tty ? SYMFILE_VERBOSE : 0);
+ symfile_add_flags add_flags = SYMFILE_DEFER_BP_RESET;
+
+ if (from_tty)
+ add_flags |= SYMFILE_VERBOSE;
for (gdb = so_list_head; gdb; gdb = gdb->next)
if (! pattern || re_exec (gdb->so_name))
@@ -1024,7 +1026,7 @@ solib_add (const char *pattern, int from_tty,
printf_unfiltered (_("Symbols already loaded for %s\n"),
gdb->so_name);
}
- else if (solib_read_symbols (gdb, flags))
+ else if (solib_read_symbols (gdb, add_flags))
loaded_any_symbols = 1;
}
}
@@ -1357,8 +1359,10 @@ reload_shared_libraries_1 (int from_tty)
char *filename, *found_pathname = NULL;
bfd *abfd;
int was_loaded = so->symbols_loaded;
- const int flags =
- SYMFILE_DEFER_BP_RESET | (from_tty ? SYMFILE_VERBOSE : 0);
+ symfile_add_flags add_flags = SYMFILE_DEFER_BP_RESET;
+
+ if (from_tty)
+ add_flags |= SYMFILE_VERBOSE;
filename = tilde_expand (so->so_original_name);
make_cleanup (xfree, filename);
@@ -1407,7 +1411,7 @@ reload_shared_libraries_1 (int from_tty)
if (!got_error
&& (auto_solib_add || was_loaded || libpthread_solib_p (so)))
- solib_read_symbols (so, flags);
+ solib_read_symbols (so, add_flags);
}
}
diff --git a/gdb/solib.h b/gdb/solib.h
index 00fd6cb..8116b1d 100644
--- a/gdb/solib.h
+++ b/gdb/solib.h
@@ -26,6 +26,8 @@ struct target_ops;
struct target_so_ops;
struct program_space;
+#include "symfile-add-flags.h"
+
/* Called when we free all symtabs, to free the shared library information
as well. */
@@ -34,7 +36,7 @@ extern void clear_solib (void);
/* Called to add symbols from a shared library to gdb's symbol table. */
extern void solib_add (const char *, int, struct target_ops *, int);
-extern int solib_read_symbols (struct so_list *, int);
+extern int solib_read_symbols (struct so_list *, symfile_add_flags);
/* Function to be called when the inferior starts up, to discover the
names of shared libraries that are dynamically linked, the base
diff --git a/gdb/symfile-add-flags.h b/gdb/symfile-add-flags.h
new file mode 100644
index 0000000..c7f5d79
--- /dev/null
+++ b/gdb/symfile-add-flags.h
@@ -0,0 +1,48 @@
+/* Definition of symfile add flags.
+
+ Copyright (C) 1990-2016 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/>. */
+
+#if !defined (SYMFILE_ADD_FLAGS_H)
+#define SYMFILE_ADD_FLAGS_H
+
+#include "common/enum-flags.h"
+
+/* This enum encodes bit-flags passed as ADD_FLAGS parameter to
+ symbol_file_add, etc. Defined in a separate file to break circular
+ header dependencies. */
+
+enum symfile_add_flag
+ {
+ /* Be chatty about what you are doing. */
+ SYMFILE_VERBOSE = 1 << 1,
+
+ /* This is the main symbol file (as opposed to symbol file for
+ dynamically loaded code). */
+ SYMFILE_MAINLINE = 1 << 2,
+
+ /* Do not call breakpoint_re_set when adding this symbol file. */
+ SYMFILE_DEFER_BP_RESET = 1 << 3,
+
+ /* Do not immediately read symbols for this file. By default,
+ symbols are read when the objfile is created. */
+ SYMFILE_NO_READ = 1 << 4
+ };
+
+DEF_ENUM_FLAGS_TYPE (enum symfile_add_flag, symfile_add_flags);
+
+#endif /* !defined(SYMFILE_ADD_FLAGS_H) */
diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
index 0792b71..a361dab 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -442,14 +442,14 @@ debug_sym_init (struct objfile *objfile)
}
static void
-debug_sym_read (struct objfile *objfile, int symfile_flags)
+debug_sym_read (struct objfile *objfile, symfile_add_flags symfile_flags)
{
const struct debug_sym_fns_data *debug_data
= ((const struct debug_sym_fns_data *)
objfile_data (objfile, symfile_debug_objfile_data_key));
fprintf_filtered (gdb_stdlog, "sf->sym_read (%s, 0x%x)\n",
- objfile_debug_name (objfile), symfile_flags);
+ objfile_debug_name (objfile), (unsigned) symfile_flags);
debug_data->real_sf->sym_read (objfile, symfile_flags);
}
diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index 79739a6..58257b9 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -91,6 +91,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr,
struct section_addr_info *sai;
unsigned int i;
struct cleanup *cleanup;
+ symfile_add_flags add_flags = 0;
if (bfd_get_flavour (templ) != bfd_target_elf_flavour)
error (_("add-symbol-file-from-memory not supported for this target"));
@@ -126,9 +127,11 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr,
}
sai->num_sections = i;
+ if (from_tty)
+ add_flags |= SYMFILE_VERBOSE;
+
objf = symbol_file_add_from_bfd (nbfd, bfd_get_filename (nbfd),
- from_tty ? SYMFILE_VERBOSE : 0,
- sai, OBJF_SHARED, NULL);
+ add_flags, sai, OBJF_SHARED, NULL);
add_target_sections_of_objfile (objf);
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 7eb6cdc..616fef0 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -85,7 +85,8 @@ int readnow_symbol_files; /* Read full symbols immediately. */
static void load_command (char *, int);
-static void symbol_file_add_main_1 (const char *args, int from_tty, int flags);
+static void symbol_file_add_main_1 (const char *args, int from_tty,
+ objfile_flags flags);
static void add_symbol_file_command (char *, int);
@@ -865,7 +866,7 @@ default_symfile_segments (bfd *abfd)
possibly force the partial symbols to be read. */
static void
-read_symbols (struct objfile *objfile, int add_flags)
+read_symbols (struct objfile *objfile, symfile_add_flags add_flags)
{
(*objfile->sf->sym_read) (objfile, add_flags);
objfile->per_bfd->minsyms_read = 1;
@@ -994,7 +995,7 @@ init_entry_point_info (struct objfile *objfile)
static void
syms_from_objfile_1 (struct objfile *objfile,
struct section_addr_info *addrs,
- int add_flags)
+ symfile_add_flags add_flags)
{
struct section_addr_info *local_addr = NULL;
struct cleanup *old_chain;
@@ -1085,7 +1086,7 @@ syms_from_objfile_1 (struct objfile *objfile,
static void
syms_from_objfile (struct objfile *objfile,
struct section_addr_info *addrs,
- int add_flags)
+ symfile_add_flags add_flags)
{
syms_from_objfile_1 (objfile, addrs, add_flags);
init_entry_point_info (objfile);
@@ -1096,7 +1097,7 @@ syms_from_objfile (struct objfile *objfile,
objfile. ADD_FLAGS is a bitmask of enum symfile_add_flags. */
static void
-finish_new_objfile (struct objfile *objfile, int add_flags)
+finish_new_objfile (struct objfile *objfile, symfile_add_flags add_flags)
{
/* If this is the main symbol file we have to clean up all users of the
old main symbol file. Otherwise it is sufficient to fixup all the
@@ -1138,9 +1139,10 @@ finish_new_objfile (struct objfile *objfile, int add_flags)
Upon failure, jumps back to command level (never returns). */
static struct objfile *
-symbol_file_add_with_addrs (bfd *abfd, const char *name, int add_flags,
+symbol_file_add_with_addrs (bfd *abfd, const char *name,
+ symfile_add_flags add_flags,
struct section_addr_info *addrs,
- int flags, struct objfile *parent)
+ objfile_flags flags, struct objfile *parent)
{
struct objfile *objfile;
const int from_tty = add_flags & SYMFILE_VERBOSE;
@@ -1164,8 +1166,9 @@ symbol_file_add_with_addrs (bfd *abfd, const char *name, int add_flags,
&& !query (_("Load new symbol table from \"%s\"? "), name))
error (_("Not confirmed."));
- objfile = allocate_objfile (abfd, name,
- flags | (mainline ? OBJF_MAINLINE : 0));
+ if (mainline)
+ flags |= OBJF_MAINLINE;
+ objfile = allocate_objfile (abfd, name, flags);
if (parent)
add_separate_debug_objfile (objfile, parent);
@@ -1242,7 +1245,8 @@ symbol_file_add_with_addrs (bfd *abfd, const char *name, int add_flags,
see allocate_objfile's definition. */
void
-symbol_file_add_separate (bfd *bfd, const char *name, int symfile_flags,
+symbol_file_add_separate (bfd *bfd, const char *name,
+ symfile_add_flags symfile_flags,
struct objfile *objfile)
{
struct section_addr_info *sap;
@@ -1268,9 +1272,10 @@ symbol_file_add_separate (bfd *bfd, const char *name, int symfile_flags,
See symbol_file_add_with_addrs's comments for details. */
struct objfile *
-symbol_file_add_from_bfd (bfd *abfd, const char *name, int add_flags,
+symbol_file_add_from_bfd (bfd *abfd, const char *name,
+ symfile_add_flags add_flags,
struct section_addr_info *addrs,
- int flags, struct objfile *parent)
+ objfile_flags flags, struct objfile *parent)
{
return symbol_file_add_with_addrs (abfd, name, add_flags, addrs, flags,
parent);
@@ -1280,8 +1285,8 @@ symbol_file_add_from_bfd (bfd *abfd, const char *name, int add_flags,
loaded file. See symbol_file_add_with_addrs's comments for details. */
struct objfile *
-symbol_file_add (const char *name, int add_flags,
- struct section_addr_info *addrs, int flags)
+symbol_file_add (const char *name, symfile_add_flags add_flags,
+ struct section_addr_info *addrs, objfile_flags flags)
{
bfd *bfd = symfile_bfd_open (name);
struct cleanup *cleanup = make_cleanup_bfd_unref (bfd);
@@ -1307,10 +1312,13 @@ symbol_file_add_main (const char *args, int from_tty)
}
static void
-symbol_file_add_main_1 (const char *args, int from_tty, int flags)
+symbol_file_add_main_1 (const char *args, int from_tty, objfile_flags flags)
{
- const int add_flags = (current_inferior ()->symfile_flags
- | SYMFILE_MAINLINE | (from_tty ? SYMFILE_VERBOSE : 0));
+ symfile_add_flags add_flags = (current_inferior ()->symfile_flags
+ | SYMFILE_MAINLINE);
+
+ if (from_tty)
+ add_flags |= SYMFILE_VERBOSE;
symbol_file_add (args, add_flags, NULL, flags);
@@ -1318,7 +1326,7 @@ symbol_file_add_main_1 (const char *args, int from_tty, int flags)
what is frameless. */
reinit_frame_cache ();
- if ((flags & SYMFILE_NO_READ) == 0)
+ if ((add_flags & SYMFILE_NO_READ) == 0)
set_initial_language ();
}
@@ -1646,7 +1654,7 @@ symbol_file_command (char *args, int from_tty)
else
{
char **argv = gdb_buildargv (args);
- int flags = OBJF_USERLOADED;
+ objfile_flags flags = OBJF_USERLOADED;
struct cleanup *cleanups;
char *name = NULL;
@@ -2222,7 +2230,6 @@ add_symbol_file_command (char *args, int from_tty)
{
struct gdbarch *gdbarch = get_current_arch ();
char *filename = NULL;
- int flags = OBJF_USERLOADED | OBJF_SHARED;
char *arg;
int section_index = 0;
int argcnt = 0;
@@ -2232,6 +2239,11 @@ add_symbol_file_command (char *args, int from_tty)
int expecting_sec_addr = 0;
char **argv;
struct objfile *objf;
+ objfile_flags flags = OBJF_USERLOADED | OBJF_SHARED;
+ symfile_add_flags add_flags = 0;
+
+ if (from_tty)
+ add_flags |= SYMFILE_VERBOSE;
struct sect_opt
{
@@ -2357,8 +2369,7 @@ add_symbol_file_command (char *args, int from_tty)
if (from_tty && (!query ("%s", "")))
error (_("Not confirmed."));
- objf = symbol_file_add (filename, from_tty ? SYMFILE_VERBOSE : 0,
- section_addrs, flags);
+ objf = symbol_file_add (filename, add_flags, section_addrs, flags);
add_target_sections_of_objfile (objf);
@@ -2937,11 +2948,11 @@ add_compunit_symtab_to_objfile (struct compunit_symtab *cu)
}
\f
-/* Reset all data structures in gdb which may contain references to symbol
- table data. ADD_FLAGS is a bitmask of enum symfile_add_flags. */
+/* Reset all data structures in gdb which may contain references to
+ symbol table data. */
void
-clear_symtab_users (int add_flags)
+clear_symtab_users (symfile_add_flags add_flags)
{
/* Someday, we should do better than this, by only blowing away
the things that really need to be blown. */
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 32902cb..cd94d85 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -23,6 +23,8 @@
/* This file requires that you first include "bfd.h". */
#include "symtab.h"
#include "probe.h"
+#include "symfile-add-flags.h"
+#include "objfile-flags.h"
/* Opaque declarations. */
struct target_section;
@@ -347,7 +349,7 @@ struct sym_fns
file we are reading. SYMFILE_FLAGS are the flags passed to
symbol_file_add & co. */
- void (*sym_read) (struct objfile *, int);
+ void (*sym_read) (struct objfile *, symfile_add_flags);
/* Read the partial symbols for an objfile. This may be NULL, in which case
gdb has to check other ways if this objfile has any symbols. This may
@@ -436,7 +438,7 @@ extern void add_compunit_symtab_to_objfile (struct compunit_symtab *cu);
extern void add_symtab_fns (enum bfd_flavour flavour, const struct sym_fns *);
-extern void clear_symtab_users (int add_flags);
+extern void clear_symtab_users (symfile_add_flags add_flags);
extern enum language deduce_language_from_filename (const char *);
@@ -445,34 +447,14 @@ extern enum language deduce_language_from_filename (const char *);
function. */
extern void add_filename_language (const char *ext, enum language lang);
-/* This enum encodes bit-flags passed as ADD_FLAGS parameter to
- symbol_file_add, etc. */
+extern struct objfile *symbol_file_add (const char *, symfile_add_flags,
+ struct section_addr_info *, objfile_flags);
-enum symfile_add_flags
- {
- /* Be chatty about what you are doing. */
- SYMFILE_VERBOSE = 1 << 1,
-
- /* This is the main symbol file (as opposed to symbol file for dynamically
- loaded code). */
- SYMFILE_MAINLINE = 1 << 2,
-
- /* Do not call breakpoint_re_set when adding this symbol file. */
- SYMFILE_DEFER_BP_RESET = 1 << 3,
-
- /* Do not immediately read symbols for this file. By default,
- symbols are read when the objfile is created. */
- SYMFILE_NO_READ = 1 << 4
- };
-
-extern struct objfile *symbol_file_add (const char *, int,
- struct section_addr_info *, int);
-
-extern struct objfile *symbol_file_add_from_bfd (bfd *, const char *, int,
+extern struct objfile *symbol_file_add_from_bfd (bfd *, const char *, symfile_add_flags,
struct section_addr_info *,
- int, struct objfile *parent);
+ objfile_flags, struct objfile *parent);
-extern void symbol_file_add_separate (bfd *, const char *, int,
+extern void symbol_file_add_separate (bfd *, const char *, symfile_add_flags,
struct objfile *);
extern char *find_separate_debug_file_by_debuglink (struct objfile *);
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 974152f..90d0753 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -195,7 +195,7 @@ eb_complaint (int arg1)
_("Mismatched .eb symbol ignored starting at symnum %d"), arg1);
}
-static void xcoff_initial_scan (struct objfile *, int);
+static void xcoff_initial_scan (struct objfile *, symfile_add_flags);
static void scan_xcoff_symtab (minimal_symbol_reader &,
struct objfile *);
@@ -2925,7 +2925,7 @@ xcoff_get_toc_offset (struct objfile *objfile)
loaded). */
static void
-xcoff_initial_scan (struct objfile *objfile, int symfile_flags)
+xcoff_initial_scan (struct objfile *objfile, symfile_add_flags symfile_flags)
{
bfd *abfd;
int val;
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 2/2] PR 20569, segv in follow_exec
2016-10-25 19:50 ` Luis Machado
@ 2016-10-25 23:40 ` Pedro Alves
2016-10-26 14:28 ` Luis Machado
2016-10-25 23:40 ` [PATCH v5 1/2] Make symfile_add_flags and objfile->flags strongly typed Pedro Alves
1 sibling, 1 reply; 13+ messages in thread
From: Pedro Alves @ 2016-10-25 23:40 UTC (permalink / raw)
To: gdb-patches; +Cc: Luis Machado, Sandra Loosemore
From: Sandra Loosemore <sandra@codesourcery.com>
Changes in v5:
- Adjusted to enum-flags-fyication.
- Fixed symfile_add_flags confusions throughout. Of note:
- Made symbol_file_add_main take a symfile_add_flags instead of a
'from_tty', and adjusted all callers (and some callers of
callers). No longer exporting symfile_add_flags_1, which expects
an objfile_flags instead.
- current_inferior ()->symfile_flags no longer hacked with
SYMFILE_DEFER_BP_RESET.
- follow_exec uses SYMFILE_DEFER_BP_RESET again.
- Commit log simplified / updated.
Tested on x86_64 Fedora 23, native and gdbserver.
The following testcases make GDB crash whenever an invalid sysroot is
provided, when GDB is unable to find a valid path to the symbol file:
gdb.base/catch-syscall.exp
gdb.base/execl-update-breakpoints.exp
gdb.base/foll-exec-mode.exp
gdb.base/foll-exec.exp
gdb.base/foll-vfork.exp
gdb.base/pie-execl.exp
gdb.multi/bkpt-multi-exec.exp
gdb.python/py-finish-breakpoint.exp
gdb.threads/execl.exp
gdb.threads/non-ldr-exc-1.exp
gdb.threads/non-ldr-exc-2.exp
gdb.threads/non-ldr-exc-3.exp
gdb.threads/non-ldr-exc-4.exp
gdb.threads/thread-execl.exp
The immediate cause of the segv is that follow_exec is passing a NULL
argument (the result of exec_file_find) to strlen.
However, the problem is deeper than that: follow_exec simply isn't
prepared for the case where sysroot translation fails to locate the
new executable. Actually all callers of exec_file_find have bugs due
to confusion between host and target pathnames. This commit attempts
to fix all that.
In terms of the testcases that were formerly segv'ing, GDB now prints
a warning but continues execution of the new program, so that the
tests now mostly FAIL instead. You could argue the FAILs are due to a
legitimate problem with the test environment setting up the sysroot
translation incorrectly.
A new representative test is added which exercises the ne wwarning
code path even with native testing.
gdb/ChangeLog:
2016-10-25 Sandra Loosemore <sandra@codesourcery.com>
Luis Machado <lgustavo@codesourcery.com>
Pedro Alves <palves@redhat.com>
PR gdb/20569
* exceptions.c (exception_print_same): Moved here from exec.c.
* exceptions.h (exception_print_same): Declare.
* exec.h: Include "symfile-add-flags.h".
(try_open_exec_file): New declaration.
* exec.c (exception_print_same): Moved to exceptions.c.
(try_open_exec_file): New function.
(exec_file_locate_attach): Rename exec_file and full_exec_path
variables to avoid confusion between target and host pathnames.
Move pathname processing logic to exec_file_find. Do not return
early if pathname lookup fails; Call try_open_exec_file.
* infrun.c (follow_exec): Split and rename execd_pathname variable
to avoid confusion between target and host pathnames. Warn if
pathname lookup fails. Pass target pathname to
target_follow_exec, not hostpathname. Call try_open_exec_file.
* main.c (symbol_file_add_main_adapter): New function.
(captured_main_1): Use it.
* solib-svr4.c (open_symbol_file_object): Adjust to pass
symfile_add_flags to symbol_file_add_main.
* solib.c (exec_file_find): Incorporate fallback logic for relative
pathnames formerly in exec_file_locate_attach.
* symfile.c (symbol_file_add_main, symbol_file_add_main_1):
Replace 'from_tty' parameter with a symfile_add_file.
(symbol_file_command): Adjust to pass symfile_add_flags to
symbol_file_add_main.
* symfile.h (symbol_file_add_main): Replace 'from_tty' parameter
with a symfile_add_file.
gdb/testsuite/ChangeLog:
2016-10-25 Luis Machado <lgustavo@codesourcery.com>
* gdb.base/exec-invalid-sysroot.exp: New file.
---
gdb/exceptions.c | 18 ++++
gdb/exceptions.h | 3 +
gdb/exec.c | 125 ++++++++++--------------
gdb/exec.h | 8 ++
gdb/inferior.c | 6 +-
gdb/infrun.c | 48 +++++----
gdb/main.c | 18 +++-
gdb/solib-svr4.c | 6 +-
gdb/solib.c | 32 ++++--
gdb/symfile.c | 21 ++--
gdb/symfile.h | 3 +-
gdb/testsuite/gdb.base/exec-invalid-sysroot.exp | 70 +++++++++++++
12 files changed, 239 insertions(+), 119 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index 9a10f66..9919938 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -256,3 +256,21 @@ catch_errors (catch_errors_ftype *func, void *func_args, char *errstring,
return 0;
return val;
}
+
+/* See exceptions.h. */
+
+int
+exception_print_same (struct gdb_exception e1, struct gdb_exception e2)
+{
+ const char *msg1 = e1.message;
+ const char *msg2 = e2.message;
+
+ if (msg1 == NULL)
+ msg1 = "";
+ if (msg2 == NULL)
+ msg2 = "";
+
+ return (e1.reason == e2.reason
+ && e1.error == e2.error
+ && strcmp (msg1, msg2) == 0);
+}
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index 5711c1a..a2d39d7 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -88,4 +88,7 @@ extern int catch_exceptions_with_msg (struct ui_out *uiout,
typedef int (catch_errors_ftype) (void *);
extern int catch_errors (catch_errors_ftype *, void *, char *, return_mask);
+/* Compare two exception objects for print equality. */
+extern int exception_print_same (struct gdb_exception e1,
+ struct gdb_exception e2);
#endif
diff --git a/gdb/exec.c b/gdb/exec.c
index 6e2a296..eeca005 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -136,73 +136,16 @@ exec_file_clear (int from_tty)
printf_unfiltered (_("No executable file now.\n"));
}
-/* Returns non-zero if exceptions E1 and E2 are equal. Returns zero
- otherwise. */
-
-static int
-exception_print_same (struct gdb_exception e1, struct gdb_exception e2)
-{
- const char *msg1 = e1.message;
- const char *msg2 = e2.message;
-
- if (msg1 == NULL)
- msg1 = "";
- if (msg2 == NULL)
- msg2 = "";
-
- return (e1.reason == e2.reason
- && e1.error == e2.error
- && strcmp (msg1, msg2) == 0);
-}
-
-/* See gdbcore.h. */
+/* See exec.h. */
void
-exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
+try_open_exec_file (const char *exec_file_host, struct inferior *inf,
+ symfile_add_flags add_flags)
{
- char *exec_file, *full_exec_path = NULL;
struct cleanup *old_chain;
struct gdb_exception prev_err = exception_none;
- /* Do nothing if we already have an executable filename. */
- exec_file = (char *) get_exec_file (0);
- if (exec_file != NULL)
- return;
-
- /* Try to determine a filename from the process itself. */
- exec_file = target_pid_to_exec_file (pid);
- if (exec_file == NULL)
- {
- warning (_("No executable has been specified and target does not "
- "support\n"
- "determining executable automatically. "
- "Try using the \"file\" command."));
- return;
- }
-
- /* If gdb_sysroot is not empty and the discovered filename
- is absolute then prefix the filename with gdb_sysroot. */
- if (*gdb_sysroot != '\0' && IS_ABSOLUTE_PATH (exec_file))
- {
- full_exec_path = exec_file_find (exec_file, NULL);
- if (full_exec_path == NULL)
- return;
- }
- else
- {
- /* It's possible we don't have a full path, but rather just a
- filename. Some targets, such as HP-UX, don't provide the
- full path, sigh.
-
- Attempt to qualify the filename against the source path.
- (If that fails, we'll just fall back on the original
- filename. Not much more we can do...) */
- if (!source_full_path_of (exec_file, &full_exec_path))
- full_exec_path = xstrdup (exec_file);
- }
-
- old_chain = make_cleanup (xfree, full_exec_path);
- make_cleanup (free_current_contents, &prev_err.message);
+ old_chain = make_cleanup (free_current_contents, &prev_err.message);
/* exec_file_attach and symbol_file_add_main may throw an error if the file
cannot be opened either locally or remotely.
@@ -217,7 +160,9 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
errors/exceptions in the following code. */
TRY
{
- exec_file_attach (full_exec_path, from_tty);
+ /* We must do this step even if exec_file_host is NULL, so that
+ exec_file_attach will clear state. */
+ exec_file_attach (exec_file_host, add_flags & SYMFILE_VERBOSE);
}
CATCH (err, RETURN_MASK_ERROR)
{
@@ -232,20 +177,58 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
}
END_CATCH
- TRY
+ if (exec_file_host != NULL)
{
- if (defer_bp_reset)
- current_inferior ()->symfile_flags |= SYMFILE_DEFER_BP_RESET;
- symbol_file_add_main (full_exec_path, from_tty);
+ TRY
+ {
+ symbol_file_add_main (exec_file_host, add_flags);
+ }
+ CATCH (err, RETURN_MASK_ERROR)
+ {
+ if (!exception_print_same (prev_err, err))
+ warning ("%s", err.message);
+ }
+ END_CATCH
}
- CATCH (err, RETURN_MASK_ERROR)
+
+ do_cleanups (old_chain);
+}
+
+/* See gdbcore.h. */
+
+void
+exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
+{
+ char *exec_file_target, *exec_file_host;
+ struct cleanup *old_chain;
+ symfile_add_flags add_flags = 0;
+
+ /* Do nothing if we already have an executable filename. */
+ if (get_exec_file (0) != NULL)
+ return;
+
+ /* Try to determine a filename from the process itself. */
+ exec_file_target = target_pid_to_exec_file (pid);
+ if (exec_file_target == NULL)
{
- if (!exception_print_same (prev_err, err))
- warning ("%s", err.message);
+ warning (_("No executable has been specified and target does not "
+ "support\n"
+ "determining executable automatically. "
+ "Try using the \"file\" command."));
+ return;
}
- END_CATCH
- current_inferior ()->symfile_flags &= ~SYMFILE_DEFER_BP_RESET;
+ exec_file_host = exec_file_find (exec_file_target, NULL);
+ old_chain = make_cleanup (xfree, exec_file_host);
+
+ if (defer_bp_reset)
+ add_flags |= SYMFILE_DEFER_BP_RESET;
+
+ if (from_tty)
+ add_flags |= SYMFILE_VERBOSE;
+
+ /* Attempt to open the exec file. */
+ try_open_exec_file (exec_file_host, current_inferior (), add_flags);
do_cleanups (old_chain);
}
diff --git a/gdb/exec.h b/gdb/exec.h
index 4044cb7..f50e9ea 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -23,6 +23,7 @@
#include "target.h"
#include "progspace.h"
#include "memrange.h"
+#include "symfile-add-flags.h"
struct target_section;
struct target_ops;
@@ -113,4 +114,11 @@ extern void print_section_info (struct target_section_table *table,
extern void exec_close (void);
+/* Helper function that attempts to open the symbol file at EXEC_FILE_HOST.
+ If successful, it proceeds to add the symbol file as the main symbol file.
+
+ ADD_FLAGS is passed on to the function adding the symbol file. */
+extern void try_open_exec_file (const char *exec_file_host,
+ struct inferior *inf,
+ symfile_add_flags add_flags);
#endif
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 1602483..de9e5ef 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -851,8 +851,12 @@ add_inferior_command (char *args, int from_tty)
int i, copies = 1;
char *exec = NULL;
char **argv;
+ symfile_add_flags add_flags = 0;
struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
+ if (from_tty)
+ add_flags |= SYMFILE_VERBOSE;
+
if (args)
{
argv = gdb_buildargv (args);
@@ -900,7 +904,7 @@ add_inferior_command (char *args, int from_tty)
switch_to_thread (null_ptid);
exec_file_attach (exec, from_tty);
- symbol_file_add_main (exec, from_tty);
+ symbol_file_add_main (exec, add_flags);
}
}
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 00bba16..42a25f1 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1077,15 +1077,17 @@ show_follow_exec_mode_string (struct ui_file *file, int from_tty,
fprintf_filtered (file, _("Follow exec mode is \"%s\".\n"), value);
}
-/* EXECD_PATHNAME is assumed to be non-NULL. */
+/* EXEC_FILE_TARGET is assumed to be non-NULL. */
static void
-follow_exec (ptid_t ptid, char *execd_pathname)
+follow_exec (ptid_t ptid, char *exec_file_target)
{
struct thread_info *th, *tmp;
struct inferior *inf = current_inferior ();
int pid = ptid_get_pid (ptid);
ptid_t process_ptid;
+ char *exec_file_host;
+ struct cleanup *old_chain;
/* This is an exec event that we actually wish to pay attention to.
Refresh our symbol table to the newly exec'd program, remove any
@@ -1155,7 +1157,7 @@ follow_exec (ptid_t ptid, char *execd_pathname)
process_ptid = pid_to_ptid (pid);
printf_unfiltered (_("%s is executing new program: %s\n"),
target_pid_to_str (process_ptid),
- execd_pathname);
+ exec_file_target);
/* We've followed the inferior through an exec. Therefore, the
inferior has essentially been killed & reborn. */
@@ -1164,14 +1166,17 @@ follow_exec (ptid_t ptid, char *execd_pathname)
breakpoint_init_inferior (inf_execd);
- if (*gdb_sysroot != '\0')
- {
- char *name = exec_file_find (execd_pathname, NULL);
+ exec_file_host = exec_file_find (exec_file_target, NULL);
+ old_chain = make_cleanup (xfree, exec_file_host);
- execd_pathname = (char *) alloca (strlen (name) + 1);
- strcpy (execd_pathname, name);
- xfree (name);
- }
+ /* If we were unable to map the executable target pathname onto a host
+ pathname, tell the user that. Otherwise GDB's subsequent behavior
+ is confusing. Maybe it would even be better to stop at this point
+ so that the user can specify a file manually before continuing. */
+ if (exec_file_host == NULL)
+ warning (_("Could not load symbols for executable %s.\n"
+ "Do you need \"set sysroot\"?"),
+ exec_file_target);
/* Reset the shared library package. This ensures that we get a
shlib event when the child reaches "_start", at which point the
@@ -1193,7 +1198,7 @@ follow_exec (ptid_t ptid, char *execd_pathname)
inf = add_inferior_with_spaces ();
inf->pid = pid;
- target_follow_exec (inf, execd_pathname);
+ target_follow_exec (inf, exec_file_target);
set_current_inferior (inf);
set_current_program_space (inf->pspace);
@@ -1212,21 +1217,14 @@ follow_exec (ptid_t ptid, char *execd_pathname)
gdb_assert (current_program_space == inf->pspace);
- /* That a.out is now the one to use. */
- exec_file_attach (execd_pathname, 0);
+ /* Attempt to open the exec file. SYMFILE_DEFER_BP_RESET is used
+ because the proper displacement for a PIE (Position Independent
+ Executable) main symbol file will only be computed by
+ solib_create_inferior_hook below. breakpoint_re_set would fail
+ to insert the breakpoints with the zero displacement. */
+ try_open_exec_file (exec_file_host, inf, SYMFILE_DEFER_BP_RESET);
- /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
- (Position Independent Executable) main symbol file will get applied by
- solib_create_inferior_hook below. breakpoint_re_set would fail to insert
- the breakpoints with the zero displacement. */
-
- symbol_file_add (execd_pathname,
- (inf->symfile_flags
- | SYMFILE_MAINLINE | SYMFILE_DEFER_BP_RESET),
- NULL, 0);
-
- if ((inf->symfile_flags & SYMFILE_NO_READ) == 0)
- set_initial_language ();
+ do_cleanups (old_chain);
/* If the target can specify a description, read it. Must do this
after flipping to the new executable (because the target supplied
diff --git a/gdb/main.c b/gdb/main.c
index 23852e2..ca6a81e 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -413,6 +413,20 @@ catch_command_errors_const (catch_command_errors_const_ftype *command,
return 1;
}
+/* Adapter for symbol_file_add_main that translates 'from_tty' to a
+ symfile_add_flags. */
+
+static void
+symbol_file_add_main_adapter (const char *arg, int from_tty)
+{
+ symfile_add_flags add_flags = 0;
+
+ if (from_tty)
+ add_flags |= SYMFILE_VERBOSE;
+
+ symbol_file_add_main (arg, add_flags);
+}
+
/* Type of this option. */
enum cmdarg_kind
{
@@ -1029,7 +1043,7 @@ captured_main_1 (struct captured_main_args *context)
catch_command_errors returns non-zero on success! */
if (catch_command_errors_const (exec_file_attach, execarg,
!batch_flag))
- catch_command_errors_const (symbol_file_add_main, symarg,
+ catch_command_errors_const (symbol_file_add_main_adapter, symarg,
!batch_flag);
}
else
@@ -1038,7 +1052,7 @@ captured_main_1 (struct captured_main_args *context)
catch_command_errors_const (exec_file_attach, execarg,
!batch_flag);
if (symarg != NULL)
- catch_command_errors_const (symbol_file_add_main, symarg,
+ catch_command_errors_const (symbol_file_add_main_adapter, symarg,
!batch_flag);
}
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 258d7dc..0e18292 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1022,6 +1022,10 @@ open_symbol_file_object (void *from_ttyp)
gdb_byte *l_name_buf = (gdb_byte *) xmalloc (l_name_size);
struct cleanup *cleanups = make_cleanup (xfree, l_name_buf);
struct svr4_info *info = get_svr4_info ();
+ symfile_add_flags add_flags = 0;
+
+ if (from_tty)
+ add_flags |= SYMFILE_VERBOSE;
if (symfile_objfile)
if (!query (_("Attempt to reload symbols from process? ")))
@@ -1071,7 +1075,7 @@ open_symbol_file_object (void *from_ttyp)
}
/* Have a pathname: read the symbol file. */
- symbol_file_add_main (filename, from_tty);
+ symbol_file_add_main (filename, add_flags);
do_cleanups (cleanups);
return 1;
diff --git a/gdb/solib.c b/gdb/solib.c
index 5067191..29b25d5 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -380,21 +380,22 @@ solib_find_1 (char *in_pathname, int *fd, int is_solib)
/* Return the full pathname of the main executable, or NULL if not
found. The returned pathname is malloc'ed and must be freed by
the caller. If FD is non-NULL, *FD is set to either -1 or an open
- file handle for the main executable.
-
- The search algorithm used is described in solib_find_1's comment
- above. */
+ file handle for the main executable. */
char *
exec_file_find (char *in_pathname, int *fd)
{
- char *result = solib_find_1 (in_pathname, fd, 0);
+ char *result;
+ const char *fskind = effective_target_file_system_kind ();
+
+ if (in_pathname == NULL)
+ return NULL;
- if (result == NULL)
+ if (*gdb_sysroot != '\0' && IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname))
{
- const char *fskind = effective_target_file_system_kind ();
+ result = solib_find_1 (in_pathname, fd, 0);
- if (fskind == file_system_kind_dos_based)
+ if (result == NULL && fskind == file_system_kind_dos_based)
{
char *new_pathname;
@@ -405,6 +406,21 @@ exec_file_find (char *in_pathname, int *fd)
result = solib_find_1 (new_pathname, fd, 0);
}
}
+ else
+ {
+ /* It's possible we don't have a full path, but rather just a
+ filename. Some targets, such as HP-UX, don't provide the
+ full path, sigh.
+
+ Attempt to qualify the filename against the source path.
+ (If that fails, we'll just fall back on the original
+ filename. Not much more we can do...) */
+
+ if (!source_full_path_of (in_pathname, &result))
+ result = xstrdup (in_pathname);
+ if (fd != NULL)
+ *fd = -1;
+ }
return result;
}
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 616fef0..f524f56 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -85,7 +85,7 @@ int readnow_symbol_files; /* Read full symbols immediately. */
static void load_command (char *, int);
-static void symbol_file_add_main_1 (const char *args, int from_tty,
+static void symbol_file_add_main_1 (const char *args, symfile_add_flags add_flags,
objfile_flags flags);
static void add_symbol_file_command (char *, int);
@@ -1306,19 +1306,16 @@ symbol_file_add (const char *name, symfile_add_flags add_flags,
command itself. */
void
-symbol_file_add_main (const char *args, int from_tty)
+symbol_file_add_main (const char *args, symfile_add_flags add_flags)
{
- symbol_file_add_main_1 (args, from_tty, 0);
+ symbol_file_add_main_1 (args, add_flags, 0);
}
static void
-symbol_file_add_main_1 (const char *args, int from_tty, objfile_flags flags)
+symbol_file_add_main_1 (const char *args, symfile_add_flags add_flags,
+ objfile_flags flags)
{
- symfile_add_flags add_flags = (current_inferior ()->symfile_flags
- | SYMFILE_MAINLINE);
-
- if (from_tty)
- add_flags |= SYMFILE_VERBOSE;
+ add_flags |= current_inferior ()->symfile_flags | SYMFILE_MAINLINE;
symbol_file_add (args, add_flags, NULL, flags);
@@ -1655,9 +1652,13 @@ symbol_file_command (char *args, int from_tty)
{
char **argv = gdb_buildargv (args);
objfile_flags flags = OBJF_USERLOADED;
+ symfile_add_flags add_flags = 0;
struct cleanup *cleanups;
char *name = NULL;
+ if (from_tty)
+ add_flags |= SYMFILE_VERBOSE;
+
cleanups = make_cleanup_freeargv (argv);
while (*argv != NULL)
{
@@ -1667,7 +1668,7 @@ symbol_file_command (char *args, int from_tty)
error (_("unknown option `%s'"), *argv);
else
{
- symbol_file_add_main_1 (*argv, from_tty, flags);
+ symbol_file_add_main_1 (*argv, add_flags, flags);
name = *argv;
}
diff --git a/gdb/symfile.h b/gdb/symfile.h
index cd94d85..59952cb 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -543,7 +543,8 @@ extern CORE_ADDR overlay_unmapped_address (CORE_ADDR, struct obj_section *);
extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
/* Load symbols from a file. */
-extern void symbol_file_add_main (const char *args, int from_tty);
+extern void symbol_file_add_main (const char *args,
+ symfile_add_flags add_flags);
/* Clear GDB symbol tables. */
extern void symbol_file_clear (int from_tty);
diff --git a/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp b/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
new file mode 100644
index 0000000..9665b5f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
@@ -0,0 +1,70 @@
+# Copyright 1997-2016 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 exercises PR20569. GDB would crash when attempting to follow
+# an exec call when it could not resolve the path to the symbol file.
+# This was the case when an invalid sysroot is provided.
+
+standard_testfile foll-exec.c
+
+global binfile
+set binfile [standard_output_file "foll-exec"]
+set testfile2 "execd-prog"
+set srcfile2 ${testfile2}.c
+set binfile2 [standard_output_file ${testfile2}]
+
+set compile_options debug
+
+# build the first test case
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable $compile_options] != "" } {
+ untested "could not compile test program ${binfile2}"
+ return -1
+}
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $compile_options] != "" } {
+ untested "could not compile test program ${binfile}"
+ return -1
+}
+
+proc do_exec_sysroot_test {} {
+ global binfile srcfile srcfile2 testfile testfile2
+ global gdb_prompt
+
+ gdb_test_no_output "set sysroot /a/path/that/does/not/exist"
+
+ # Start the program running, and stop at main.
+ #
+ if ![runto_main] then {
+ fail "Couldn't run ${testfile}"
+ return
+ }
+
+ # Verify that the system supports "catch exec".
+ gdb_test "catch exec" "Catchpoint \[0-9\]* \\(exec\\)" "insert exec catchpoint"
+ set test "continue to exec catchpoint"
+ gdb_test_multiple "continue" $test {
+ -re ".*Your system does not support this type\r\nof catchpoint.*$gdb_prompt $" {
+ unsupported $test
+ return
+ }
+ -re ".*Could not load symbols for executable.*$gdb_prompt $" {
+ pass $test
+ }
+ }
+}
+
+# Start with a fresh gdb
+clean_restart $binfile
+do_exec_sysroot_test
--
2.5.5
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/2] PR 20569, segv in follow_exec
2016-10-25 23:40 ` [PATCH v5 2/2] " Pedro Alves
@ 2016-10-26 14:28 ` Luis Machado
2016-10-26 16:05 ` Luis Machado
0 siblings, 1 reply; 13+ messages in thread
From: Luis Machado @ 2016-10-26 14:28 UTC (permalink / raw)
To: Pedro Alves, gdb-patches; +Cc: Sandra Loosemore
On 10/25/2016 06:39 PM, Pedro Alves wrote:
> From: Sandra Loosemore <sandra@codesourcery.com>
>
> Changes in v5:
>
> - Adjusted to enum-flags-fyication.
>
> - Fixed symfile_add_flags confusions throughout. Of note:
>
> - Made symbol_file_add_main take a symfile_add_flags instead of a
> 'from_tty', and adjusted all callers (and some callers of
> callers). No longer exporting symfile_add_flags_1, which expects
> an objfile_flags instead.
>
> - current_inferior ()->symfile_flags no longer hacked with
> SYMFILE_DEFER_BP_RESET.
>
> - follow_exec uses SYMFILE_DEFER_BP_RESET again.
>
> - Commit log simplified / updated.
>
> Tested on x86_64 Fedora 23, native and gdbserver.
>
> The following testcases make GDB crash whenever an invalid sysroot is
> provided, when GDB is unable to find a valid path to the symbol file:
>
> gdb.base/catch-syscall.exp
> gdb.base/execl-update-breakpoints.exp
> gdb.base/foll-exec-mode.exp
> gdb.base/foll-exec.exp
> gdb.base/foll-vfork.exp
> gdb.base/pie-execl.exp
> gdb.multi/bkpt-multi-exec.exp
> gdb.python/py-finish-breakpoint.exp
> gdb.threads/execl.exp
> gdb.threads/non-ldr-exc-1.exp
> gdb.threads/non-ldr-exc-2.exp
> gdb.threads/non-ldr-exc-3.exp
> gdb.threads/non-ldr-exc-4.exp
> gdb.threads/thread-execl.exp
>
> The immediate cause of the segv is that follow_exec is passing a NULL
> argument (the result of exec_file_find) to strlen.
>
> However, the problem is deeper than that: follow_exec simply isn't
> prepared for the case where sysroot translation fails to locate the
> new executable. Actually all callers of exec_file_find have bugs due
> to confusion between host and target pathnames. This commit attempts
> to fix all that.
>
> In terms of the testcases that were formerly segv'ing, GDB now prints
> a warning but continues execution of the new program, so that the
> tests now mostly FAIL instead. You could argue the FAILs are due to a
> legitimate problem with the test environment setting up the sysroot
> translation incorrectly.
>
> A new representative test is added which exercises the ne wwarning
> code path even with native testing.
>
> gdb/ChangeLog:
> 2016-10-25 Sandra Loosemore <sandra@codesourcery.com>
> Luis Machado <lgustavo@codesourcery.com>
> Pedro Alves <palves@redhat.com>
>
> PR gdb/20569
> * exceptions.c (exception_print_same): Moved here from exec.c.
> * exceptions.h (exception_print_same): Declare.
> * exec.h: Include "symfile-add-flags.h".
> (try_open_exec_file): New declaration.
> * exec.c (exception_print_same): Moved to exceptions.c.
> (try_open_exec_file): New function.
> (exec_file_locate_attach): Rename exec_file and full_exec_path
> variables to avoid confusion between target and host pathnames.
> Move pathname processing logic to exec_file_find. Do not return
> early if pathname lookup fails; Call try_open_exec_file.
> * infrun.c (follow_exec): Split and rename execd_pathname variable
> to avoid confusion between target and host pathnames. Warn if
> pathname lookup fails. Pass target pathname to
> target_follow_exec, not hostpathname. Call try_open_exec_file.
> * main.c (symbol_file_add_main_adapter): New function.
> (captured_main_1): Use it.
> * solib-svr4.c (open_symbol_file_object): Adjust to pass
> symfile_add_flags to symbol_file_add_main.
> * solib.c (exec_file_find): Incorporate fallback logic for relative
> pathnames formerly in exec_file_locate_attach.
> * symfile.c (symbol_file_add_main, symbol_file_add_main_1):
> Replace 'from_tty' parameter with a symfile_add_file.
> (symbol_file_command): Adjust to pass symfile_add_flags to
> symbol_file_add_main.
> * symfile.h (symbol_file_add_main): Replace 'from_tty' parameter
> with a symfile_add_file.
>
> gdb/testsuite/ChangeLog:
> 2016-10-25 Luis Machado <lgustavo@codesourcery.com>
>
> * gdb.base/exec-invalid-sysroot.exp: New file.
> ---
> gdb/exceptions.c | 18 ++++
> gdb/exceptions.h | 3 +
> gdb/exec.c | 125 ++++++++++--------------
> gdb/exec.h | 8 ++
> gdb/inferior.c | 6 +-
> gdb/infrun.c | 48 +++++----
> gdb/main.c | 18 +++-
> gdb/solib-svr4.c | 6 +-
> gdb/solib.c | 32 ++++--
> gdb/symfile.c | 21 ++--
> gdb/symfile.h | 3 +-
> gdb/testsuite/gdb.base/exec-invalid-sysroot.exp | 70 +++++++++++++
> 12 files changed, 239 insertions(+), 119 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
>
> diff --git a/gdb/exceptions.c b/gdb/exceptions.c
> index 9a10f66..9919938 100644
> --- a/gdb/exceptions.c
> +++ b/gdb/exceptions.c
> @@ -256,3 +256,21 @@ catch_errors (catch_errors_ftype *func, void *func_args, char *errstring,
> return 0;
> return val;
> }
> +
> +/* See exceptions.h. */
> +
> +int
> +exception_print_same (struct gdb_exception e1, struct gdb_exception e2)
> +{
> + const char *msg1 = e1.message;
> + const char *msg2 = e2.message;
> +
> + if (msg1 == NULL)
> + msg1 = "";
> + if (msg2 == NULL)
> + msg2 = "";
> +
> + return (e1.reason == e2.reason
> + && e1.error == e2.error
> + && strcmp (msg1, msg2) == 0);
> +}
> diff --git a/gdb/exceptions.h b/gdb/exceptions.h
> index 5711c1a..a2d39d7 100644
> --- a/gdb/exceptions.h
> +++ b/gdb/exceptions.h
> @@ -88,4 +88,7 @@ extern int catch_exceptions_with_msg (struct ui_out *uiout,
> typedef int (catch_errors_ftype) (void *);
> extern int catch_errors (catch_errors_ftype *, void *, char *, return_mask);
>
> +/* Compare two exception objects for print equality. */
> +extern int exception_print_same (struct gdb_exception e1,
> + struct gdb_exception e2);
> #endif
> diff --git a/gdb/exec.c b/gdb/exec.c
> index 6e2a296..eeca005 100644
> --- a/gdb/exec.c
> +++ b/gdb/exec.c
> @@ -136,73 +136,16 @@ exec_file_clear (int from_tty)
> printf_unfiltered (_("No executable file now.\n"));
> }
>
> -/* Returns non-zero if exceptions E1 and E2 are equal. Returns zero
> - otherwise. */
> -
> -static int
> -exception_print_same (struct gdb_exception e1, struct gdb_exception e2)
> -{
> - const char *msg1 = e1.message;
> - const char *msg2 = e2.message;
> -
> - if (msg1 == NULL)
> - msg1 = "";
> - if (msg2 == NULL)
> - msg2 = "";
> -
> - return (e1.reason == e2.reason
> - && e1.error == e2.error
> - && strcmp (msg1, msg2) == 0);
> -}
> -
> -/* See gdbcore.h. */
> +/* See exec.h. */
>
> void
> -exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
> +try_open_exec_file (const char *exec_file_host, struct inferior *inf,
> + symfile_add_flags add_flags)
> {
> - char *exec_file, *full_exec_path = NULL;
> struct cleanup *old_chain;
> struct gdb_exception prev_err = exception_none;
>
> - /* Do nothing if we already have an executable filename. */
> - exec_file = (char *) get_exec_file (0);
> - if (exec_file != NULL)
> - return;
> -
> - /* Try to determine a filename from the process itself. */
> - exec_file = target_pid_to_exec_file (pid);
> - if (exec_file == NULL)
> - {
> - warning (_("No executable has been specified and target does not "
> - "support\n"
> - "determining executable automatically. "
> - "Try using the \"file\" command."));
> - return;
> - }
> -
> - /* If gdb_sysroot is not empty and the discovered filename
> - is absolute then prefix the filename with gdb_sysroot. */
> - if (*gdb_sysroot != '\0' && IS_ABSOLUTE_PATH (exec_file))
> - {
> - full_exec_path = exec_file_find (exec_file, NULL);
> - if (full_exec_path == NULL)
> - return;
> - }
> - else
> - {
> - /* It's possible we don't have a full path, but rather just a
> - filename. Some targets, such as HP-UX, don't provide the
> - full path, sigh.
> -
> - Attempt to qualify the filename against the source path.
> - (If that fails, we'll just fall back on the original
> - filename. Not much more we can do...) */
> - if (!source_full_path_of (exec_file, &full_exec_path))
> - full_exec_path = xstrdup (exec_file);
> - }
> -
> - old_chain = make_cleanup (xfree, full_exec_path);
> - make_cleanup (free_current_contents, &prev_err.message);
> + old_chain = make_cleanup (free_current_contents, &prev_err.message);
>
> /* exec_file_attach and symbol_file_add_main may throw an error if the file
> cannot be opened either locally or remotely.
> @@ -217,7 +160,9 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
> errors/exceptions in the following code. */
> TRY
> {
> - exec_file_attach (full_exec_path, from_tty);
> + /* We must do this step even if exec_file_host is NULL, so that
> + exec_file_attach will clear state. */
> + exec_file_attach (exec_file_host, add_flags & SYMFILE_VERBOSE);
> }
> CATCH (err, RETURN_MASK_ERROR)
> {
> @@ -232,20 +177,58 @@ exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
> }
> END_CATCH
>
> - TRY
> + if (exec_file_host != NULL)
> {
> - if (defer_bp_reset)
> - current_inferior ()->symfile_flags |= SYMFILE_DEFER_BP_RESET;
> - symbol_file_add_main (full_exec_path, from_tty);
> + TRY
> + {
> + symbol_file_add_main (exec_file_host, add_flags);
> + }
> + CATCH (err, RETURN_MASK_ERROR)
> + {
> + if (!exception_print_same (prev_err, err))
> + warning ("%s", err.message);
> + }
> + END_CATCH
> }
> - CATCH (err, RETURN_MASK_ERROR)
> +
> + do_cleanups (old_chain);
> +}
> +
> +/* See gdbcore.h. */
> +
> +void
> +exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
> +{
> + char *exec_file_target, *exec_file_host;
> + struct cleanup *old_chain;
> + symfile_add_flags add_flags = 0;
> +
> + /* Do nothing if we already have an executable filename. */
> + if (get_exec_file (0) != NULL)
> + return;
> +
> + /* Try to determine a filename from the process itself. */
> + exec_file_target = target_pid_to_exec_file (pid);
> + if (exec_file_target == NULL)
> {
> - if (!exception_print_same (prev_err, err))
> - warning ("%s", err.message);
> + warning (_("No executable has been specified and target does not "
> + "support\n"
> + "determining executable automatically. "
> + "Try using the \"file\" command."));
> + return;
> }
> - END_CATCH
> - current_inferior ()->symfile_flags &= ~SYMFILE_DEFER_BP_RESET;
>
> + exec_file_host = exec_file_find (exec_file_target, NULL);
> + old_chain = make_cleanup (xfree, exec_file_host);
> +
> + if (defer_bp_reset)
> + add_flags |= SYMFILE_DEFER_BP_RESET;
> +
> + if (from_tty)
> + add_flags |= SYMFILE_VERBOSE;
> +
> + /* Attempt to open the exec file. */
> + try_open_exec_file (exec_file_host, current_inferior (), add_flags);
> do_cleanups (old_chain);
> }
>
> diff --git a/gdb/exec.h b/gdb/exec.h
> index 4044cb7..f50e9ea 100644
> --- a/gdb/exec.h
> +++ b/gdb/exec.h
> @@ -23,6 +23,7 @@
> #include "target.h"
> #include "progspace.h"
> #include "memrange.h"
> +#include "symfile-add-flags.h"
>
> struct target_section;
> struct target_ops;
> @@ -113,4 +114,11 @@ extern void print_section_info (struct target_section_table *table,
>
> extern void exec_close (void);
>
> +/* Helper function that attempts to open the symbol file at EXEC_FILE_HOST.
> + If successful, it proceeds to add the symbol file as the main symbol file.
> +
> + ADD_FLAGS is passed on to the function adding the symbol file. */
> +extern void try_open_exec_file (const char *exec_file_host,
> + struct inferior *inf,
> + symfile_add_flags add_flags);
> #endif
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index 1602483..de9e5ef 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -851,8 +851,12 @@ add_inferior_command (char *args, int from_tty)
> int i, copies = 1;
> char *exec = NULL;
> char **argv;
> + symfile_add_flags add_flags = 0;
> struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
>
> + if (from_tty)
> + add_flags |= SYMFILE_VERBOSE;
> +
> if (args)
> {
> argv = gdb_buildargv (args);
> @@ -900,7 +904,7 @@ add_inferior_command (char *args, int from_tty)
> switch_to_thread (null_ptid);
>
> exec_file_attach (exec, from_tty);
> - symbol_file_add_main (exec, from_tty);
> + symbol_file_add_main (exec, add_flags);
> }
> }
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 00bba16..42a25f1 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -1077,15 +1077,17 @@ show_follow_exec_mode_string (struct ui_file *file, int from_tty,
> fprintf_filtered (file, _("Follow exec mode is \"%s\".\n"), value);
> }
>
> -/* EXECD_PATHNAME is assumed to be non-NULL. */
> +/* EXEC_FILE_TARGET is assumed to be non-NULL. */
>
> static void
> -follow_exec (ptid_t ptid, char *execd_pathname)
> +follow_exec (ptid_t ptid, char *exec_file_target)
> {
> struct thread_info *th, *tmp;
> struct inferior *inf = current_inferior ();
> int pid = ptid_get_pid (ptid);
> ptid_t process_ptid;
> + char *exec_file_host;
> + struct cleanup *old_chain;
>
> /* This is an exec event that we actually wish to pay attention to.
> Refresh our symbol table to the newly exec'd program, remove any
> @@ -1155,7 +1157,7 @@ follow_exec (ptid_t ptid, char *execd_pathname)
> process_ptid = pid_to_ptid (pid);
> printf_unfiltered (_("%s is executing new program: %s\n"),
> target_pid_to_str (process_ptid),
> - execd_pathname);
> + exec_file_target);
>
> /* We've followed the inferior through an exec. Therefore, the
> inferior has essentially been killed & reborn. */
> @@ -1164,14 +1166,17 @@ follow_exec (ptid_t ptid, char *execd_pathname)
>
> breakpoint_init_inferior (inf_execd);
>
> - if (*gdb_sysroot != '\0')
> - {
> - char *name = exec_file_find (execd_pathname, NULL);
> + exec_file_host = exec_file_find (exec_file_target, NULL);
> + old_chain = make_cleanup (xfree, exec_file_host);
>
> - execd_pathname = (char *) alloca (strlen (name) + 1);
> - strcpy (execd_pathname, name);
> - xfree (name);
> - }
> + /* If we were unable to map the executable target pathname onto a host
> + pathname, tell the user that. Otherwise GDB's subsequent behavior
> + is confusing. Maybe it would even be better to stop at this point
> + so that the user can specify a file manually before continuing. */
> + if (exec_file_host == NULL)
> + warning (_("Could not load symbols for executable %s.\n"
> + "Do you need \"set sysroot\"?"),
> + exec_file_target);
>
> /* Reset the shared library package. This ensures that we get a
> shlib event when the child reaches "_start", at which point the
> @@ -1193,7 +1198,7 @@ follow_exec (ptid_t ptid, char *execd_pathname)
>
> inf = add_inferior_with_spaces ();
> inf->pid = pid;
> - target_follow_exec (inf, execd_pathname);
> + target_follow_exec (inf, exec_file_target);
>
> set_current_inferior (inf);
> set_current_program_space (inf->pspace);
> @@ -1212,21 +1217,14 @@ follow_exec (ptid_t ptid, char *execd_pathname)
>
> gdb_assert (current_program_space == inf->pspace);
>
> - /* That a.out is now the one to use. */
> - exec_file_attach (execd_pathname, 0);
> + /* Attempt to open the exec file. SYMFILE_DEFER_BP_RESET is used
> + because the proper displacement for a PIE (Position Independent
> + Executable) main symbol file will only be computed by
> + solib_create_inferior_hook below. breakpoint_re_set would fail
> + to insert the breakpoints with the zero displacement. */
> + try_open_exec_file (exec_file_host, inf, SYMFILE_DEFER_BP_RESET);
>
> - /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
> - (Position Independent Executable) main symbol file will get applied by
> - solib_create_inferior_hook below. breakpoint_re_set would fail to insert
> - the breakpoints with the zero displacement. */
> -
> - symbol_file_add (execd_pathname,
> - (inf->symfile_flags
> - | SYMFILE_MAINLINE | SYMFILE_DEFER_BP_RESET),
> - NULL, 0);
> -
> - if ((inf->symfile_flags & SYMFILE_NO_READ) == 0)
> - set_initial_language ();
> + do_cleanups (old_chain);
>
> /* If the target can specify a description, read it. Must do this
> after flipping to the new executable (because the target supplied
> diff --git a/gdb/main.c b/gdb/main.c
> index 23852e2..ca6a81e 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -413,6 +413,20 @@ catch_command_errors_const (catch_command_errors_const_ftype *command,
> return 1;
> }
>
> +/* Adapter for symbol_file_add_main that translates 'from_tty' to a
> + symfile_add_flags. */
> +
> +static void
> +symbol_file_add_main_adapter (const char *arg, int from_tty)
> +{
> + symfile_add_flags add_flags = 0;
> +
> + if (from_tty)
> + add_flags |= SYMFILE_VERBOSE;
> +
> + symbol_file_add_main (arg, add_flags);
> +}
> +
> /* Type of this option. */
> enum cmdarg_kind
> {
> @@ -1029,7 +1043,7 @@ captured_main_1 (struct captured_main_args *context)
> catch_command_errors returns non-zero on success! */
> if (catch_command_errors_const (exec_file_attach, execarg,
> !batch_flag))
> - catch_command_errors_const (symbol_file_add_main, symarg,
> + catch_command_errors_const (symbol_file_add_main_adapter, symarg,
> !batch_flag);
> }
> else
> @@ -1038,7 +1052,7 @@ captured_main_1 (struct captured_main_args *context)
> catch_command_errors_const (exec_file_attach, execarg,
> !batch_flag);
> if (symarg != NULL)
> - catch_command_errors_const (symbol_file_add_main, symarg,
> + catch_command_errors_const (symbol_file_add_main_adapter, symarg,
> !batch_flag);
> }
>
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index 258d7dc..0e18292 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -1022,6 +1022,10 @@ open_symbol_file_object (void *from_ttyp)
> gdb_byte *l_name_buf = (gdb_byte *) xmalloc (l_name_size);
> struct cleanup *cleanups = make_cleanup (xfree, l_name_buf);
> struct svr4_info *info = get_svr4_info ();
> + symfile_add_flags add_flags = 0;
> +
> + if (from_tty)
> + add_flags |= SYMFILE_VERBOSE;
>
> if (symfile_objfile)
> if (!query (_("Attempt to reload symbols from process? ")))
> @@ -1071,7 +1075,7 @@ open_symbol_file_object (void *from_ttyp)
> }
>
> /* Have a pathname: read the symbol file. */
> - symbol_file_add_main (filename, from_tty);
> + symbol_file_add_main (filename, add_flags);
>
> do_cleanups (cleanups);
> return 1;
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 5067191..29b25d5 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -380,21 +380,22 @@ solib_find_1 (char *in_pathname, int *fd, int is_solib)
> /* Return the full pathname of the main executable, or NULL if not
> found. The returned pathname is malloc'ed and must be freed by
> the caller. If FD is non-NULL, *FD is set to either -1 or an open
> - file handle for the main executable.
> -
> - The search algorithm used is described in solib_find_1's comment
> - above. */
> + file handle for the main executable. */
>
> char *
> exec_file_find (char *in_pathname, int *fd)
> {
> - char *result = solib_find_1 (in_pathname, fd, 0);
> + char *result;
> + const char *fskind = effective_target_file_system_kind ();
> +
> + if (in_pathname == NULL)
> + return NULL;
>
> - if (result == NULL)
> + if (*gdb_sysroot != '\0' && IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname))
> {
> - const char *fskind = effective_target_file_system_kind ();
> + result = solib_find_1 (in_pathname, fd, 0);
>
> - if (fskind == file_system_kind_dos_based)
> + if (result == NULL && fskind == file_system_kind_dos_based)
> {
> char *new_pathname;
>
> @@ -405,6 +406,21 @@ exec_file_find (char *in_pathname, int *fd)
> result = solib_find_1 (new_pathname, fd, 0);
> }
> }
> + else
> + {
> + /* It's possible we don't have a full path, but rather just a
> + filename. Some targets, such as HP-UX, don't provide the
> + full path, sigh.
> +
> + Attempt to qualify the filename against the source path.
> + (If that fails, we'll just fall back on the original
> + filename. Not much more we can do...) */
> +
> + if (!source_full_path_of (in_pathname, &result))
> + result = xstrdup (in_pathname);
> + if (fd != NULL)
> + *fd = -1;
> + }
>
> return result;
> }
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 616fef0..f524f56 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -85,7 +85,7 @@ int readnow_symbol_files; /* Read full symbols immediately. */
>
> static void load_command (char *, int);
>
> -static void symbol_file_add_main_1 (const char *args, int from_tty,
> +static void symbol_file_add_main_1 (const char *args, symfile_add_flags add_flags,
> objfile_flags flags);
>
> static void add_symbol_file_command (char *, int);
> @@ -1306,19 +1306,16 @@ symbol_file_add (const char *name, symfile_add_flags add_flags,
> command itself. */
>
> void
> -symbol_file_add_main (const char *args, int from_tty)
> +symbol_file_add_main (const char *args, symfile_add_flags add_flags)
> {
> - symbol_file_add_main_1 (args, from_tty, 0);
> + symbol_file_add_main_1 (args, add_flags, 0);
> }
>
> static void
> -symbol_file_add_main_1 (const char *args, int from_tty, objfile_flags flags)
> +symbol_file_add_main_1 (const char *args, symfile_add_flags add_flags,
> + objfile_flags flags)
> {
> - symfile_add_flags add_flags = (current_inferior ()->symfile_flags
> - | SYMFILE_MAINLINE);
> -
> - if (from_tty)
> - add_flags |= SYMFILE_VERBOSE;
> + add_flags |= current_inferior ()->symfile_flags | SYMFILE_MAINLINE;
>
> symbol_file_add (args, add_flags, NULL, flags);
>
> @@ -1655,9 +1652,13 @@ symbol_file_command (char *args, int from_tty)
> {
> char **argv = gdb_buildargv (args);
> objfile_flags flags = OBJF_USERLOADED;
> + symfile_add_flags add_flags = 0;
> struct cleanup *cleanups;
> char *name = NULL;
>
> + if (from_tty)
> + add_flags |= SYMFILE_VERBOSE;
> +
> cleanups = make_cleanup_freeargv (argv);
> while (*argv != NULL)
> {
> @@ -1667,7 +1668,7 @@ symbol_file_command (char *args, int from_tty)
> error (_("unknown option `%s'"), *argv);
> else
> {
> - symbol_file_add_main_1 (*argv, from_tty, flags);
> + symbol_file_add_main_1 (*argv, add_flags, flags);
> name = *argv;
> }
>
> diff --git a/gdb/symfile.h b/gdb/symfile.h
> index cd94d85..59952cb 100644
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -543,7 +543,8 @@ extern CORE_ADDR overlay_unmapped_address (CORE_ADDR, struct obj_section *);
> extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
>
> /* Load symbols from a file. */
> -extern void symbol_file_add_main (const char *args, int from_tty);
> +extern void symbol_file_add_main (const char *args,
> + symfile_add_flags add_flags);
>
> /* Clear GDB symbol tables. */
> extern void symbol_file_clear (int from_tty);
> diff --git a/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp b/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
> new file mode 100644
> index 0000000..9665b5f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
> @@ -0,0 +1,70 @@
> +# Copyright 1997-2016 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 exercises PR20569. GDB would crash when attempting to follow
> +# an exec call when it could not resolve the path to the symbol file.
> +# This was the case when an invalid sysroot is provided.
> +
> +standard_testfile foll-exec.c
> +
> +global binfile
> +set binfile [standard_output_file "foll-exec"]
> +set testfile2 "execd-prog"
> +set srcfile2 ${testfile2}.c
> +set binfile2 [standard_output_file ${testfile2}]
> +
> +set compile_options debug
> +
> +# build the first test case
> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" executable $compile_options] != "" } {
> + untested "could not compile test program ${binfile2}"
> + return -1
> +}
> +
> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $compile_options] != "" } {
> + untested "could not compile test program ${binfile}"
> + return -1
> +}
> +
> +proc do_exec_sysroot_test {} {
> + global binfile srcfile srcfile2 testfile testfile2
> + global gdb_prompt
> +
> + gdb_test_no_output "set sysroot /a/path/that/does/not/exist"
> +
> + # Start the program running, and stop at main.
> + #
> + if ![runto_main] then {
> + fail "Couldn't run ${testfile}"
> + return
> + }
> +
> + # Verify that the system supports "catch exec".
> + gdb_test "catch exec" "Catchpoint \[0-9\]* \\(exec\\)" "insert exec catchpoint"
> + set test "continue to exec catchpoint"
> + gdb_test_multiple "continue" $test {
> + -re ".*Your system does not support this type\r\nof catchpoint.*$gdb_prompt $" {
> + unsupported $test
> + return
> + }
> + -re ".*Could not load symbols for executable.*$gdb_prompt $" {
> + pass $test
> + }
> + }
> +}
> +
> +# Start with a fresh gdb
> +clean_restart $binfile
> +do_exec_sysroot_test
>
Thanks for fixing the other confusion with the symbol file flags.
This version looks OK to me and matches the fixes i had for a v5.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/2] PR 20569, segv in follow_exec
2016-10-26 14:28 ` Luis Machado
@ 2016-10-26 16:05 ` Luis Machado
2016-10-27 14:09 ` Pedro Alves
0 siblings, 1 reply; 13+ messages in thread
From: Luis Machado @ 2016-10-26 16:05 UTC (permalink / raw)
To: Pedro Alves, gdb-patches; +Cc: Sandra Loosemore
On 10/26/2016 09:28 AM, Luis Machado wrote:
> On 10/25/2016 06:39 PM, Pedro Alves wrote:
>> From: Sandra Loosemore <sandra@codesourcery.com>
>>
>> Changes in v5:
>>
>> - Adjusted to enum-flags-fyication.
>>
>> - Fixed symfile_add_flags confusions throughout. Of note:
>>
>> - Made symbol_file_add_main take a symfile_add_flags instead of a
>> 'from_tty', and adjusted all callers (and some callers of
>> callers). No longer exporting symfile_add_flags_1, which expects
>> an objfile_flags instead.
>>
>> - current_inferior ()->symfile_flags no longer hacked with
>> SYMFILE_DEFER_BP_RESET.
>>
>> - follow_exec uses SYMFILE_DEFER_BP_RESET again.
>>
>> - Commit log simplified / updated.
>>
>> Tested on x86_64 Fedora 23, native and gdbserver.
>>
>> The following testcases make GDB crash whenever an invalid sysroot is
>> provided, when GDB is unable to find a valid path to the symbol file:
>>
>> gdb.base/catch-syscall.exp
>> gdb.base/execl-update-breakpoints.exp
>> gdb.base/foll-exec-mode.exp
>> gdb.base/foll-exec.exp
>> gdb.base/foll-vfork.exp
>> gdb.base/pie-execl.exp
>> gdb.multi/bkpt-multi-exec.exp
>> gdb.python/py-finish-breakpoint.exp
>> gdb.threads/execl.exp
>> gdb.threads/non-ldr-exc-1.exp
>> gdb.threads/non-ldr-exc-2.exp
>> gdb.threads/non-ldr-exc-3.exp
>> gdb.threads/non-ldr-exc-4.exp
>> gdb.threads/thread-execl.exp
>>
>> The immediate cause of the segv is that follow_exec is passing a NULL
>> argument (the result of exec_file_find) to strlen.
>>
>> However, the problem is deeper than that: follow_exec simply isn't
>> prepared for the case where sysroot translation fails to locate the
>> new executable. Actually all callers of exec_file_find have bugs due
>> to confusion between host and target pathnames. This commit attempts
>> to fix all that.
>>
>> In terms of the testcases that were formerly segv'ing, GDB now prints
>> a warning but continues execution of the new program, so that the
>> tests now mostly FAIL instead. You could argue the FAILs are due to a
>> legitimate problem with the test environment setting up the sysroot
>> translation incorrectly.
>>
>> A new representative test is added which exercises the ne wwarning
>> code path even with native testing.
>>
>> gdb/ChangeLog:
>> 2016-10-25 Sandra Loosemore <sandra@codesourcery.com>
>> Luis Machado <lgustavo@codesourcery.com>
>> Pedro Alves <palves@redhat.com>
>>
>> PR gdb/20569
>> * exceptions.c (exception_print_same): Moved here from exec.c.
>> * exceptions.h (exception_print_same): Declare.
>> * exec.h: Include "symfile-add-flags.h".
>> (try_open_exec_file): New declaration.
>> * exec.c (exception_print_same): Moved to exceptions.c.
>> (try_open_exec_file): New function.
>> (exec_file_locate_attach): Rename exec_file and full_exec_path
>> variables to avoid confusion between target and host pathnames.
>> Move pathname processing logic to exec_file_find. Do not return
>> early if pathname lookup fails; Call try_open_exec_file.
>> * infrun.c (follow_exec): Split and rename execd_pathname variable
>> to avoid confusion between target and host pathnames. Warn if
>> pathname lookup fails. Pass target pathname to
>> target_follow_exec, not hostpathname. Call try_open_exec_file.
>> * main.c (symbol_file_add_main_adapter): New function.
>> (captured_main_1): Use it.
>> * solib-svr4.c (open_symbol_file_object): Adjust to pass
>> symfile_add_flags to symbol_file_add_main.
>> * solib.c (exec_file_find): Incorporate fallback logic for relative
>> pathnames formerly in exec_file_locate_attach.
>> * symfile.c (symbol_file_add_main, symbol_file_add_main_1):
>> Replace 'from_tty' parameter with a symfile_add_file.
>> (symbol_file_command): Adjust to pass symfile_add_flags to
>> symbol_file_add_main.
>> * symfile.h (symbol_file_add_main): Replace 'from_tty' parameter
>> with a symfile_add_file.
>>
>> gdb/testsuite/ChangeLog:
>> 2016-10-25 Luis Machado <lgustavo@codesourcery.com>
>>
>> * gdb.base/exec-invalid-sysroot.exp: New file.
>> ---
>> gdb/exceptions.c | 18 ++++
>> gdb/exceptions.h | 3 +
>> gdb/exec.c | 125
>> ++++++++++--------------
>> gdb/exec.h | 8 ++
>> gdb/inferior.c | 6 +-
>> gdb/infrun.c | 48 +++++----
>> gdb/main.c | 18 +++-
>> gdb/solib-svr4.c | 6 +-
>> gdb/solib.c | 32 ++++--
>> gdb/symfile.c | 21 ++--
>> gdb/symfile.h | 3 +-
>> gdb/testsuite/gdb.base/exec-invalid-sysroot.exp | 70 +++++++++++++
>> 12 files changed, 239 insertions(+), 119 deletions(-)
>> create mode 100644 gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
>>
>> diff --git a/gdb/exceptions.c b/gdb/exceptions.c
>> index 9a10f66..9919938 100644
>> --- a/gdb/exceptions.c
>> +++ b/gdb/exceptions.c
>> @@ -256,3 +256,21 @@ catch_errors (catch_errors_ftype *func, void
>> *func_args, char *errstring,
>> return 0;
>> return val;
>> }
>> +
>> +/* See exceptions.h. */
>> +
>> +int
>> +exception_print_same (struct gdb_exception e1, struct gdb_exception e2)
>> +{
>> + const char *msg1 = e1.message;
>> + const char *msg2 = e2.message;
>> +
>> + if (msg1 == NULL)
>> + msg1 = "";
>> + if (msg2 == NULL)
>> + msg2 = "";
>> +
>> + return (e1.reason == e2.reason
>> + && e1.error == e2.error
>> + && strcmp (msg1, msg2) == 0);
>> +}
>> diff --git a/gdb/exceptions.h b/gdb/exceptions.h
>> index 5711c1a..a2d39d7 100644
>> --- a/gdb/exceptions.h
>> +++ b/gdb/exceptions.h
>> @@ -88,4 +88,7 @@ extern int catch_exceptions_with_msg (struct ui_out
>> *uiout,
>> typedef int (catch_errors_ftype) (void *);
>> extern int catch_errors (catch_errors_ftype *, void *, char *,
>> return_mask);
>>
>> +/* Compare two exception objects for print equality. */
>> +extern int exception_print_same (struct gdb_exception e1,
>> + struct gdb_exception e2);
>> #endif
>> diff --git a/gdb/exec.c b/gdb/exec.c
>> index 6e2a296..eeca005 100644
>> --- a/gdb/exec.c
>> +++ b/gdb/exec.c
>> @@ -136,73 +136,16 @@ exec_file_clear (int from_tty)
>> printf_unfiltered (_("No executable file now.\n"));
>> }
>>
>> -/* Returns non-zero if exceptions E1 and E2 are equal. Returns zero
>> - otherwise. */
>> -
>> -static int
>> -exception_print_same (struct gdb_exception e1, struct gdb_exception e2)
>> -{
>> - const char *msg1 = e1.message;
>> - const char *msg2 = e2.message;
>> -
>> - if (msg1 == NULL)
>> - msg1 = "";
>> - if (msg2 == NULL)
>> - msg2 = "";
>> -
>> - return (e1.reason == e2.reason
>> - && e1.error == e2.error
>> - && strcmp (msg1, msg2) == 0);
>> -}
>> -
>> -/* See gdbcore.h. */
>> +/* See exec.h. */
>>
>> void
>> -exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
>> +try_open_exec_file (const char *exec_file_host, struct inferior *inf,
>> + symfile_add_flags add_flags)
>> {
>> - char *exec_file, *full_exec_path = NULL;
>> struct cleanup *old_chain;
>> struct gdb_exception prev_err = exception_none;
>>
>> - /* Do nothing if we already have an executable filename. */
>> - exec_file = (char *) get_exec_file (0);
>> - if (exec_file != NULL)
>> - return;
>> -
>> - /* Try to determine a filename from the process itself. */
>> - exec_file = target_pid_to_exec_file (pid);
>> - if (exec_file == NULL)
>> - {
>> - warning (_("No executable has been specified and target does not "
>> - "support\n"
>> - "determining executable automatically. "
>> - "Try using the \"file\" command."));
>> - return;
>> - }
>> -
>> - /* If gdb_sysroot is not empty and the discovered filename
>> - is absolute then prefix the filename with gdb_sysroot. */
>> - if (*gdb_sysroot != '\0' && IS_ABSOLUTE_PATH (exec_file))
>> - {
>> - full_exec_path = exec_file_find (exec_file, NULL);
>> - if (full_exec_path == NULL)
>> - return;
>> - }
>> - else
>> - {
>> - /* It's possible we don't have a full path, but rather just a
>> - filename. Some targets, such as HP-UX, don't provide the
>> - full path, sigh.
>> -
>> - Attempt to qualify the filename against the source path.
>> - (If that fails, we'll just fall back on the original
>> - filename. Not much more we can do...) */
>> - if (!source_full_path_of (exec_file, &full_exec_path))
>> - full_exec_path = xstrdup (exec_file);
>> - }
>> -
>> - old_chain = make_cleanup (xfree, full_exec_path);
>> - make_cleanup (free_current_contents, &prev_err.message);
>> + old_chain = make_cleanup (free_current_contents, &prev_err.message);
>>
>> /* exec_file_attach and symbol_file_add_main may throw an error if
>> the file
>> cannot be opened either locally or remotely.
>> @@ -217,7 +160,9 @@ exec_file_locate_attach (int pid, int
>> defer_bp_reset, int from_tty)
>> errors/exceptions in the following code. */
>> TRY
>> {
>> - exec_file_attach (full_exec_path, from_tty);
>> + /* We must do this step even if exec_file_host is NULL, so that
>> + exec_file_attach will clear state. */
>> + exec_file_attach (exec_file_host, add_flags & SYMFILE_VERBOSE);
>> }
>> CATCH (err, RETURN_MASK_ERROR)
>> {
>> @@ -232,20 +177,58 @@ exec_file_locate_attach (int pid, int
>> defer_bp_reset, int from_tty)
>> }
>> END_CATCH
>>
>> - TRY
>> + if (exec_file_host != NULL)
>> {
>> - if (defer_bp_reset)
>> - current_inferior ()->symfile_flags |= SYMFILE_DEFER_BP_RESET;
>> - symbol_file_add_main (full_exec_path, from_tty);
>> + TRY
>> + {
>> + symbol_file_add_main (exec_file_host, add_flags);
>> + }
>> + CATCH (err, RETURN_MASK_ERROR)
>> + {
>> + if (!exception_print_same (prev_err, err))
>> + warning ("%s", err.message);
>> + }
>> + END_CATCH
>> }
>> - CATCH (err, RETURN_MASK_ERROR)
>> +
>> + do_cleanups (old_chain);
>> +}
>> +
>> +/* See gdbcore.h. */
>> +
>> +void
>> +exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
>> +{
>> + char *exec_file_target, *exec_file_host;
>> + struct cleanup *old_chain;
>> + symfile_add_flags add_flags = 0;
>> +
>> + /* Do nothing if we already have an executable filename. */
>> + if (get_exec_file (0) != NULL)
>> + return;
>> +
>> + /* Try to determine a filename from the process itself. */
>> + exec_file_target = target_pid_to_exec_file (pid);
>> + if (exec_file_target == NULL)
>> {
>> - if (!exception_print_same (prev_err, err))
>> - warning ("%s", err.message);
>> + warning (_("No executable has been specified and target does not "
>> + "support\n"
>> + "determining executable automatically. "
>> + "Try using the \"file\" command."));
>> + return;
>> }
>> - END_CATCH
>> - current_inferior ()->symfile_flags &= ~SYMFILE_DEFER_BP_RESET;
>>
>> + exec_file_host = exec_file_find (exec_file_target, NULL);
>> + old_chain = make_cleanup (xfree, exec_file_host);
>> +
>> + if (defer_bp_reset)
>> + add_flags |= SYMFILE_DEFER_BP_RESET;
>> +
>> + if (from_tty)
>> + add_flags |= SYMFILE_VERBOSE;
>> +
>> + /* Attempt to open the exec file. */
>> + try_open_exec_file (exec_file_host, current_inferior (), add_flags);
>> do_cleanups (old_chain);
>> }
>>
>> diff --git a/gdb/exec.h b/gdb/exec.h
>> index 4044cb7..f50e9ea 100644
>> --- a/gdb/exec.h
>> +++ b/gdb/exec.h
>> @@ -23,6 +23,7 @@
>> #include "target.h"
>> #include "progspace.h"
>> #include "memrange.h"
>> +#include "symfile-add-flags.h"
>>
>> struct target_section;
>> struct target_ops;
>> @@ -113,4 +114,11 @@ extern void print_section_info (struct
>> target_section_table *table,
>>
>> extern void exec_close (void);
>>
>> +/* Helper function that attempts to open the symbol file at
>> EXEC_FILE_HOST.
>> + If successful, it proceeds to add the symbol file as the main
>> symbol file.
>> +
>> + ADD_FLAGS is passed on to the function adding the symbol file. */
>> +extern void try_open_exec_file (const char *exec_file_host,
>> + struct inferior *inf,
>> + symfile_add_flags add_flags);
>> #endif
>> diff --git a/gdb/inferior.c b/gdb/inferior.c
>> index 1602483..de9e5ef 100644
>> --- a/gdb/inferior.c
>> +++ b/gdb/inferior.c
>> @@ -851,8 +851,12 @@ add_inferior_command (char *args, int from_tty)
>> int i, copies = 1;
>> char *exec = NULL;
>> char **argv;
>> + symfile_add_flags add_flags = 0;
>> struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
>>
>> + if (from_tty)
>> + add_flags |= SYMFILE_VERBOSE;
>> +
>> if (args)
>> {
>> argv = gdb_buildargv (args);
>> @@ -900,7 +904,7 @@ add_inferior_command (char *args, int from_tty)
>> switch_to_thread (null_ptid);
>>
>> exec_file_attach (exec, from_tty);
>> - symbol_file_add_main (exec, from_tty);
>> + symbol_file_add_main (exec, add_flags);
>> }
>> }
>>
>> diff --git a/gdb/infrun.c b/gdb/infrun.c
>> index 00bba16..42a25f1 100644
>> --- a/gdb/infrun.c
>> +++ b/gdb/infrun.c
>> @@ -1077,15 +1077,17 @@ show_follow_exec_mode_string (struct ui_file
>> *file, int from_tty,
>> fprintf_filtered (file, _("Follow exec mode is \"%s\".\n"), value);
>> }
>>
>> -/* EXECD_PATHNAME is assumed to be non-NULL. */
>> +/* EXEC_FILE_TARGET is assumed to be non-NULL. */
>>
>> static void
>> -follow_exec (ptid_t ptid, char *execd_pathname)
>> +follow_exec (ptid_t ptid, char *exec_file_target)
>> {
>> struct thread_info *th, *tmp;
>> struct inferior *inf = current_inferior ();
>> int pid = ptid_get_pid (ptid);
>> ptid_t process_ptid;
>> + char *exec_file_host;
>> + struct cleanup *old_chain;
>>
>> /* This is an exec event that we actually wish to pay attention to.
>> Refresh our symbol table to the newly exec'd program, remove any
>> @@ -1155,7 +1157,7 @@ follow_exec (ptid_t ptid, char *execd_pathname)
>> process_ptid = pid_to_ptid (pid);
>> printf_unfiltered (_("%s is executing new program: %s\n"),
>> target_pid_to_str (process_ptid),
>> - execd_pathname);
>> + exec_file_target);
>>
>> /* We've followed the inferior through an exec. Therefore, the
>> inferior has essentially been killed & reborn. */
>> @@ -1164,14 +1166,17 @@ follow_exec (ptid_t ptid, char *execd_pathname)
>>
>> breakpoint_init_inferior (inf_execd);
>>
>> - if (*gdb_sysroot != '\0')
>> - {
>> - char *name = exec_file_find (execd_pathname, NULL);
>> + exec_file_host = exec_file_find (exec_file_target, NULL);
>> + old_chain = make_cleanup (xfree, exec_file_host);
>>
>> - execd_pathname = (char *) alloca (strlen (name) + 1);
>> - strcpy (execd_pathname, name);
>> - xfree (name);
>> - }
>> + /* If we were unable to map the executable target pathname onto a host
>> + pathname, tell the user that. Otherwise GDB's subsequent behavior
>> + is confusing. Maybe it would even be better to stop at this point
>> + so that the user can specify a file manually before continuing. */
>> + if (exec_file_host == NULL)
>> + warning (_("Could not load symbols for executable %s.\n"
>> + "Do you need \"set sysroot\"?"),
>> + exec_file_target);
>>
>> /* Reset the shared library package. This ensures that we get a
>> shlib event when the child reaches "_start", at which point the
>> @@ -1193,7 +1198,7 @@ follow_exec (ptid_t ptid, char *execd_pathname)
>>
>> inf = add_inferior_with_spaces ();
>> inf->pid = pid;
>> - target_follow_exec (inf, execd_pathname);
>> + target_follow_exec (inf, exec_file_target);
>>
>> set_current_inferior (inf);
>> set_current_program_space (inf->pspace);
>> @@ -1212,21 +1217,14 @@ follow_exec (ptid_t ptid, char *execd_pathname)
>>
>> gdb_assert (current_program_space == inf->pspace);
>>
>> - /* That a.out is now the one to use. */
>> - exec_file_attach (execd_pathname, 0);
>> + /* Attempt to open the exec file. SYMFILE_DEFER_BP_RESET is used
>> + because the proper displacement for a PIE (Position Independent
>> + Executable) main symbol file will only be computed by
>> + solib_create_inferior_hook below. breakpoint_re_set would fail
>> + to insert the breakpoints with the zero displacement. */
>> + try_open_exec_file (exec_file_host, inf, SYMFILE_DEFER_BP_RESET);
>>
>> - /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
>> - (Position Independent Executable) main symbol file will get
>> applied by
>> - solib_create_inferior_hook below. breakpoint_re_set would fail
>> to insert
>> - the breakpoints with the zero displacement. */
>> -
>> - symbol_file_add (execd_pathname,
>> - (inf->symfile_flags
>> - | SYMFILE_MAINLINE | SYMFILE_DEFER_BP_RESET),
>> - NULL, 0);
>> -
>> - if ((inf->symfile_flags & SYMFILE_NO_READ) == 0)
>> - set_initial_language ();
>> + do_cleanups (old_chain);
>>
>> /* If the target can specify a description, read it. Must do this
>> after flipping to the new executable (because the target supplied
>> diff --git a/gdb/main.c b/gdb/main.c
>> index 23852e2..ca6a81e 100644
>> --- a/gdb/main.c
>> +++ b/gdb/main.c
>> @@ -413,6 +413,20 @@ catch_command_errors_const
>> (catch_command_errors_const_ftype *command,
>> return 1;
>> }
>>
>> +/* Adapter for symbol_file_add_main that translates 'from_tty' to a
>> + symfile_add_flags. */
>> +
>> +static void
>> +symbol_file_add_main_adapter (const char *arg, int from_tty)
>> +{
>> + symfile_add_flags add_flags = 0;
>> +
>> + if (from_tty)
>> + add_flags |= SYMFILE_VERBOSE;
>> +
>> + symbol_file_add_main (arg, add_flags);
>> +}
>> +
>> /* Type of this option. */
>> enum cmdarg_kind
>> {
>> @@ -1029,7 +1043,7 @@ captured_main_1 (struct captured_main_args
>> *context)
>> catch_command_errors returns non-zero on success! */
>> if (catch_command_errors_const (exec_file_attach, execarg,
>> !batch_flag))
>> - catch_command_errors_const (symbol_file_add_main, symarg,
>> + catch_command_errors_const (symbol_file_add_main_adapter, symarg,
>> !batch_flag);
>> }
>> else
>> @@ -1038,7 +1052,7 @@ captured_main_1 (struct captured_main_args
>> *context)
>> catch_command_errors_const (exec_file_attach, execarg,
>> !batch_flag);
>> if (symarg != NULL)
>> - catch_command_errors_const (symbol_file_add_main, symarg,
>> + catch_command_errors_const (symbol_file_add_main_adapter, symarg,
>> !batch_flag);
>> }
>>
>> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
>> index 258d7dc..0e18292 100644
>> --- a/gdb/solib-svr4.c
>> +++ b/gdb/solib-svr4.c
>> @@ -1022,6 +1022,10 @@ open_symbol_file_object (void *from_ttyp)
>> gdb_byte *l_name_buf = (gdb_byte *) xmalloc (l_name_size);
>> struct cleanup *cleanups = make_cleanup (xfree, l_name_buf);
>> struct svr4_info *info = get_svr4_info ();
>> + symfile_add_flags add_flags = 0;
>> +
>> + if (from_tty)
>> + add_flags |= SYMFILE_VERBOSE;
>>
>> if (symfile_objfile)
>> if (!query (_("Attempt to reload symbols from process? ")))
>> @@ -1071,7 +1075,7 @@ open_symbol_file_object (void *from_ttyp)
>> }
>>
>> /* Have a pathname: read the symbol file. */
>> - symbol_file_add_main (filename, from_tty);
>> + symbol_file_add_main (filename, add_flags);
>>
>> do_cleanups (cleanups);
>> return 1;
>> diff --git a/gdb/solib.c b/gdb/solib.c
>> index 5067191..29b25d5 100644
>> --- a/gdb/solib.c
>> +++ b/gdb/solib.c
>> @@ -380,21 +380,22 @@ solib_find_1 (char *in_pathname, int *fd, int
>> is_solib)
>> /* Return the full pathname of the main executable, or NULL if not
>> found. The returned pathname is malloc'ed and must be freed by
>> the caller. If FD is non-NULL, *FD is set to either -1 or an open
>> - file handle for the main executable.
>> -
>> - The search algorithm used is described in solib_find_1's comment
>> - above. */
>> + file handle for the main executable. */
>>
>> char *
>> exec_file_find (char *in_pathname, int *fd)
>> {
>> - char *result = solib_find_1 (in_pathname, fd, 0);
>> + char *result;
>> + const char *fskind = effective_target_file_system_kind ();
>> +
>> + if (in_pathname == NULL)
>> + return NULL;
>>
>> - if (result == NULL)
>> + if (*gdb_sysroot != '\0' && IS_TARGET_ABSOLUTE_PATH (fskind,
>> in_pathname))
>> {
>> - const char *fskind = effective_target_file_system_kind ();
>> + result = solib_find_1 (in_pathname, fd, 0);
>>
>> - if (fskind == file_system_kind_dos_based)
>> + if (result == NULL && fskind == file_system_kind_dos_based)
>> {
>> char *new_pathname;
>>
>> @@ -405,6 +406,21 @@ exec_file_find (char *in_pathname, int *fd)
>> result = solib_find_1 (new_pathname, fd, 0);
>> }
>> }
>> + else
>> + {
>> + /* It's possible we don't have a full path, but rather just a
>> + filename. Some targets, such as HP-UX, don't provide the
>> + full path, sigh.
>> +
>> + Attempt to qualify the filename against the source path.
>> + (If that fails, we'll just fall back on the original
>> + filename. Not much more we can do...) */
>> +
>> + if (!source_full_path_of (in_pathname, &result))
>> + result = xstrdup (in_pathname);
>> + if (fd != NULL)
>> + *fd = -1;
>> + }
>>
>> return result;
>> }
>> diff --git a/gdb/symfile.c b/gdb/symfile.c
>> index 616fef0..f524f56 100644
>> --- a/gdb/symfile.c
>> +++ b/gdb/symfile.c
>> @@ -85,7 +85,7 @@ int readnow_symbol_files; /* Read full symbols
>> immediately. */
>>
>> static void load_command (char *, int);
>>
>> -static void symbol_file_add_main_1 (const char *args, int from_tty,
>> +static void symbol_file_add_main_1 (const char *args,
>> symfile_add_flags add_flags,
>> objfile_flags flags);
>>
>> static void add_symbol_file_command (char *, int);
>> @@ -1306,19 +1306,16 @@ symbol_file_add (const char *name,
>> symfile_add_flags add_flags,
>> command itself. */
>>
>> void
>> -symbol_file_add_main (const char *args, int from_tty)
>> +symbol_file_add_main (const char *args, symfile_add_flags add_flags)
>> {
>> - symbol_file_add_main_1 (args, from_tty, 0);
>> + symbol_file_add_main_1 (args, add_flags, 0);
>> }
>>
>> static void
>> -symbol_file_add_main_1 (const char *args, int from_tty, objfile_flags
>> flags)
>> +symbol_file_add_main_1 (const char *args, symfile_add_flags add_flags,
>> + objfile_flags flags)
>> {
>> - symfile_add_flags add_flags = (current_inferior ()->symfile_flags
>> - | SYMFILE_MAINLINE);
>> -
>> - if (from_tty)
>> - add_flags |= SYMFILE_VERBOSE;
>> + add_flags |= current_inferior ()->symfile_flags | SYMFILE_MAINLINE;
>>
>> symbol_file_add (args, add_flags, NULL, flags);
>>
>> @@ -1655,9 +1652,13 @@ symbol_file_command (char *args, int from_tty)
>> {
>> char **argv = gdb_buildargv (args);
>> objfile_flags flags = OBJF_USERLOADED;
>> + symfile_add_flags add_flags = 0;
>> struct cleanup *cleanups;
>> char *name = NULL;
>>
>> + if (from_tty)
>> + add_flags |= SYMFILE_VERBOSE;
>> +
>> cleanups = make_cleanup_freeargv (argv);
>> while (*argv != NULL)
>> {
>> @@ -1667,7 +1668,7 @@ symbol_file_command (char *args, int from_tty)
>> error (_("unknown option `%s'"), *argv);
>> else
>> {
>> - symbol_file_add_main_1 (*argv, from_tty, flags);
>> + symbol_file_add_main_1 (*argv, add_flags, flags);
>> name = *argv;
>> }
>>
>> diff --git a/gdb/symfile.h b/gdb/symfile.h
>> index cd94d85..59952cb 100644
>> --- a/gdb/symfile.h
>> +++ b/gdb/symfile.h
>> @@ -543,7 +543,8 @@ extern CORE_ADDR overlay_unmapped_address
>> (CORE_ADDR, struct obj_section *);
>> extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct
>> obj_section *);
>>
>> /* Load symbols from a file. */
>> -extern void symbol_file_add_main (const char *args, int from_tty);
>> +extern void symbol_file_add_main (const char *args,
>> + symfile_add_flags add_flags);
>>
>> /* Clear GDB symbol tables. */
>> extern void symbol_file_clear (int from_tty);
>> diff --git a/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
>> b/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
>> new file mode 100644
>> index 0000000..9665b5f
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/exec-invalid-sysroot.exp
>> @@ -0,0 +1,70 @@
>> +# Copyright 1997-2016 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 exercises PR20569. GDB would crash when attempting to
>> follow
>> +# an exec call when it could not resolve the path to the symbol file.
>> +# This was the case when an invalid sysroot is provided.
>> +
>> +standard_testfile foll-exec.c
>> +
>> +global binfile
>> +set binfile [standard_output_file "foll-exec"]
On further inspection, i think this violates the testcase cookbook's
rules? It is reusing the foll-exec.c sources, but it should compile to a
unique executable name.
On the other hand, foll-exec.c relies on its own name to do the trick.
It replaces foll-exec with execd-prog.
Is this still a problem now that we have unique directories for each
testcase?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v5 2/2] PR 20569, segv in follow_exec
2016-10-26 16:05 ` Luis Machado
@ 2016-10-27 14:09 ` Pedro Alves
0 siblings, 0 replies; 13+ messages in thread
From: Pedro Alves @ 2016-10-27 14:09 UTC (permalink / raw)
To: Luis Machado, gdb-patches; +Cc: Sandra Loosemore
On 10/26/2016 05:04 PM, Luis Machado wrote:
> On 10/26/2016 09:28 AM, Luis Machado wrote:
>> On 10/25/2016 06:39 PM, Pedro Alves wrote:
>>> +# This test exercises PR20569. GDB would crash when attempting to
>>> follow
>>> +# an exec call when it could not resolve the path to the symbol file.
>>> +# This was the case when an invalid sysroot is provided.
>>> +
>>> +standard_testfile foll-exec.c
>>> +
>>> +global binfile
>>> +set binfile [standard_output_file "foll-exec"]
>
[FYI, I pushed this in already yesterday.]
> On further inspection, i think this violates the testcase cookbook's
> rules? It is reusing the foll-exec.c sources, but it should compile to a
> unique executable name.
>
> On the other hand, foll-exec.c relies on its own name to do the trick.
> It replaces foll-exec with execd-prog.
>
> Is this still a problem now that we have unique directories for each
> testcase?
Yeah, I think it's probably not such a big deal nowadays as it used to be.
The case where it'd maybe still help a little bit is the remote host/target
case. In that case, we'll still download the files to a single remote
directory. We can't do parallel testing in that case, so the downside in
general is that the overwriting may make it a bit harder to debug failing
testcases after the fact. Maybe not that much of a problem, since we have
the separate copies on the host side. We already have this overwriting
on the target side in case we have tests with the same name in
different gdb.foo/ subdirectories, I think:
$ ls testsuite/gdb.*/*.exp | sed 's,testsuite/gdb.[a-z]*/,,g' | sort | uniq -c | sort -nr | head -n 30
3 types.exp
3 print.exp
3 gcore.exp
3 exprs.exp
2 vla-datatypes.exp
2 start.exp
2 solib-symbol.exp
2 solib.exp
2 sizeof.exp
2 range-stepping.exp
2 ptype.exp
2 pr11022.exp
2 pending.exp
2 methods.exp
2 maint.exp
2 logical.exp
2 integers.exp
2 hello.exp
2 exception.exp
2 demangle.exp
2 debug-expr.exp
2 data.exp
2 complex.exp
2 charset.exp
2 callfuncs.exp
2 break.exp
2 backtrace.exp
2 arrayidx.exp
2 annota3.exp
1 xfullpath.exp
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-10-27 14:09 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-25 17:12 [PATCH, v4] PR 20569, segv in follow_exec Luis Machado
2016-10-25 18:04 ` Pedro Alves
2016-10-25 18:16 ` Luis Machado
2016-10-25 18:20 ` Pedro Alves
2016-10-25 18:31 ` Luis Machado
2016-10-25 19:41 ` Luis Machado
2016-10-25 19:46 ` Pedro Alves
2016-10-25 19:50 ` Luis Machado
2016-10-25 23:40 ` [PATCH v5 2/2] " Pedro Alves
2016-10-26 14:28 ` Luis Machado
2016-10-26 16:05 ` Luis Machado
2016-10-27 14:09 ` Pedro Alves
2016-10-25 23:40 ` [PATCH v5 1/2] Make symfile_add_flags and objfile->flags strongly typed Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox