# HG changeset patch
# User Jan Vrany <jan.vrany@fit.cvut.cz>
# Date 1524201755 -3600
# Fri Apr 20 06:22:35 2018 +0100
# Branch jv
# Node ID 9777fd81cae8c076d1bb05a24844e4f20ae8e47f
# Parent c02671bebd0a80223fd0a6dc776f2ed96a88a92c
Issue #205: fixed compilation of assignment into inlined block argument
Due to a bug in, compiler generated a wrong store when compiling an assignment
to a block argument in inlined block. Consider following method:
foo
1 to: 10 do:[:i | i := 100]
For this method it generated following (wrong) code:
decompiling nA: 0 nV: 1 nT: 3
1: 79 push1
2: 08 01 LINE [1]
4: 2F dup
5: 10 0A pushNum 10
7: 91 01 send1 #> [1]
9: 33 0A trueJump 10 (21)
11: 2F dup
12: 25 01 storeMethodVar 1
14: 10 64 pushNum 100
16: 65 storeMethodVar2 "<-- wrong"
17: 7B 01 sendPlus1 [1]
19: 36 ED jump -19 (2)
21: 05 retSelf
22: 05 retSelf
it should generate storeMethodVar1 insteead. When this code is executed,
it stores the value to a first slot in method's operation stack,
overwriting whatever was there.
This could have caused a segmentation violation as demonstrated in
issue #205. The reason is rather interesting:
The bytecode interpreter (in `interpret.c`) maintains a pointer
to the next bytecode as pointer into a `ByteArray` instance that
represents the bytecode. Since the GC may move the byte array at
"any" time, interpreter has to adjust BP ("bytecode pointer") each
time a GC could have happened. To do so, it stores the reference
to the bytearray in first slot of operation stack adjusting SP
accordingly. When interpreter interpreted the code above, the
bytearray got overwritten so BP got messed up after interpreter
adjusted BP after a possible GC.
Interestingly, the code from the issue works for small number of
iterations, because all the code inside inlined block is optimized
and does not involve any message send and thus there cannot be a GC
and thus no need to adjust the BP.
For large amount of interations, however, what happnes is that
some kind of an interrupt happens (IO / timer interrupt likely),
the interpreter has to give control to interrupt handler (which may
involve a GC) and when handled, it adjusts BP to a wrong value
and on subsequent intepretation step it crashes.
Note, that currently this trick with keeping bytearray in first
stack slot is not needed as we keep method in context and we can
reach method's bytecode from there.
Another lesson learned from this is that smalltalk implementation
should have some sort of bytecode verifiers, just like Java does.
https://swing.fit.cvut.cz/projects/stx-jv/ticket/205
diff -r c02671bebd0a -r 9777fd81cae8 VariableNode.st
a
|
b
|
|
628 | 628 | ]. |
629 | 629 | |
630 | 630 | ((type == #BlockVariable) |
631 | | "/ the following cannot be encountered with Smalltalk code; |
632 | | "/ however, the VM supports it for other languages (JavaScript, in particular) |
| 631 | "/ Normally one cannot assign to a block argument in smalltalk. |
| 632 | "/ However, this is supported in Smalltalk/X if ParserFlags allowAssignmentToBlockArgument: |
| 633 | "/ is set to true or when compiling other languages (JavaScript in particular). |
633 | 634 | or:[(type == #BlockArg)]) ifTrue:[ |
634 | 635 | bvIdx := index. |
635 | 636 | |
… |
… |
|
655 | 656 | "/ Generate a storeMVAR |
656 | 657 | |
657 | 658 | bvIdx := block indexOfFirstTemp + index - 1. |
658 | | bvIdx := bvIdx + block numArgs. |
| 659 | type == #BlockVariable ifTrue:[ |
| 660 | bvIdx := bvIdx + block numArgs. |
| 661 | ]. |
659 | 662 | ^ self |
660 | 663 | codeStoreOn:aStream |
661 | 664 | type:#MethodVariable index:bvIdx |
… |
… |
|
667 | 670 | "/ Generate a pushBVAR |
668 | 671 | |
669 | 672 | bvIdx := block indexOfFirstTemp + index - 1. |
670 | | bvIdx := bvIdx + block numArgs. |
| 673 | type == #BlockVariable ifTrue:[ |
| 674 | bvIdx := bvIdx + block numArgs. |
| 675 | ]. |
671 | 676 | ] ifFalse:[ |
672 | 677 | block isInlineBlock ifTrue:[ |
673 | 678 | "/ a var of a block which is inlined in another block. |
674 | 679 | "/ Generate a pushBVAR / pushOuterBVAR |
675 | 680 | bvIdx := block indexOfFirstTemp + index - 1. |
676 | | bvIdx := bvIdx + block numArgs. |
| 681 | type == #BlockVariable ifTrue:[ |
| 682 | bvIdx := bvIdx + block numArgs. |
| 683 | ]. |
677 | 684 | ] |
678 | 685 | ]. |
679 | 686 | |
… |
… |
|
806 | 813 | |
807 | 814 | "Created: / 25-06-1997 / 16:14:40 / cg" |
808 | 815 | "Modified: / 25-10-2011 / 23:41:59 / cg" |
| 816 | "Modified: / 18-04-2018 / 23:34:33 / Jan Vrany <jan.vrany@fit.cvut.cz>" |
| 817 | "Modified (comment): / 20-04-2018 / 06:21:11 / Jan Vrany <jan.vrany@fit.cvut.cz>" |
809 | 818 | ! ! |
810 | 819 | |
811 | 820 | !VariableNode methodsFor:'enumerating'! |
… |
… |
|
1195 | 1204 | |
1196 | 1205 | version_CVS |
1197 | 1206 | ^ '$Header$' |
| 1207 | ! |
| 1208 | |
| 1209 | version_HG |
| 1210 | |
| 1211 | ^ '$Changeset: <not expanded> $' |
1198 | 1212 | ! ! |
1199 | 1213 | |