From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47582 invoked by alias); 20 Mar 2015 00:15:53 -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 47572 invoked by uid 89); 20 Mar 2015 00:15:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-ob0-f201.google.com Received: from mail-ob0-f201.google.com (HELO mail-ob0-f201.google.com) (209.85.214.201) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 20 Mar 2015 00:15:51 +0000 Received: by obcwp18 with SMTP id wp18so5367790obc.1 for ; Thu, 19 Mar 2015 17:15:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:mime-version:content-type :content-transfer-encoding:message-id:date:to:cc:subject:in-reply-to :references; bh=kYXBKP4I1JfYwicJhn2gsTmqGI7nRY9/9wy0LtUIflA=; b=UqJB76obH+EdVXgk2/72RuJlCu7JdORdg6hwvf7PVfl3nb/6d+GMqtZ1P2kFCfaq2q TyXv/wOz9qSel/ItnlyCM+2WRFGN6LbbWSXRsCgTZcK0ER2Qz2dzrlfGRD4hz/Uqy/OH YcTlEj5B53ljE0J+k/9W5FalLxrwUK1b55mjpNI7oiwzBk59JyhrNbnT/NaxJQ/znjAl PNC/bc9X3CWu7S9XaSWYN3XByCtFu8YeeyQkrpkDpJMu3lS14ao7V4gWCgqjLsSnMSq1 RQUa1Z1ivptEgUPcwLphrm4kHTgZjCVAsfZONWj6uOogl97ALbMrsMz0nqlt8yLT7hq4 /zhA== X-Gm-Message-State: ALoCoQlkANd9aNP8JISq9x9nvyUEj+uo6eAHSWz/gNS52yjiYr+AFNHrpU/ccaawYKrzYWGWS5PQYh/9WRZ/Jyj19KjNLQMJG1kofcXGRAMWRnYkzXkHfo2Q5EaTLUonZcbU3gpKn7HfBeCq44QKbQ1z1H7VtGqxDLpTLOGuJvNPv1YUgM33KN0= X-Received: by 10.182.168.105 with SMTP id zv9mr81137995obb.6.1426810549495; Thu, 19 Mar 2015 17:15:49 -0700 (PDT) Received: from corpmail-nozzle1-2.hot.corp.google.com ([100.108.1.103]) by gmr-mx.google.com with ESMTPS id l42si198848yhq.1.2015.03.19.17.15.48 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Mar 2015 17:15:49 -0700 (PDT) Received: from ruffy2.mtv.corp.google.com ([172.17.128.107]) by corpmail-nozzle1-2.hot.corp.google.com with ESMTP id eP4QZJie.1; Thu, 19 Mar 2015 17:15:49 -0700 From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <21771.26292.146330.287991@ruffy2.mtv.corp.google.com> Date: Fri, 20 Mar 2015 00:15:00 -0000 To: Andy Wingo , Alexander Smundak cc: gdb-patches Subject: Re: [RFC] [PATCH] Provide the ability to write the frame unwinder in Python In-Reply-To: <87fv918kyf.fsf@igalia.com> 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> <87ioe1dvu2.fsf@igalia.com> <87sid4atms.fsf@igalia.com> <87fv918kyf.fsf@igalia.com> X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00613.txt.bz2 Andy Wingo writes: > On Thu 19 Mar 2015 01:36, Alexander Smundak writes: > > >> Regarding the result of an unwinder/sniffer, > >> If I approach this from the point of view of what's > >> easy to explain, it feels like the result of an Unwinder > >> is a Frame. > > It is logical, but that's not what GDB core expects from the unwinder. > > I hesitate to have Python frame unwinder API differ too much from > > the underlying GDB core API. > > I agree with Alexander. If it were really a frame you'd have to expose > a frame constructor to Python/GDB, and then would the resulting frame be > interned? What would happen if you built a frame but then returned > None, or threw an exception? Would there be side effects to the frame > cache? And if not, could you hold on to the frame? Would it be equal > to frame.newer().older() ? Better to return data instead, which GDB > uses to build the actual frame. Yeah. Having read everything through last night, here are some thoughts in no particular order. Don't rush off and resubmit another patch (unless you want to). I think there's enough on the table now to get the high level details decided on. I do see a few implementation issues in the patches but I don't want to discuss them prematurely. 1) I like the idea of the input to the unwinder being a frame (of some kind, ephemeral or whatever). In python, let's use EphemeralFrame instead of SnifferInfo. But the only real output of the sniffing action is a boolean saying "I recognize this frame". ref: frame-unwind.c:frame_unwind_try_unwinder(). If the unwinder wants to cache something during sniffing that's its business, and it can cache whatever is appropriate. IIUC, What the patches do is use the result of the sniffing action to both say "I do/don't recognize this frame" and to return an object that is the cache of random data (which it calls "UnwindInfo"). Thus "UnwindInfo" is as good a name as anything. 2) IIUC, setting aside hitting the outermost frame and such, it's a rule that some unwinder will recognize the frame. Therefore "ephemeral" isn't quite right, even "real" frames are ephemeral since we toss the frame cache when the inferior resumes. OTOH, we need to make progress and I'm just throwing this out there. "ephemeral" is fine with me: we don't use that term for anything else which is nice. 3) We need to allow sniffers to record anything they want in gdb.UnwindInfo/. In Python I'm guessing one can just add extra attributes to the object or subclass. In Guile I guess one could use an external container (weakly?) indexed by the object. In both cases the documentation should make recommendations to the reader. [If it does and I missed it apologies.] 4) I gather the UnwindInfo/ objects provide direct support for recording registers so that one doesn't have to go back out to the extension language to implement frame_unwind.prev_register. Plus, they also create the frame id when creating the UnwindInfo object so that frame_unwind.this_id doesn't have to call back out to the extension language. That's fine. Just wanted to write this down to confirm. Both implementations allow for adding future support for having frame_unwind.prev_register/this_id/stop_reason calling back out to the extension language if we need it. 5) The docs don't make any mention of target endianness in the saved register values. They probably should. 6) I noticed several routines for building frame ids in Python. Both Python and Guile support keyword based arguments to functions. Can we just have one frame id builder function and each take sp and pc (and special if we need to) as keyword arguments? E.g. (make-unwind-info ephem-frame #:sp sp #:pc pc) But see (7) below. 7) If we ever need to use frame ids in the extension language IWBN if they were their own object, in which case we might have (make-unwind-info ephem-frame #:frame-id frame-id) but then I'm wondering if there should be an unwind-info-set-frame-id! function and move sp,pc there. IOW change make-unwind-info to this so that the various ways of creating a frame idea are don't complicate make-unwind-info. [I realize one might want to not let make-unwind-info return an object without a frame id, but I don't see this as a problem. uwscm_sniffer could verify a frame id is present.] (let ((unwind-info (make-unwind-info ephem-frame))) (unwind-info-set-frame-id! #:sp sp #:pc pc) (unwind-info-add-saved-register! unwind-info "rip" rip) ...) And for python: I'm not sure where to put the UnwindInfo creator (factory method). Do we need one? Can we just create them via normal object construction? unwind_info = gdb.UnwindInfo(ephem_frame) unwind_info.set_frame_id(...) unwind_info.set_previous_frame_register(...) This could all be done in pure Python (I think), and then pyuw_sniffer could process the object looking for expected members(attributes) with expected types (and throw an error if there's a problem). At the least let's combine unwind_info_with_id, frame_id_build_special, and frame_id_build_wild into one function that takes keyword arguments for each of sp, pc, special. 8) Re: set_previous_frame_register vs unwind-info-add-saved-register! Dunno. On this particular point I guess I don't have a strong enough interesting in being consistent. I'll let you guys decide. --- That's it for now.