[Pharo-project] Compiler bug!!!
Here the simple method: at: index <primitive: 60 error: x > ^ x And its bytecode: <primitive: 60 error: x> 13 <81 41> storeIntoTemp: 1 15 <11> pushTemp: 1 16 <7C> returnTop Now, if i add this: at: index <primitive: 60 error: x > [:foo | 5]. ^ x I get following bytecode: <primitive: 60 error: x> 17 <81 42> storeIntoTemp: 2 19 <8F 01 00 02> closureNumCopied: 0 numArgs: 1 bytes 23 to 24 23 <20> pushConstant: 5 24 <7D> blockReturn 25 <87> pop 26 <12> pushTemp: 2 27 <7C> returnTop so, if we ignore the block closure bytecodes, we have: <primitive: 60 error: x> 17 <81 42> storeIntoTemp: 2 26 <12> pushTemp: 2 27 <7C> returnTop The problem is that for method context there are only 2 temps (argument + error code) .. but not 3 (index is zero-based). The fix should be simple.. in code which calculates the temp index for 'error code' temp. -- Best regards, Igor Stasenko.
The problem is with #newTemp: used for binding a temporary in a block argument - [:foo <<< ] which increments the total number of temps in encoder (nTemps).. and as result, at the point where #fixTemp: is sent, the nTemps = 2, while actually should be = 1. resulting that error code temp having wrong index. and if you look later, the total number of temps in compiled method are not based on nTemps variable of encoder, but actually calculated from scratch: aCompiledMethodClass newBytes: blkSize trailerBytes: trailer nArgs: arguments size nTemps: (encoder supportsClosureOpcodes ifTrue: [| locals | locals := arguments, temporaries, (primErrNode ifNil: [#()] ifNotNil: [{primErrNode}]). encoder noteBlockExtent: block blockExtent hasLocals: locals. locals size] ifFalse: [encoder maxTemp]) here, locals size = 2, not 3 which means, that (storeIntoTemp: 2) won't work, because it lies outside of method's temps range. Here's even more crazy... if we compile code like that: badat: index <primitive: 60 error: x > [:foo | | xx y z | 5]. ^ x Bytecode: <primitive: 60 error: x> 17 <81 45> storeIntoTemp: 5 19 <8F 01 00 05> closureNumCopied: 0 numArgs: 1 bytes 23 to 27 23 <73> pushConstant: nil 24 <73> pushConstant: nil 25 <73> pushConstant: nil 26 <20> pushConstant: 5 27 <7D> blockReturn 28 <87> pop 29 <15> pushTemp: 5 30 <7C> returnTop -- Best regards, Igor Stasenko.
Igor, great catch. I prefer the attached. Strictly, the index needs to be fixed correctly before the node's size is determined, since it could change (not in this bytecode set, but in some future one it could). On Tue, Mar 20, 2012 at 8:38 AM, Igor Stasenko <siguctua@gmail.com> wrote:
Here the simple method:
at: index <primitive: 60 error: x > ^ x
And its bytecode:
<primitive: 60 error: x> 13 <81 41> storeIntoTemp: 1 15 <11> pushTemp: 1 16 <7C> returnTop
Now, if i add this:
at: index <primitive: 60 error: x > [:foo | 5]. ^ x
I get following bytecode:
<primitive: 60 error: x> 17 <81 42> storeIntoTemp: 2 19 <8F 01 00 02> closureNumCopied: 0 numArgs: 1 bytes 23 to 24 23 <20> pushConstant: 5 24 <7D> blockReturn 25 <87> pop 26 <12> pushTemp: 2 27 <7C> returnTop
so, if we ignore the block closure bytecodes, we have:
<primitive: 60 error: x> 17 <81 42> storeIntoTemp: 2 26 <12> pushTemp: 2 27 <7C> returnTop
The problem is that for method context there are only 2 temps (argument + error code) .. but not 3 (index is zero-based).
The fix should be simple.. in code which calculates the temp index for 'error code' temp.
-- Best regards, Igor Stasenko.
-- best, Eliot
On 20 March 2012 20:00, Eliot Miranda <eliot.miranda@gmail.com> wrote:
Igor,
  great catch.
well.. i lies on the surface, but since nobody else tried to play with error codes, i was sitting undiscovered :)
 I prefer the attached.  Strictly, the index needs to be fixed correctly before the node's size is determined, since it could change (not in this bytecode set, but in some future one it could).
yes, that's why i CCed you , because as author, you know better what is more appropriate fix to it.
On Tue, Mar 20, 2012 at 8:38 AM, Igor Stasenko <siguctua@gmail.com> wrote:
Here the simple method:
at: index     <primitive: 60 error: x >     ^ x
And its bytecode:
<primitive: 60 error: x> 13 <81 41> storeIntoTemp: 1 15 <11> pushTemp: 1 16 <7C> returnTop
Now, if i add this:
at: index     <primitive: 60 error: x >     [:foo | 5].     ^ x
I get following bytecode:
<primitive: 60 error: x> 17 <81 42> storeIntoTemp: 2 19 <8F 01 00 02> closureNumCopied: 0 numArgs: 1 bytes 23 to 24 23 Â Â Â <20> pushConstant: 5 24 Â Â Â <7D> blockReturn 25 <87> pop 26 <12> pushTemp: 2 27 <7C> returnTop
so, if we ignore the block closure bytecodes, we have:
<primitive: 60 error: x> 17 <81 42> storeIntoTemp: 2 26 <12> pushTemp: 2 27 <7C> returnTop
The problem is that for method context there are only 2 temps (argument + error code) .. but not 3 (index is zero-based).
The fix should be simple.. in code which calculates the temp index for 'error code' temp.
-- Best regards, Igor Stasenko.
-- best, Eliot
-- Best regards, Igor Stasenko.
participants (2)
-
Eliot Miranda -
Igor Stasenko