From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@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 16:34:00 -0000 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 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2014-03/txt/msg00318.txt.bz2 Doug Evans writes: > On Mon, Mar 3, 2014 at 1:17 AM, Ludovic Court=C3=A8s 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 =3D 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=C3=A8s 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 =3D 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); =20 /* 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.guil= e/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 \$" "=3D 42" =20 + # Verify the recorded history value survives a gc. + gdb_test_no_output "guile (gc)" + gdb_test "p \$\$" "=3D 42" + # Test dereferencing the argv pointer. =20 # Just get inferior variable argv the value history, available to guil= e.