From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18049 invoked by alias); 10 Mar 2015 17:36:54 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 17967 invoked by uid 89); 10 Mar 2015 17:36:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_NEUTRAL autolearn=ham version=3.3.2 X-HELO: sasl.smtp.pobox.com Received: from pb-sasl1.int.icgroup.com (HELO sasl.smtp.pobox.com) (208.72.237.25) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 10 Mar 2015 17:36:51 +0000 Received: from sasl.smtp.pobox.com (unknown [127.0.0.1]) by pb-sasl1.pobox.com (Postfix) with ESMTP id D91303B5A4; Tue, 10 Mar 2015 13:36:49 -0400 (EDT) Received: from pb-sasl1.int.icgroup.com (unknown [127.0.0.1]) by pb-sasl1.pobox.com (Postfix) with ESMTP id D068E3B5A3; Tue, 10 Mar 2015 13:36:49 -0400 (EDT) Received: from rusty (unknown [88.160.190.192]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by pb-sasl1.pobox.com (Postfix) with ESMTPSA id E514B3B5A2; Tue, 10 Mar 2015 13:36:48 -0400 (EDT) From: Andy Wingo To: Pedro Alves Cc: gdb-patches , Alexander Smundak , Doug Evans Subject: Re: Frame sniffers in Python/Guile/* References: <87d24r4jgx.fsf@igalia.com> <54FDBE7B.9070405@redhat.com> <87lhj6q8rl.fsf@igalia.com> <54FF1D65.8020401@redhat.com> Date: Tue, 10 Mar 2015 17:36:00 -0000 In-Reply-To: <54FF1D65.8020401@redhat.com> (Pedro Alves's message of "Tue, 10 Mar 2015 16:35:49 +0000") Message-ID: <8761a8oitv.fsf@igalia.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Pobox-Relay-ID: 07D9F890-C74C-11E4-B76E-B058D0B8C469-02397024!pb-sasl1.pobox.com X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00258.txt.bz2 Greets :) On Tue 10 Mar 2015 17:35, Pedro Alves writes: > On 03/09/2015 07:18 PM, Andy Wingo wrote: > >> On Mon 09 Mar 2015 16:38, Pedro Alves writes: >> >>> The sniffer callback is only supposed to identify whether the >>> unwinder can unwind, not do any unwinding at all. :-/ >> >> Unfortunately you have to do some unwinding in order to compute a frame >> ID, and the "unwind->this_id" callback runs in approximately the same >> environment as the sniffer. > > Can you give an example, so we can understand the requirements better? Yes. For example we are unwinding the innermost frame, frame 0, from the sentinel. The Guile sniffer is called to see if any Guile unwinder can handle the frame. I have a Guile unwinder that checks if the PC is within any JIT-compiled page in V8. It has to traverse the V8 heap, basically, to see if it can handle the page. Once the Guile V8 sniffer has enough information to know whether it can unwind the frame or not, it _also_ has enough information to compute the frame ID for frame 0 -- namely, since it knows what V8 object contains the code, it knows how to interpret the registers to find the frame pointer and it knows about the stack layout, so it can compute the start PC and the start SP easily, which is the frame ID. For the same reason it also knows how to unwind the PC, SP, and FP. I guess by "you have to do some unwinding in the sniffer" I just meant that users (ignorant ones like me :) of the Guile/Python interface to unwinders will have to do some computation over the state of the inferior in the sniffer callback to know whether to handle the frame or not -- and that relatively speaking the other parts of the unwinder interface are trivial. > The main problem would be unwind-past-this_frame: that is, something doing > get_prev_frame(this_frame), get_frame_id (this_frame) or something like that > while inside the sniffer. This is impossible in the proposed Guile or Python patches because the wrappers for this_frame are not frames. You can't actually even get to any linked frame from the "ephemeral frame object" (in Guile) or the "sniffer info object" (in Python). What can happen is that someone could inadvertently cause recursion to this_frame's unwinder -- for example via get_prev_frame on get_selected_frame(), or even just by calling get_selected_frame() while unwinding frame 0. In this case -- somehow recursing to unwind this_frame -- my patch makes get_prev_frame(next_frame) return NULL. But you'd have to have a next_frame to begin with, so this is OK. It doesn't add strangeness. True, this can make get_selected_frame() return the sentinel frame if recursion happens while unwinding frame 0, but that case appears to be already reachable otherwise -- see unwind_to_current_frame. > Unwinding registers from the next frame to build a cache to compute > the frame id or figure out where registers were saved is ok. Yep, no problem here. >>> See here for example: >>> >>> https://www.sourceware.org/ml/gdb-patches/2013-11/msg00602.html >> >> Is it important to use frame_unwind_got_memory et al for results? > > I don't think I understand this question. I think I misunderstood the issue, please ignore the question. (I thought it was that because you used frame_unwind_got_memory to produce GDB values for saved registers, and those values linked to a frame by ID, that somehow caused recursion when computing the frame_id for this_frame. I was wondering if it were somehow necessary to build register values by frame_unwind_got_memory and not the value-building API in Python or Guile. It seems that is not the case.) >> In my unwinders I am just returning values using e.g. > >> >> (value-dereference >> (value-cast (ephemeral-frame-read-register ef "rbp") void**-type)) >> >> or something similar. If this is important, we will need to provide an >> interface that looks much more like the unwind interface instead of a >> simple Maybe :( > > I'm afraid I don't know what "Maybe" is. My apologies! The Python patch calls the return value of the sniffer the "unwind info". If the sniffer returns None, then the sniffer is considered to fail, and otherwise the return value is the "unwind info". A Maybe is just an idiom for a data type that is either an instance of T or None. Although I'm a schemer at heart I have enjoyed thinking about program design in types recently, but I see I have added to the confusion. What I meant was: in both the Python and the Guile patches, the unwinder interface is essentially a function that either punts, or it does all of the unwinding at once. It doesn't reflect the nuances that the sniffer, the this_id callback, and the prev_register callbacks are all called at different times. I like the simplification but I do see why the low-level GDB interface is different (avoiding unnecessary prev_register computation, chiefly). The question was, is there a problem with the all-at-a-time approach that the Guile and Python patches use? I am thinking that no, that it's OK to do it all at once. >> [Should the Guile bindings avoid assuming that] it's sensible to get >> the current architecture, or the selected or current frame? > > It probably makes sense to refer to those. As long as e.g., > can e.g., make get_current_frame() really return the current frame. > But from what I understood, we end up instead falling back to NULL? > If we're going to give the wrong answer, it makes me nervous to have > to support doing it going forward. :-/ If I understand the concern, I think I answered it above when referring to the new way that get_prev_frame could return NULL. >> I was pleased that $pc, $sp, and $fp >> "worked" otherwise, so I hoped to avoid encoding architecture-specific >> things. Alas. (Incidentally, my patch still has the problem that >> passing in a pseudo register will barf.) > > Speaking of which, it occurred to me that we'll likely have problems > on architectures that hide all raw registers (by making them unnamed) > behind pseudo registers, like MIPS. What's the solution here then? Perhaps Alexander was right that register numbers have a place :) Thank you again for the review and for thinking this through, Pedro. I know that it takes time and I really do appreciate it. Cheers, Andy