Re: [PATCH v5 2/2] PR 20569, segv in follow_exec

[Luis Machado <lgustavo@codesourcery.com>]

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?