From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 37046 invoked by alias); 23 Jan 2017 22:43:55 -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 35544 invoked by uid 89); 23 Jan 2017 22:43:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-HELO: sesbmg23.ericsson.net Received: from sesbmg23.ericsson.net (HELO sesbmg23.ericsson.net) (193.180.251.37) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 23 Jan 2017 22:43:41 +0000 Received: from ESESSHC018.ericsson.se (Unknown_Domain [153.88.253.124]) by (Symantec Mail Security) with SMTP id 68.BD.14025.51786885; Mon, 23 Jan 2017 23:43:34 +0100 (CET) Received: from EUR01-HE1-obe.outbound.protection.outlook.com (153.88.183.145) by oa.msg.ericsson.com (153.88.183.72) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 23 Jan 2017 23:42:26 +0100 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; Received: from elxcz23q12-y4.ca.am.ericsson.se (192.75.88.130) by DBXPR07MB399.eurprd07.prod.outlook.com (10.141.14.149) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.874.6; Mon, 23 Jan 2017 22:40:39 +0000 From: Simon Marchi To: CC: Simon Marchi Subject: [PATCH 4/5] Make Python inferior-related internal functions return a gdbpy_inf_ref Date: Mon, 23 Jan 2017 22:43:00 -0000 Message-ID: <20170123224004.8893-5-simon.marchi@ericsson.com> In-Reply-To: <20170123224004.8893-1-simon.marchi@ericsson.com> References: <20170123224004.8893-1-simon.marchi@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain X-ClientProxiedBy: CY4PR13CA0008.namprd13.prod.outlook.com (10.168.161.146) To DBXPR07MB399.eurprd07.prod.outlook.com (10.141.14.149) X-MS-Office365-Filtering-Correlation-Id: 4a64ea94-5007-4629-4bcc-08d443e0dbbd X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:DBXPR07MB399; X-Microsoft-Exchange-Diagnostics: 1;DBXPR07MB399;3:z9IN9VC+lf01K/06IIQIXqTAMwlKs8KwH5rqnG/Cj9uy6rN+lavLV+l28d7FillPQu208meot97EOe7HCYzFXb9wFF3q6f9C+r7pjlal18CaqkrNIFrpI9xDHCp0SFRT9nAXE7GVJff84zKhNDhW2GCce1xmR6LUPu7JQDlYx3HIv5Eww1h6l5VlTqQ6JRGPRiRUlgU72kJbscLJKyhjZWo2EGFSLAId/iR19XIU3bwWeyRf9Neg1IZQsmyLFySvr21zB/7eNVOjl1tTIqsHfA==;25:Cr12SihKptW0Y8wJepAhFnwmqYYcZS1y/PcgtcQ9ZbMILhqa3NECIHBc8VKTxKMtzDbgpWipIEF7wvfym6oB7dvoUsytG0vejTjXLSZvhAjRuhXcRnoHSKZUbzD8Eoa9DaYYl5cilmoIm7uUjDNBLLvhMDs+0TVLrkP8WfhdhYgX5ch/qvXuVZ7ni5djGE7pz+KZzVILsvEcC63Njmdw7VHrJT7L3PM00UNNH3s/60vYed5cDv3EjGAi4COwTZDnAju4RU+VszxC0EZEt2z+fXPSYiJkI5e8PL97qpp82YEB6/K3nqq4dRxLRp0TJ4FxVkdxQzKrhqZXwkRF9Pmqww+TIwDuUQ5JyBWyXue+5VJGb6Pf4leL/kI6Sv3wVdcUGx2+c2vzxGSa6Pkcz9FKq7EjZK7gVzZ0VoUOJBikeLFd8k9qyVaNIXszfi2f97q/l894ZOJ/N8hOwgJegjB1JQ== X-Microsoft-Exchange-Diagnostics: 1;DBXPR07MB399;31:GSiFgECE66bVU+A9tFfkWObxZ3EWW/sr0ldEQkRyWDvk38/8R24StL5kcq3ZZZYVATEUaZ9jGCWY0RDqeaFjMMkqNuIy3HeYfRYRnl2v5f2O0N3cL1+ATLYXL9s9ovFfdK8Y/4aq/GM9voZb+ef+qSN0aPX5IZDmE82PA7b8knZ7wDZ+lE4uO7GLV0oNlq6HnSnKAzFY0s2GQfalL6O5Vxn8k+da6UPpVaKccp/0L0rEugeoRqk5hUiQlc+JL92+GgbibjPKYDHYyqbhMeyqkWLQDhXG8sL6oq1HyipONhk=;20:0181RLcCq+8UVK1W6hUhgVcUYZ/gUj93/XJ85d5RUxV+k32oOcFJK5NhEYrdAuoe+Wlg/WqYn+HHqMWhi0MxRakL2bx+Qp+MjhVNuKPXOZK5SP2iXjl/9s/jK1yLIe6mUU7A4T0HHERUz05KXKvIddr0tMDR40laO4M8YsnjYuyYHPTkVJqK16SIXjTON94Y3oWXaY+4+208PuaZX3PblhK2Y0FPpRw4jaVDmWisQE0auZcCm9XujDBGzvvyuzR2uMIlzpj5I4XEHjS89E9MsUTGJVzqV566aL2Xw9oOKLigJLlblx1GJ21PPRMTndNiCulAsk3aYKH+bGOAgCaHihhLK91mM9q9QpapFJYAcw9qISRveoQLIkkS7kr4uNS82xmGzc1e5M6Tctm0gN8xqNiD33E74q455+Uh1Y7hYz6rBnYvB+y0tKcoOJiy9sH/jb+Nb+LZMns5t6zd+Mc/9ZpHStohOrcxhRevTss3wMN17A47d7J1K0mpcBEWSx/q X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(60795455431006); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6041248)(20161123560025)(20161123558021)(20161123555025)(20161123562025)(20161123564025)(6072148);SRVR:DBXPR07MB399;BCL:0;PCL:0;RULEID:;SRVR:DBXPR07MB399; X-Microsoft-Exchange-Diagnostics: 1;DBXPR07MB399;4:cmwMgxp/xM638IUXgT5LS48cCq1J0syFEgI8kAplkXKHzv6KF4vUPUXcfAhnOknhbDnD4Z7anULsE4YYi0+JpPyb97VaDrMkrqnzjQd6V2ComaDJGby+T53/h6g4LZa3rOmLrzuQEiRjjv3dcy0gxTYSCJbgtqAlM65hlZrcODFvTEDPuJnMSyF3UtUjsXuCRkvDHD0e7nUUvBBuUI64BnW8bjbwVUa3PM6HI0ykLcqYIt5DtFK/VWeAcNCOVNIi2LxfdLPM9RNONdufsoEt3vLGti1msJnrqTXZBzyVghhj8LDoHgRyiSoobXMipb+6HE/t48rjYt5VOr33VaeJYmzw2hXxfCz3Lllo1IPceszr4gclE3e/A5M4b7xErosD840TFbmzpuuKJgXPa23TlJA48tCuwBspVX3fykI9a1fgNzXF8QzmSihqvaWfxiyTEEdLo53Kt19hs3L0Il7dDeBY+7EByyBNSYgZK55J1TESHepxJMTCRIz7s//kR3Sx0kvb+fvU673ukxjqw2wmPVOIh4goYDInyR5Lsft67ZnGN6wbt+itVpgv+5mSFxrmbNalkkzmxe4ouvlOYox1PnFpw6Wqc4E6SnltXSNhdhXWTb40zMT0xNqNqAU+Be4Cj1n/jQVssQvNgnnlpw872g== X-Forefront-PRVS: 0196A226D1 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(7916002)(39450400003)(43544003)(199003)(54534003)(189002)(6916009)(8676002)(110136003)(48376002)(4326007)(5660300001)(2906002)(105586002)(81156014)(81166006)(106356001)(2351001)(189998001)(7736002)(2950100002)(6666003)(50466002)(50226002)(68736007)(1076002)(5003940100001)(92566002)(305945005)(3846002)(6116002)(97736004)(6512007)(86362001)(25786008)(6486002)(6506006)(38730400001)(50986999)(33646002)(76176999)(42186005)(66066001)(47776003)(36756003)(53936002)(101416001);DIR:OUT;SFP:1101;SCL:1;SRVR:DBXPR07MB399;H:elxcz23q12-y4.ca.am.ericsson.se;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;DBXPR07MB399;23:trGPt5mV5qoc3FF5rVrP9J/c0VsLM3R2RpptvDmJuQ?= =?us-ascii?Q?+JyFrrlcpg4hV+9wAbEJbrXOVyqFkzIZftOaOaCunMIFh/4ZuhTZJ0pCfNXI?= =?us-ascii?Q?RsIPcCt6CN8xEXEGdttDC8ZBynEYpq5FJ1PRomn7LhBMnJ0vDifSvV0XWblA?= =?us-ascii?Q?g5On32RUCKdNhHiNTNwUTQ5XXSnVKtkeuCu/AnkvnnBFv79SjI1LNy2rmsoZ?= =?us-ascii?Q?sQJVsMHRUCS8NUvGKjxvXJvrphUs0BLzAvjzNr8/AXyEvAAdd00t7/4r2AfO?= =?us-ascii?Q?WS7P+cmE9o5D3pGxz6pndfE/j2rfSJCBPUIpbPWaPjR/p1O7sG8dvYKo4Mwg?= =?us-ascii?Q?pV3xP13CEzFNet4CwTiTg9Nkw0/0Qy4mxbHhmiYA0lxBUrM1BxIMPmXAOtd0?= =?us-ascii?Q?crqLAtTLBLCr45O0HLnhEQMWTfdvb40nk3R3FFMs28Vr8c22AflGZb9vo/zQ?= =?us-ascii?Q?UqaZQX9PztVzU7MuHiMwBiqCH+ZUwAI9CSHX4rBRJn+jLFA2ZJXqeztd6HzM?= =?us-ascii?Q?sH+bVqqpqrhX02CgGbSpHjbdatAbVGqUXufx5AbnwsGQ6E211SsoUXTjNprc?= =?us-ascii?Q?2TormJZpdJHMGxUI3c0flxRDEqlitIvCzvk2Rzx3dHgXmWCe36nC5sRG/spu?= =?us-ascii?Q?PeNeb+5c9NaDyaqWccivkR9341nrbB4ZXix/91jzDnHz90aaIXtePwwVFceT?= =?us-ascii?Q?syzcFRgonGZKKUQ8HpugL/wuhUedaHs6Osl0kZRcbk990FFrO3f+kjv8zVYf?= =?us-ascii?Q?sSe6wC5m/RvH8qwlGCtrMCfvJG+NVFZWx108Ov3msWo79ubkg8+ySfnjoLI+?= =?us-ascii?Q?a2h2jfA7KAX38stwObUwss03YCYnVRHTXUrSi6w9K7qOL3hBsPrM1Zjs0edL?= =?us-ascii?Q?mVasmKeemQuFvQlbpmGBitw+7012alpYOsjOAYJDfhVyhRFMNQP6GopRu/Yd?= =?us-ascii?Q?6XTfoJwgCbLy5/Rpms7ZpWFfDCX5mrCHpIxcc25zjfZkpRkBLPQm4W52u/k+?= =?us-ascii?Q?/ZMx3DErtyLPqDiU3+BuIir66kym7/nyPvyfVkiyGZR+Fz4r1Gr9W6a2gc/P?= =?us-ascii?Q?l0che+Ei9wEYXKZM041Gtlaer+jPf1zPuEJlwImf0cjLChQcPyONtOI1r/UR?= =?us-ascii?Q?iYnZwWozR6+CY3R0H2of/89RajDOPn0mH0UMsDwzfupi+UtOnclAmJC4uLAW?= =?us-ascii?Q?z9IUtG7P+yiSs=3D?= X-Microsoft-Exchange-Diagnostics: 1;DBXPR07MB399;6:yTUBIKATCQWk7anbGhEwhpJD1dZ3HUsTpJ8TeJmWNq4amTRAPlnteh31FtF9a/49OvYWFjya1IkcpQ8TZvh7Fvu5WsINPItNYGc6gwxhs8l02E5FkI3H93YVXPXICjKn7sS5JQE5KhFahHFpD9ClqT66C5nApW2wZcYgK5SpW7baNxQpAAwXec2GrZy/cR4jZlFKsLI7uYAKcmnnznxKX4WfNf1Eq7ixBu/g+V6P76/CN/J4Wy1mC6l5yenNMkRjEqCWdxWr0oWwJxyaDEVbhpqqCP7mF/mBVT5bP2EpiTGikObu3KBfC77PrZMLRMZ8bPk7DrE7yb9H80ZGf9nGkmkrkUgpBwjUNUbgTKylm6V0iTe9wyLvs3X+q/bOP98/P7zdjZyKFdOReFRAwi8BdxdG9iOpqWzfCQJ+qiQHvHQ=;5:KUnGj2dbksVwO4Cn2+d1EA9CmuTjjs/YzIGolO38Qp2It31McxT2neHE8DHUeFa3ct9hh3ybkP5IborDSr6Z1f/VjWE9UWOPFjO6l6pbs1iMcdijQjsvW+EgQ05W5PMOup+T+PX9H89nX8gf0mEbwA==;24:/Qc5+aYlNCXyHGhy8hFTH2DSKe2knOsFORy3jbPIFNvcfcIV73mSoIrNQBZ9V1Pq9234gTOyihGtflKEGF7XR2tuXZ332/Z0dpz4hBN8+HU= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;DBXPR07MB399;7:FcBqsKFCnWJcW5QUGSiyAI8XM7ASfHqhJNSX0pNC86q8qTkTXcAx+hnoLgpcNGJRJwW5K782QiLuVnTJmppqJEI3joHq177W6US8NjZB8iEyCzZIlEoIe8PEb2VXkBaFlYcTpa271rGyrfF4SezSgM1y+10DAQsG6iPQstwSWpRkbhQRrkytgTY7uzahJRGj1BK3M1x1wAuHpc5pUD2n7jHV79SmiGEbaV7E87Xl3z0hvPFEQSfmEtQHusWHnLYyu0vyKl90yKnLcE2AbJyJlzxCadSUGMLQba3m39g/KlQCKkd/HZKA/mWYjtV3/UJq5ha69c6cVxaolQPNKheaOD6qy3lR+iVsSP0wqnaUcw/lYkr8hs2FQgGMI10VAWypHmlwycnJPxEgLvBUjnVrKGtbdj1DNijAidSOvAcOwXF/ryMFPdPIPfKJtwBqImiGlOoVhfCnCUzAZKhqEk6VSQ== X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jan 2017 22:40:39.7090 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBXPR07MB399 X-OriginatorOrg: ericsson.com X-IsSubscribed: yes X-SW-Source: 2017-01/txt/msg00475.txt.bz2 From: Simon Marchi The functions inferior_to_inferior_object and find_inferior_object return a new reference to an inferior_object. This means that the caller owns that reference and is responsible for decrementing it when it's done. To avoid the possibility of the caller forgetting to DECREF when it's done with the reference, make those functions return a gdbpy_inf_ref instead of a plain pointer. If the caller doesn't need the reference after it has used it, gdbpy_inf_ref will take care of removing that reference. If the reference needs to outlive the gdbpy_inf_ref object (e.g. because we are return the value to Python, which will take ownership of the reference), the caller will have to release the pointer. At least it will be explicit and it won't be ambiguous. I added comments in inferior_to_inferior_object for the poor souls who will have to deal with this again in the future. A couple of things I am not sure about: * I am not sure whether the behaviour is right with the assignment operator in delete_thread_object, so if somebody could take a look at that in particular it would be appreciated: gdbpy_inf_ref inf_obj_ref = find_inferior_object (ptid_get_pid (tp->ptid)); I suppose it's the operator= version which moves the reference that is invoked? * I used .release() on the reference in create_thread_object with a comment explaining why, but I would need some more pairs of eyes on that to see if it's right. gdb/ChangeLog: * python/py-inferior.c (inferior_to_inferior_object): Return a gdbpy_inf_ref. (find_inferior_object): Return a gdbpy_inf_ref. (delete_thread_object): Use gdbpy_inf_ref. (gdbpy_selected_inferior): Likewise. * python/py-infthread.c (create_thread_object): Adapt to gdbpy_inf_ref. * python/python-internal.h: Include py-ref.h. (inferior_to_inferior_object): Return a gdbpy_inf_ref. (find_inferior_object): Likewise. --- gdb/python/py-inferior.c | 41 +++++++++++++++++++---------------------- gdb/python/py-infthread.c | 6 +++++- gdb/python/py-ref.h | 2 ++ gdb/python/python-internal.h | 6 ++++-- 4 files changed, 30 insertions(+), 25 deletions(-) diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c index b6b43af7cd..340dddcfbd 100644 --- a/gdb/python/py-inferior.c +++ b/gdb/python/py-inferior.c @@ -207,39 +207,38 @@ python_new_objfile (struct objfile *objfile) representing INFERIOR. If the object has already been created, return it and increment the reference count, otherwise, create it. Return NULL on failure. */ -inferior_object * +gdbpy_inf_ref inferior_to_inferior_object (struct inferior *inferior) { inferior_object *inf_obj; inf_obj = (inferior_object *) inferior_data (inferior, infpy_inf_data_key); - if (!inf_obj) + if (inf_obj == NULL) { if (debug_python) printf_filtered ("Creating Python Inferior object inf = %d\n", inferior->num); inf_obj = PyObject_New (inferior_object, &inferior_object_type); - if (!inf_obj) - return NULL; + if (inf_obj == NULL) + return gdbpy_inf_ref (); inf_obj->inferior = inferior; inf_obj->threads = NULL; inf_obj->nthreads = 0; set_inferior_data (inferior, infpy_inf_data_key, inf_obj); - } else Py_INCREF ((PyObject *)inf_obj); - return inf_obj; + return gdbpy_inf_ref (inf_obj); } /* Finds the Python Inferior object for the given PID. Returns a reference, or NULL if PID does not match any inferior object. */ -inferior_object * +gdbpy_inf_ref find_inferior_object (int pid) { struct inferior *inf = find_inferior_pid (pid); @@ -247,7 +246,7 @@ find_inferior_object (int pid) if (inf) return inferior_to_inferior_object (inf); - return NULL; + return gdbpy_inf_ref (); } thread_object * @@ -304,39 +303,34 @@ add_thread_object (struct thread_info *tp) static void delete_thread_object (struct thread_info *tp, int ignore) { - inferior_object *inf_obj; struct threadlist_entry **entry, *tmp; if (!gdb_python_initialized) return; gdbpy_enter enter_py (python_gdbarch, python_language); + gdbpy_inf_ref inf_obj_ref = find_inferior_object (ptid_get_pid (tp->ptid)); - inf_obj - = (inferior_object *) find_inferior_object (ptid_get_pid (tp->ptid)); - if (!inf_obj) + if (inf_obj_ref == NULL) return; /* Find thread entry in its inferior's thread_list. */ - for (entry = &inf_obj->threads; *entry != NULL; entry = - &(*entry)->next) + for (entry = &inf_obj_ref.get ()->threads; + *entry != NULL; + entry = &(*entry)->next) if ((*entry)->thread_obj->thread == tp) break; - if (!*entry) - { - Py_DECREF (inf_obj); - return; - } + if (*entry == NULL) + return; tmp = *entry; tmp->thread_obj->thread = NULL; *entry = (*entry)->next; - inf_obj->nthreads--; + inf_obj_ref.get ()->nthreads--; Py_DECREF (tmp->thread_obj); - Py_DECREF (inf_obj); xfree (tmp); } @@ -815,7 +809,10 @@ py_free_inferior (struct inferior *inf, void *datum) PyObject * gdbpy_selected_inferior (PyObject *self, PyObject *args) { - return (PyObject *) inferior_to_inferior_object (current_inferior ()); + gdbpy_inf_ref inf_obj_ref (inferior_to_inferior_object (current_inferior ())); + + /* Release the reference, it will now be managed by Python. */ + return (PyObject *) inf_obj_ref.release (); } int diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c index c7553310c3..79fb5d12d1 100644 --- a/gdb/python/py-infthread.c +++ b/gdb/python/py-infthread.c @@ -46,7 +46,11 @@ create_thread_object (struct thread_info *tp) return NULL; thread_obj->thread = tp; - thread_obj->inf_obj = find_inferior_object (ptid_get_pid (tp->ptid)); + + /* The thread holds a weak reference to its inferior to avoid creating a + reference loop between the inferior and its threads. */ + gdbpy_inf_ref inf_obj_ref = find_inferior_object (ptid_get_pid (tp->ptid)); + thread_obj->inf_obj = inf_obj_ref.release (); return thread_obj; } diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h index b212ef195f..03c0304c74 100644 --- a/gdb/python/py-ref.h +++ b/gdb/python/py-ref.h @@ -67,6 +67,8 @@ public: /* Specializations of gdbpy_ref_base for concrete Python object types. */ typedef gdbpy_ref_base gdbpy_ref; + +struct inferior_object; typedef gdbpy_ref_base gdbpy_inf_ref; #endif /* GDB_PYTHON_REF_H */ diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h index 62a834d403..a7fa9aa4bc 100644 --- a/gdb/python/python-internal.h +++ b/gdb/python/python-internal.h @@ -94,6 +94,8 @@ #include #include +#include "py-ref.h" + #if PY_MAJOR_VERSION >= 3 #define IS_PY3K 1 #endif @@ -421,8 +423,8 @@ PyObject *gdbarch_to_arch_object (struct gdbarch *gdbarch); thread_object *create_thread_object (struct thread_info *tp); thread_object *find_thread_object (ptid_t ptid) CPYCHECKER_RETURNS_BORROWED_REF; -inferior_object *find_inferior_object (int pid); -inferior_object *inferior_to_inferior_object (struct inferior *inferior); +gdbpy_inf_ref find_inferior_object (int pid); +gdbpy_inf_ref inferior_to_inferior_object (struct inferior *inferior); const struct block *block_object_to_block (PyObject *obj); struct symbol *symbol_object_to_symbol (PyObject *obj); -- 2.11.0