From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10270 invoked by alias); 16 Mar 2015 11:29:35 -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 10255 invoked by uid 89); 16 Mar 2015 11:29:34 -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; Mon, 16 Mar 2015 11:29:32 +0000 Received: from sasl.smtp.pobox.com (unknown [127.0.0.1]) by pb-sasl1.pobox.com (Postfix) with ESMTP id 8036337003; Mon, 16 Mar 2015 07:29:30 -0400 (EDT) Received: from pb-sasl1.int.icgroup.com (unknown [127.0.0.1]) by pb-sasl1.pobox.com (Postfix) with ESMTP id 6C61337001; Mon, 16 Mar 2015 07:29:30 -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 8F93934000; Mon, 16 Mar 2015 07:29:29 -0400 (EDT) From: Andy Wingo To: Alexander Smundak Cc: Doug Evans , gdb-patches Subject: Re: [RFC] [PATCH] Provide the ability to write the frame unwinder in Python References: <21714.40641.510825.30998@ruffy2.mtv.corp.google.com> <54E71694.1080304@redhat.com> <87ioei31uj.fsf@igalia.com> <87d24p19tt.fsf@igalia.com> <54FD7DAA.7010603@redhat.com> <87twxrncld.fsf@igalia.com> Date: Mon, 16 Mar 2015 11:29:00 -0000 In-Reply-To: (Alexander Smundak's message of "Wed, 11 Mar 2015 11:48:21 -0700") Message-ID: <87ioe1dvu2.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: B5D3E956-CBCF-11E4-86D1-96E29252DF99-02397024!pb-sasl1.pobox.com X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00443.txt.bz2 Hi :) [-pmuldoon as he has already given an LGTM] On Wed 11 Mar 2015 19:48, Alexander Smundak writes: >> What do you think about merging the SnifferInfo and UnwindInfo objects >> into an EphemeralFrame object? It would be nice to use the same nouns >> on the Guile and Python sides. > > I prefer the separation of the input (SnifferInfo) and output (UnwindInfo). > I am surprised someone from Guile side is advocating stateful interface :-) I know what you mean, but because there should be a link between them -- in that saved register set should be checked for validity against the architecture of the incoming frame -- then given that, joining them makes sense to me. Otherwise you'd have to pass the SnifferInfo to the UnwindInfo constructor, which would be fine but what if we could avoid it by joining them... And then there's the question of what do you call the two things -- SnifferInfo is a bad name, for starters. It's a frame, albeit some ephemeral kind of frame. It's literally a struct frame_info* under the hood. Currently you can read a register, but you might want to do other frame things on it -- get_next_frame, get_frame_arch, frame_relative_level, etc. WDYT about the name EphemeralFrame ? > In fact, I'd rather avoid having modifiable UnwindInfo at all -- a sniffer can > return a 2-tuple of (, ), > where FrameId instance can be created by a class method > gdb.SnifferInfo.frame_id_build_xxx(). Can we agree on that? Regarding the return value, I think returning a 2-tuple is a bad idea for all of the reasons I have previously mentioned and which you have not responded to :) * bad error reporting (e.g. gdb.Value width doesn't match register width, how to map that back to some source location or program state) * bad extensibility (e.g. what about stop_reason? what about other register states?) * bad ergonomics (is the frame id first, or is it the saved registers?) On the other hand to have to give a name the result object is painful too -- "this is an UnwindInfo and it is composed of blah blah". For that reason I joined the result with the ephemeral frame object, to avoid introducing another concept. But checking "did this unwinder succeed" via "did the unwinder set the frame ID on this ephemeral frame" is not nice either. How about let's meet somewhat halfway. * We rename SnifferInfo to EphemeralFrame. * Unwinders return UnwindInfo (better name welcome) on success or None/#f otherwise * UnwindInfo takes EphemeralFrame as constructor arg - the EphemeralFrame must be valid - in practice there is only ever one EphemeralFrame alive because unwinding is not recursive * UnwindInfo also takes frame ID as positional constructor args - setting frame_id is the only thing an unwinder *must* do, so this makes an invariant "if return value is an UnwindInfo, then it is valid and has all necessary info" * UnwindInfo has add_saved_register() API (see discussion with Pedro here: http://article.gmane.org/gmane.comp.gdb.patches/105538) * After accepting an UnwindInfo as an unwinder return result, py-unwinders.c / scm-frame-unwinders.c marks the UnwindInfo as frozen so that add_saved_register() etc can't alter the state * continuation of unwinder call also checks that the ephemeral frame on the unwindinfo is valid Example of use: def unwind(frame): if we_can_handle(frame): var ret = UnwindInfo(frame, fp, pc) ret.add_saved_register(r0) return ret I will rework the Guile patch along these lines, and then hopefully I am done reworking :) Andy