On Tue, Dec 17, 2013 at 11:54:44PM +0100, Jonathan Worthington wrote:
> On 12/15/2013 21:50, Nicholas Clark wrote:
> > I don't know for sure what the right fix is.
> Hopefully, 191c383 is the right fix. Confirmation welcome. :-)
I can confirm that with it, the error goes away and I can build the setting.
I'm also going to paste it here because I like the elegance - move the
creation of both objects (value_obj and base_obj) before taking interior
pointers of either, rendering the entire issue of stale interior pointers
moot.
And yes, also need to temporarily root value_obj in case creating base_obj
moves it, but don't need to root base_obj as after its creation there are
no other calls that could allocate and hence might move.
But this is all suggesting to me that writing code in C that correctly
follows the GC rules is hard, because there are lots of subtle traps, like
those fixed here. And once you factor in all the time lost to errors and
debugging, it's suggesting that writing the code in a higher level language
(such as NQP) is usually going to be the right approach.
commit 191c383d4e02455ca21c63d09525777670fe4407
Author: jnthn <jn...@jnthn.net>
Date: Tue Dec 17 23:28:39 2013 +0100
Make radix_I careful with interior pointers.
Before, the object could move during their lifetime. This fixes the
issue. nwc10++ for finding it.
diff --git a/src/math/bigintops.c b/src/math/bigintops.c
index 09e936c..e1e218c 100644
--- a/src/math/bigintops.c
+++ b/src/math/bigintops.c
@@ -366,12 +366,12 @@ MVMObject * MVM_bigint_radix(MVMThreadContext *tc,
MVMint64 radix, MVMString *st
value_obj = MVM_repr_alloc_init(tc, type);
MVM_repr_push_o(tc, result, value_obj);
-
- value = get_bigint(tc, value_obj);
+ MVM_gc_root_temp_push(tc, (MVMCollectable **)&value_obj);
base_obj = MVM_repr_alloc_init(tc, type);
MVM_repr_push_o(tc, result, base_obj);
+ value = get_bigint(tc, value_obj);
base = get_bigint(tc, base_obj);
mp_set_int(base, 1);
@@ -412,7 +412,8 @@ MVMObject * MVM_bigint_radix(MVMThreadContext *tc, MVMint64
radix, MVMString *st
pos_obj = MVM_repr_alloc_init(tc, type);
MVM_repr_set_int(tc, pos_obj, pos);
MVM_repr_push_o(tc, result, pos_obj);
- MVM_gc_root_temp_pop_n(tc, 3);
+
+ MVM_gc_root_temp_pop_n(tc, 4);
return result;
}
Nicholas Clark
