From patchwork Wed May 21 15:15:34 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 1054 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx21.g.dreamhost.com (peon2454.g.dreamhost.com [208.113.200.127]) by wilcox.dreamhost.com (Postfix) with ESMTP id 3EE4B360079 for ; Wed, 21 May 2014 08:15:47 -0700 (PDT) Received: by homiemail-mx21.g.dreamhost.com (Postfix, from userid 14314964) id DC81519CAF6B; Wed, 21 May 2014 08:15:46 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx21.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx21.g.dreamhost.com (Postfix) with ESMTPS id 7DD7E19CAF9C for ; Wed, 21 May 2014 08:15:46 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:references:date:in-reply-to :message-id:mime-version:content-type; q=dns; s=default; b=M3cd/ T3hH3jFVC6/xftpgaHokB6mApT1zDdunI/vRKMrum7a08SKSa0ZgBmQfY/h2y1U/ wpC4IFJQX4BR8ETM+BlmZoLUW6UBNM7VaTa1TYMe0k/2QxwVaALZGtqV8Goz8u0r g1xg8tTXpkkjxCOOp9x3LBzehX9Txyp+MtXUHQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:references:date:in-reply-to :message-id:mime-version:content-type; s=default; bh=X+quOvmZnrX cf5fYY1AwRCOSyw0=; b=q78WMtKSHJDqBVWXRZJ+lHhpGKHx4EfGgM9Xju7WoG3 gugo8DluZPTW/ql+EztCumSup52orjeaA+5gNgmPHKcrbUlOyz0qYw8R/TG6qdrT +5H1Ed6JflzF4erUkC7QzgIpgyZcq7SHwJuiNHSYm610L+FThtanI2Wwf+50nYNs = Received: (qmail 12777 invoked by alias); 21 May 2014 15:15:45 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 12761 invoked by uid 89); 21 May 2014 15:15:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 21 May 2014 15:15:43 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s4LFFan7003941 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 21 May 2014 11:15:37 -0400 Received: from barimba (ovpn-113-182.phx2.redhat.com [10.3.113.182]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s4LFFYti003732 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Wed, 21 May 2014 11:15:35 -0400 From: Tom Tromey To: Don Breazeal Cc: , Don Breazeal Subject: Re: [PATCH 1/4][REPOST] Remote Linux ptrace exit events References: <1398885482-8449-1-git-send-email-donb@codesourcery.com> <1398885482-8449-2-git-send-email-donb@codesourcery.com> Date: Wed, 21 May 2014 09:15:34 -0600 In-Reply-To: <1398885482-8449-2-git-send-email-donb@codesourcery.com> (Don Breazeal's message of "Wed, 30 Apr 2014 12:17:59 -0700") Message-ID: <87ppj7nox5.fsf@fleche.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-DH-Original-To: gdb@patchwork.siddhesh.in >>>>> "Don" == Don Breazeal writes: Don> This patch implements support for the extended ptrace event Don> PTRACE_EVENT_EXIT on Linux. This is a preparatory patch for exec event Don> support. Thanks. I had a few questions about this patch. Don> * common/linux-ptrace.c (linux_test_for_tracefork) Don> [GDBSERVER]: Add support for PTRACE_EVENT_EXIT if the OS Don> supports it. I'm curious why PTRACE_O_TRACEEXIT is needed by gdbserver to implement this feature. It isn't needed by gdb. And, I think it's preferable to try to push the two back ends closer together -- it's a long-term goal -- so new divergences are subject to special scrutiny. Don> +#ifdef GDBSERVER Don> + /* PTRACE_O_FORK is supported, so now test for PTRACE_O_TRACEEXIT. Don> + First try to set the option. If this fails, we know for sure that Don> + it is not supported. */ Don> + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, Don> + (PTRACE_TYPE_ARG4) PTRACE_O_TRACEEXIT); Don> + if (ret != 0) Don> + return; [...] It would be much nicer to reduce the use of #ifdef GDBSERVER, rather than to add to it. I think this could be done a different way, say by having the clients of this interface specify which flags they're interested in. Then gdbserver could ask for PTRACE_O_TRACEEXIT and gdb could refrain. This would be just as simple but still be a good fit for the long-term goal. I've appended a patch I wrote along these lines -- perhaps hacky, definitely untested -- from my experiment with moving gdbserver to top-level. Feel free to use or ignore it. Don> + else if (event == PTRACE_EVENT_EXIT) Don> + { Don> + unsigned long exit_status; Don> + unsigned long lwpid = lwpid_of (event_thr); Don> + int ret; Don> + Don> + if (debug_threads) Don> + debug_printf ("LHEW: Got exit event from LWP %ld\n", Don> + lwpid_of (event_thr)); Don> + Don> + ptrace (PTRACE_GETEVENTMSG, lwpid_of (event_thr), (PTRACE_TYPE_ARG3) 0, Don> + &exit_status); Don> + Don> + if (num_lwps (pid_of (event_thr)) > 1) Don> + { Don> + /* If there is at least one more LWP, then the program still Don> + exists and the exit should not be reported to GDB. */ I thought, from the man page, that PTRACE_EVENT_EXIT was for a process exit event only. So checking num_lwps doesn't seem correct here. But after seeing your patch I am not sure; and I would like to know the answer. Tom commit a1b6a7417e0e192c8f925ac94491a819c384c7e9 Author: Tom Tromey Date: Fri Jan 3 10:55:52 2014 -0700 remove some GDBSERVER checks from linux-ptrace diff --git a/gdb/common/linux-ptrace.c b/gdb/common/linux-ptrace.c index 7c1b78a..d1659e0 100644 --- a/gdb/common/linux-ptrace.c +++ b/gdb/common/linux-ptrace.c @@ -37,6 +37,10 @@ there are no supported features. */ static int current_ptrace_options = -1; +/* Additional flags to test. */ + +static int additional_flags; + /* Find all possible reasons we could fail to attach PID and append these newline terminated reason strings to initialized BUFFER. '\0' termination of BUFFER must be done by the caller. */ @@ -359,16 +363,15 @@ linux_check_ptrace_features (void) static void linux_test_for_tracesysgood (int child_pid) { -#ifdef GDBSERVER - /* gdbserver does not support PTRACE_O_TRACESYSGOOD. */ -#else - int ret; + if ((additional_flags & PTRACE_O_TRACESYSGOOD) != 0) + { + int ret; - ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, - (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD); - if (ret == 0) - current_ptrace_options |= PTRACE_O_TRACESYSGOOD; -#endif + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, + (PTRACE_TYPE_ARG4) PTRACE_O_TRACESYSGOOD); + if (ret != 0) + additional_flags &= ~PTRACE_O_TRACESYSGOOD; + } } /* Determine if PTRACE_O_TRACEFORK can be used to follow fork @@ -388,16 +391,15 @@ linux_test_for_tracefork (int child_pid) if (ret != 0) return; -#ifdef GDBSERVER - /* gdbserver does not support PTRACE_O_TRACEVFORKDONE yet. */ -#else - /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */ - ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, - (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK - | PTRACE_O_TRACEVFORKDONE)); - if (ret == 0) - current_ptrace_options |= PTRACE_O_TRACEVFORKDONE; -#endif + if ((additional_flags & PTRACE_O_TRACEVFORKDONE) != 0) + { + /* Check if the target supports PTRACE_O_TRACEVFORKDONE. */ + ret = ptrace (PTRACE_SETOPTIONS, child_pid, (PTRACE_TYPE_ARG3) 0, + (PTRACE_TYPE_ARG4) (PTRACE_O_TRACEFORK + | PTRACE_O_TRACEVFORKDONE)); + if (ret != 0) + additional_flags &= ~PTRACE_O_TRACEVFORKDONE; + } /* Setting PTRACE_O_TRACEFORK did not cause an error, however we don't know for sure that the feature is available; old @@ -433,18 +435,7 @@ linux_test_for_tracefork (int child_pid) /* We got the PID from the grandchild, which means fork tracing is supported. */ -#ifdef GDBSERVER - /* Do not enable all the options for now since gdbserver does not - properly support them. This restriction will be lifted when - gdbserver is augmented to support them. */ - current_ptrace_options |= PTRACE_O_TRACECLONE; -#else - current_ptrace_options |= PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK - | PTRACE_O_TRACECLONE | PTRACE_O_TRACEEXEC; - - /* Do not enable PTRACE_O_TRACEEXIT until GDB is more prepared to - support read-only process state. */ -#endif + current_ptrace_options |= PTRACE_O_TRACECLONE | additional_flags; /* Do some cleanup and kill the grandchild. */ my_waitpid (second_pid, &second_status, 0); @@ -542,3 +533,9 @@ linux_ptrace_init_warnings (void) linux_ptrace_test_ret_to_nx (); } + +void +linux_ptrace_set_additional_flags (int flags) +{ + additional_flags = flags; +} diff --git a/gdb/common/linux-ptrace.h b/gdb/common/linux-ptrace.h index 38bb9ea..e5094df 100644 --- a/gdb/common/linux-ptrace.h +++ b/gdb/common/linux-ptrace.h @@ -90,5 +90,6 @@ extern int linux_supports_tracefork (void); extern int linux_supports_traceclone (void); extern int linux_supports_tracevforkdone (void); extern int linux_supports_tracesysgood (void); +extern void linux_ptrace_set_additional_flags (int); #endif /* COMMON_LINUX_PTRACE_H */ diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index bf6f586..289ae41 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -4980,6 +4980,12 @@ Enables printf debugging output."), sigdelset (&suspend_mask, SIGCHLD); sigemptyset (&blocked_mask); + + linux_ptrace_set_additional_flags (PTRACE_O_TRACESYSGOOD + | PTRACE_O_TRACEVFORKDONE + | PTRACE_O_TRACEVFORK + | PTRACE_O_TRACEFORK + | PTRACE_O_TRACEEXEC); }