From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 64835 invoked by alias); 20 Mar 2015 18:32:15 -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 64808 invoked by uid 89); 20 Mar 2015 18:32:14 -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-pa0-f74.google.com Received: from mail-pa0-f74.google.com (HELO mail-pa0-f74.google.com) (209.85.220.74) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Fri, 20 Mar 2015 18:32:13 +0000 Received: by pablj1 with SMTP id lj1so7221885pab.0 for ; Fri, 20 Mar 2015 11:32:11 -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=PzN3/JWkzHSNYzWBIuT52aTIJ4KI9dM95cXlDTB3ORU=; b=l/XIkTyznuLMzB2tm29NBdOqdLOh1GfLvBaVd3LOVEuJquKy5Fqk/jo4HHdQCCROk+ KLD4DtLGTySg+kpA1IrS9LxNN7u0QQQW+LOiNeYA4mBG3alAKfBDEBUJ9ai56WWg052F Ducy2pUfu2UHknEZbx7QPW2g0tt9S0PxyA7gbRZEyL5RLODyh1kJIThT6wfHBWjO/87s KJcMNyI4XFF+VB8VFeGMOhZM68UFGmIUMgMnTX+gWAlB8kfqQAqNMwN5Je5LVtdjxwcz tBxyFzTmItBdF9hQeRumwG5PISKJnsV1obSUHi7qBf+OEf2QFuKsHF9C3WjsT3T3MwOu ZwuA== X-Gm-Message-State: ALoCoQm/kwdnjEC+EotGdZv0eP81gJRYQtGJGkjB2X3+SCNh5y2t/jYUOEUHrFQlx11KTtkVWCUj7DQBgr8/eUdT0UAQcrI1L2vfxc/5s76Uvrp1czSk4AKj3hHCA/+UhEBDqLIQ1ocuEcu2f1b0hvxV2tqwA2aTTBxrBYTlYtO9MkelS783EyY= X-Received: by 10.66.66.196 with SMTP id h4mr88832079pat.5.1426876331552; Fri, 20 Mar 2015 11:32:11 -0700 (PDT) Received: from corpmail-nozzle1-1.hot.corp.google.com ([100.108.1.104]) by gmr-mx.google.com with ESMTPS id 3si368735yhe.0.2015.03.20.11.32.10 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 20 Mar 2015 11:32:11 -0700 (PDT) Received: from ruffy2.mtv.corp.google.com ([172.17.128.107]) by corpmail-nozzle1-1.hot.corp.google.com with ESMTP id qQjHDFYP.1; Fri, 20 Mar 2015 11:32:11 -0700 From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <21772.26533.9361.394318@ruffy2.mtv.corp.google.com> Date: Fri, 20 Mar 2015 18:32:00 -0000 To: Andy Wingo Cc: Alexander Smundak , gdb-patches Subject: Re: [RFC] [PATCH] Provide the ability to write the frame unwinder in Python In-Reply-To: <87619w6pmt.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> <21771.26292.146330.287991@ruffy2.mtv.corp.google.com> <87619w6pmt.fsf@igalia.com> X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00667.txt.bz2 Andy Wingo writes: > Hi, > > Thanks for taking the time to look at the patches, Doug! > > On Fri 20 Mar 2015 01:15, Doug Evans writes: > > > 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. > > Just to throw out some other names -- PendingFrame. Alexander mentioned > InspectedFrame. IntermediateFrame. FrameTemplate. SkeletonFrame. > MinimalFrame. FrameStub. > > Dunno, just some ideas :) I like PendingFrame. It's a frame, but not a real one yet. > > 3) We need to allow sniffers to record anything they > > want in gdb.UnwindInfo/. > > I see from your point (4) later that you are thinking that if we add > some kind of callback interface to UnwindInfo that e.g. Guile code might > need to fetch arbitrary data from the UnwindInfo. > > You also note correctly that this is not currently something that the > patches require -- the UnwindInfo contains all of the information, and > the C prev_register/this_id callbacks just look into the UnwindInfo > data. > > If we did add a callback interface I don't think there would be a > problem. Cool. > Let's say we want to add a way for prev_register to call back into > Guile. If the prev-register callback is added to the unwind info via > unwind-info-set-prev-register-callback! or something then the callback > can be a closure that captures the data it needs. Or it can use an > external weak hash table. Or when we add the > set-prev-register-callback! interface, we can add some other associated > interface to store data in the unwind-info. There are multiple fine > options here. > > > 5) The docs don't make any mention of target endianness > > in the saved register values. They probably should. > > +1 to Alexander's answer -- as they are GDB values, there shouldn't be > an issue, should there? Well, as I wrote in my reply to Alexander, we can't control how the user will create these values. They *might* want to do something off the beaten path, so to speak. We should specify what's required. > > 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) > > This would be fine; and actually at this point the kwarg is unnecessary. I'd have make-frame-id take kwargs though. > (make-unwind-info ephem-frame frame-id) > > > but then I'm wondering if there should be an > > unwind-info-set-frame-id! function and move sp,pc there. > > I agree with Alexander that this isn't great. The only thing that an > unwinder *must* do is set a frame ID. It must produce a frame_id at the > same time as the sniffer returns TRUE. (They are different callbacks > but always called one after the other.) Therefore it makes sense to > have the invariant: > > Is it an UnwindInfo? Then it definitely has a frame ID. > > That way it takes away one possible error case. I recognize these points, but I guess in this particular case I don't feel as strongly. But I don't mind making frame-id a required argument to make-unwind-info. So go for it. > Also you wouldn't want > to set the frame ID on an UnwindInfo twice, it doesn't make sense. One could say the same of saved registers. > > 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? > > In Guile you have to give it a name of course -- make-unwind-info. But > in Python I liked Alexander's use of a factory method. But I dunno, > I'll leave the Python discussion to you all :) A factory method is a layer of abstraction which is nice (e.g., if later we need to change/extend things it'd be easier to add a new method or whatever than to play with the constructor). So ok let's use a factory method. > > 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. > > If you don't have such an interest on being consistent for this piece > then I guess the easy way is to leave these as they are then. Avoids > multiple-day awkward backs and forths :) But if you decide they need to > be the same let us know. > > Speaking for myself I prefer the Guile one of course, but I prefer > executive decisions over having to endure more days of naming > discussions over email :) I'll make an executive decision to let you guys decide. 1/2 :-) "But seriously, ..." Let's go with add_saved_register / unwind-info-add-saved-register! Sorry Alexander. :-) I have to pause whenever I read "previous" vs "next": the meaning is clear in frame-unwind.h, but it just doesn't stick. E.g., The next frame to unwind is the previous frame on the stack. Thank goodness frame.c uses all of next/down/inner/younger and prev/up/outer/older.