[OpenSmalltalk/opensmalltalk-vm] pushRcvr fails for context instances whose sender has been set to an Integer instance (Issue #654)
Example to reproduce (in Squeak):
```smalltalk (Context basicNew: 16) privSender: 1; pc ```
For me, this reproducibly crashes the VM.
<details><summary>Stack backtrace</summary><pre><code> [00007ff7460a73f7] ??? + 0x173f7 in SqueakConsole.exe [00007ff7465372cc] Cog method with nil selector + 0xbc in CogCode [00007ff746401520] ceReturnToInterpreterTrampoline + 0x0 in CogCode [00007ff7478f04f6] ??? + 0x0 in (null) [00007ff7478f09e8] ??? + 0x0 in (null) [00007ff746498543] Cog method with nil selector + 0x213 in CogCode [00007ff746402906] on:do: + 0xa6 in CogCode [00007ff746401520] ceReturnToInterpreterTrampoline + 0x0 in CogCode [00007ff746401550] ceBaseFrameReturnTrampoline + 0x0 in CogCode</code></pre></details>
Other examples:
```smalltalk (Context basicNew: 16) privSender: 1; method. "nil -- doesn't crash" (Context basicNew: 16) privSender: 1; receiver. "nil -- doesn't crash" (Context basicNew: 16) privSender: 1; sender. "crashes!" (Context basicNew: 16) privSender: 1; isMorph. "false - doesn't crash" (Context basicNew: 16) privSender: 1; yourself. "aContext - beware! crashes one or two seconds later without a backtrace" ```
Unless the context instance is executed by the VM, this should not happen. This is an annoying limitation for "heap fuzzing", i.e., randomly creating and assigning object instances, as done in SimulationStudio, for instance.
I’m sorry but this counts as shooting oneself in the foot. This should be handled in the fuzzed as an exception. Contexts are created via Context class>>#sender:receiver:method:arguments:, or if via newForMethod:, properly initialized with a method soon thereafter. Why? Because supporting methodless contexts adds checks in critical paths that will slow down not only process switch, but also return.
If there is a bug it is that Context class>>#newMethod: does not initialize the method inst var. I shall fix this in trunk. Then any fuzzer must be written to test contexts as a special case and instantiate them via Context class>>#newMethod: or Context class>>#sender:receiver:method:arguments:
Please see Kernel-eem.1489 in the inbox.
Thanks for the reply! My concrete issue is not related to setting method to nil but to setting sender to an Integer. Kernel-eem.1489 looks logical, but I can still do `([] asContextWithSender: 1) pc` to crash the VM.
I do understand that the way the VM treats Context instances is an important optimization, but can we maybe have an explicit contract for what an image must not not do with any (non-married) Context instances in order to prevent the VM from crashing? For instance, maybe: A Context's sender must be either nil or a (sub)instance of Context. A Context's stackp must be either nil or an Integer and it must be changed via primitiveStoreStackp only but not via primitiveSlotAtPut. Etc. ...
Such a contract would help me design my fuzzer in a way that keeps the VM alive. Additionally, we *could* consider checking the class of the argument in Context>>#privSender: et al (if this does not impact performance too much). What do you think? :)
Ah, of course. I understand now. Setting to an integer has disastrous consequences for the vm. That must be prevented.
Possibly by coincidence Jaromir brought up the issue on squeak-dev, suggesting
```Context>>privSender: aContextOrNil
sender := aContextOrNil ifNotNil: [aContextOrNil asContext]```
which I think is good. I would validate by sending asContext in the relevant places. If necessary we can introduce something only understood by Context and not by BlockClosure, eg raiseErrorIfNotContext, which I suppose could be implemented in Object to raise an error.
vm-dev@lists.squeakfoundation.org
-
Christoph Thiede -
Eliot Miranda