From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47681 invoked by alias); 26 Oct 2016 14:28:26 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 47575 invoked by uid 89); 26 Oct 2016 14:28:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 26 Oct 2016 14:28:14 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1bzPBc-0001FA-SQ from Luis_Gustavo@mentor.com ; Wed, 26 Oct 2016 07:28:12 -0700 Received: from [172.30.1.187] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Wed, 26 Oct 2016 07:28:09 -0700 Reply-To: Luis Machado Subject: Re: [PATCH v5 2/2] PR 20569, segv in follow_exec References: <1477438784-16583-2-git-send-email-palves@redhat.com> To: Pedro Alves , CC: Sandra Loosemore From: Luis Machado Message-ID: <5de934fd-8ce5-9d97-75c2-61d7769b339d@codesourcery.com> Date: Wed, 26 Oct 2016 14:28:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <1477438784-16583-2-git-send-email-palves@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-04.mgc.mentorg.com (147.34.90.204) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2016-10/txt/msg00719.txt.bz2 On 10/25/2016 06:39 PM, Pedro Alves wrote: > From: Sandra Loosemore > > 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 > Luis Machado > Pedro Alves > > 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 > > * 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 . > + > +# 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.