Faculty of Information Technology
Software Engineering Group

Ticket #205: libcomp_fix_1_of_1_rev_9777fd81cae8_Issue__205__fixed_compilation_of_assignment_into_inlined_block_argument.patch

File libcomp_fix_1_of_1_rev_9777fd81cae8_Issue__205__fixed_compilation_of_assignment_into_inlined_block_argument.patch, 5.1 KB (added by Jan Vrany, 19 months ago)
  • VariableNode.st

    # 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  
    628628    ].
    629629
    630630    ((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).
    633634    or:[(type == #BlockArg)]) ifTrue:[
    634635        bvIdx := index.
    635636       
     
    655656                "/ Generate a storeMVAR
    656657
    657658                bvIdx := block indexOfFirstTemp + index - 1.
    658                 bvIdx := bvIdx + block numArgs.
     659                type == #BlockVariable ifTrue:[
     660                    bvIdx := bvIdx + block numArgs.
     661                ].
    659662                ^ self
    660663                    codeStoreOn:aStream
    661664                    type:#MethodVariable index:bvIdx
     
    667670            "/ Generate a pushBVAR
    668671
    669672            bvIdx := block indexOfFirstTemp + index - 1.
    670             bvIdx := bvIdx + block numArgs.
     673            type == #BlockVariable ifTrue:[     
     674                bvIdx := bvIdx + block numArgs.
     675            ].
    671676        ] ifFalse:[
    672677            block isInlineBlock ifTrue:[
    673678                "/ a var of a block which is inlined in another block.
    674679                "/ Generate a pushBVAR / pushOuterBVAR
    675680                bvIdx := block indexOfFirstTemp + index - 1.
    676                 bvIdx := bvIdx + block numArgs.
     681                type == #BlockVariable ifTrue:[     
     682                    bvIdx := bvIdx + block numArgs.
     683                ].
    677684            ]
    678685        ].
    679686
     
    806813
    807814    "Created: / 25-06-1997 / 16:14:40 / cg"
    808815    "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>"
    809818! !
    810819
    811820!VariableNode methodsFor:'enumerating'!
     
    11951204
    11961205version_CVS
    11971206    ^ '$Header$'
     1207!
     1208
     1209version_HG
     1210
     1211    ^ '$Changeset: <not expanded> $'
    11981212! !
    11991213