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: Antoine Tremblay <antoine dot tremblay at ericsson dot com>
- To: Pedro Alves <palves at redhat dot com>
- Cc: Antoine Tremblay <antoine dot tremblay at ericsson dot com>, <gdb-patches at sourceware dot org>
- Date: Fri, 3 Jun 2016 11:32:16 -0400
- 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> <f6d86604-6cf9-3ba4-bec7-958b19e82400 at redhat dot com>
Pedro Alves writes:
> 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.
>
Indeed, OK.
>> + 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.:
>
OK.
> extern void inf_ptrace_attach_success (struct target_ops *ops);
>
OK. (detach_success)
>> + }
>> + }
>> + }
>> + 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.
>
Oops thanks.
>> + 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.
>
Right, I tested that before without sucess but had not the perfect regex
yet...
I'll use this now. thanks.
>> 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.
>
OK.
>> + #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.
>
This one is needed since on the teardown of the .exp test gdb_exit is
called and calls monitor_exit which fails with a timeout this avoids
this and the original gdb_exit is called.
>> + 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)"
>
>> + }
>> +}
>>
Noted, thx.