[squeak-dev] Bug in primitivePerform (was: The Inbox: KernelTests-ct.382.mcz)

Thiede, Christoph Christoph.Thiede at student.hpi.uni-potsdam.de
Wed Jan 6 00:22:46 UTC 2021


Agreed. Put this back onto my to-do list. Please don't merge until then. :-)


Still, could we at least make primitive 118 a bit more robust against missing arguments? Its name implies trial and error, so it's a little disappointing to be kicked out of the VM in case of "error". Or is #tryPrimitive:withArgs: simply not meant to be a public selector but rather a private one that you should wrap with LBYL?


Best,

Christoph

<http://www.hpi.de/>
________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
Gesendet: Dienstag, 5. Januar 2021 12:00:39
An: squeak-dev
Betreff: Re: [squeak-dev] Bug in primitivePerform (was: The Inbox: KernelTests-ct.382.mcz)

>  IMO the VM isn't obligated to implement checking for nonsensical definitions.  That would slow the VM down unnecessarily.  Instead one should *never* use primitive 83 in a method that takes no arguments.

+1

Best,
Marcel

Am 02.01.2021 02:39:55 schrieb Eliot Miranda <eliot.miranda at gmail.com>:

Hi Christoph,

On Fri, Jan 1, 2021 at 11:07 AM Thiede, Christoph <Christoph.Thiede at student.hpi.uni-potsdam.de<mailto:Christoph.Thiede at student.hpi.uni-potsdam.de>> wrote:

Hi all,


please note that primitive 83 is currently crashing the VM (tested with VMMaker.oscog-eem.2850) when called without any arguments, so #testPrimitive83 in the patch below will break the test execution at the moment. However, primitive 84 works well, so to me this looks like a trivial mistake only.

Hang on.  When would one include primitive 83 in a method that takes no arguments?  Implementing e.g.
Object>>perform
    "Invoke the perform primitive with no arguments cuz there's nothing to perform"
    <primitive: 83>
   self error: 'you don''t say'

makes no sense.  IMO the VM isn't obligated to implement checking for nonsensical definitions.  That would slow the VM down unnecessarily.  Instead one should *never* use primitive 843 in a method that takes no arguments.

Hence in your simulation tests you can exclude the case where someone tries tryPrimitive: 83 withArguments: #() because it constitutes undefined behaviour and the VM crashing is IMO to be expected.


I have (still!) not yet made myself the gift of exploring VMMaker, so I won't be able to fix a patch at the moment. But if someone can solve this, it would be great if you could give me a short pointer to the corresponding VMMaker patch because studying small interesting patches appear as a very nice way to explore an unknown framework to me. :-)

Best,
Christoph

________________________________
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org<mailto:squeak-dev-bounces at lists.squeakfoundation.org>> im Auftrag von commits at source.squeak.org<mailto:commits at source.squeak.org> <commits at source.squeak.org<mailto:commits at source.squeak.org>>
Gesendet: Freitag, 1. Januar 2021 19:59 Uhr
An: squeak-dev at lists.squeakfoundation.org<mailto:squeak-dev at lists.squeakfoundation.org>
Betreff: [squeak-dev] The Inbox: KernelTests-ct.382.mcz

Christoph Thiede uploaded a new version of KernelTests to project The Inbox:
http://source.squeak.org/inbox/KernelTests-ct.382.mcz

==================== Summary ====================

Name: KernelTests-ct.382
Author: ct
Time: 1 January 2021, 7:59:13.17819 pm
UUID: 812293dd-db7d-4c03-a33d-8b6f4354d1fa
Ancestors: KernelTests-ct.375, KernelTests-tonyg.381

Tests simulation of #perform:... primitives 83, 84, and 100. Complements Kernel-ct.1367.

Depends indeed not only on KernelTests-tonyg.381 but also on KernelTests-ct.375, it would be nice if we could get the latter merged soon, this has already been causing too many merge conflicts in the past. :-)

=============== Diff against KernelTests-ct.375 ===============

Item was added:
+ ----- Method: ContextTest>>testPrimitive100 (in category 'tests') -----
+ testPrimitive100
+
+        {
+                {#isNil. {}. Object}. "valid 0-arg message"
+                {#=. {true}. UndefinedObject}. "valid unary message"
+                {#ifNil:ifNotNil:. {[2]. [:x | x]}. Object}. "valid binary message"
+                {}. "missing selector"
+                {#isNil}. "missing arguments"
+                {#isNil. 'not an array'}. "invalid arguments"
+                {#isNil. {}}. "missing lookupClass"
+                {#isNil. {'excess arg'}. Object}. "too many arguments"
+                {#=. {}. UndefinedObject}. "missing argument"
+                {#isNil. {}. Boolean}. "lookupClass not in inheritance chain"
+        } do: [:args |
+                self
+                        assert: (Context runSimulated: [nil tryPrimitive: 100 withArgs: args])
+                        equals: (nil tryPrimitive: 100 withArgs: args)].!

Item was added:
+ ----- Method: ContextTest>>testPrimitive83 (in category 'tests') -----
+ testPrimitive83
+
+        {
+                {#isNil}. "valid 0-arg message"
+                {#=. true}. "valid unary message"
+                {#ifNil:ifNotNil:. [2]. [:x | x]}. "valid binary message"
+                {}. "missing selector"
+                {#isNil. 'excess arg'}. "too many arguments"
+                {#=}. "missing argument"
+        } do: [:args |
+                self
+                        assert: (Context runSimulated: [nil tryPrimitive: 83 withArgs: args])
+                        equals: (nil tryPrimitive: 83 withArgs: args)].!

Item was added:
+ ----- Method: ContextTest>>testPrimitive84 (in category 'tests') -----
+ testPrimitive84
+
+        {
+                {#isNil. {}}. "valid 0-arg message"
+                {#=. {true}}. "valid unary message"
+                {#ifNil:ifNotNil:. {[2]. [:x | x]}}. "valid binary message"
+                {}. "missing selector"
+                {#isNil}. "missing arguments"
+                {#isNil. 'not an array'}. "invalid arguments"
+                {#isNil. {'excess arg'}}. "too many arguments"
+                {#=. {}}. "missing argument"
+        } do: [:args |
+                self
+                        assert: (Context runSimulated: [nil tryPrimitive: 84 withArgs: args])
+                        equals: (nil tryPrimitive: 84 withArgs: args)].!





--
_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20210106/9a9908d5/attachment.html>


More information about the Squeak-dev mailing list