Hello,
I found a fondamental bug in Squeak and Pharo. The next code 11 / 2
gives the fraction (11/2). It's correct. But the next code (SmallInteger>>#/) valueWithReceiver: 11 arguments: {2} gives 1 ! The problem is the method valueWithReceiver:arguements: is used hugely with method wrappers...
After long time of debugging, I found a point to debug: this method don't have the good behavior with compiled methods having a primitive that fails and executes some code after (as in SmallInteger>>#/ method when the division don't give a whole integer). In fact, when I send this message, the vm executes normally the compiled method but, in place of returns simply the good result, seems to rerun the the compiled method with other arguments (completly wrong) and returns so a wrong result.
For example, (SmallInteger>>#/) valueWithReceiver: 11 arguments: {2}
has the following execution trace :
2 isZero | 2 = 0
| returns: false returns: false
2 isMemberOf: SmallInteger | 2 class
| returns: SmallInteger | SmallInteger == SmallInteger
| returns: true returns: true
Fraction numerator: 101 denominator: 2 | Fraction new
| | Fraction basicNew | | returns: a Fraction instance
| | (a Fraction instance) initialize | | returns: a Fraction instance
| returns: a Fraction instance | a Fraction instance setNumerator: 101 denominator: 2
| | 2 = 0 | | returns: false
| | 101 asInteger | | returns: 101
| | 2 asInteger | | returns: 2
| | 2 abs | | | 2 < 0
| | | returns: false | | returns: 2
| | 2 < 0 | | returns: false
| returns: (101/2) returns: (101/2)
(101/2) reduced | 101 = 0
| returns: false | 101 gcd: 2
| | 101 = 0 | | returns: false
| | 2 \\ 101 | | returns: 2
| | 2 = 0 | | returns: false
| | 101 \\ 2 | | returns: 1
| | 1 = 0 | | returns: false
| | 2 \\ 1 | | returns: 0
| | 0 = 0 | | returns: true
| | 1 abs | | | 1 < 0
| | | returns: false | | returns: 1
| returns: 1 | 101 // 1
| returns: 101 | 2 // 1
| returns: 2 | 2 = 1
| returns: false | Fraction numerator: 101 denominator: 2
| | Fraction new | | | Fraction basicNew
| | | returns: a Fraction instance | | | (a Fraction instance) initialize
| | | returns: a Fraction instance | | returns: a Fraction instance
| | (a Fraction instance) setNumerator: 101 denominator: 2 | | | 2 = 0
| | | returns: false | | | 101 asInteger
| | | returns: 101 | | | 2 asInteger
| | | returns: 2 | | | 2 abs
| | | | 2 < 0 | | | | returns: false
| | | returns: 2 | | | 2 < 0
| | | returns: false | | returns: (101/2)
| returns: (101/2) returns: (101/2)
2 isZero | 2 = 0
| returns: false returns: false
false isMemberOf: SmallInteger | false class
| returns: False | False == SmallInteger
| returns: false returns: false
Please help me to fix this bug. I really need it works fine ! Fréd
-- Frédéric Pluquet Université Libre de Bruxelles (ULB) Assistant http://www.ulb.ac.be/di/fpluquet |
Hi Frederic -
Congrats! You found the first real VM bug in five years or so ;-) I'm not completely sure what the problem is but there is obviously something wrong here. It's easiest to recreate the problem by copying the #/ method and simply do something like: SmallInteger>>foo: aNumber "(SmallInteger>>#foo:) valueWithReceiver: 11 arguments: {2}" <primitive: 10> self halt. aNumber isZero ifTrue: [^(ZeroDivide dividend: self) signal]. (aNumber isMemberOf: SmallInteger) ifTrue: [^(Fraction numerator: self denominator: aNumber) reduced] ifFalse: [^super / aNumber] When you run this, you'll see that the debugger shows *two* frames with SmallInteger>>foo: on them. Inspecting the "parent" frame will eventually crash the system since it has a completely bogus stack pointer (-1). It seems that in a failing primitive method we end up with a bogus activation record in primitiveExecMethodWithArgs (removing the primitive from the above makes it work fine). I'm somewhat at a loss here as to what the problem might be given that the implementations of primPerformWithArgs and primExecWithArgs are so similar but interestingly, the equivalent: 11 perform: #foo: withArguments:{2}. works fine with or without the primitive. Somewhere there must be a difference ... Cheers, - Andreas Frederic Pluquet wrote: > Hello, > > I found a fondamental bug in Squeak and Pharo. The next code > > 11 / 2 > > gives the fraction (11/2). It's correct. But the next code > > (SmallInteger>>#/) valueWithReceiver: 11 arguments: {2} > > gives 1 ! > > The problem is the method valueWithReceiver:arguements: is used hugely > with method wrappers... > > After long time of debugging, I found a point to debug: this method > don't have the good behavior with compiled methods having a primitive > that fails and executes some code after (as in SmallInteger>>#/ method > when the division don't give a whole integer). In fact, when I send this > message, the vm executes normally the compiled method but, in place of > returns simply the good result, seems to rerun the the compiled method > with other arguments (completly wrong) and returns so a wrong result. > > For example, > (SmallInteger>>#/) valueWithReceiver: 11 arguments: {2} > > has the following execution trace : > > 2 isZero > | 2 = 0 > | returns: false > returns: false > 2 isMemberOf: SmallInteger > | 2 class > | returns: SmallInteger > | SmallInteger == SmallInteger > | returns: true > returns: true > Fraction numerator: 101 denominator: 2 > | Fraction new > | | Fraction basicNew > | | returns: a Fraction instance > | | (a Fraction instance) initialize > | | returns: a Fraction instance > | returns: a Fraction instance > | a Fraction instance setNumerator: 101 denominator: 2 > | | 2 = 0 > | | returns: false > | | 101 asInteger > | | returns: 101 > | | 2 asInteger > | | returns: 2 > | | 2 abs > | | | 2 < 0 > | | | returns: false > | | returns: 2 > | | 2 < 0 > | | returns: false > | returns: (101/2) > returns: (101/2) > (101/2) reduced > | 101 = 0 > | returns: false > | 101 gcd: 2 > | | 101 = 0 > | | returns: false > | | 2 \\ 101 > | | returns: 2 > | | 2 = 0 > | | returns: false > | | 101 \\ 2 > | | returns: 1 > | | 1 = 0 > | | returns: false > | | 2 \\ 1 > | | returns: 0 > | | 0 = 0 > | | returns: true > | | 1 abs > | | | 1 < 0 > | | | returns: false > | | returns: 1 > | returns: 1 > | 101 // 1 > | returns: 101 > | 2 // 1 > | returns: 2 > | 2 = 1 > | returns: false > | Fraction numerator: 101 denominator: 2 > | | Fraction new > | | | Fraction basicNew > | | | returns: a Fraction instance > | | | (a Fraction instance) initialize > | | | returns: a Fraction instance > | | returns: a Fraction instance > | | (a Fraction instance) setNumerator: 101 denominator: 2 > | | | 2 = 0 > | | | returns: false > | | | 101 asInteger > | | | returns: 101 > | | | 2 asInteger > | | | returns: 2 > | | | 2 abs > | | | | 2 < 0 > | | | | returns: false > | | | returns: 2 > | | | 2 < 0 > | | | returns: false > | | returns: (101/2) > | returns: (101/2) > returns: (101/2) > 2 isZero > | 2 = 0 > | returns: false > returns: false > false isMemberOf: SmallInteger > | false class > | returns: False > | False == SmallInteger > | returns: false > returns: false > > > Please help me to fix this bug. I really need it works fine ! > > Fréd > -- > Frédéric Pluquet > Université Libre de Bruxelles (ULB) > Assistant > http://www.ulb.ac.be/di/fpluquet > > > ------------------------------------------------------------------------ > > |
On Sun, 26 Oct 2008 13:39:04 +0100, Andreas Raab wrote:
> Hi Frederic - > > Congrats! You found the first real VM bug in five years or so ;-) I'm > not completely sure what the problem is but there is obviously something > wrong here. It's easiest to recreate the problem by copying the #/ > method and simply do something like: > > SmallInteger>>foo: aNumber > "(SmallInteger>>#foo:) valueWithReceiver: 11 arguments: {2}" > <primitive: 10> > self halt. > aNumber isZero ifTrue: [^(ZeroDivide dividend: self) signal]. > (aNumber isMemberOf: SmallInteger) > ifTrue: [^(Fraction numerator: self denominator: aNumber) reduced] > ifFalse: [^super / aNumber] > > When you run this, you'll see that the debugger shows *two* frames with > SmallInteger>>foo: on them. Inspecting the "parent" frame will > eventually crash the system since it has a completely bogus stack > pointer (-1). > > It seems that in a failing primitive method we end up with a bogus > activation record in primitiveExecMethodWithArgs (removing the primitive > from the above makes it work fine). I'm somewhat at a loss here as to > what the problem might be given that the implementations of > primPerformWithArgs and primExecWithArgs are so similar but > interestingly, the equivalent: > > 11 perform: #foo: withArguments:{2}. > > works fine with or without the primitive. Somewhere there must be a > difference ... The difference can be this: #primitiveExecuteMethod is called as part of #primitiveResponse, and in turn calls #executeNewMethod which again does #primitiveResponse. If the latter does #primitiveFail, the now *two* activations of #primitiveResponse want to handle the situation - without knowing from each other. Perhaps #primitiveExecuteMethod should end with an explicit [ ... ^ self success: true] in its true-branch (this is just a thought, I have as yet not looked deeper). Besides: if #primitiveExecuteMethod would do its own #primitiveFail, it looks like its arguments are no longer on the stack. /Klaus > Cheers, > - Andreas > > Frederic Pluquet wrote: >> Hello, >> I found a fondamental bug in Squeak and Pharo. The next code >> 11 / 2 gives the fraction (11/2). It's correct. But the next code >> (SmallInteger>>#/) valueWithReceiver: 11 arguments: {2} >> gives 1 ! >> The problem is the method valueWithReceiver:arguements: is used hugely >> with method wrappers... >> After long time of debugging, I found a point to debug: this method >> don't have the good behavior with compiled methods having a primitive >> that fails and executes some code after (as in SmallInteger>>#/ method >> when the division don't give a whole integer). In fact, when I send >> this message, the vm executes normally the compiled method but, in >> place of returns simply the good result, seems to rerun the the >> compiled method with other arguments (completly wrong) and returns so a >> wrong result. >> For example, (SmallInteger>>#/) valueWithReceiver: 11 arguments: {2} >> has the following execution trace : >> 2 isZero >> | 2 = 0 >> | returns: false >> returns: false >> 2 isMemberOf: SmallInteger >> | 2 class >> | returns: SmallInteger >> | SmallInteger == SmallInteger >> | returns: true >> returns: true >> Fraction numerator: 101 denominator: 2 >> | Fraction new >> | | Fraction basicNew >> | | returns: a Fraction instance >> | | (a Fraction instance) initialize >> | | returns: a Fraction instance >> | returns: a Fraction instance >> | a Fraction instance setNumerator: 101 denominator: 2 >> | | 2 = 0 >> | | returns: false >> | | 101 asInteger >> | | returns: 101 >> | | 2 asInteger >> | | returns: 2 >> | | 2 abs >> | | | 2 < 0 >> | | | returns: false >> | | returns: 2 >> | | 2 < 0 >> | | returns: false >> | returns: (101/2) >> returns: (101/2) >> (101/2) reduced >> | 101 = 0 >> | returns: false >> | 101 gcd: 2 >> | | 101 = 0 >> | | returns: false >> | | 2 \\ 101 >> | | returns: 2 >> | | 2 = 0 >> | | returns: false >> | | 101 \\ 2 >> | | returns: 1 >> | | 1 = 0 >> | | returns: false >> | | 2 \\ 1 >> | | returns: 0 >> | | 0 = 0 >> | | returns: true >> | | 1 abs >> | | | 1 < 0 >> | | | returns: false >> | | returns: 1 >> | returns: 1 >> | 101 // 1 >> | returns: 101 >> | 2 // 1 >> | returns: 2 >> | 2 = 1 >> | returns: false >> | Fraction numerator: 101 denominator: 2 >> | | Fraction new >> | | | Fraction basicNew >> | | | returns: a Fraction instance >> | | | (a Fraction instance) initialize >> | | | returns: a Fraction instance >> | | returns: a Fraction instance >> | | (a Fraction instance) setNumerator: 101 denominator: 2 >> | | | 2 = 0 >> | | | returns: false >> | | | 101 asInteger >> | | | returns: 101 >> | | | 2 asInteger >> | | | returns: 2 >> | | | 2 abs >> | | | | 2 < 0 >> | | | | returns: false >> | | | returns: 2 >> | | | 2 < 0 >> | | | returns: false >> | | returns: (101/2) >> | returns: (101/2) >> returns: (101/2) >> 2 isZero >> | 2 = 0 >> | returns: false >> returns: false >> false isMemberOf: SmallInteger >> | false class >> | returns: False >> | False == SmallInteger >> | returns: false >> returns: false >> Please help me to fix this bug. I really need it works fine ! >> Fréd >> -- Frédéric Pluquet >> Université Libre de Bruxelles (ULB) >> Assistant >> http://www.ulb.ac.be/di/fpluquet >> >> ------------------------------------------------------------------------ >> > > > -- “If at first, the idea is not absurd, then there is no hope for it”. Albert Einstein |
On Sun, Oct 26, 2008 at 6:21 AM, Klaus D. Witzel <[hidden email]> wrote:
I think you're exactly right. This fixes it for me.
Thanks! Besides: if #primitiveExecuteMethod would do its own #primitiveFail, it looks like its arguments are no longer on the stack. |
Er, my only comment on this is that although
in C it's defined as sqInt success(sqInt); but it's written as success: successValue successFlag := successValue & successFlag. which resolves as sqInt success(sqInt successValue) { register struct foo * foo = &fum; foo->successFlag = successValue && foo->successFlag; } or sqInt success(sqInt successValue) { foo->successFlag = successValue && foo->successFlag; } or sqInt success(sqInt successValue) { successFlag = successValue && successFlag; } so what does [ ... ^ self success: true] actually return? On Oct 26, 2008, at 7:13 PM, Eliot Miranda wrote: > Perhaps #primitiveExecuteMethod should end with an explicit [ ... ^ > self success: true] in its true-branch (this is just a thought, I > have as yet not -- = = = ======================================================================== John M. McIntosh <[hidden email]> Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com = = = ======================================================================== |
On Mon, 27 Oct 2008 06:45:20 +0100, John M McIntosh wrote:
> Er, my only comment on this is that although > in C it's defined as > > sqInt success(sqInt); > > but it's written as > success: successValue > successFlag := successValue & successFlag. > ... > so what does > [ ... ^ self success: true] > actually return? I didn't think about possible implementation (not deep enough, as said earlier). So how about [ ... ^ successFlag := true] perhaps without ^ (except for documentation). Would that tell the outer #primitiveResponse to not activate the same method a 2nd time. > On Oct 26, 2008, at 7:13 PM, Eliot Miranda wrote: > >> Perhaps #primitiveExecuteMethod should end with an explicit [ ... ^ >> self success: true] in its true-branch (this is just a thought, I have >> as yet not > > -- > === > ======================================================================== > John M. McIntosh <[hidden email]> > Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com > === > ======================================================================== > -- "If at first, the idea is not absurd, then there is no hope for it". Albert Einstein |
In reply to this post by johnmci
Well I actually wrote the fix as
primitiveExecuteMethod "receiver, args, then method are on top of stack. Execute method against receiver and args"
newMethod := self popStack. primitiveIndex := self primitiveIndexOf: newMethod.
self success: argumentCount - 1 = (self argumentCountOf: newMethod). successFlag
ifTrue: [argumentCount := argumentCount - 1. self executeNewMethod. "Recursive xeq affects successFlag"
successFlag := true] ifFalse: [self unPop: 1] to agree with the perform code. The fix is to ensure that successFlag is true.
On Sun, Oct 26, 2008 at 10:45 PM, John M McIntosh <[hidden email]> wrote: Er, my only comment on this is that although |
Free forum by Nabble | Edit this page |