From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27109 invoked by alias); 8 Feb 2013 18:05:11 -0000 Received: (qmail 27080 invoked by uid 22791); 8 Feb 2013 18:05:08 -0000 X-SWARE-Spam-Status: No, hits=-5.6 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail-vb0-f52.google.com (HELO mail-vb0-f52.google.com) (209.85.212.52) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 08 Feb 2013 18:05:02 +0000 Received: by mail-vb0-f52.google.com with SMTP id fa15so2466014vbb.39 for ; Fri, 08 Feb 2013 10:05:01 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type:x-gm-message-state; bh=Z8A/NZnEMV95PwxgdLHtwRRG58wX1TQX7vmd5eb25kw=; b=eb4EBZPQn/Ufxx5ck7+gowZezDSmc2dUaqMCm3tNpt/ZLH6mUohw/Tyukmep9VHI/4 MMzM+724HoXbYYTMUAEvz1dvJNbZJIM9v5Ua+/atGmOkyqXwTUCoutOKhJkRpwmt2pyD pvINFR4JtwBzEHhrN7fyCjJwLFF/cWAcz473XMiBXXVlDG0vobBBHCarX0ZCewBtdpP2 Gazt63loPqv2XAx0RJLjK3bIzLAeYoMX3crUPulY+LVXQJ7zEQSpW00UGq5FZYaDitVy 8xKrOnTPQIa8vBRyAh3NPBQEMPuNEsmJ8yj2izlfGMo0io75xfJzo3BRmbWgTOOCge92 kbxg== MIME-Version: 1.0 X-Received: by 10.220.149.198 with SMTP id u6mr7630766vcv.52.1360346701026; Fri, 08 Feb 2013 10:05:01 -0800 (PST) Received: by 10.220.5.130 with HTTP; Fri, 8 Feb 2013 10:05:00 -0800 (PST) In-Reply-To: References: <20753.38272.55066.651097@ruffy2.mtv.corp.google.com> Date: Fri, 08 Feb 2013 18:05:00 -0000 Message-ID: Subject: Re: [RFC - Python Scripting] New method gdb.Architecture.disassemble From: Doug Evans To: Siva Chandra Cc: gdb-patches Content-Type: text/plain; charset=ISO-8859-1 X-Gm-Message-State: ALoCoQl+86ZAOyT/t2Se3Eu2DPqdGh2hfJMmdT2krCT8hMmsm/mWN3toQXkVyC7RSnPRcg0c3tUbW4ZRf/GNRQxPUWzYfVX2XHspaGRh2mQFqjb/I1cCPFB2tyWyeQ01jr7qYWpTT0NiepPooKWF03SBX5MEkNRtEFeXZznx14Gz/srGjTrWUY7jpURqZ31mp6jzPOMXbfokl4ArPcE0xQ65o3vLw4YRlg== X-IsSubscribed: yes 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 X-SW-Source: 2013-02/txt/msg00218.txt.bz2 On Tue, Feb 5, 2013 at 5:53 PM, Siva Chandra wrote: > On Tue, Feb 5, 2013 at 3:28 PM, Doug Evans wrote: >> I like the idea, but for an API I wouldn't mind seeing something >> a bit lower level. E.g., skip the higher level disassembler entry >> points in gdb (mixed source/assembly support and all that), and provide >> more direct access to the disassembler. > > The only useful entry point currently available is gdb_disassembly and > I do not think it is a bad entry point. Other disassembly functions in > disasm.c are static. Well, it begs the question what entrypoint into libopcodes does gdb_disassembly use? :-) Anyways, my point is gdb_disassembly is a bit high level for the Python API for me. > However, for the Python API, my patch provides > only one option of whether to include or exclude opcodes in the > disassembled output. Today. Someone will think "Oh look, I can just add a flag and get more features." with no real consideration if that level in the API is the right place for those features. >> I didn't go through it with a fine toothed comb, but here are some questions. >> 1) Can we remove the py_out global? > > At what level do you not want this to be global? I have made it static > to the file in the attached patch. This has been addressed elsewhere. >> 2) It seems like this will export a lot of struct ui_out to the user. >> I'd rather provide disassembly without having to commit to supporting >> struct ui_out in Python. > > I am not very sure I understand this point. My patch does not expose > anything about the struct ui_out to the user/Python API. Python ui_out > is only a way to get results from GDB internals into a Python data > structure. Also, this Python data structure does not depend on > gdb_disassembly's display format. Let's take a step back and assume one wants to add disassembly to Python, without knowing anything about what gdb does and doesn't already provide. If I ask to disassemble an instruction at a particular address I expect to get back a string (with possibly an error thrown or some such if the memory at that address isn't readable). Let's see an enumeration of the other things one might want to get back. - symbol name of a referenced address? - or at least the address so that the caller can do the symbol lookup, which might be preferable - raw bytes of the instruction? - length of the instruction? - anything else? Hmmm, another thing occurs to me: Global State In An API Is Bad. What are the other inputs to disassembly, and should they be parameters? - syntax? - for now we could use the CLI setting as the default - architecture variant? (including endianness) - all aspects of this are presumably encoded in the gdb.Architecture class so no need for an explicit parameter, included here for completeness sake - anything else? If not, we're forcing the caller to do a "save current global state, change it, catch the call to the disassembler so we can properly restore state, restore global state" dance. We need to move away from this in APIs (we're suffering from aspects of this in gdb's internal APIs, let's learn this lesson and apply it). > 2013-02-05 Siva Chandra Reddy > > Add a new method 'disassemble' to gdb.Architecture class. > * Makefile.in: Add entries for the new file python/py-out.c > * python/py-arch.c (archpy_disassmble): Implementation of the > new method gdb.Architecture.disassemble. > (arch_object_methods): Add entry for the new method. > * python/py-out.c: Implementation of a Python ui_out. > * python/python-internal.h: Add declarations for new utility > functions. > * python/python.c (_initialize_python): Initialize Python > ui_out.