Fix "-gdb-set logging redirect on" crash
This commit fixes a "-gdb-set logging redirect on" crash by not handling "logging redirect on" on the fly. Previous discussion here: https://sourceware.org/ml/gdb-patches/2017-01/msg00467.html Code for handling "logging redirect on" on the fly was added here: https://sourceware.org/ml/gdb-patches/2010-08/msg00202.html Meanwhile, MI gained support for logging, but flipping redirect "on" on the fly was not considered. The result is that this sequence of commands crashes GDB: -gdb-set logging on -gdb-set logging redirect on Program received signal SIGSEGV, Segmentation fault. 0x00000000008dd7bc in gdb_flush (file=0x2a097f0) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/ui-file.c:95 194 file->to_flush (file); (top-gdb) bt #0 0x00000000008dd7bc in gdb_flush(ui_file*) (file=0x2a097f0) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/ui-file.c:95 #1 0x00000000007b5f34 in gdb_wait_for_event(int) (block=0) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:752 #2 0x00000000007b52b6 in gdb_do_one_event() () at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:322 #3 0x00000000007b5362 in start_event_loop() () at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/event-loop.c:371 #4 0x000000000082704a in captured_command_loop(void*) (data=0x0) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/main.c:325 #5 0x00000000007b8d7c in catch_errors(int (*)(void*), void*, char*, return_mask) (func=0x827008 <captured_command_loop(void*)>, func_args=0x0, errstring=0x11dee51 "", mask=RETURN_MASK_ALL) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/exceptions.c:236 #6 0x000000000082839b in captured_main(void*) (data=0x7fffffffd820) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/main.c:1148 During symbol reading, cannot get low and high bounds for subprogram DIE at 24065. #7 0x00000000008283c4 in gdb_main(captured_main_args*) (args=0x7fffffffd820) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/main.c:1158 #8 0x0000000000412d4d in main(int, char**) (argc=4, argv=0x7fffffffd928) at /home/pedro/gdb/mygit/cxx-convertion/src/gdb/gdb.c:32 The handling of redirect on the fly is not really a use case we need to handle, IMO. Its inconsistent (other "set logging foo" commands aren't handled on the fly), and complicates the code significantly. Instead of complicating it further for MI, go back to the original idea of warning, only: https://sourceware.org/ml/gdb-patches/2010-08/msg00083.html New test included. gdb/ChangeLog: 2017-02-02 Pedro Alves <palves@redhat.com> * cli/cli-logging.c (maybe_warn_already_logging): New factored out from ... (set_logging_overwrite): ... here. (logging_no_redirect_file): Delete. (set_logging_redirect): Don't handle redirection on the fly. Instead warn that "logging off" / "logging on" is necessary. (pop_output_files): Delete references to logging_no_redirect_file. (show_logging_command): Always speak in terms of what will happen once logging is reenabled. gdb/testsuite/ChangeLog: 2017-02-02 Pedro Alves <palves@redhat.com> * gdb.mi/mi-logging.exp: Add "redirect while already logging" tests.
This commit is contained in:
parent
c99cc448c8
commit
5be5dbf0ce
|
@ -1,3 +1,15 @@
|
||||||
|
2017-02-02 Pedro Alves <palves@redhat.com>
|
||||||
|
|
||||||
|
* cli/cli-logging.c (maybe_warn_already_logging): New factored out
|
||||||
|
from ...
|
||||||
|
(set_logging_overwrite): ... here.
|
||||||
|
(logging_no_redirect_file): Delete.
|
||||||
|
(set_logging_redirect): Don't handle redirection on the fly.
|
||||||
|
Instead warn that "logging off" / "logging on" is necessary.
|
||||||
|
(pop_output_files): Delete references to logging_no_redirect_file.
|
||||||
|
(show_logging_command): Always speak in terms of what will happen
|
||||||
|
once logging is reenabled.
|
||||||
|
|
||||||
2017-02-02 Pedro Alves <palves@redhat.com>
|
2017-02-02 Pedro Alves <palves@redhat.com>
|
||||||
|
|
||||||
* disasm.h (gdb_pretty_print_disassembler): Tweak intro comment.
|
* disasm.h (gdb_pretty_print_disassembler): Tweak intro comment.
|
||||||
|
|
|
@ -47,13 +47,19 @@ show_logging_filename (struct ui_file *file, int from_tty,
|
||||||
static int logging_overwrite;
|
static int logging_overwrite;
|
||||||
|
|
||||||
static void
|
static void
|
||||||
set_logging_overwrite (char *args, int from_tty, struct cmd_list_element *c)
|
maybe_warn_already_logging ()
|
||||||
{
|
{
|
||||||
if (saved_filename)
|
if (saved_filename)
|
||||||
warning (_("Currently logging to %s. Turn the logging off and on to "
|
warning (_("Currently logging to %s. Turn the logging off and on to "
|
||||||
"make the new setting effective."), saved_filename);
|
"make the new setting effective."), saved_filename);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static void
|
||||||
|
set_logging_overwrite (char *args, int from_tty, struct cmd_list_element *c)
|
||||||
|
{
|
||||||
|
maybe_warn_already_logging ();
|
||||||
|
}
|
||||||
|
|
||||||
static void
|
static void
|
||||||
show_logging_overwrite (struct ui_file *file, int from_tty,
|
show_logging_overwrite (struct ui_file *file, int from_tty,
|
||||||
struct cmd_list_element *c, const char *value)
|
struct cmd_list_element *c, const char *value)
|
||||||
|
@ -67,70 +73,10 @@ show_logging_overwrite (struct ui_file *file, int from_tty,
|
||||||
/* Value as configured by the user. */
|
/* Value as configured by the user. */
|
||||||
static int logging_redirect;
|
static int logging_redirect;
|
||||||
|
|
||||||
/* The on-disk file in use if logging is currently active together
|
|
||||||
with redirection turned off (and therefore using tee_file_new).
|
|
||||||
For active logging with redirection the on-disk file is directly in
|
|
||||||
GDB_STDOUT and this variable is NULL. */
|
|
||||||
static struct ui_file *logging_no_redirect_file;
|
|
||||||
|
|
||||||
static void
|
static void
|
||||||
set_logging_redirect (char *args, int from_tty, struct cmd_list_element *c)
|
set_logging_redirect (char *args, int from_tty, struct cmd_list_element *c)
|
||||||
{
|
{
|
||||||
ui_file_up destroy_old_stdout;
|
maybe_warn_already_logging ();
|
||||||
struct ui_file *output, *new_logging_no_redirect_file;
|
|
||||||
struct ui_out *uiout = current_uiout;
|
|
||||||
|
|
||||||
if (saved_filename == NULL
|
|
||||||
|| (logging_redirect != 0 && logging_no_redirect_file == NULL)
|
|
||||||
|| (logging_redirect == 0 && logging_no_redirect_file != NULL))
|
|
||||||
return;
|
|
||||||
|
|
||||||
if (logging_redirect != 0)
|
|
||||||
{
|
|
||||||
gdb_assert (logging_no_redirect_file != NULL);
|
|
||||||
|
|
||||||
/* ui_out_redirect still has not been called for next
|
|
||||||
gdb_stdout. */
|
|
||||||
destroy_old_stdout.reset (gdb_stdout);
|
|
||||||
|
|
||||||
output = logging_no_redirect_file;
|
|
||||||
new_logging_no_redirect_file = NULL;
|
|
||||||
|
|
||||||
if (from_tty)
|
|
||||||
fprintf_unfiltered (saved_output.out, "Redirecting output to %s.\n",
|
|
||||||
logging_filename);
|
|
||||||
}
|
|
||||||
else
|
|
||||||
{
|
|
||||||
gdb_assert (logging_no_redirect_file == NULL);
|
|
||||||
output = new tee_file (saved_output.out, 0, gdb_stdout, 0);
|
|
||||||
new_logging_no_redirect_file = gdb_stdout;
|
|
||||||
|
|
||||||
if (from_tty)
|
|
||||||
fprintf_unfiltered (saved_output.out, "Copying output to %s.\n",
|
|
||||||
logging_filename);
|
|
||||||
}
|
|
||||||
|
|
||||||
/* Give the current interpreter a chance to do anything special that
|
|
||||||
it might need for logging, such as updating other channels. */
|
|
||||||
if (current_interp_set_logging (1, output, NULL) == 0)
|
|
||||||
{
|
|
||||||
gdb_stdout = output;
|
|
||||||
gdb_stdlog = output;
|
|
||||||
gdb_stderr = output;
|
|
||||||
gdb_stdtarg = output;
|
|
||||||
gdb_stdtargerr = output;
|
|
||||||
}
|
|
||||||
|
|
||||||
logging_no_redirect_file = new_logging_no_redirect_file;
|
|
||||||
|
|
||||||
/* There is a former output pushed on the ui_out_redirect stack. We
|
|
||||||
want to replace it by OUTPUT so we must pop the former value
|
|
||||||
first. Ideally, we should either do both the pop and push or do
|
|
||||||
neither of them. */
|
|
||||||
|
|
||||||
uiout->redirect (NULL);
|
|
||||||
uiout->redirect (output);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static void
|
static void
|
||||||
|
@ -144,12 +90,6 @@ show_logging_redirect (struct ui_file *file, int from_tty,
|
||||||
static void
|
static void
|
||||||
pop_output_files (void)
|
pop_output_files (void)
|
||||||
{
|
{
|
||||||
if (logging_no_redirect_file)
|
|
||||||
{
|
|
||||||
delete logging_no_redirect_file;
|
|
||||||
logging_no_redirect_file = NULL;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (current_interp_set_logging (0, NULL, NULL) == 0)
|
if (current_interp_set_logging (0, NULL, NULL) == 0)
|
||||||
{
|
{
|
||||||
/* Only delete one of the files -- they are all set to the same
|
/* Only delete one of the files -- they are all set to the same
|
||||||
|
@ -204,7 +144,6 @@ handle_redirections (int from_tty)
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
gdb_assert (logging_no_redirect_file == NULL);
|
|
||||||
output = std::move (log);
|
output = std::move (log);
|
||||||
|
|
||||||
if (from_tty)
|
if (from_tty)
|
||||||
|
@ -231,7 +170,7 @@ handle_redirections (int from_tty)
|
||||||
}
|
}
|
||||||
|
|
||||||
output.release ();
|
output.release ();
|
||||||
logging_no_redirect_file = no_redirect_file.release ();
|
no_redirect_file.release ();
|
||||||
|
|
||||||
/* Don't do the redirect for MI, it confuses MI's ui-out scheme. */
|
/* Don't do the redirect for MI, it confuses MI's ui-out scheme. */
|
||||||
if (!current_uiout->is_mi_like_p ())
|
if (!current_uiout->is_mi_like_p ())
|
||||||
|
@ -290,20 +229,10 @@ show_logging_command (char *args, int from_tty)
|
||||||
else
|
else
|
||||||
printf_unfiltered (_("Logs will be appended to the log file.\n"));
|
printf_unfiltered (_("Logs will be appended to the log file.\n"));
|
||||||
|
|
||||||
if (saved_filename)
|
if (logging_redirect)
|
||||||
{
|
printf_unfiltered (_("Output will be sent only to the log file.\n"));
|
||||||
if (logging_redirect)
|
|
||||||
printf_unfiltered (_("Output is being sent only to the log file.\n"));
|
|
||||||
else
|
|
||||||
printf_unfiltered (_("Output is being logged and displayed.\n"));
|
|
||||||
}
|
|
||||||
else
|
else
|
||||||
{
|
printf_unfiltered (_("Output will be logged and displayed.\n"));
|
||||||
if (logging_redirect)
|
|
||||||
printf_unfiltered (_("Output will be sent only to the log file.\n"));
|
|
||||||
else
|
|
||||||
printf_unfiltered (_("Output will be logged and displayed.\n"));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Provide a prototype to silence -Wmissing-prototypes. */
|
/* Provide a prototype to silence -Wmissing-prototypes. */
|
||||||
|
|
|
@ -1,3 +1,8 @@
|
||||||
|
2017-02-02 Pedro Alves <palves@redhat.com>
|
||||||
|
|
||||||
|
* gdb.mi/mi-logging.exp: Add "redirect while already logging"
|
||||||
|
tests.
|
||||||
|
|
||||||
2017-02-01 Andreas Arnez <arnez@linux.vnet.ibm.com>
|
2017-02-01 Andreas Arnez <arnez@linux.vnet.ibm.com>
|
||||||
|
|
||||||
* gdb.dwarf2/dw2-op-stack-value.exp: Adjust expected result of
|
* gdb.dwarf2/dw2-op-stack-value.exp: Adjust expected result of
|
||||||
|
|
|
@ -82,6 +82,20 @@ if [regexp "1001\\^done\[\r\n\]+$mi_log_prompt.*1002\\^running\[\r\n\]+\\*runnin
|
||||||
fail "redirect log file contents"
|
fail "redirect log file contents"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# Now try enabling a redirect while GDB is already logging. This used
|
||||||
|
# to crash GDB.
|
||||||
|
with_test_prefix "redirect while already logging" {
|
||||||
|
mi_gdb_test "-gdb-set logging redirect off" ".*" \
|
||||||
|
"logging redirect off"
|
||||||
|
mi_gdb_test "-gdb-set logging on" ".*" \
|
||||||
|
"logging on"
|
||||||
|
mi_gdb_test "-gdb-set logging redirect on" \
|
||||||
|
".*warning: Currently logging .*Turn the logging off and on to make the new setting effective.*" \
|
||||||
|
"logging redirect on"
|
||||||
|
mi_gdb_test "-gdb-set logging off" ".*" \
|
||||||
|
"logging off"
|
||||||
|
}
|
||||||
|
|
||||||
mi_gdb_exit
|
mi_gdb_exit
|
||||||
|
|
||||||
remote_file host delete $milogfile
|
remote_file host delete $milogfile
|
||||||
|
|
Loading…
Reference in New Issue