Message ID | yjt2wqdhwli8.fsf@ruffy.mtv.corp.google.com |
---|---|
State | Committed |
Headers |
Return-Path: <x14314964@homiemail-mx20.g.dreamhost.com> X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx20.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id E494A360098 for <siddhesh@wilcox.dreamhost.com>; Mon, 19 May 2014 13:40:05 -0700 (PDT) Received: by homiemail-mx20.g.dreamhost.com (Postfix, from userid 14314964) id 6D68441C67C0F; Mon, 19 May 2014 13:40:05 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx20.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx20.g.dreamhost.com (Postfix) with ESMTPS id 4977E41C67C0A for <gdb@patchwork.siddhesh.in>; Mon, 19 May 2014 13:40:05 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-type; q=dns; s=default; b=SJzQ2O0p0AkYtAFe71l6hBsONI04m k6O1a3dEbaBL8NNIQ6fcyxki2VHihajq8gFCAwifdi+hlRRhcZksALNIEJtxXBXb 9N8pCrZSR2CIBgNRgB5f9SmfoTDDD2RbWACtRruVz1mN7o5d4xHChux9qLWYczkO kRcFBfLTnDsfdo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-type; s=default; bh=bxXYvv71NWe0/yq9W3lG7GfO+SQ=; b=tGV 8aw3FAY6exsbDH+WpKRpFBMwvruDDsvpEHY89nb4Q9CSTaz2EXZS8j8AsB10EZHf lT2vgd+7VJysFIy5SjG9jPn/DFLYw/v5RXfP3HmG5dXGuZsfsH9RKsb/QEgTyQEP 5zY7nElPwIYfPs9f8I6y34R2GgMCM04tqqlHUBM8= Received: (qmail 27965 invoked by alias); 19 May 2014 20:40:03 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-gdb=patchwork.siddhesh.in@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 27954 invoked by uid 89); 19 May 2014 20:40:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS 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-SHA encrypted) ESMTPS; Mon, 19 May 2014 20:40:01 +0000 Received: by mail-pa0-f74.google.com with SMTP id rd3so1052633pab.1 for <gdb-patches@sourceware.org>; Mon, 19 May 2014 13:40:00 -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:to:subject:date:message-id:mime-version :content-type; bh=/bHeZAYxBmItPIs+7h59tMZpLMKaAvtCBAh5I8vA624=; b=LPVQsL83jNXMrxvElLJ+vWRCoR7/90QkfWVqDjTf9BIU7oCFrzNEeoQw+5n2DF+LBz Sllu25S6yFL+S7sNb7MRhT995o2DHgSYTXvSaIizgv9DbMV9vMGiKWkhqHrUP++Ja96v EgT5d/HAi09OsJVDQgAS0SwiVQVi4TotVT0NNEMJIBookYtW0ypkvmZa4RVsKGKTgpKL BMb6wyg5Ca92i73eFJjh4h/vWeA7TWWsNrHzAF+padd7/jnmZ66hCj4HvJacCm2ZqIwj aQYvJRryGYHUv1mNKzv3bdrOyVEHl83sMf5gLjse1DYOIvauDYp4CNL2lbAvnhwhSGvR 1WJw== X-Gm-Message-State: ALoCoQml4Mp0+HpW9Dai7exTRQZQsRbBijPRCv6Kpv3Luq57U+hLcQQKp1bvYZxSKHFF759OkCt/W7qS7SNM8RTG87nx/O3muVtZU9lQHGDTNz+WtvCOeOwgcnTwMId9SIsy6RQ+P7XZ X-Received: by 10.66.240.4 with SMTP id vw4mr16977904pac.10.1400532000055; Mon, 19 May 2014 13:40:00 -0700 (PDT) Received: from corp2gmr1-2.hot.corp.google.com (corp2gmr1-2.hot.corp.google.com [172.24.189.93]) by gmr-mx.google.com with ESMTPS id n43si655751yhe.1.2014.05.19.13.40.00 for <gdb-patches@sourceware.org> (version=TLSv1.1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 19 May 2014 13:40:00 -0700 (PDT) Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.17.128.44]) by corp2gmr1-2.hot.corp.google.com (Postfix) with ESMTP id A51795A427D for <gdb-patches@sourceware.org>; Mon, 19 May 2014 13:39:59 -0700 (PDT) From: Doug Evans <dje@google.com> To: gdb-patches@sourceware.org Subject: [PATCH] Fix gdb.multi/base.exp, gdb memory corruption Date: Mon, 19 May 2014 13:39:59 -0700 Message-ID: <yjt2wqdhwli8.fsf@ruffy.mtv.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in |
Commit Message
Doug Evans
May 19, 2014, 8:39 p.m. UTC
Hi. My prune_inferiors patch 2014-05-17 Doug Evans <xdje42@gmail.com> * inferior.c (prune_inferiors): Fix comment. (remove_inferior_command): Call prune_program_spaces. is causing gdb.multi/base.exp to fail: UNRESOLVED: gdb.multi/base.exp: remove-inferiors 2-3 UNRESOLVED: gdb.multi/base.exp: check remove-inferiors gdb is crashing because it's accessing/freeing already freed memory. I was running the testsuite all weekend, sorry I only saw this now. E.g. ==16368== Invalid read of size 4 ==16368== at 0x660A9D: find_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:1349) ==16368== by 0x663ECB: lookup_minimal_symbol_by_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/minsyms.c:734) ==16368== by 0x5D987A: find_pc_sect_symtab (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/symtab.c:2153) ==16368== by 0x5D4D77: blockvector_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:168) ==16368== by 0x5D4F59: block_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:246) ==16368== by 0x5D4F9B: block_for_pc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:258) ==16368== by 0x734C5D: inline_frame_sniffer (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inline-frame.c:218) ==16368== by 0x732104: frame_unwind_try_unwinder (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:108) ==16368== by 0x73223F: frame_unwind_find_by_frame (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:159) ==16368== by 0x72D5AA: compute_frame_id (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:453) ==16368== by 0x7300EC: get_prev_frame_if_no_cycle (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1758) ==16368== by 0x73079A: get_prev_frame_always (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1931) ==16368== Address 0x5b13500 is 16 bytes inside a block of size 24 free'd ==16368== at 0x403072E: free (valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c:445) ==16368== by 0x762134: xfree (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/common/common-utils.c:108) ==16368== by 0x65DACF: objfiles_pspace_data_cleanup (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:91) ==16368== by 0x75E546: program_spaceregistry_callback_adaptor (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45) ==16368== by 0x7644F6: registry_clear_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:82) ==16368== by 0x7645AB: registry_container_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:95) ==16368== by 0x75E5B4: program_space_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45) ==16368== by 0x75E9BA: release_program_space (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:167) ==16368== by 0x75EB9B: prune_program_spaces (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:269) ==16368== by 0x75303D: remove_inferior_command (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inferior.c:792) ==16368== by 0x50B5FD: do_cfunc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:107) ==16368== by 0x50E6F2: cmd_func (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:1886) The problem originates from the get_current_arch call in py-progspace.c:py_free_pspace. I think the comment in this patch explains things pretty well. Basically, we're in the pspace destructor, and thus there's not much we can rely on. The Python machinery needs an arch, so we give it one, albeit a fiction. I think(!) it doesn't matter. I'm going with this fix because it's not clear to me what The Right fix is, short of removing global state in gdb which is non-trivial. Since "there is always an inferior" calling target_gdbarch seems pretty safe here. 2014-05-19 Doug Evans <dje@google.com> * python/py-progspace.c (py_free_pspace): Use target_gdbarch instead of get_current_arch.
Comments
On Mon, May 19, 2014 at 1:39 PM, Doug Evans <dje@google.com> wrote: > Hi. > > My prune_inferiors patch > > 2014-05-17 Doug Evans <xdje42@gmail.com> > > * inferior.c (prune_inferiors): Fix comment. > (remove_inferior_command): Call prune_program_spaces. > > is causing gdb.multi/base.exp to fail: > > UNRESOLVED: gdb.multi/base.exp: remove-inferiors 2-3 > UNRESOLVED: gdb.multi/base.exp: check remove-inferiors > > gdb is crashing because it's accessing/freeing already freed memory. > I was running the testsuite all weekend, sorry I only saw this now. > > E.g. > ==16368== Invalid read of size 4 > ==16368== at 0x660A9D: find_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:1349) > ==16368== by 0x663ECB: lookup_minimal_symbol_by_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/minsyms.c:734) > ==16368== by 0x5D987A: find_pc_sect_symtab (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/symtab.c:2153) > ==16368== by 0x5D4D77: blockvector_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:168) > ==16368== by 0x5D4F59: block_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:246) > ==16368== by 0x5D4F9B: block_for_pc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:258) > ==16368== by 0x734C5D: inline_frame_sniffer (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inline-frame.c:218) > ==16368== by 0x732104: frame_unwind_try_unwinder (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:108) > ==16368== by 0x73223F: frame_unwind_find_by_frame (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:159) > ==16368== by 0x72D5AA: compute_frame_id (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:453) > ==16368== by 0x7300EC: get_prev_frame_if_no_cycle (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1758) > ==16368== by 0x73079A: get_prev_frame_always (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1931) > ==16368== Address 0x5b13500 is 16 bytes inside a block of size 24 free'd > ==16368== at 0x403072E: free (valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c:445) > ==16368== by 0x762134: xfree (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/common/common-utils.c:108) > ==16368== by 0x65DACF: objfiles_pspace_data_cleanup (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:91) > ==16368== by 0x75E546: program_spaceregistry_callback_adaptor (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45) > ==16368== by 0x7644F6: registry_clear_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:82) > ==16368== by 0x7645AB: registry_container_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:95) > ==16368== by 0x75E5B4: program_space_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45) > ==16368== by 0x75E9BA: release_program_space (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:167) > ==16368== by 0x75EB9B: prune_program_spaces (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:269) > ==16368== by 0x75303D: remove_inferior_command (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inferior.c:792) > ==16368== by 0x50B5FD: do_cfunc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:107) > ==16368== by 0x50E6F2: cmd_func (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:1886) > > The problem originates from the get_current_arch call in > py-progspace.c:py_free_pspace. > I think the comment in this patch explains things pretty well. > Basically, we're in the pspace destructor, and thus there's not much we can > rely on. The Python machinery needs an arch, so we give it one, > albeit a fiction. I think(!) it doesn't matter. > I'm going with this fix because it's not clear to me what The Right fix > is, short of removing global state in gdb which is non-trivial. > Since "there is always an inferior" calling target_gdbarch seems > pretty safe here. > > 2014-05-19 Doug Evans <dje@google.com> > > * python/py-progspace.c (py_free_pspace): Use target_gdbarch instead > of get_current_arch. > > diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c > index cda5a86..c787c69 100644 > --- a/gdb/python/py-progspace.c > +++ b/gdb/python/py-progspace.c > @@ -241,7 +241,16 @@ py_free_pspace (struct program_space *pspace, void *datum) > { > struct cleanup *cleanup; > pspace_object *object = datum; > - struct gdbarch *arch = get_current_arch (); > + /* This is a fiction, but we're in a nasty spot: The pspace is in the > + process of being deleted, we can't rely on anything in it. Plus > + this is one time when the current program space and current inferior > + are not in sync. The architecture comes from the inferior, which cannot > + be the current one because we wouldn't be deleting its pspace. > + We don't need to do much here so this fiction suffices. > + Note: We cannot call get_current_arch because it may try to access > + the target, which may involve accessing data in the pspace currently > + being deleted. */ > + struct gdbarch *arch = target_gdbarch (); > > cleanup = ensure_python_env (arch, current_language); > object->pspace = NULL; Hmmm... I looked at py_free_inferior to see what it does: ---snip--- static void py_free_inferior (struct inferior *inf, void *datum) { struct cleanup *cleanup; inferior_object *inf_obj = datum; struct threadlist_entry *th_entry, *th_tmp; if (!gdb_python_initialized) return; cleanup = ensure_python_env (python_gdbarch, python_language); ---snip--- Just passing python_gdbarch back into ensure_python_env doesn't feel right. One might try to get the arch from INF but it is being destructed and we it would be preferable to not have to rely on it being available. One solution would be to have a special "dummy" or some such arch for use during these kinds of situations. Plus I see python_on_normal_stop calling get_current_arch whereas python_on_resume calling target_gdbarch. There are no comments in the code explaining why things are the way they are, thus it's hard to reason whether these choices have been deliberately made or are essentially random. It could be the case that either choice is fine because it will never be used, but IWBN to have that documented (or better yet just be consistent and document it some place). It feels like there's a good cleanup project here. Am I missing something?
On Mon, May 19, 2014 at 2:52 PM, Doug Evans <dje@google.com> wrote: > On Mon, May 19, 2014 at 1:39 PM, Doug Evans <dje@google.com> wrote: >> Hi. >> >> My prune_inferiors patch >> >> 2014-05-17 Doug Evans <xdje42@gmail.com> >> >> * inferior.c (prune_inferiors): Fix comment. >> (remove_inferior_command): Call prune_program_spaces. >> >> is causing gdb.multi/base.exp to fail: >> >> UNRESOLVED: gdb.multi/base.exp: remove-inferiors 2-3 >> UNRESOLVED: gdb.multi/base.exp: check remove-inferiors >> >> gdb is crashing because it's accessing/freeing already freed memory. >> I was running the testsuite all weekend, sorry I only saw this now. >> >> E.g. >> ==16368== Invalid read of size 4 >> ==16368== at 0x660A9D: find_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:1349) >> ==16368== by 0x663ECB: lookup_minimal_symbol_by_pc_section (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/minsyms.c:734) >> ==16368== by 0x5D987A: find_pc_sect_symtab (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/symtab.c:2153) >> ==16368== by 0x5D4D77: blockvector_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:168) >> ==16368== by 0x5D4F59: block_for_pc_sect (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:246) >> ==16368== by 0x5D4F9B: block_for_pc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/block.c:258) >> ==16368== by 0x734C5D: inline_frame_sniffer (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inline-frame.c:218) >> ==16368== by 0x732104: frame_unwind_try_unwinder (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:108) >> ==16368== by 0x73223F: frame_unwind_find_by_frame (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame-unwind.c:159) >> ==16368== by 0x72D5AA: compute_frame_id (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:453) >> ==16368== by 0x7300EC: get_prev_frame_if_no_cycle (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1758) >> ==16368== by 0x73079A: get_prev_frame_always (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/frame.c:1931) >> ==16368== Address 0x5b13500 is 16 bytes inside a block of size 24 free'd >> ==16368== at 0x403072E: free (valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c:445) >> ==16368== by 0x762134: xfree (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/common/common-utils.c:108) >> ==16368== by 0x65DACF: objfiles_pspace_data_cleanup (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/objfiles.c:91) >> ==16368== by 0x75E546: program_spaceregistry_callback_adaptor (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45) >> ==16368== by 0x7644F6: registry_clear_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:82) >> ==16368== by 0x7645AB: registry_container_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/registry.c:95) >> ==16368== by 0x75E5B4: program_space_free_data (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:45) >> ==16368== by 0x75E9BA: release_program_space (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:167) >> ==16368== by 0x75EB9B: prune_program_spaces (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/progspace.c:269) >> ==16368== by 0x75303D: remove_inferior_command (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/inferior.c:792) >> ==16368== by 0x50B5FD: do_cfunc (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:107) >> ==16368== by 0x50E6F2: cmd_func (/usr/local/lib/debug/debug-src/obj64/gdb/../../binutils-gdb/gdb/cli/cli-decode.c:1886) >> >> The problem originates from the get_current_arch call in >> py-progspace.c:py_free_pspace. >> I think the comment in this patch explains things pretty well. >> Basically, we're in the pspace destructor, and thus there's not much we can >> rely on. The Python machinery needs an arch, so we give it one, >> albeit a fiction. I think(!) it doesn't matter. >> I'm going with this fix because it's not clear to me what The Right fix >> is, short of removing global state in gdb which is non-trivial. >> Since "there is always an inferior" calling target_gdbarch seems >> pretty safe here. >> >> 2014-05-19 Doug Evans <dje@google.com> >> >> * python/py-progspace.c (py_free_pspace): Use target_gdbarch instead >> of get_current_arch. >> >> diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c >> index cda5a86..c787c69 100644 >> --- a/gdb/python/py-progspace.c >> +++ b/gdb/python/py-progspace.c >> @@ -241,7 +241,16 @@ py_free_pspace (struct program_space *pspace, void *datum) >> { >> struct cleanup *cleanup; >> pspace_object *object = datum; >> - struct gdbarch *arch = get_current_arch (); >> + /* This is a fiction, but we're in a nasty spot: The pspace is in the >> + process of being deleted, we can't rely on anything in it. Plus >> + this is one time when the current program space and current inferior >> + are not in sync. The architecture comes from the inferior, which cannot >> + be the current one because we wouldn't be deleting its pspace. >> + We don't need to do much here so this fiction suffices. >> + Note: We cannot call get_current_arch because it may try to access >> + the target, which may involve accessing data in the pspace currently >> + being deleted. */ >> + struct gdbarch *arch = target_gdbarch (); >> >> cleanup = ensure_python_env (arch, current_language); >> object->pspace = NULL; > > Hmmm... > > I looked at py_free_inferior to see what it does: > > ---snip--- > static void > py_free_inferior (struct inferior *inf, void *datum) > { > > struct cleanup *cleanup; > inferior_object *inf_obj = datum; > struct threadlist_entry *th_entry, *th_tmp; > > if (!gdb_python_initialized) > return; > > cleanup = ensure_python_env (python_gdbarch, python_language); > ---snip--- > > Just passing python_gdbarch back into ensure_python_env doesn't feel right. > One might try to get the arch from INF but it is being destructed and > we it would be preferable to not have to rely on it being available. > One solution would be to have a special "dummy" or some such arch for > use during these kinds of situations. > > Plus I see python_on_normal_stop calling get_current_arch whereas > python_on_resume calling target_gdbarch. > There are no comments in the code explaining why things are the way > they are, thus it's hard to reason whether these choices have been > deliberately made or are essentially random. It could be the case > that either choice is fine because it will never be used, but IWBN to > have that documented (or better yet just be consistent and document it > some place). > > It feels like there's a good cleanup project here. > Am I missing something? Curiouser and curiouser ... progspace.c:release_program_space has this: if (!gdbarch_has_shared_address_space (target_gdbarch ())) free_address_space (pspace->aspace); That also can't be right. [Right?] We haven't (and can't) set current_inferior to one that used this program space: it's gone. So target_gdbarch() is essentially returning a random value upon which we will decide whether to call free_address_space. Since it's the arch that decides "has_shared_address_space", and since we're allocating an object stored in a pspace based on this decision, ISTM that a program space also has an arch.
On 05/20/2014 04:39 AM, Doug Evans wrote: > The problem originates from the get_current_arch call in > py-progspace.c:py_free_pspace. The backtrace below can make the problem clear, the per-pspace data is used (frame #0) when it is being released (frame #20). #0 get_objfile_pspace_data (pspace=0x86e5ad0) at ../../../git/gdb/objfiles.c:102 #1 0x08239496 in find_pc_section (pc=134513888) at ../../../git/gdb/objfiles.c:1348 #2 0x0823c255 in lookup_minimal_symbol_by_pc_section (pc=134513888, section=0x0) at ../../../git/gdb/minsyms.c:734 #3 0x081c1dea in find_pc_sect_symtab (pc=pc@entry=134513888, section=section@entry=0x0) at ../../../git/gdb/symtab.c:2153 ..... #17 0x082ed6db in get_current_frame () at ../../../git/gdb/frame.c:1485 #18 0x082ed7fc in get_selected_frame (message=0x0) at ../../../git/gdb/frame.c:1540 #19 0x08212d92 in get_current_arch () at ../../../git/gdb/arch-utils.c:758 #20 0x0813d788 in py_free_pspace (pspace=0x86e5ad0, datum=0xb7bf9ad0) at ../../../git/gdb/python/py-progspace.c:244 #21 0x08313d4a in program_spaceregistry_callback_adaptor (func=0x813d777 <py_free_pspace>, container=0x86e5ad0, data=0xb7bf9ad0) at ../../../git/gdb/progspace.c:45 #22 0x08318d6b in registry_clear_data (data_registry=0x85c425c <program_space_data_registry>, adaptor=0x8313d1d <program_spaceregistry_callback_adaptor>, container=0x86e5ad0, fields=0x86e5b10) at ../../../git/gdb/registry.c:82 #23 0x08318e16 in registry_container_free_data (data_registry=0x85c425c <program_space_data_registry>, adaptor=0x8313d1d <program_spaceregistry_callback_adaptor>, container=0x86e5ad0, fields=0x86e5b10) at ../../../git/gdb/registry.c:95 #24 0x08313db0 in program_space_free_data (container=0x86e5ad0) at ../../../git/gdb/progspace.c:45 #25 0x0831412f in release_program_space (pspace=0x86e5ad0) at ../../../git/gdb/progspace.c:167 #26 0x083142cd in prune_program_spaces () at ../../../git/gdb/progspace.c:269 > I think the comment in this patch explains things pretty well. > Basically, we're in the pspace destructor, and thus there's not much we can > rely on. The Python machinery needs an arch, so we give it one, > albeit a fiction. I think(!) it doesn't matter. > I'm going with this fix because it's not clear to me what The Right fix > is, short of removing global state in gdb which is non-trivial. > Since "there is always an inferior" calling target_gdbarch seems > pretty safe here. The patch looks reasonable to me. Although I tried to do something different at first, remove pspace earlier, say, when delete the inferior, delete the pspace too if it isn't shared with other inferiors. This crash isn't fixed unfortunately. > > 2014-05-19 Doug Evans <dje@google.com> > > * python/py-progspace.c (py_free_pspace): Use target_gdbarch instead > of get_current_arch. > > diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c > index cda5a86..c787c69 100644 > --- a/gdb/python/py-progspace.c > +++ b/gdb/python/py-progspace.c > @@ -241,7 +241,16 @@ py_free_pspace (struct program_space *pspace, void *datum) > { > struct cleanup *cleanup; > pspace_object *object = datum; > - struct gdbarch *arch = get_current_arch (); > + /* This is a fiction, but we're in a nasty spot: The pspace is in the > + process of being deleted, we can't rely on anything in it. Plus > + this is one time when the current program space and current inferior > + are not in sync. The architecture comes from the inferior, which cannot > + be the current one because we wouldn't be deleting its pspace. > + We don't need to do much here so this fiction suffices. > + Note: We cannot call get_current_arch because it may try to access > + the target, which may involve accessing data in the pspace currently > + being deleted. */ There are five sentences in this paragraph, but only the first and the last are necessary, IMO. The rest doesn't bring any useful information, if not confusion, into this comment. > + struct gdbarch *arch = target_gdbarch (); > > cleanup = ensure_python_env (arch, current_language); > object->pspace = NULL;
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c index cda5a86..c787c69 100644 --- a/gdb/python/py-progspace.c +++ b/gdb/python/py-progspace.c @@ -241,7 +241,16 @@ py_free_pspace (struct program_space *pspace, void *datum) { struct cleanup *cleanup; pspace_object *object = datum; - struct gdbarch *arch = get_current_arch (); + /* This is a fiction, but we're in a nasty spot: The pspace is in the + process of being deleted, we can't rely on anything in it. Plus + this is one time when the current program space and current inferior + are not in sync. The architecture comes from the inferior, which cannot + be the current one because we wouldn't be deleting its pspace. + We don't need to do much here so this fiction suffices. + Note: We cannot call get_current_arch because it may try to access + the target, which may involve accessing data in the pspace currently + being deleted. */ + struct gdbarch *arch = target_gdbarch (); cleanup = ensure_python_env (arch, current_language); object->pspace = NULL;