record_latest_value: Call release_value_or_incref instead of release_value
Commit Message
Doug Evans <xdje42@gmail.com> writes:
> On Mon, Mar 3, 2014 at 1:17 AM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Doug Evans <xdje42@gmail.com> 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 <xdje42@gmail.com>
* value.c (record_latest_value): Call release_value_or_incref
instead of release_value.
testsuite/
2014-03-13 Ludovic Courtès <ludo@gnu.org>
Doug Evans <xdje42@gmail.com>
* gdb.guile/scm-value.exp (test_value_in_inferior): Verify value added
to history survives a gc.
@@ -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. */
@@ -67,6 +67,10 @@ proc test_value_in_inferior {} {
gdb_test "gu (history-ref i)" "#<gdb:value 42>"
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.