diff options
author | Tom Tromey <tromey@redhat.com> | 2014-02-28 09:47:34 -0700 |
---|---|---|
committer | Tom Tromey <tromey@redhat.com> | 2014-03-12 13:05:58 -0600 |
commit | b3ccfe11d3b0fc84f8ccd4e4fa25b75d1dc71cfc (patch) | |
tree | bcaf99bcdc993f80db556b6851fa051a00f55fdf /gdb/infcmd.c | |
parent | 55d9b4c146716a683d9fea769e5f4106eadb30fc (diff) | |
download | gdb-b3ccfe11d3b0fc84f8ccd4e4fa25b75d1dc71cfc.zip gdb-b3ccfe11d3b0fc84f8ccd4e4fa25b75d1dc71cfc.tar.gz gdb-b3ccfe11d3b0fc84f8ccd4e4fa25b75d1dc71cfc.tar.bz2 |
fix regressions with target-async
A patch in the target cleanup series caused a regression when using
record with target-async. Version 4 of the patch is here:
https://sourceware.org/ml/gdb-patches/2014-03/msg00159.html
The immediate problem is that record supplies to_can_async_p and
to_is_async_p methods, but does not supply a to_async method. So,
when target-async is set, record claims to support async -- but if the
underlying target does not support async, then the to_async method
call will end up in that method's default implementation, namely
tcomplain.
This worked previously because the record target used to provide a
to_async method; one that (erroneously, only at push time) checked the
other members of the target stack, and then simply dropped to_async
calls in the "does not implement async" case.
My first thought was to simply drop tcomplain as the default for
to_async. This works, but Pedro pointed out that the only reason
record has to supply to_can_async_p and to_is_async_p is that these
default to using the find_default_run_target machinery -- and these
defaults are only needed by "run" and "attach".
So, a nicer solution presents itself: change run and attach to
explicitly call into the default run target when needed; and change
to_is_async_p and to_can_async_p to default to "return 0". This makes
the target stack simpler to use and lets us remove the method
implementations from record. This is also in harmony with other plans
for the target stack; namely trying to reduce the impact of
find_default_run_target. This approach makes it clear that
find_default_is_async_p is not needed -- it is asking whether a target
that may not even be pushed is actually async, which seems like a
nonsensical question.
While an improvement, this approach proved to introduce the same bug
when using the core target. Looking a bit deeper, the issue is that
code in "attach" and "run" may need to use either the current target
stack or the default run target -- but different calls into the target
API in those functions could wind up querying different targets.
This new patch makes the target to use more explicit in "run" and
"attach". Then these commands explicitly make the needed calls
against that target. This ensures that a single target is used for
all relevant operations. This lets us remove a couple find_default_*
functions from various targets, including the dummy target. I think
this is a decent understandability improvement.
One issue I see with this patch is that the new calls in "run" and
"attach" are not very much like the rest of the target API. I think
fundamentally this is due to bad factoring in the target API, which
may need to be fixed for multi-target. Tackling that seemed ambitious
for a regression fix.
While working on this I noticed that there don't seem to be any test
cases that involve both target-async and record, so this patch changes
break-precsave.exp to add some. It also changes corefile.exp to add
some target-async tests; these pass with current trunk and with this
patch applied, but fail with the v1 patch.
This patch differs from v4 in that it moves initialization of
to_can_async_p and to_supports_non_stop into inf-child, adds some
assertions to complete_target_initialization, and adds some comments
to target.h.
Built and regtested on x86-64 Fedora 20.
2014-03-12 Tom Tromey <tromey@redhat.com>
* inf-child.c (return_zero): New function.
(inf_child_target): Set to_can_async_p, to_supports_non_stop.
* aix-thread.c (aix_thread_inferior_created): New function.
(aix_thread_attach): Remove.
(init_aix_thread_ops): Don't set to_attach.
(_initialize_aix_thread): Register inferior_created observer.
* corelow.c (init_core_ops): Don't set to_attach or
to_create_inferior.
* exec.c (init_exec_ops): Don't set to_attach or
to_create_inferior.
* infcmd.c (run_command_1): Use find_run_target. Make direct
target calls.
(attach_command): Use find_attach_target. Make direct target
calls.
* record-btrace.c (init_record_btrace_ops): Don't set
to_create_inferior.
* record-full.c (record_full_can_async_p, record_full_is_async_p):
Remove.
(init_record_full_ops, init_record_full_core_ops): Update. Don't
set to_create_inferior.
* target.c (complete_target_initialization): Add assertion.
(target_create_inferior): Remove.
(find_default_attach, find_default_create_inferior): Remove.
(find_attach_target, find_run_target): New functions.
(find_default_is_async_p, find_default_can_async_p)
(target_supports_non_stop, target_attach): Remove.
(init_dummy_target): Don't set to_create_inferior or
to_supports_non_stop.
* target.h (struct target_ops) <to_attach>: Add comment. Remove
TARGET_DEFAULT_FUNC.
<to_create_inferior>: Add comment.
<to_can_async_p, to_is_async_p, to_supports_non_stop>: Use
TARGET_DEFAULT_RETURN.
<to_can_async_p, to_supports_non_stop, to_can_run>: Add comments.
(find_attach_target, find_run_target): Declare.
(target_create_inferior): Remove.
(target_has_execution_1): Update comment.
(target_supports_non_stop): Remove.
* target-delegates.c: Rebuild.
2014-03-12 Tom Tromey <tromey@redhat.com>
* gdb.base/corefile.exp (corefile_test_run, corefile_test_attach):
New procs. Add target-async tests.
* gdb.reverse/break-precsave.exp (precsave_tests): New proc.
Add target-async tests.
Diffstat (limited to 'gdb/infcmd.c')
-rw-r--r-- | gdb/infcmd.c | 34 |
1 files changed, 23 insertions, 11 deletions
diff --git a/gdb/infcmd.c b/gdb/infcmd.c index c59d0be..75d37fd 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -503,6 +503,7 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main) struct cleanup *old_chain; ptid_t ptid; struct ui_out *uiout = current_uiout; + struct target_ops *run_target; dont_repeat (); @@ -531,7 +532,9 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main) exec_file = (char *) get_exec_file (0); - if (non_stop && !target_supports_non_stop ()) + run_target = find_run_target (); + + if (non_stop && !run_target->to_supports_non_stop (run_target)) error (_("The target does not support running in non-stop mode.")); /* We keep symbols from add-symbol-file, on the grounds that the @@ -544,7 +547,7 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main) if (!args) { - if (target_can_async_p ()) + if (run_target->to_can_async_p (run_target)) async_disable_stdin (); } else @@ -553,12 +556,12 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main) /* If we get a request for running in the bg but the target doesn't support it, error out. */ - if (async_exec && !target_can_async_p ()) + if (async_exec && !run_target->to_can_async_p (run_target)) error (_("Asynchronous execution not supported on this target.")); /* If we don't get a request of running in the bg, then we need to simulate synchronous (fg) execution. */ - if (!async_exec && target_can_async_p ()) + if (!async_exec && run_target->to_can_async_p (run_target)) { /* Simulate synchronous execution. */ async_disable_stdin (); @@ -585,9 +588,12 @@ run_command_1 (char *args, int from_tty, int tbreak_at_main) /* We call get_inferior_args() because we might need to compute the value now. */ - target_create_inferior (exec_file, get_inferior_args (), - environ_vector (current_inferior ()->environment), - from_tty); + run_target->to_create_inferior (run_target, exec_file, get_inferior_args (), + environ_vector (current_inferior ()->environment), + from_tty); + /* to_create_inferior should push the target, so after this point we + shouldn't refer to run_target again. */ + run_target = NULL; /* We're starting off a new process. When we get out of here, in non-stop mode, finish the state of all threads of that process, @@ -2515,6 +2521,7 @@ attach_command (char *args, int from_tty) { int async_exec = 0; struct cleanup *back_to = make_cleanup (null_cleanup, NULL); + struct target_ops *attach_target; dont_repeat (); /* Not for the faint of heart */ @@ -2534,7 +2541,9 @@ attach_command (char *args, int from_tty) this function should probably be moved into target_pre_inferior. */ target_pre_inferior (from_tty); - if (non_stop && !target_supports_non_stop ()) + attach_target = find_attach_target (); + + if (non_stop && !attach_target->to_supports_non_stop (attach_target)) error (_("Cannot attach to this target in non-stop mode")); if (args) @@ -2543,20 +2552,23 @@ attach_command (char *args, int from_tty) /* If we get a request for running in the bg but the target doesn't support it, error out. */ - if (async_exec && !target_can_async_p ()) + if (async_exec && !attach_target->to_can_async_p (attach_target)) error (_("Asynchronous execution not supported on this target.")); } /* If we don't get a request of running in the bg, then we need to simulate synchronous (fg) execution. */ - if (!async_exec && target_can_async_p ()) + if (!async_exec && attach_target->to_can_async_p (attach_target)) { /* Simulate synchronous execution. */ async_disable_stdin (); make_cleanup ((make_cleanup_ftype *)async_enable_stdin, NULL); } - target_attach (args, from_tty); + attach_target->to_attach (attach_target, args, from_tty); + /* to_attach should push the target, so after this point we + shouldn't refer to attach_target again. */ + attach_target = NULL; /* Set up the "saved terminal modes" of the inferior based on what modes we are starting it with. */ |