From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 61454 invoked by alias); 4 Mar 2015 02:36:46 -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 61433 invoked by uid 89); 4 Mar 2015 02:36:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 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-ig0-f174.google.com Received: from mail-ig0-f174.google.com (HELO mail-ig0-f174.google.com) (209.85.213.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 04 Mar 2015 02:36:44 +0000 Received: by igbhn18 with SMTP id hn18so33365212igb.2 for ; Tue, 03 Mar 2015 18:36:42 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=Rj5iuAvV+12OCnQ5RurakY8TFxUNdU5j9KFDn/GFXRU=; b=PolWjZRcMVkCNt/dHbFsiXrriEvVaI8DusgS2xSdPFlgyT5wT50XS0qTYJxZ0o4sz/ 8GXtSIRJr7TvXQLCehfyB+ApLV2x/5ZDWXNrIBlnv+wadYGPdycYkoN101liHjHAoINv HZkxVZLLNCphffQB/6dbpI0fh9yg3Vxu9ECpboXqybcvHcHIdS2KIjKAaCAoOVeCBSWb lsZW/i1k5qE0+yrINNMadbNou4tUlBot0xDbYtJlKT8KFVssqcJiuKgPoVfQr4uBVDL5 zc9LwHzcq3urr3/yDr0614GM4PiUyPrsrCgPo6f/8XZpCaVYNXUQn1h4OsxhtiJzm7NM WB8w== X-Gm-Message-State: ALoCoQlFcuHbtL6IuxLLt0KuBScST7cx5GSjCz2n5kAkpDk2Tfieu0bP+wQdwrQAqd8shEh0qouW MIME-Version: 1.0 X-Received: by 10.107.36.9 with SMTP id k9mr7298180iok.2.1425436602615; Tue, 03 Mar 2015 18:36:42 -0800 (PST) Received: by 10.64.227.110 with HTTP; Tue, 3 Mar 2015 18:36:42 -0800 (PST) In-Reply-To: <87ioei31uj.fsf@igalia.com> References: <21714.40641.510825.30998@ruffy2.mtv.corp.google.com> <54E71694.1080304@redhat.com> <87ioei31uj.fsf@igalia.com> Date: Wed, 04 Mar 2015 02:36:00 -0000 Message-ID: Subject: Re: [RFC] [PATCH] Provide the ability to write the frame unwinder in Python From: Alexander Smundak To: Andy Wingo Cc: Phil Muldoon , Doug Evans , gdb-patches Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-03/txt/msg00113.txt.bz2 >> So here's the new proposal for the Python API, hopefully in >> line with what you have in mind for Guile: >> >> If a sniffer is able to unwind a frame, it should return an instance of >> gdb.sniffer.UnwindInfo class, which has the following methods: >> * UnwindInfo(registers) >> Constructor. `registers' is a tuple of (register_number, register_value) >> 2-tuples for the registers that can be unwound. >> * frame_id_build_wild(SP) >> frame_id_build(SP, PC) >> frame_id_build_special(SP, PC, SPECIAL) >> Sets frame ID by calling the corresponding GDB function. It is an error >> to return UnwindInfo object before one of these methods is called (a >> sniffer should return None if it cannot unwind a frame) >> * set_register(register_number, register_value) >> Adds a 2-tuple to the list of unwound registers. Not sure this is needed. > > You'll need a link to the sniffer_info in order to be able to give good > errors for set_register, to check that the register exists and that the > value is of the correct type and size. For that reason, in my first > draft of a Guile interface, the "ephemeral frame" is like your > sniffer_info and unwind_info together. Perhaps this is a bad idea > though. I am somewhat confused, so maybe my proposal wasn't clear, let me try again. In the current implementation for Python a sniffer return something similar to this (x86_64 architecture): return (((AMD64_RBP_NUM, fp), (AMD64_RIP_NUM, pc), (AMD64_RSP_NUM, sp)), (AMD64_RSP_NUM, AMD64_RIP_NUM)) (the variables `fp', 'pc', 'sp' contain the values of the respective variables in the returned frame). I am proposing to return an object as follows: previous_frame = UnwindInfo(((AMD64_RBP_NUM, fp), (AMD64_RIP_NUM, pc), (AMD64_RSP_NUM, sp))) previous_frame.frame_id_build(sp, pc) return previous_frame or previous_frame = UnwindInfo(((AMD64_RBP_NUM, fp), (AMD64_RIP_NUM, pc))) previous_frame.set_register(AMD64_RSP_NUM, sp) previous_frame.frame_id_build(sp, pc) return previous_frame Checking the type and size of the a register in the implementation of the `set_register' method depends only on the current architecture, not on what's available in the sniffer_info. >>> [W]hy not specify registers as strings, as elsewhere >>> (e.g. gdb.Frame.read_register)? >> My concern is that name lookups are expensive > > Are they? I wouldn't think so, no more than anything that happens in > Python. The function mapping a register name to the number, `user_reg_map_name_to_regnum', compares given register name with all known register names, one at a time. I thought Python interpreter is somewhat smarter than that and use dictionaries for lookups. Even if it is not, there is no need to pile up inefficiency. Actually, it's not that difficult to be able to pass either a register number, or register name. >>> The sniffer_info object is unfortunate -- it's a frame, but without >>> frame methods. You can't get its architecture from python, for >>> example, or get the next frame. More about that later. >> I guess you know by now that it is not a frame. The interface >> reflects that. > > Well. I mean, it's not a frame to Python, but its only state is a > "struct frame_info" pointer, and its only method is also present on > gdb.Frame, so it looks a lot like a frame to me :) > >>> In the read_register() function, I believe you can use >>> get_frame_register_value instead of deprecated_frame_register_read. >> You can't, get frame_register_value wiil assert because the frame >> has no frame ID yet. > > The comment in the source says: > > /* Call `deprecated_frame_register_read' -- calling > `value_of_register' would an assert in `get_frame_id' > because our frame is incomplete. */ > > Whereas get_frame_register_value looks something like this: > > struct value * > frame_unwind_register_value (struct frame_info *frame, int regnum) > { > /* Find the unwinder. */ > if (frame->unwind == NULL) > frame_unwind_find_by_frame (frame, &frame->prologue_cache); > > /* Ask this frame to unwind its register. */ > return frame->unwind->prev_register (frame, &frame->prologue_cache, regnum); > } > > struct value * > get_frame_register_value (struct frame_info *frame, int regnum) > { > return frame_unwind_register_value (frame->next, regnum); > } > > So it doesn't touch THIS_FRAME. Here's traceback I get when I try to call value_of_register: #5 0x00000000006b5212 in internal_error (file=file@entry=0x838760 "<...>/gdb/frame.c", line=line@entry=478, fmt=) at gdb/common/errors.c:55 #6 0x0000000000688796 in get_frame_id (fi=fi@entry=0x24b63f0) at gdb/frame.c:478 #7 0x0000000000551ea3 in value_of_register_lazy (frame=frame@entry=0x24b63f0, regnum=regnum@entry=6) at gdb/findvar.c:290 #8 0x0000000000551fef in value_of_register (regnum=6, frame=0x24b63f0) at gdb/findvar.c:271 I am not sure why this particular comment I've added attracts so much attention :-) > Alexander, did you not run into nasty crashes while doing random Python > things inside your unwind handler? I used to have random crashes before I learned to call `ensure_python_env' to establish Python runtime environment before suing Python API, but after that, no. Do you have an example that you can share?