From patchwork Thu Mar 13 16:33:51 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 71 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx20.g.dreamhost.com (caibbdcaaahc.dreamhost.com [208.113.200.72]) by wilcox.dreamhost.com (Postfix) with ESMTP id 91C86360167 for ; Thu, 13 Mar 2014 09:34:07 -0700 (PDT) Received: by homiemail-mx20.g.dreamhost.com (Postfix, from userid 14314964) id 46EBC4081EC10; Thu, 13 Mar 2014 09:34:06 -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 A54764080C02D for ; Thu, 13 Mar 2014 09:34:06 -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:cc:subject:references:date:in-reply-to :message-id:mime-version:content-type:content-transfer-encoding; q=dns; s=default; b=JpnCFPjlJ5meP7ajZEt/c+FGbmuTf6hr5OXrBeTQsW2 oBBkGfA4y5pL/CK6Ka3yOClXC8vVEuBXMTtjaahamjY4Ei5wkqqqS6onoFtF04R3 EnJuAtrEyUEZWRhcRk6lCmL4ZVEnguYF9DDtGJDElS442XthsZSdgHQm5cfziRBQ = 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:cc:subject:references:date:in-reply-to :message-id:mime-version:content-type:content-transfer-encoding; s=default; bh=SP+KrB4/hHbzyI1vpvk7Vgb1KzU=; b=HosvFPLno/Nuqfc8k QyMR5iwQ/BB72Hw+YbuMRoJPUV6pbbldiZzmyCnxhA9x812Fnw0ASF7tx6QTNVrw AmiOmsbvnM6VmaUlNYCLmtA4uboqb4vwSvhzWnERQ4lSsPSBLS7vPvKvvxWdjzDz gYWCsoSzS1+XzDW/STeJu9Rzyw= Received: (qmail 7478 invoked by alias); 13 Mar 2014 16:34:04 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 7468 invoked by uid 89); 13 Mar 2014 16:34:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-pd0-f169.google.com Received: from mail-pd0-f169.google.com (HELO mail-pd0-f169.google.com) (209.85.192.169) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 13 Mar 2014 16:34:02 +0000 Received: by mail-pd0-f169.google.com with SMTP id fp1so1294713pdb.14 for ; Thu, 13 Mar 2014 09:34:01 -0700 (PDT) X-Received: by 10.68.12.133 with SMTP id y5mr3500953pbb.134.1394728440974; Thu, 13 Mar 2014 09:34:00 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-53-sfba.hfc.comcastbusiness.net. [173.13.178.53]) by mx.google.com with ESMTPSA id sy2sm8454488pbc.28.2014.03.13.09.33.59 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Mar 2014 09:34:00 -0700 (PDT) From: Doug Evans To: Ludovic =?utf-8?Q?Court=C3=A8s?= Cc: "gdb-patches\@sourceware.org" Subject: Re: [PATCH] record_latest_value: Call release_value_or_incref instead of release_value References: <87wqgbsktg.fsf@gnu.org> Date: Thu, 13 Mar 2014 09:33:51 -0700 In-Reply-To: (Doug Evans's message of "Mon, 3 Mar 2014 08:22:15 -0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in Doug Evans writes: > On Mon, Mar 3, 2014 at 1:17 AM, Ludovic Courtès wrote: >> Doug Evans skribis: >> >>> I think this is because its reference counting is wrong. >>> Upon return from record_latest_value, its reference count is still one. >>> However it was one upon entry. It should be two, right? >>> One for the Scheme wrapper and one for the history entry. >>> >>> (gdb) guile (define histnum (history-append! (make-value 42))) >> >> Indeed, good catch. Here's how I reproduced it: >> >> --8<---------------cut here---------------start------------->8--- >> (gdb) guile (use-modules (gdb)) >> (gdb) guile (history-append! (make-value 42)) >> 1 >> (gdb) p $1 >> $2 = 42 >> (gdb) guile (gc) >> (gdb) p $1 >> Segmentation fault >> --8<---------------cut here---------------end--------------->8--- >> >> What about adding this to the patch as a test case? > > Shall do. Thanks. Here's what I committed. 2014-03-13 Doug Evans * value.c (record_latest_value): Call release_value_or_incref instead of release_value. testsuite/ 2014-03-13 Ludovic Courtès Doug Evans * gdb.guile/scm-value.exp (test_value_in_inferior): Verify value added to history survives a gc. diff --git a/gdb/value.c b/gdb/value.c index 4e8d1fe..27043ee 100644 --- a/gdb/value.c +++ b/gdb/value.c @@ -1659,7 +1659,11 @@ record_latest_value (struct value *val) from. This is a bit dubious, because then *&$1 does not just return $1 but the current contents of that location. c'est la vie... */ val->modifiable = 0; - release_value (val); + + /* The value may have already been released, in which case we're adding a + new reference for its entry in the history. That is why we call + release_value_or_incref here instead of release_value. */ + release_value_or_incref (val); /* Here we treat value_history_count as origin-zero and applying to the value being stored now. */ diff --git a/gdb/testsuite/gdb.guile/scm-value.exp b/gdb/testsuite/gdb.guile/scm-value.exp index 89f0ff1..a85d5bd 100644 --- a/gdb/testsuite/gdb.guile/scm-value.exp +++ b/gdb/testsuite/gdb.guile/scm-value.exp @@ -67,6 +67,10 @@ proc test_value_in_inferior {} { gdb_test "gu (history-ref i)" "#" gdb_test "p \$" "= 42" + # Verify the recorded history value survives a gc. + gdb_test_no_output "guile (gc)" + gdb_test "p \$\$" "= 42" + # Test dereferencing the argv pointer. # Just get inferior variable argv the value history, available to guile.