From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23700 invoked by alias); 16 Aug 2015 20:12:19 -0000 Mailing-List: contact archer-commits-help@sourceware.org; run by ezmlm Sender: Precedence: bulk List-Post: List-Help: List-Subscribe: Received: (qmail 23682 invoked by uid 9674); 16 Aug 2015 20:12:18 -0000 Date: Sun, 16 Aug 2015 20:12:00 -0000 Message-ID: <20150816201218.23654.qmail@sourceware.org> From: jkratoch@sourceware.org To: archer-commits@sourceware.org Subject: [SCM] jankratochvil/gdbserverbuildid: sticky X-Git-Refname: refs/heads/jankratochvil/gdbserverbuildid X-Git-Reftype: branch X-Git-Oldrev: 7a947a6bd868ee8559dca4a2b16b894ea654b8fd X-Git-Newrev: ea8ad7ecbd272bdfaaa28fbef8dbd24cd14caa8a X-SW-Source: 2015-q3/txt/msg00032.txt.bz2 List-Id: The branch, jankratochvil/gdbserverbuildid has been updated discards 7a947a6bd868ee8559dca4a2b16b894ea654b8fd (commit) via ea8ad7ecbd272bdfaaa28fbef8dbd24cd14caa8a (commit) from 7a947a6bd868ee8559dca4a2b16b894ea654b8fd (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email. - Log ----------------------------------------------------------------- commit ea8ad7ecbd272bdfaaa28fbef8dbd24cd14caa8a Author: Jan Kratochvil Date: Sun Aug 16 22:11:19 2015 +0200 sticky Message-ID: <559A7C37.6020501@redhat.com> On 07/03/2015 04:44 PM, Pedro Alves wrote: > (I still suspect that if we reverse the sense of the flag, then > its management ends up being more centralized, as then the > place that sets it is also the place that needs to check it, > instead of doing that in multiple places. But, see below.) It didn't seem fair to impose a subjective preference, so I tried this in order to understand it myself, and I now agree that is really doesn't make much difference, as then we'd have to mark auto-discovered in a few more places that I wasn't originally seeing. There's at least the execd handling in infrun.c, and also spu_symbol_file_add_from_memory. As I was playing with this already, I poked at the other review points I made, and came up with the variant of the patch below. > This function is called while the passed in PID is not the > current inferior. E.g., the remote_add_inferior path. > > Therefore seems to me that these symbol_file_add_main / exec_file_attach > calls can change the symbols of the wrong inferior. ... > I also notice that reread_symbols has an exec_file_attach > call which seems to me results in losing the is_user_supplied > flag if the executable's timestamp changed, at least here: > >> + /* Attempt to open the file that was executed to create this >> + inferior. If the user has explicitly specified executable >> + and/or symbol files then warn the user if their choices do >> + not match. Otherwise, set exec_file and symfile_objfile to >> + the new file. */ >> + exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty); >> + >> + if (exec_file_is_user_supplied) >> { >> reopen_exec_file (); >> reread_symbols (); > > > I also notice that the clone-inferior command ends up with > an exec_file_attach call in clone_program_space. That one > should probably be setting the is_user_supplied flag too, > I'd think. Or at least, copying it from the original. > > At this point, I'm wondering about adding a parameter to > exec_file_attach to force considering (now and in future) > the right value to put in the flag in each case. I tried this too (see patch below). As this requires touching the user-supplied paths anyway, it didn't really matter to tag files user-supplied or auto-discovered, so I reverted back to user-supplied. For symbols, we already have the add_flags, so I added SYMFILE_USER_SUPPLIED instead of a new boolean. > >> @@ -2490,11 +2490,14 @@ attach_command_post_wait (char *args, int from_tty, int async_exec) >> inferior = current_inferior (); >> inferior->control.stop_soon = NO_STOP_QUIETLY; >> >> - /* If no exec file is yet known, try to determine it from the >> - process itself. */ >> - if (get_exec_file (0) == NULL) >> - exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty); >> - else >> + /* Attempt to open the file that was executed to create this >> + inferior. If the user has explicitly specified executable >> + and/or symbol files then warn the user if their choices do >> + not match. Otherwise, set exec_file and symfile_objfile to >> + the new file. */ >> + exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty); >> + >> + if (exec_file_is_user_supplied) >> { >> reopen_exec_file (); >> reread_symbols (); > > It seems to me that we should be able to move these reopen/reread > calls inside exec_file_locate_attach. I did this too. What do you think of the version below? I also tweaked the warnings a bit, to explicitly say "mismatch". This would still need docs/NEWS changes, and a test would be good too. ----------------------------------------------------------------------- Summary of changes: gdb/corefile.c | 2 +- gdb/exec.c | 94 +++++++++++++++++++++++++++++++----------------------- gdb/exec.h | 4 +- gdb/gdbcore.h | 25 ++++++++++---- gdb/infcmd.c | 8 +---- gdb/inferior.c | 9 ++--- gdb/infrun.c | 2 +- gdb/main.c | 56 +++++++++++++++++++++----------- gdb/progspace.c | 5 ++- gdb/progspace.h | 24 +++++++------- gdb/remote.c | 2 +- gdb/solib-svr4.c | 2 +- gdb/symfile.c | 26 +++++++++------ gdb/symfile.h | 9 ++++- 14 files changed, 156 insertions(+), 112 deletions(-) First 500 lines of diff: diff --git a/gdb/corefile.c b/gdb/corefile.c index 5246f71..5d861de 100644 --- a/gdb/corefile.c +++ b/gdb/corefile.c @@ -146,7 +146,7 @@ reopen_exec_file (void) res = stat (filename, &st); if (exec_bfd_mtime && exec_bfd_mtime != st.st_mtime) - exec_file_attach (filename, 0); + exec_file_attach (exec_file_was_user_supplied, filename, 0); else /* If we accessed the file since last opening it, close it now; this stops GDB from holding the executable open after it diff --git a/gdb/exec.c b/gdb/exec.c index 4d0e646..4ec321b 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -78,7 +78,7 @@ static void exec_open (const char *args, int from_tty) { target_preopen (from_tty); - exec_file_attach (args, from_tty); + exec_file_attach (1, args, from_tty); } /* Close and clear exec_bfd. If we end up with no target sections to @@ -102,7 +102,7 @@ exec_close (void) xfree (exec_filename); exec_filename = NULL; - exec_file_is_user_supplied = 0; + exec_file_was_user_supplied = 0; } } @@ -139,54 +139,67 @@ exec_file_clear (int from_tty) /* See gdbcore.h. */ void -exec_file_locate_attach (int pid, int from_tty) +exec_file_and_symbols_resync (struct inferior *inf, int from_tty) { char *exec_file, *full_exec_path = NULL; + struct cleanup *old_chain = save_current_program_space (); - /* Try to determine a filename from the process itself. */ - exec_file = target_pid_to_exec_file (pid); - if (exec_file == NULL) - return; + /* Switch over temporarily, while reading executable and + symbols. */ + set_current_program_space (inf->pspace); - /* 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) + /* Try to determine a filename from the process itself. */ + exec_file = target_pid_to_exec_file (inf->pid); + if (exec_file != NULL) { - /* 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); + /* 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) + { + /* 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); + } } - if (exec_file_is_user_supplied) + if (exec_filename != NULL && exec_file_was_user_supplied) { - if (strcmp (full_exec_path, exec_filename) != 0) - warning (_("Process %d has executable file %s," - " but executable file is currently set to %s"), - pid, full_exec_path, exec_filename); + if (full_exec_path != NULL && strcmp (full_exec_path, exec_filename) != 0) + warning (_("Detected exec-file mismatch on %s. Running %s; Loaded %s"), + target_pid_to_str (pid_to_ptid (inf->pid)), + full_exec_path, exec_filename); + reopen_exec_file (); } - else - exec_file_attach (full_exec_path, from_tty); + else if (full_exec_path != NULL) + exec_file_attach (0, full_exec_path, from_tty); - if (symfile_objfile_is_user_supplied) + if (symfile_objfile != NULL && symfile_objfile_was_user_supplied) { const char *symbol_filename = objfile_filename (symfile_objfile); - if (strcmp (full_exec_path, symbol_filename) != 0) - warning (_("Process %d has symbol file %s," - " but symbol file is currently set to %s"), - pid, full_exec_path, symbol_filename); + if (full_exec_path != NULL + && strcmp (full_exec_path, symbol_filename) != 0) + warning (_("Detected symbol-file mismatch on %s. Running %s; Loaded %s"), + target_pid_to_str (pid_to_ptid (inf->pid)), + full_exec_path, symbol_filename); } - else - symbol_file_add_main (full_exec_path, from_tty); + else if (full_exec_path != NULL) + symbol_file_add_main (0, full_exec_path, from_tty); + + /* Re-read symbol files that were modified since we last loaded + them. */ + reread_symbols (); + + do_cleanups (old_chain); } /* Set FILENAME as the new exec file. @@ -207,7 +220,7 @@ exec_file_locate_attach (int pid, int from_tty) we're supplying the exec pathname late for good reason.) */ void -exec_file_attach (const char *filename, int from_tty) +exec_file_attach (int user_supplied, const char *filename, int from_tty) { struct cleanup *cleanups; @@ -294,6 +307,8 @@ exec_file_attach (const char *filename, int from_tty) do_cleanups (cleanups); + exec_file_was_user_supplied = user_supplied; + bfd_cache_close_all (); observer_notify_executable_changed (); } @@ -335,13 +350,12 @@ exec_file_command (char *args, int from_tty) filename = tilde_expand (*argv); make_cleanup (xfree, filename); - exec_file_attach (filename, from_tty); - exec_file_is_user_supplied = 1; + exec_file_attach (1, filename, from_tty); do_cleanups (cleanups); } else - exec_file_attach (NULL, from_tty); + exec_file_attach (1, NULL, from_tty); } /* Set both the exec file and the symbol file, in one command. diff --git a/gdb/exec.h b/gdb/exec.h index d984e97..aef2926 100644 --- a/gdb/exec.h +++ b/gdb/exec.h @@ -32,8 +32,8 @@ struct objfile; #define exec_bfd current_program_space->ebfd #define exec_bfd_mtime current_program_space->ebfd_mtime #define exec_filename current_program_space->pspace_exec_filename -#define exec_file_is_user_supplied \ - current_program_space->pspace_exec_file_is_user_supplied +#define exec_file_was_user_supplied \ + current_program_space->pspace_exec_file_was_user_supplied /* Builds a section table, given args BFD, SECTABLE_PTR, SECEND_PTR. Returns 0 if OK, 1 on error. */ diff --git a/gdb/gdbcore.h b/gdb/gdbcore.h index 0c08b37..f34ee77 100644 --- a/gdb/gdbcore.h +++ b/gdb/gdbcore.h @@ -146,14 +146,23 @@ extern int write_files; extern void core_file_command (char *filename, int from_tty); -extern void exec_file_attach (const char *filename, int from_tty); - -/* If the filename of the main executable is unknown, attempt to - determine it. If a filename is determined, proceed as though - it was just specified with the "file" command. Do nothing if - the filename of the main executable is already known. */ - -extern void exec_file_locate_attach (int pid, int from_tty); +extern void exec_file_attach (int user_supplied, const char *filename, + int from_tty); + +/* Resync executable and symbols. Fetch the filename of the main + executable based on what the target is running. If a filename can + be determined, and the current exec file loaded was not previously + user supplied, then load the discovered filename as exec file. If + the executable loaded had been user supplied (with e.g., the "file" + command), issue a warning if there's a mismatch, and reload the + previously user-specified executable (in case the program was + recompiled since the last time we loaded it). Likewise, if the + currenly load main symbol file was not user supplied, load the + discovered filename as symbol file; otherwise, if there's a + mismatch, warn. End by reading all symbol files that have been + modified since the last time we loaded them. */ + +extern void exec_file_and_symbols_resync (struct inferior *inf, int from_tty); extern void exec_file_clear (int from_tty); diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 1f9d94a..b6d9474 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -2497,13 +2497,7 @@ attach_command_post_wait (char *args, int from_tty, int async_exec) and/or symbol files then warn the user if their choices do not match. Otherwise, set exec_file and symfile_objfile to the new file. */ - exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty); - - if (exec_file_is_user_supplied) - { - reopen_exec_file (); - reread_symbols (); - } + exec_file_and_symbols_resync (inferior, from_tty); /* Take any necessary post-attaching actions for this platform. */ target_post_attach (ptid_get_pid (inferior_ptid)); diff --git a/gdb/inferior.c b/gdb/inferior.c index 118aca6..30d0f25 100644 --- a/gdb/inferior.c +++ b/gdb/inferior.c @@ -859,16 +859,13 @@ add_inferior_command (char *args, int from_tty) if (exec != NULL) { /* Switch over temporarily, while reading executable and - symbols.q. */ + symbols. */ set_current_program_space (inf->pspace); set_current_inferior (inf); switch_to_thread (null_ptid); - exec_file_attach (exec, from_tty); - exec_file_is_user_supplied = 1; - - symbol_file_add_main (exec, from_tty); - symfile_objfile_is_user_supplied = 1; + exec_file_attach (1, exec, from_tty); + symbol_file_add_main (SYMFILE_USER_SPECIFIED, exec, from_tty); } } diff --git a/gdb/infrun.c b/gdb/infrun.c index a695f2e..44d2e06 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1211,7 +1211,7 @@ 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); + exec_file_attach (0, execd_pathname, 0); /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE (Position Independent Executable) main symbol file will get applied by diff --git a/gdb/main.c b/gdb/main.c index 2c63e80..e7f5086 100644 --- a/gdb/main.c +++ b/gdb/main.c @@ -405,6 +405,29 @@ catch_command_errors_const (catch_command_errors_const_ftype *command, return 1; } +/* Like catch_command_errors, but specifically for symbol/exec load + routines. */ + +typedef void (catch_exec_or_symbol_errors_ftype) (int, const char *, int); + +static int +catch_exec_or_symbol_errors (catch_exec_or_symbol_errors_ftype *func, + int arg, const char *filename, int from_tty) +{ + TRY + { + func (arg, filename, from_tty); + } + CATCH (e, RETURN_MASK_ALL) + { + exception_print (gdb_stderr, e); + return 0; + } + END_CATCH + + return 1; +} + /* Type of this option. */ enum cmdarg_kind { @@ -1050,28 +1073,23 @@ captured_main (void *data) { /* The exec file and the symbol-file are the same. If we can't open it, better only print one error message. - catch_command_errors returns non-zero on success! */ - if (catch_command_errors_const (exec_file_attach, execarg, - !batch_flag)) - { - exec_file_is_user_supplied = 1; - - if (catch_command_errors_const (symbol_file_add_main, symarg, - !batch_flag)) - symfile_objfile_is_user_supplied = 1; - } + catch_exec_or_symbol_errors returns non-zero on success. */ + if (catch_exec_or_symbol_errors (exec_file_attach, + 1, execarg, !batch_flag)) + catch_exec_or_symbol_errors (symbol_file_add_main, + SYMFILE_USER_SPECIFIED, + symarg, !batch_flag); } else { - if (execarg != NULL - && catch_command_errors_const (exec_file_attach, execarg, - !batch_flag)) - exec_file_is_user_supplied = 1; - - if (symarg != NULL - && catch_command_errors_const (symbol_file_add_main, symarg, - !batch_flag)) - symfile_objfile_is_user_supplied = 1; + if (execarg != NULL) + catch_exec_or_symbol_errors (exec_file_attach, 1, + execarg, !batch_flag); + + if (symarg != NULL) + catch_exec_or_symbol_errors (symbol_file_add_main, + SYMFILE_USER_SPECIFIED, + symarg, !batch_flag); } if (corearg && pidarg) diff --git a/gdb/progspace.c b/gdb/progspace.c index ea2f8ec..1b3f689 100644 --- a/gdb/progspace.c +++ b/gdb/progspace.c @@ -182,10 +182,11 @@ clone_program_space (struct program_space *dest, struct program_space *src) set_current_program_space (dest); if (src->pspace_exec_filename != NULL) - exec_file_attach (src->pspace_exec_filename, 0); + exec_file_attach (1, src->pspace_exec_filename, 0); if (src->symfile_object_file != NULL) - symbol_file_add_main (objfile_name (src->symfile_object_file), 0); + symbol_file_add_main (SYMFILE_USER_SPECIFIED, + objfile_name (src->symfile_object_file), 0); do_cleanups (old_chain); return dest; diff --git a/gdb/progspace.h b/gdb/progspace.h index 4761781..01fd9a3 100644 --- a/gdb/progspace.h +++ b/gdb/progspace.h @@ -155,10 +155,10 @@ struct program_space char *pspace_exec_filename; /* Nonzero if pspace_exec_filename was supplied by the user, - either at startup (on the command-line) or via the "file" - or "add-inferior -exec" commands. Zero if - pspace_exec_filename is unset or was discovered by GDB. */ - int pspace_exec_file_is_user_supplied; + either at startup (on the command-line) or via the "file" or + "add-inferior -exec" commands. Zero if pspace_exec_filename is + unset or was discovered by GDB. */ + int pspace_exec_file_was_user_supplied; /* The address space attached to this program space. More than one program space may be bound to the same address space. In the @@ -189,11 +189,11 @@ struct program_space (e.g. the argument to the "symbol-file" or "file" command). */ struct objfile *symfile_object_file; - /* Nonzero if symfile_object_file was supplied by the user, - either at startup (on the command-line) or via the "file", - "symbol-file" or "add-inferior -exec" commands. Zero if - symfile_object_file is unset or was discovered by GDB. */ - int symfile_object_file_is_user_supplied; + /* Nonzero if symfile_object_file is set and was discovered by + GDB. Zero if symfile_object_file is not set or was supplied + by the user, either at startup (on the command-line) or via the + "file" or "add-inferior -exec" commands. */ + int symfile_object_file_was_user_supplied; /* All known objfiles are kept in a linked list. This points to the head of this list. */ @@ -227,10 +227,10 @@ struct program_space #define symfile_objfile current_program_space->symfile_object_file -/* See program_space->symfile_object_file_is_user_supplied. */ +/* See program_space->symfile_object_file_was_user_supplied. */ -#define symfile_objfile_is_user_supplied \ - current_program_space->symfile_object_file_is_user_supplied +#define symfile_objfile_was_user_supplied \ + current_program_space->symfile_object_file_was_user_supplied /* All known objfiles are kept in a linked list. This points to the root of this list. */ diff --git a/gdb/remote.c b/gdb/remote.c index 8ff71c2..0aea212 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1644,7 +1644,7 @@ remote_add_inferior (int fake_pid_p, int pid, int attached, not match. Otherwise, set exec_file and symfile_objfile to the new file. */ if (try_open_exec) - exec_file_locate_attach (pid, 1); + exec_file_and_symbols_resync (inf, 1); return inf; } diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c index 8cc7cc1..f15ea02 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -1073,7 +1073,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 (0, filename, from_tty); do_cleanups (cleanups); return 1; diff --git a/gdb/symfile.c b/gdb/symfile.c index 9a1785d..a943ec0 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -85,7 +85,8 @@ int readnow_symbol_files; /* Read full symbols immediately. */ static void load_command (char *, int); -static void symbol_file_add_main_1 (const char *args, int from_tty, int flags); +static void symbol_file_add_main_1 (int add_flags, const char *args, + int from_tty, int flags); static void add_symbol_file_command (char *, int); @@ -1306,16 +1307,17 @@ symbol_file_add (const char *name, int add_flags, command itself. */ void -symbol_file_add_main (const char *args, int from_tty) +symbol_file_add_main (int add_flags, const char *args, int from_tty) { - symbol_file_add_main_1 (args, from_tty, 0); + symbol_file_add_main_1 (add_flags, args, from_tty, 0); } static void -symbol_file_add_main_1 (const char *args, int from_tty, int flags) +symbol_file_add_main_1 (int add_flags, const char *args, int from_tty, + int flags) { - const int add_flags = (current_inferior ()->symfile_flags - | SYMFILE_MAINLINE | (from_tty ? SYMFILE_VERBOSE : 0)); + add_flags |= (current_inferior ()->symfile_flags + | SYMFILE_MAINLINE | (from_tty ? SYMFILE_VERBOSE : 0)); symbol_file_add (args, add_flags, NULL, flags); @@ -1325,6 +1327,9 @@ symbol_file_add_main_1 (const char *args, int from_tty, int flags) if ((flags & SYMFILE_NO_READ) == 0) set_initial_language (); + + symfile_objfile_was_user_supplied + = (add_flags & SYMFILE_USER_SPECIFIED) != 0; } void @@ -1348,7 +1353,7 @@ symbol_file_clear (int from_tty) if (from_tty) printf_unfiltered (_("No symbol file now.\n")); hooks/post-receive -- Repository for Project Archer.