Euphoria
Ticket #448:
splice() fails when splicing in long sequences
-
Reported by
DerekParnell
Nov 26, 2010
I've added a new test in t_sequence.e that demonstrates this.
Details
1. Comment by mattlewis
Nov 26, 2010
Interestingly, this passes for me interpreted and bound, but I get memory corruption when translated.
2. Comment by DerekParnell
Nov 26, 2010
Yes, the issue is with the translator.
I found it because I'm now using splice() inside the map.e module and everything was working well until I translated it. I then spent some time trying to track down what exactly was failing and it turns out the splice() is the culprit.
3. Comment by mattlewis
Nov 26, 2010
Here is some valgrind output from running a translated t_sequence:
==4759== Invalid read of size 4
==4759== at 0x80554DE: main (main-.c:3411)
==4759== Address 0x6e91df4 is 4 bytes inside a block of size 36 free'd
==4759== at 0x48CDCCD: realloc (vg_replace_malloc.c:525)
==4759== by 0x8086ED0: Add_internal_space (be_runtime.c:806)
==4759== by 0x8055439: main (main-.c:3387)
Here is main-.c:3387:
assign_space = Add_internal_space( _8427, insert_pos,((s1_ptr)SEQ_PTR(_8498))->length);
This is part of the code for the test previous to the new one:
test_equal("splice() string", "John", splice("Jon", "h", 3))*
_8427 is the "Jon" literal. So somewhere, that's getting incorrectly freed or realloc'd. I'm guessing the problem exists in the interpreter, too, but we just haven't found the right condition.
4. Comment by DerekParnell
Nov 26, 2010
Hmmm... in my map.e the failing statement is ...
map_data[KEY_BUCKETS][bucket_] = splice(map_data[KEY_BUCKETS][bucket_], repeat(-2, threshold_size),2)
5. Comment by mattlewis
Nov 26, 2010
Actually, it may not be an interpreter issue at all. The implementation in the interpreter is different. It looks like SPLICE and INSERT share some implementation in the interpreter, but are separate in the translator.
I haven't gone through opSPLICE carefully in compile.e, but I'd bet that's the issue.
6. Comment by mattlewis
Nov 28, 2010
I updated translated splice() to be the same as interpreted. There were some discrepancies with respect to in-place modifications and reference counts.
The t_sequence.e test now passes. Derek, please confirm that your other code works after this change.
7. Comment by SDPringle
Nov 28, 2010
The reported release must be wrong. Didn't you mean 4271?
8. Comment by SDPringle
Nov 28, 2010
I was able to reproduce the bug on 4302 on Windows XP. I am now confirming it fixed. I am using 4333.
9. Comment by mattlewis
Nov 28, 2010
Thanks for the confirmation, Shawn, but Derek actually had some code that lead him to finding this. I'd like to see if that code is also working now, in addition to the test written to trigger the bug.
10. Comment by DerekParnell
Nov 28, 2010
Is all good now.
GitHub