Hi all,
apologies if this doesn't belong here. Mantis is a usability-nightmare and I didn't know where else to report a bug in the NewCompiler. The corresponding ML on squeakfoundation.org hasn't been posted to in over six months. I've noticed that BytecodeGenerator>>#jump:if: has a bug when given a distance >= 1024. It states hi := distance // 256. and then goes on checking if hi < 8. If that's false, the jump is assumed to be too big. However, the check should be hi < 4. Assume the method gets an argument of 1024, and hi is 4. The following self nextPut: (Bytecodes at: #longJumpIfFalse) first + hi. generates bytecode 176, which is an arithmetic message send instead of a conditional jump. I've attached a changeset that checks both jump conditions for hi < 4. Regards, André 'From Squeak3.10.1 of ''26 May 2008'' [latest update: #7175] on 22 June 2008 at 12:36:17 pm'! !BytecodeGenerator methodsFor: 'private' stamp: 'awe 6/22/2008 12:35'! jump: distance if: condition | hi | distance = 0 ifTrue: [ "jumps to fall through, no-op" ^ self nextPut: (Bytecodes at: #popStackBytecode)]. condition ifTrue: [ hi := distance // 256. hi < 4 ifFalse: [self error: 'true jump too big']. self nextPut: (Bytecodes at: #longJumpIfTrue) first + hi. self nextPut: distance \\ 256. ] ifFalse: [ distance <= 8 ifTrue: [ self nextPut: (Bytecodes at: #shortConditionalJump) first + distance - 1. ] ifFalse: [ hi := distance // 256. hi < 4 ifFalse: [self error: 'false jump too big']. self nextPut: (Bytecodes at: #longJumpIfFalse) first + hi. self nextPut: distance \\ 256. ]. ] ! ! |
Hello André,
Good find! I confirm only 4 bytes codes are reserved for longJumpIfFalse and 4 bytes code for longJumpIfTrue. So this seems like a MAJOR bug. The kind of bug able to crash squeak if you generate and execute a method with a jump > 1024 and < 2048... I recommend you post to NewCompiler mailing list, even if not very active, some members don't listen at squeak-dev. I recommend you also check latest squeak source Monticello updates for NewCompiler. (MCHttpRepository location: 'http://www.squeaksource.com/NewCompiler' user: '' password: '') Last, I recommend you persist a little bit with Mantis. Creating an account and posting a bugform should not be that difficult. What is the problem you encountered with Mantis? Nicolas André Wendt wrote: > Hi all, > > apologies if this doesn't belong here. Mantis is a usability-nightmare > and I didn't know where else to report a bug in the NewCompiler. The > corresponding ML on squeakfoundation.org hasn't been posted to in over > six months. > > I've noticed that BytecodeGenerator>>#jump:if: has a bug when given a > distance >= 1024. > > It states > > hi := distance // 256. > > and then goes on checking if hi < 8. If that's false, the jump is > assumed to be too big. However, the check should be hi < 4. > > Assume the method gets an argument of 1024, and hi is 4. The following > > self nextPut: (Bytecodes at: #longJumpIfFalse) first + hi. > > generates bytecode 176, which is an arithmetic message send instead of a > conditional jump. > > I've attached a changeset that checks both jump conditions for hi < 4. > > Regards, > André > > > ------------------------------------------------------------------------ > > |
nice wrote:
> Hello André, > > Good find! > I confirm only 4 bytes codes are reserved for longJumpIfFalse and 4 > bytes code for longJumpIfTrue. > So this seems like a MAJOR bug. The kind of bug able to crash squeak if > you generate and execute a method with a jump > 1024 and < 2048... > Just for the fun of it: how do you write non regression tests for such ones? self shouldnt: [SmalltalkImage crash] ??? |
On Sun, 22 Jun 2008 16:05:57 +0200, Nicolas wrote:
> nice wrote: >> Hello André, >> Good find! >> I confirm only 4 bytes codes are reserved for longJumpIfFalse and 4 >> bytes code for longJumpIfTrue. >> So this seems like a MAJOR bug. The kind of bug able to crash squeak if >> you generate and execute a method with a jump > 1024 and < 2048... >> > > Just for the fun of it: how do you write non regression tests for such > ones? > > self shouldnt: [SmalltalkImage crash] ??? You might want to check the CompiledMethod of the following method (the bytecode which in my .image has offset 34): !UndefinedObject methodsFor: 'as yet unclassified' stamp: 'kwl 6/22/2008 16:37'! longJumpIfFalseTest "(nil class>>#longJumpIfFalseTest) inspect" true ifTrue:[ {42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42 } ]! ! /Klaus > |
In reply to this post by Nicolas Cellier-3
nice schrieb:
> nice wrote: >> Hello André, >> >> Good find! >> I confirm only 4 bytes codes are reserved for longJumpIfFalse and 4 >> bytes code for longJumpIfTrue. >> So this seems like a MAJOR bug. The kind of bug able to crash squeak >> if you generate and execute a method with a jump > 1024 and < 2048... >> > > Just for the fun of it: how do you write non regression tests for such > ones? > > self shouldnt: [SmalltalkImage crash] ??? > > > through the simulator. The simulator would most likely generate some kind of array out of bounds exception, but it can't crash the image. Cheers, Hans-Martin |
In reply to this post by Klaus D. Witzel
Klaus D. Witzel wrote:
> On Sun, 22 Jun 2008 16:05:57 +0200, Nicolas wrote: > >> nice wrote: >>> Hello André, >>> Good find! >>> I confirm only 4 bytes codes are reserved for longJumpIfFalse and 4 >>> bytes code for longJumpIfTrue. >>> So this seems like a MAJOR bug. The kind of bug able to crash squeak >>> if you generate and execute a method with a jump > 1024 and < 2048... >>> >> >> Just for the fun of it: how do you write non regression tests for such >> ones? >> >> self shouldnt: [SmalltalkImage crash] ??? > > You might want to check the CompiledMethod of the following method (the > bytecode which in my .image has offset 34): > > !UndefinedObject methodsFor: 'as yet unclassified' stamp: 'kwl 6/22/2008 > 16:37'! > longJumpIfFalseTest > "(nil class>>#longJumpIfFalseTest) inspect" > > true ifTrue:[ > {42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. 42. > 42. 42. 42. 42. 42. 42 > } > ]! ! > > /Klaus > >> Thanks Klaus, i know how to generate the bug - believe me, I'm really good at generating bugs :) My problem is with the quality dogma: "provide a test that fails before the patch and pass after". So what would you assert about longJumpIfFalseTest? We could assert that trying to compile such a method should raise an Exception (let's say a ByteCodeLimitException). But if we install a handler that overcome the limitation (like compiling above example using a BlockClosure, true ifTrue: [aBlock value]) the test would not raise the ByteCodeLimitException and thus would fail! This would not be a very good test... A better test should be to try and compile, and if it compiles to assert the result. And if the result is a crash then... self shouldnt: [...] raise: SmalltalkImageCrash. Ah Ah, would i be the maintainer of an image containing this bug, that i would not wait for a non regression test. I would correct it as quickly as possible. If a good non regression tests comes, that's a plus, I take it. But should never be a dogma. Just take a look at http://bugs.squeak.org/view.php?id=6584 i bet the bug still is in the latests images. Nicolas |
In reply to this post by Hans-Martin Mosner
Hans-Martin Mosner wrote:
> You'd probably compile a method which would trigger the bug and run it > through the simulator. The simulator would most likely generate some > kind of array out of bounds exception, but it can't crash the image. > > Cheers, > Hans-Martin > > That's a clever way to follow. Eventually, if the Simulator doesn't raise an Exception we can still assert the result of execution. But my joke was about the dogma "provide a failure test before your change is accepted in Squeak". Correcting such bug is ten times easier than writing a TestCase, and is an emergency. So in some exceptional cases like this the dogma should be "Correct immediately then provide a non regression test case". Nicolas |
nice schrieb:
> > > But my joke was about the dogma "provide a failure test before your > change is accepted in Squeak". > > Correcting such bug is ten times easier than writing a TestCase, and > is an emergency. So in some exceptional cases like this the dogma > should be "Correct immediately then provide a non regression test case". <offtopic>As a christian I'm pretty cautious about dogmata - they are always formulated by people who have only partial insight and don't know the whole truth - as Paul wrote in 1 Cor. 13,12... So I take them with a grain of salt.</offtopic> If you only accept SUnit tests, you're bound to encounter cases such as this where it is not possible to write a meaningful failure test. But when you accept other kinds of tests, it's pretty simple. The test would be a method which when compiled and executed crashes the image. With the fix in place, the image does not crash. The most important feature of tests is that they are repeatable, which this test would obviously be. Cheers, Hans-Martin |
Hans-Martin Mosner wrote:
> nice schrieb: >> >> But my joke was about the dogma "provide a failure test before your >> change is accepted in Squeak". >> >> Correcting such bug is ten times easier than writing a TestCase, and >> is an emergency. So in some exceptional cases like this the dogma >> should be "Correct immediately then provide a non regression test case". > <offtopic>As a christian I'm pretty cautious about dogmata - they are > always formulated by people who have only partial insight and don't know > the whole truth - as Paul wrote in 1 Cor. 13,12... So I take them with a > grain of salt.</offtopic> > > If you only accept SUnit tests, you're bound to encounter cases such as > this where it is not possible to write a meaningful failure test. > But when you accept other kinds of tests, it's pretty simple. The test > would be a method which when compiled and executed crashes the image. > With the fix in place, the image does not crash. > The most important feature of tests is that they are repeatable, which > this test would obviously be. > > Cheers, > Hans-Martin > > You get it, thus the joke self shouldnt: [SmalltalkImage crash]. |
In reply to this post by Nicolas Cellier-3
On Sun, 22 Jun 2008 17:16:37 +0200, Nicolas wrote:
> So what would you assert about longJumpIfFalseTest? ;) > We could assert that trying to compile such a method should raise an > Exception (let's say a ByteCodeLimitException). No, I wouldn't do that. I'd wrap the instance of CompiledMethod into an InstructionStream using temps | is msg | and check it on the meta level, without executing the method and without the possibility that this crashes the VM, like (msg := is nextInstruction) selector == #pushConstant: and: [msg arguments = #(true)]. (msg := is nextInstruction) selector == #jump:if: and: [msg arguments = #(offset false)] where offset is the one expected. /Klaus |
In reply to this post by Nicolas Cellier-3
Hello Nicolas,
nice wrote: > Hello André, > > Good find! > I confirm only 4 bytes codes are reserved for longJumpIfFalse and 4 > bytes code for longJumpIfTrue. > So this seems like a MAJOR bug. The kind of bug able to crash squeak if > you generate and execute a method with a jump > 1024 and < 2048... Well, not in my case, I was wondering why I got a True>>doesNotUnderstand: #adaptToNumberAndSend: The reason was that a send:#+ was generated after a Boolean got pushed. So no crash, but weird behavior. YMMV. > I recommend you post to NewCompiler mailing list, even if not very > active, some members don't listen at squeak-dev. OK, I'll do that. > I recommend you also check latest squeak source Monticello updates for > NewCompiler. > (MCHttpRepository > location: 'http://www.squeaksource.com/NewCompiler' > user: '' > password: '') Well, I have confirmed this with NewCompiler-md.281.mcz, there doesn't semm to be a more recent version on [1]. > Last, I recommend you persist a little bit with Mantis. Creating an > account and posting a bugform should not be that difficult. What is the > problem you encountered with Mantis? Well, for starters the font is so small it makes my eyes hurt. I know I can increase it with my browser, but why isn't there a usable default value? :( For obvious reasons, I always want to make sure the bug isn't reported yet before filing it -- but there's no search on the homepage. :( What kind of "project" does this bug belong to? Why do I have to care? Oh wait, there it is: "View Issues". I can apply "filters", but why not a general fuzzy search? A big fat "SEARCH: [__________] [GO]" that doesn't give me a zillion options would be nice. Let's put it the marketing way: "There's room for improvement." ;-) Anyway, I'll forward this to the NewCompiler list and will file a bug in Mantis. Regards, André [1] http://www.squeaksource.com/NewCompiler/ > > Nicolas > > André Wendt wrote: >> Hi all, >> >> apologies if this doesn't belong here. Mantis is a usability-nightmare >> and I didn't know where else to report a bug in the NewCompiler. The >> corresponding ML on squeakfoundation.org hasn't been posted to in over >> six months. >> >> I've noticed that BytecodeGenerator>>#jump:if: has a bug when given a >> distance >= 1024. >> >> It states >> >> hi := distance // 256. >> >> and then goes on checking if hi < 8. If that's false, the jump is >> assumed to be too big. However, the check should be hi < 4. >> >> Assume the method gets an argument of 1024, and hi is 4. The following >> >> self nextPut: (Bytecodes at: #longJumpIfFalse) first + hi. >> >> generates bytecode 176, which is an arithmetic message send instead of a >> conditional jump. >> >> I've attached a changeset that checks both jump conditions for hi < 4. >> >> Regards, >> André >> >> >> ------------------------------------------------------------------------ >> >> > |
In reply to this post by André Wendt-3
Normally there is a mailing-list for the newcompiler but indeed we
should have a bugtracker. Stef On Jun 22, 2008, at 12:54 PM, André Wendt wrote: > Hi all, > > apologies if this doesn't belong here. Mantis is a usability-nightmare > and I didn't know where else to report a bug in the NewCompiler. The > corresponding ML on squeakfoundation.org hasn't been posted to in over > six months. > > I've noticed that BytecodeGenerator>>#jump:if: has a bug when given a > distance >= 1024. > > It states > > hi := distance // 256. > > and then goes on checking if hi < 8. If that's false, the jump is > assumed to be too big. However, the check should be hi < 4. > > Assume the method gets an argument of 1024, and hi is 4. The following > > self nextPut: (Bytecodes at: #longJumpIfFalse) first + hi. > > generates bytecode 176, which is an arithmetic message send instead > of a > conditional jump. > > I've attached a changeset that checks both jump conditions for hi < 4. > > Regards, > André > 'From Squeak3.10.1 of ''26 May 2008'' [latest update: #7175] on 22 > June 2008 at 12:36:17 pm'! > > !BytecodeGenerator methodsFor: 'private' stamp: 'awe 6/22/2008 12:35'! > jump: distance if: condition > > | hi | > distance = 0 ifTrue: [ > "jumps to fall through, no-op" > ^ self nextPut: (Bytecodes at: #popStackBytecode)]. > condition ifTrue: [ > hi := distance // 256. > hi < 4 ifFalse: [self error: 'true jump too big']. > self nextPut: (Bytecodes at: #longJumpIfTrue) first + hi. > self nextPut: distance \\ 256. > ] ifFalse: [ > distance <= 8 ifTrue: [ > self nextPut: (Bytecodes at: #shortConditionalJump) first + > distance - 1. > ] ifFalse: [ > hi := distance // 256. > hi < 4 ifFalse: [self error: 'false jump too big']. > self nextPut: (Bytecodes at: #longJumpIfFalse) first + hi. > self nextPut: distance \\ 256. > ]. > ] > ! ! > > |
In reply to this post by Klaus D. Witzel
Klaus D. Witzel wrote:
> On Sun, 22 Jun 2008 17:16:37 +0200, Nicolas wrote: > >> So what would you assert about longJumpIfFalseTest? > > ;) > >> We could assert that trying to compile such a method should raise an >> Exception (let's say a ByteCodeLimitException). > > No, I wouldn't do that. I'd wrap the instance of CompiledMethod into an > InstructionStream using temps | is msg | and check it on the meta level, > without executing the method and without the possibility that this > crashes the VM, like > > (msg := is nextInstruction) selector == #pushConstant: and: [msg > arguments = #(true)]. > (msg := is nextInstruction) selector == #jump:if: and: [msg arguments > = #(offset false)] > > where offset is the one expected. > > /Klaus > > > Thanks, that's a nice alternative which is future proof toward Bytecode definition change. However, i have two objections: 1) the test would still not pass if i install a handler that can overcome ByteCodeLimitException 2) the test won't avoid para-phrasing ByteCodeGenerator job. Because you need to calculate the offset... In other words, I would always prefer higher level tests if possible (like asserting the result of execution of ifTrue:ifFalse:). Nicolas |
In reply to this post by Nicolas Cellier-3
nice wrote:
> Hello André, > > Good find! > I confirm only 4 bytes codes are reserved for longJumpIfFalse and 4 > bytes code for longJumpIfTrue. > So this seems like a MAJOR bug. The kind of bug able to crash squeak if > you generate and execute a method with a jump > 1024 and < 2048... > > I recommend you post to NewCompiler mailing list, even if not very > active, some members don't listen at squeak-dev. > > I recommend you also check latest squeak source Monticello updates for > NewCompiler. > (MCHttpRepository > location: 'http://www.squeaksource.com/NewCompiler' > user: '' > password: '') > > Last, I recommend you persist a little bit with Mantis. Creating an > account and posting a bugform should not be that difficult. What is the > problem you encountered with Mantis? I've reported a bug on this, see http://bugs.squeak.org/view.php?id=7100 André > Nicolas > > André Wendt wrote: >> Hi all, >> >> apologies if this doesn't belong here. Mantis is a usability-nightmare >> and I didn't know where else to report a bug in the NewCompiler. The >> corresponding ML on squeakfoundation.org hasn't been posted to in over >> six months. >> >> I've noticed that BytecodeGenerator>>#jump:if: has a bug when given a >> distance >= 1024. >> >> It states >> >> hi := distance // 256. >> >> and then goes on checking if hi < 8. If that's false, the jump is >> assumed to be too big. However, the check should be hi < 4. >> >> Assume the method gets an argument of 1024, and hi is 4. The following >> >> self nextPut: (Bytecodes at: #longJumpIfFalse) first + hi. >> >> generates bytecode 176, which is an arithmetic message send instead of a >> conditional jump. >> >> I've attached a changeset that checks both jump conditions for hi < 4. >> >> Regards, >> André >> >> >> ------------------------------------------------------------------------ >> >> > |
In reply to this post by André Wendt-3
André Wendt wrote:
> Hello Nicolas, > > nice wrote: >> Hello André, >> >> So this seems like a MAJOR bug. The kind of bug able to crash squeak if >> you generate and execute a method with a jump > 1024 and < 2048... > > Well, not in my case, I was wondering why I got a > True>>doesNotUnderstand: #adaptToNumberAndSend: > > The reason was that a send:#+ was generated after a Boolean got pushed. > So no crash, but weird behavior. YMMV. > You're lucky, i crashed a st-80 image 20 years ago for the very same long jump reasons (with automatically generated code). Nicolas |
In reply to this post by André Wendt-3
André Wendt wrote:
> > I've reported a bug on this, see http://bugs.squeak.org/view.php?id=7100 > Thanks! |
In reply to this post by Nicolas Cellier-3
On Sun, 22 Jun 2008 18:17:45 +0200, Nicolas wrote:
> Klaus D. Witzel wrote: >> On Sun, 22 Jun 2008 17:16:37 +0200, Nicolas wrote: >> >>> So what would you assert about longJumpIfFalseTest? >> ;) >> >>> We could assert that trying to compile such a method should raise an >>> Exception (let's say a ByteCodeLimitException). >> No, I wouldn't do that. I'd wrap the instance of CompiledMethod into >> an InstructionStream using temps | is msg | and check it on the meta >> level, without executing the method and without the possibility that >> this crashes the VM, like >> (msg := is nextInstruction) selector == #pushConstant: and: [msg >> arguments = #(true)]. >> (msg := is nextInstruction) selector == #jump:if: and: [msg arguments >> = #(offset false)] >> where offset is the one expected. >> /Klaus >> > > Thanks, that's a nice alternative which is future proof toward Bytecode > definition change. > > However, i have two objections: > 1) the test would still not pass if i install a handler that can > overcome ByteCodeLimitException This your ififthenthenelseelse did not parse here :( > 2) the test won't avoid para-phrasing ByteCodeGenerator job. Because you > need to calculate the offset... You could use a computer, perhaps programmed in Smalltalk, for a loop which creates a CompiledMethod which is guaranteed to exhibit the desired offset? ;) > In other words, I would always prefer higher level tests if possible > (like asserting the result of execution of ifTrue:ifFalse:). > > Nicolas > |
In reply to this post by stephane ducasse
stephane ducasse wrote:
> Normally there is a mailing-list for the newcompiler but indeed we > should have a bugtracker. > > Stef > Yes, at least a NewCompiler category in Mantis. |
In reply to this post by stephane ducasse
On Sun, Jun 22, 2008 at 6:11 PM, stephane ducasse <[hidden email]> wrote: Normally there is a mailing-list for the newcompiler but indeed we should have a bugtracker. I have now set up a google code bugtracker for the NewCompiler: Marcus
|
Cool
On Jun 24, 2008, at 10:51 AM, Marcus Denker wrote: > > > On Sun, Jun 22, 2008 at 6:11 PM, stephane ducasse <[hidden email] > > wrote: > Normally there is a mailing-list for the newcompiler but indeed we > should have a bugtracker. > > I have now set up a google code bugtracker for the NewCompiler: > > http://code.google.com/p/squeaknewcompiler/issues/ > > Marcus > > > > Stef > > On Jun 22, 2008, at 12:54 PM, André Wendt wrote: > > Hi all, > > apologies if this doesn't belong here. Mantis is a usability-nightmare > and I didn't know where else to report a bug in the NewCompiler. The > corresponding ML on squeakfoundation.org hasn't been posted to in over > six months. > > I've noticed that BytecodeGenerator>>#jump:if: has a bug when given a > distance >= 1024. > > It states > > hi := distance // 256. > > and then goes on checking if hi < 8. If that's false, the jump is > assumed to be too big. However, the check should be hi < 4. > > Assume the method gets an argument of 1024, and hi is 4. The following > > self nextPut: (Bytecodes at: #longJumpIfFalse) first + hi. > > generates bytecode 176, which is an arithmetic message send instead > of a > conditional jump. > > I've attached a changeset that checks both jump conditions for hi < 4. > > Regards, > André > 'From Squeak3.10.1 of ''26 May 2008'' [latest update: #7175] on 22 > June 2008 at 12:36:17 pm'! > > !BytecodeGenerator methodsFor: 'private' stamp: 'awe 6/22/2008 12:35'! > jump: distance if: condition > > | hi | > distance = 0 ifTrue: [ > "jumps to fall through, no-op" > ^ self nextPut: (Bytecodes at: #popStackBytecode)]. > condition ifTrue: [ > hi := distance // 256. > hi < 4 ifFalse: [self error: 'true jump too big']. > self nextPut: (Bytecodes at: #longJumpIfTrue) first + > hi. > self nextPut: distance \\ 256. > ] ifFalse: [ > distance <= 8 ifTrue: [ > self nextPut: (Bytecodes at: > #shortConditionalJump) first + distance - 1. > ] ifFalse: [ > hi := distance // 256. > hi < 4 ifFalse: [self error: 'false jump too > big']. > self nextPut: (Bytecodes at: > #longJumpIfFalse) first + hi. > self nextPut: distance \\ 256. > ]. > ] > ! ! > > > > > Mth |
Free forum by Nabble | Edit this page |