This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2] Fix failure to detach if threads exit while detaching on linux
- From: Pedro Alves <palves at redhat dot com>
- To: Antoine Tremblay <antoine dot tremblay at ericsson dot com>, gdb-patches at sourceware dot org
- Date: Fri, 3 Jun 2016 16:16:23 +0100
- Subject: Re: [PATCH v2] Fix failure to detach if threads exit while detaching on linux
- Authentication-results: sourceware.org; auth=none
- References: <bf6cd7bf-4f1a-b5c6-b396-e5ae42ed9c70 at redhat dot com> <1464965361-25399-1-git-send-email-antoine dot tremblay at ericsson dot com>
On 06/03/2016 03:49 PM, Antoine Tremblay wrote:
> @@ -1491,7 +1523,28 @@ linux_nat_detach (struct target_ops *ops, const char *args, int from_tty)
> linux_fork_detach (args, from_tty);
> }
> else
> - linux_ops->to_detach (ops, args, from_tty);
> + {
> + TRY
> + {
> + linux_ops->to_detach (ops, args, from_tty);
> + }
> + CATCH (ex, RETURN_MASK_ERROR)
> + {
> + if (!check_ptrace_stopped_lwp_gone (main_lwp))
> + {
> + delete_lwp (main_lwp->ptid);
Is deleting the right thing to do? I think we'll likely
crash, the next time we try to do something (e.g., try
to detach again). Seems like on the gdbserver side,
the lwp isn't deleted in this case.
> + throw_exception (ex);
> + }
> + /* Ignore the error since the thread is gone already. */
> + else
> + {
> + inferior_ptid = null_ptid;
> + detach_inferior (pid);
> + inf_child_maybe_unpush_target (ops);
Please factor these three lines out of inf_ptrace_attach into
a separate function, exported from inf-ptrace.h and called
from both places. E.g.:
extern void inf_ptrace_attach_success (struct target_ops *ops);
> + }
> + }
> + }
> + delete_lwp (main_lwp->ptid);
> }
>
> +clean_restart ${testfile}
> +
> +if ![runto_main] {
> + fail "Can't run to main to check for trace support"
"to check for trace support" is stale.
> + return -1
> +}
> +
> +gdb_breakpoint "_exit"
> +gdb_continue_to_breakpoint "_exit" ".*_exit.*"
> +
> +# Since the gdbserver-native board ends remote debugging on detach,
> +# we can't use gdb_test as this function intercepts the Ending remote
> +# debugging message and tries to restart gdb.
Eh, that looks like a bogus think for gdb_test_multiple to be doing.
Seems like it's been there since "forever":
commit 19fa4a0af34ff07f260fc75e903ab92d8ca03d33
Author: Mike Werner <mtw@cygnus>
AuthorDate: Sun Feb 21 20:03:55 1993 +0000
* gdb/testsuite: Initial creation of gdb/testsuite.
Migrated dejagnu testcases and support files for testing nm to
gdb/testsuite from deja-gnu. These files were moved "as is"
with no modifications. This migration is part of a major overhaul
of dejagnu. The modifications to these testcases, etc., which
will allow them to work with the new version of dejagnu will be
made in a future update.
I'd support just removing that bit.
In any case, there's no need to avoid gdb_test/gdb_test_multiple.
Any user-specified pattern overrides gdb_test_multiple's internal
patterns. So you just need to intercept the "Ending remote debugging"
message yourself.
> However, it can't do that
> +# as gdb_exit on this board will call monitor exit which will fail as
> +# we're not remote debugging anymore after a detach in this case.
> +send_gdb "detach\n"
> +gdb_expect {
set test "detach"
gdb_test_multiple $test $test {
-re "Detaching from .*, process $decimal\r\nEnding remote debugging\.\r\n$gdb_prompt $" {
# This is what you get with "target remote".
pass $test
}
-re "Detaching from .*, process $decimal\r\n$gdb_prompt $" {
pass $test
}
}
> + -re "Detaching from .*, process $decimal\r\nEnding remote debugging\." {
Missing $gdb_prompt match.
> + #Don't try to exit gdbserver see above comment.
> + set gdbserver_reconnect_p 1
I don't understand this one. It shouldn't be needed.
> + pass "detach non-extended"
Please use the same test message for all branches:
pass $test
> + }
> + -re "Detaching from .*, process $decimal\r\n$gdb_prompt $" {
> + pass "detach"
> + }
> + -re ".*$gdb_prompt $" {
> + fail "detach"
> + }
> + timeout {
> + fail "detach timeout"
FYI, this would have been the standard:
> + fail "$test (timeout)"
> + }
> +}
>
Thanks,
Pedro Alves