Skip to content

Fix meta try primitive #968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: pharo-12
Choose a base branch
from

Conversation

guillep
Copy link
Member

@guillep guillep commented May 7, 2025

Bug

Debugging the follign code (Cmd+Shift+D)

"Call primitive 60 (array at:) doing an out of bounds"
#( 177 ) tryPrimitive: 118 withArgs: #( 60 #( 2 ) )

And stepping into tryPrimitive:withArgs: crashes the VM.

Cause

This happens because the primitive stores intermediate data in special vm variables (tempOop and tempOop2) that are remapped in case a GC passes and later restored if the called primitive fails.

In this case, the call to primitive 118 is reentrant: primitive 118 calls primitive 118.
And this causes the lose of the tempOop and tempOop2 values of the first call.

TODOs after this PR

  • The error code of the original primitive is lost in the reentrant call
  • These special variables are there for a single case that is extremely unlikely: when the primitive called by 118 is an external primitive(117) and that external primitive fails with PrimErrNoMemory, the VM performs a GC during the primitive. This seems to break some other system invariants, this could be reviewed to work more alike to basicNew et al

Comment on lines +980 to +984
savedTempOop2 := tempOop2.
tempOop2 := self stackValue: 3. "actual receiver"
rcvr := self stackValue: 2. "receiver for primitive"
(objectMemory isOopForwarded: rcvr) ifTrue: [
rcvr := objectMemory followForwarded: rcvr ].
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a nice side effect, fixing the indexes here (0 based) "fixes" the following test in Pharo:

MirrorPrimitivesTest >> testExecutingPrimitive
	| actual |
	<expectedFailure> "it will be supported by VM at some point"

	actual := MirrorPrimitives withReceiver: 100 tryPrimitive: 1 withArguments: #(2).

	self assert: actual equals: 102

self pop: 4; push: rcvr] "use first arg as receiver"
ifFalse:
[self pop: 2].
ifTrue: [ "use first arg as receiver""...and receiver if the three arg form"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This first comment got moved by the reformatting, I'll put it back

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant