[v5,2/2] PR 20569, segv in follow_exec
Commit Message
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
Comments
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.
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?
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
@@ -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);
+}
@@ -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
@@ -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);
}
@@ -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
@@ -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);
}
}
@@ -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
@@ -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);
}
@@ -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;
@@ -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;
}
@@ -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;
}
@@ -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);
new file mode 100644
@@ -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