From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from omta036.useast.a.cloudfilter.net (omta036.useast.a.cloudfilter.net [44.202.169.35]) by sourceware.org (Postfix) with ESMTPS id 549A23858D33 for ; Tue, 6 Feb 2024 17:50:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 549A23858D33 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tromey.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 549A23858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=44.202.169.35 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707241816; cv=none; b=ug4zqaRBsCRbUCOPtCw+X19+dg8jKq6j+QLeVwGSAypIKzVLn9XjzElP7Ae9FUXfHsVo9XafTzZWtjtiFPZIWL9tpQnfAI707clDIjWMpCxh549jy3Gvp7Cj2EL9OvtVzxyl9XLdQdikvbaYD00dk/KECRWH6dXg9ME8nuwNOvQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707241816; c=relaxed/simple; bh=08f4ofrDuQGuNb3zSLhCv7sarZXLQt2rsthFgwguH3A=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=C3+ZPpaun0i80pZq4vn11L7s3ErNtKhqn/J0ByHB1GHxMxDoy/M+g5133Op7hbZUmQvThfeJDO+h1kDnsHCCFzyLwhphikPAFX9JR2RJzYDbMgoj3/GJ/xMH5M3ERFuwx1kE3lj8MJ5k+7gw8rObzYnVb5etlu87hZopCzRLCP4= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from eig-obgw-6007a.ext.cloudfilter.net ([10.0.30.247]) by cmsmtp with ESMTPS id XKporvHMV8uLRXPaHroH2L; Tue, 06 Feb 2024 17:50:14 +0000 Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTPS id XPaHruwFbR2g0XPaHr7fba; Tue, 06 Feb 2024 17:50:13 +0000 X-Authority-Analysis: v=2.4 cv=WvOIMsfv c=1 sm=1 tr=0 ts=65c27155 a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=k7vzHIieQBIA:10 a=Qbun_eYptAEA:10 a=pGLkceISAAAA:8 a=7K-zUSnz8Fxgumx0-FkA:9 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date:References :Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=VQYrZrEkjxfk/TZd8X0THuazp8kryXRx5E4fSp24Jsc=; b=O8LI2WyxQQqFhQOxQg+QVG961D YjmVPXBe2gkuKeJ7skURXx/yP8amsIAxIB50KY9AO4+2vm1PjBIJeSVRBicOKaGnS2dMKJojNx4xo MfLCRvgN+lhe1l+0o8wJMvq5j; Received: from 97-122-68-157.hlrn.qwest.net ([97.122.68.157]:56872 helo=murgatroyd) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96.2) (envelope-from ) id 1rXPaG-000UCL-2T; Tue, 06 Feb 2024 10:50:12 -0700 From: Tom Tromey To: Matheus Branco Borella Cc: gdb-patches@sourceware.org, aburgess@redhat.com Subject: Re: [PATCH v2] Add support for symbol addition to the Python API References: <20230707231352.61735-1-dark.ryu.550@gmail.com> <20240113013631.109348-1-dark.ryu.550@gmail.com> X-Attribution: Tom Date: Tue, 06 Feb 2024 10:50:11 -0700 In-Reply-To: <20240113013631.109348-1-dark.ryu.550@gmail.com> (Matheus Branco Borella's message of "Fri, 12 Jan 2024 22:36:32 -0300") Message-ID: <875xz1eb3g.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - box5379.bluehost.com X-AntiAbuse: Original Domain - sourceware.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - tromey.com X-BWhitelist: no X-Source-IP: 97.122.68.157 X-Source-L: No X-Exim-ID: 1rXPaG-000UCL-2T X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 97-122-68-157.hlrn.qwest.net (murgatroyd) [97.122.68.157]:56872 X-Source-Auth: tom+tromey.com X-Email-Count: 7 X-Org: HG=bhshared;ORG=bluehost; X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-CMAE-Envelope: MS4xfF7M/LBuNktfqt+qBhRsHM1ifxl33jOraooWxnt7/FbG6qmyBGy/gZTwVN1StKdP7033OPTDBVm6SMPd0+YeAEVPXojpO6O9ibYsw1UQ8U1jppIZf+Zy d5fZOue1VySvDg1lArQjP2F3UvZLZ81EMHJbeeiLfysiPiJ4CqVQjOfO64waIlZeXOaqV6yMhMzlYBwxEx1NmHFfJjTRTCtv0ik= X-Spam-Status: No, score=-3016.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,JMQ_SPF_NEUTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: >>>>> Matheus Branco Borella writes: > I had to walk away from this for a while. I'm pinging it now and I've updated > the code so that it works on master. Thank you for the patch. > This patch adds support for symbol creation and registration. It currently > supports adding type symbols (VAR_DOMAIN/LOC_TYPEDEF), static symbols > (VAR_DOMAIN/LOC_STATIC) and goto target labels (LABEL_DOMAIN/LOC_LABEL). Symbol domains recently went through a change. Also, a patch that changes the Python API requires a documentation change and also an entry in NEWS. > In the same vein, PC-based function name lookup also does not work, although > there may be a way to have the feature work using overlays. I guess because nothing makes any blocks. However this seems like a kind of big issue to me, because it means that by-name lookups will appear to succeed ("function xyz is at address 0xaaaaa") but then stopping in that function won't show the name. > + virtual void register_msymbol (const std::string& name, gdb style puts the "&" next to "name", not next to "string". There's a lot of instances of this. > +/* Data being held by the gdb.ObjfileBuilder. > + * > + * This structure needs to have its constructor run in order for its lifetime > + * to begin. Because of how Python handles its objects, we can't just reconstruct > + * the object structure as a whole, as that would overwrite things the runtime > + * cares about, so these fields had to be broken off into their own structure. */ gdb doesn't use the "leading *" style of comment. > + /* We need to tell GDB what architecture the objfile uses. */ > + if (has_stack_frames ()) > + of->per_bfd->gdbarch = get_frame_arch (get_selected_frame (nullptr)); > + else > + of->per_bfd->gdbarch = current_inferior ()->arch (); gdb exposes a gdb.Architecture, maybe we could let the Python code specify this. > +/* Parses a language from a string (coming from Python) into a language > + * variant. */ > + > +static enum language > +parse_language (const char *language) > +{ > + if (strcmp (language, "c") == 0) > + return language_c; > + else if (strcmp (language, "objc") == 0) > + return language_objc; I think this should call language_enum instead. > + if (language_name == nullptr) > + language_name = "auto"; I think it's kind of weird to use auto here. > +/* Builds the object file. */ > +static PyObject * > +objbdpy_build (PyObject *self, PyObject *args) > +{ > + auto builder = validate_objfile_builder_object (self); > + if (builder == nullptr) > + return nullptr; > + > + if (builder->inner.installed) > + { > + PyErr_SetString (PyExc_ValueError, "build() cannot be run twice on the \ > + same object"); > + return nullptr; > + } > + auto of = build_new_objfile (*builder); There's a rule in the Python layer in gdb that code that calls into gdb has to wrap the call in a try/catch and use GDB_PY_HANDLE_EXCEPTION. This is because a lot of gdb code can throw exceptions, but letting an exception cross the Python boundary is catastrophic. > + auto objpy = objfile_to_objfile_object (of).get (); > + Py_INCREF(objpy); > + return objpy; objfile_to_objfile_object returns a new reference so I think the incref is wrong here. We try to avoid explicit inc/dec-refs in gdb anyway. Tom