From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23275 invoked by alias); 30 Oct 2019 23:49:32 -0000 Mailing-List: contact elfutils-devel-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Post: List-Help: List-Subscribe: Sender: elfutils-devel-owner@sourceware.org Received: (qmail 23264 invoked by uid 89); 30 Oct 2019 23:49:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.3 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=poke, HX-Spam-Relays-External:209.85.210.195, H*RU:209.85.210.195, life X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on sourceware.org X-Spam-Level: X-HELO: mail-pf1-f195.google.com Received: from mail-pf1-f195.google.com (HELO mail-pf1-f195.google.com) (209.85.210.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 30 Oct 2019 23:49:30 +0000 Received: by mail-pf1-f195.google.com with SMTP id u9so2846309pfn.4 for ; Wed, 30 Oct 2019 16:49:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osandov-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=y6spMPgMG0UjPlL4AAjwoue4rkB6lZPOYP4MBvp+AfE=; b=0Xepud9vwSk9edcqS1jYyNG8gLYzH4sx9ciItZ1U3RdZeDB6mDEVtfHq3VvkhbC42f 1LgyczWQnew4vhaJhvmG/qmI0RxnQpSLnISJljTeMQwQ9VISxpYtAuJ51pOtFWzv+OPq rzN8I0Z4E4mcNkzMFtZ9YVtP4yS/Vs9ja7c785UPjSpT8YgANZXtNMVDdNU4mdZWmrED ujAoUCSbs3VdZyCZm8N66BSgVWIO5XuDLk5TDocW570KX+ke/fFvCERXLfU0kvx139qw ZRw9aXqbYYSxY/scnItRzJIzqrjo2C5ynud53ObAiQFCXrkMxKh9AhkZOTdE+jPG9obL i77w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=y6spMPgMG0UjPlL4AAjwoue4rkB6lZPOYP4MBvp+AfE=; b=RuFetW1OCfOgKnGNAu2wZ3y4+/GtQeec+JFFnNhnWUkXtFBZAht49MaxAWRwxPw42b V9QPS9fEDq++9WZK556f3aDMgF34QwHXKDUlgkA/VnmCxVteb2J2r5w0UcJmwMxZBAKj YRDTX5f2okuUwZihtU2TThVw0t+g3PlQghb1JEoNzQe/Mz1NUQkqs3p/F/UsaylnIqNP bCE5w/wwDyVkHmuU9kb9irzz3eKWQGFIqXGCCegFTC8BHlcEeO+FYLmxOcDc9MW1TEGI vHXiXssc+1ukr0zowBGHLYLsJ6jeksnsqrjGv2MHF4waqlBPIPT38Uc+tUJtVw3uK7bb FSLw== X-Gm-Message-State: APjAAAW3UWLGSFo3tAeuhooVPUsrf/BG5+bKe7BANrviv6ta/HIhxKnV WGbLgMGvolvTlhrqHTxqBqL+OA== X-Google-Smtp-Source: APXvYqximZGLV+QHqL7Mhs/gmH3ehxKPfqKqBVBuNmsm7wc5H9eSUZYz58B3X5RgZj4KuAsnp3bzEA== X-Received: by 2002:a62:5442:: with SMTP id i63mr2251011pfb.220.1572479367793; Wed, 30 Oct 2019 16:49:27 -0700 (PDT) Received: from vader ([2620:10d:c090:180::3912]) by smtp.gmail.com with ESMTPSA id z18sm999947pfq.182.2019.10.30.16.49.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Oct 2019 16:49:27 -0700 (PDT) Date: Wed, 30 Oct 2019 23:49:00 -0000 From: Omar Sandoval To: Mark Wielaard Cc: elfutils-devel@sourceware.org Subject: Re: [PATCH 3/5] libdwfl: add interface for attaching to/detaching from threads Message-ID: <20191030234926.GI326591@vader> References: <06926ca241daa563aba5398977b3d4acd70e47cd.1570438723.git.osandov@fb.com> <1ce04b119fd9f326a9a7a2d1c0630b45afe35f6e.camel@klomp.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1ce04b119fd9f326a9a7a2d1c0630b45afe35f6e.camel@klomp.org> User-Agent: Mutt/1.12.2 (2019-09-21) X-IsSubscribed: yes X-SW-Source: 2019-q4/txt/msg00087.txt.bz2 On Wed, Oct 30, 2019 at 01:47:52PM +0100, Mark Wielaard wrote: > Hi Omar, > > On Mon, 2019-10-07 at 02:05 -0700, Omar Sandoval wrote: > > libdwfl has implementations of attaching to/detaching from threads > > and > > unwinding stack traces. However, that functionality is only available > > through the dwfl_thread_getframes interface, which isn't very flexible. > > This adds two new functions, dwfl_attach_thread and dwfl_detach_thread, > > which separate the thread stopping functionality out of > > dwfl_thread_getframes. Additionally, it makes dwfl_thread_getframes > > cache the stack trace for threads stopped this way. This makes it > > possible to use the frames after dwfl_thread_getframes returns. > > The advantage of the dwfl_thread_getframes interface is that you cannot > forget to "detach", so no Dwfl_Frames get dangling and (if the process > is life) you don't disrupt it more than necessary. This new interface > seems very simple to get wrong causing leaks and locking up > processes/threads. That is true, although I imagine that use cases which don't need the additional flexibility would continue to use the simpler API. > Also, if we would adopt dwfl_attach_thread then I think it should take > a Dwfl_Thread object not a pid/tid as argument. In that case, we'd probably want to expose the internal getthread function with something like: /* Find the thread with the given thread ID. Returns zero if the thread was processed, -1 on error (including when no thread with the given thread ID exists), or the return value of the callback when not DWARF_CB_OK. */ int dwfl_getthread (Dwfl *dwfl, pid_t tid, int (*callback) (Dwfl_Thread *thread, void *arg), void *arg) __nonnull_attribute__ (1, 3); Then dwfl_attach_thread could be used with either dwfl_getthread or dwfl_getthreads, which is nice. However, a crucial part of this feature is being able to access the Dwfl_Thread outside of the callback. Since the Dwfl_Thread is currently on the stack, I see a couple of options: 1. We keep Dwfl_Thread on the stack and make dwfl_attach_thread() return a copy (like it does in this patch). 2. We always allocate the Dwfl_Thread on the heap and free it before returning from dwfl_getthread(s) _unless_ dwfl_attach_thread() was called. If it was, it will be freed by dwfl_detach_thread() instead. Option 2 sounds better to me. What do you think? > Could you provide some examples where this new interface/style is > beneficial? dwfl_attach_thread could be used to implement something like `gdb -p $pid`: attach to a thread, stop it, and poke at it further. dwfl_detach_thread would be kind of like GDB's `continue` command. I applied these patches to the version of elfutils that I bundle with drgn; you can see how I use the interface here [1]. Basically, drgn has an API to get the stack trace for a thread [2] and get the value of registers (and in the future, local variables) at each stack frame [3]. When I get the stack trace, I use dwfl_attach_thread and dwfl_thread_getframes. The user can then access the frames to their heart's content. When they're done with it, I free everything with dwfl_detach_thread. (The attach/detach is currently tied to the lifetime of the stack trace because I don't yet support actually pausing threads, but I plan to support that, hopefully using the same libdwfl attach/detach API.) Thanks, and sorry for the wall of text :) 1: https://github.com/osandov/drgn/blob/master/libdrgn/stack_trace.c 2: https://drgn.readthedocs.io/en/latest/api_reference.html#drgn.Program.stack_trace 3: https://drgn.readthedocs.io/en/latest/api_reference.html#stack-traces