I'm not yet sure what to do about it, but the problem was introduced last April in VMMaker-dtl.415. Apparently the obsolete primitiveInvokeObjectAsMethod was not quite as obsolete as I thought it was.

Dave

Name: VMMaker-dtl.415 Author: dtl Time: 19 April 2020, 5:30:30.208 pm UUID: 747f8591-57e6-4950-858a-c7fbc22ad1c2 Ancestors: VMMaker-dtl.414

VMMaker 4.19.1 Required for Squeak trunk Collections-eem.885 and above.

Install primitiveArrayBecomeOneWayNoCopyHash as primitive 248, replacing obsolete primitiveInvokeObjectAsMethod.

Fix 128 primitiveArrayBecome to match oscog logic. This a fix from VMMaker.oscog-eem.647 which Eliot explained as follows: Fix primitiveArrayBecome (the two-way become); it should /not/ specify copyHash. ObjectMemory ignores the copyHash flag when doing a two-way become, hence the wrong sense of the flag had no effect. Hence correctly evaluate testBecomeIdentityHash.

The primitives now do this: primitive 72: twoWay: false copyHash: true primitive 128: twoWay: true copyHash: false primitive 248: twoWay: false copyHash: false primitive 249: twoWay: false copyHash: <arg 3 value>

On Sun, Feb 21, 2021 at 11:29:13AM -0500, David T. Lewis wrote:

Thanks.

Confirmed on Linux amd64, so it is not Solaris/OpenIndiana issue.

Dave

On Sun, Feb 21, 2021 at 04:02:38PM +0100, stes@PANDORA.BE wrote:

...

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256

As can be seen in the screenshot at:

http://docs.openindiana.org/handbook/community/squeak/index.html

I'm deselecting the Tests-ObjectsAsMethods test (1 test), because it causes (reproducible) SIGSEGV on Solaris cc/OpenIndiana gcc.

I think the segmentation fault is new in recent 4.19, I think it didn't happen a while ago in 4.16.

I can test this as follows: when I install an older version

squeak -version 4.16.7-3775

then I go into test runner: Tests-ObjectsAsMethods and select TestObjectsAsMethods that works in 4.16.7

Test Runner

... TestObjectsAsMethods

3 run, 3 passes, 0 expected failures, 0 failures, 0 errors, 0 unexpected passes

But it stopped working in 4.19.x which is from I believe from:

ContextInterpreter VMMaker-dtl.422 uuid: e72b95a0-204e-45a1-a4e4-3ac3c9e7a51a

the interp.c file is automatically generated from VMMaker-dtl.422.

It's reproducible in the sense that if I deselect all tests, and just select that one single test, I can repeatedly and reproducible SIGSEGV the VM.

When I run the VM under a debugger:

dbx: warning: Bad transition in runtime linker interface. CONSISTENT->CONSISTENT t@1 (l@1) signal SEGV (no mapping at the fault address) in interpret at line 9120 in file "interp.c" 9120 foo->freeContexts = longAt((newContext + (BASE_HEADER_SIZE)) + (0 << (SHIFT_FOR_WORD)));

(dbx) where current thread: t@1 =>[1] interpret(), line 9120 in "interp.c" [2] main(argc = 1, argv = 0xfeffe250, envp = 0xfeffe258), line 1484 in "sqUnixMain.c"

The above is from Solaris with cc/dbx but the same thing appears to happen for me on OpenIndiana with gcc/gdb.

Unfortunately because the code of interp.c is automatically generated, it looks complicated to me and I don't see what's wrong with those " freeContext" code.

The crash appears to be in:

                            /* begin internalActivateNewMethod */
                            methodHeader = longAt((foo->newMethod + (BASE_HEADER_SIZE)) + (HeaderIndex << (SHIFT_FOR_WORD)));
                            needsLarge = methodHeader & LargeContextBit;
                            if ((needsLarge == 0) && (foo->freeContexts != NilContext)) {
                                    newContext = foo->freeContexts;
                                    /* begin setFreeContextsAfter: */
                                    foo->freeContexts = longAt((newContext + (BASE_HEADER_SIZE)) + (0 << (SHIFT_FOR_WORD)));
                            } else {
                                    /* begin externalizeIPandSP */

Has anyone seen this ?

Also what is the test TestObjectsAsMethods actually doing please ? what is it testing ?

Regards, David Stes

-----BEGIN PGP SIGNATURE----- Version: GnuPG v2

iQEcBAEBCAAGBQJgMnV5AAoJEAwpOKXMq1MafEgH/3EWQxaSaVm2g4r/2p99Wc21 P+U+ijqKpVTDfJ1smwV/GsgF0V8ZrZky0k7BzRDAyq3Gi/HGVm0e2bqOAKa1fo2Y MUS9JHOW4Lys+9qWgT0aLiWypjYlzThtYS0/Lfh013tsF1bBv2eppTceUyq/Zitv 6J0IFvDOspMN/zHwBw/ux3H6uR049boZ3mvk23sp3KIHDc2Yw2kF4TAXBwjZXmVO UFlIAC4EAahrtNZyLZSIBDbsXOl+wJGmQTsOIBG81pfSFpP6RBrIARcu6enZC3Wc bwsvWYADs49SKgVq3NBovfyzkZBIW30V82xlVKpOnp6A4FnOYXxQiVm9sNaOVXc= =34TJ -----END PGP SIGNATURE-----

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256

Install primitiveArrayBecomeOneWayNoCopyHash as primitive 248 replacing obsolete primitiveInvokeObjectAsMethod.

Any news on this Tests-ObjectsAsMethods issue ?

The purpose of those tests are not clear to me;

Perhaps the change for the "primitives" which was made, can be reverted ?

Possibly that creates a new problem for primitiveArrayBecomeOneWayNoCopyHash.

David Stes

On Mon, Mar 1, 2021 at 16:05 David T. Lewis lewis@mail.msen.com wrote:

On Mon, Mar 01, 2021 at 06:20:58PM +0100, stes@PANDORA.BE wrote:

...

...

Install primitiveArrayBecomeOneWayNoCopyHash as primitive 248 replacing obsolete primitiveInvokeObjectAsMethod.

Any news on this Tests-ObjectsAsMethods issue ?

The purpose of those tests are not clear to me;

Perhaps the change for the "primitives" which was made, can be reverted ?

Possibly that creates a new problem for

primitiveArrayBecomeOneWayNoCopyHash.

...

David Stes

You can consider this a bug in the most recent interpreter VMs. I had failed to notice that there is a hard coded reference to that primitive number within the VM itself, and this reference is now resulting in invocation of the wrong function.

A trivial fix is to rearrange some primtive number assignments, but the correct thing to do is adopt the logic that Eliot uses in oscog so that the function is not invoked as a "primitive" at all.

I'll try to follow up on this next week.

Dave

This test crashes SqueakJS, too. Could be the same primitive reassignment confusion.

Can you describe this “hard coded reference”? Under what circumstances is a primitive being called without being an actual primitive?

Vanessa

On Fri, Mar 19, 2021 at 11:47 AM David T. Lewis lewis@mail.msen.com wrote:

On Fri, Mar 19, 2021 at 09:11:52AM -0700, Vanessa Freudenberg wrote:

...

On Mon, Mar 1, 2021 at 16:05 David T. Lewis lewis@mail.msen.com wrote:

...

On Mon, Mar 01, 2021 at 06:20:58PM +0100, stes@PANDORA.BE wrote:

...

...

Install primitiveArrayBecomeOneWayNoCopyHash as primitive 248 replacing obsolete primitiveInvokeObjectAsMethod.

Any news on this Tests-ObjectsAsMethods issue ?

The purpose of those tests are not clear to me;

Perhaps the change for the "primitives" which was made, can be

reverted ?

...

...

...

Possibly that creates a new problem for

primitiveArrayBecomeOneWayNoCopyHash.

...

David Stes

You can consider this a bug in the most recent interpreter VMs. I had failed to notice that there is a hard coded reference to that primitive number within the VM itself, and this reference is now resulting in invocation of the wrong function.

A trivial fix is to rearrange some primtive number assignments, but the correct thing to do is adopt the logic that Eliot uses in oscog so that the function is not invoked as a "primitive" at all.

I'll try to follow up on this next week.

Dave

This test crashes SqueakJS, too. Could be the same primitive reassignment confusion.

Can you describe this ???hard coded reference???? Under what

circumstances is a

...

primitive being called without being an actual primitive?

Hi Vanessa,

Sorry I did not think about SqueakJS and yes I am sure it is the same issue.

The hard coded reference to primitive 248 is in ContextInterpreter>>lookupMethodInDictionary: which assumes that primitiveInvokeObjectAsMethod is installed as primitive 248.

I tried assigning primitiveInvokeObjectAsMethod to primitive number 575 in initializePrimitiveTable (last available entry in the current table, which appeared to be unused), and changed lookupMethodInDictionary: to call 575 rather than 248.

That worked fine until I tried running a trunk-level V3 image, which failed because we are now calling primitive 575 for primitiveHighBit.

I'm sure there's some other primitive number we could use to hide primitiveInvokeObjectAsMethod elsewhere in the primitive table, but this approach seems obviously wrong to me, and the right thing would be to do whatever Eliot did in oscog to avoid trying to pretend that this method is a primitive (which it is not despite its name).

At that point I had no more time to follow up on this, sorry. But that's where the problem lies, and I'm sure there is a way to do this correctly based on Eliot's implementation.

Dave

Ah that makes sense. Yes, in SqueakJS it is used directly too:

https://github.com/codefrau/SqueakJS/blob/02d88e6cd5592029071c1939d85181b79a...

... and the easiest fix would indeed be to assign an unused primitive number, because that would require no changes to the rest of the execution mechanism. Putting it in the lookup phase, and caching it, seems quite elegant. Otherwise we would have to find another place to check that the method dict entry is actually a method, and invoke the run:with:in: mechanism if not.

Vanessa

On 2021-03-20, at 9:01 AM, David T. Lewis lewis@mail.msen.com wrote:

That protects it against accidental invocation from the image as more numbered primitives get added (yuk). I'll give that a try and see how it looks.

I really wish for no numbered primitives at all. I thought I had worked out how once upon a time but not quite.

tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Diagnostics are the programs that run when nothing else will.

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256

A trivial fix is to rearrange some primtive number assignments, but the correct thing to do is adopt the logic that Eliot uses in oscog so that the function is not invoked as a "primitive" at all.

I'll try to follow up on this next week.

Would it please be possible to commit to subversion the trivial fix, while waiting for the correct fix.

As I understand it, this means to undo the "primitives" change.

David Stes

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256

If a good solution exists, it's better of course than a quick (and possibly) wrong short term solution ...

Thanks for having looked into this.

David Stes

On Sat, Mar 20, 2021 at 10:37 AM David T. Lewis lewis@mail.msen.com wrote:

On Thu, Mar 18, 2021 at 03:05:29PM +0100, stes@PANDORA.BE wrote:

...

If a good solution exists, it's better of course than a quick (and possibly) wrong short term solution ...

Thanks for having looked into this.

David Stes

Hi David,

I found a "good enough" solution. Sources are updated in SVN r3799.

Dave

Wow, I didn't realize until just now you're still maintaining the interpreter sources on SVN. I mean I knew you did it somewhere, I just never bothered to look at it 😳

Also found your VMMaker-dtl.423 with the changes. I just did the same hack in SqueakJS. It was good enough to not crash the image anymore.

I also had to add a VMMaker version to vm attribute 1004, because of ImageSegmentTest>>testSelectors 😬

Then 2 out of 3 objects-as-methods test were working, the 3rd one revealed a bug, with the second line in testAddNumbers erroring.

Tracking that one down was slightly tricky because it looked like it had to do with the perform. But after banging on it for a while, I found that if the method/object was found in the cache, the current message selector global would not be updated. The only place where that is used is in primitiveInvokeObjectAsMethod. 😜

Now all 3 tests are green. Yay! Thanks for the original report, David S!

- Vanessa -

On Sat, Mar 20, 2021 at 08:14:30PM +0100, stes@PANDORA.BE wrote:

Also I'll try to test SqueakJS which is something I have no experience with, at all (JS = Javascript).

As far as I understand, SqueakJS is a Javascript runtime for Squeak.

I'll try to test that as well ...

I'm learning about all the different runtimes ...

• squeak-5 (OpenSmalltalk) and squeak-5 Cog Spur (COG VM)
• squeak-4 classical VM
• squeakJS javascript

I certainly encourage you to get acquainted with SqueakJS. It is not the fastest VM for Squeak, but is it certainly the most portable with respect to supporting various Squeak images, and it demonstrates that the virtual machine that runs the Squeak image does not necessarily need to be tied to any specific operating system or implementation language.

Dave

Hi David,

Sorry I did not reply earlier,

On Sun, Mar 14, 2021 at 07:30:25PM +0100, stes@PANDORA.BE wrote:

...

A trivial fix is to rearrange some primtive number assignments, but the correct thing to do is adopt the logic that Eliot uses in oscog so that the function is not invoked as a "primitive" at all.

I'll try to follow up on this next week.

Would it please be possible to commit to subversion the trivial fix, while waiting for the correct fix.

Unfortunately no. I tried reassigning primitiveInvokeObjectAsMethod to an unused primitive number. That worked fine, but I quickly found that more recent versions of Squeak trunk were using that "unused" primitive number for some other purpose, so this is a no-go.

A quick and dirty fix is not feasible, it needs to be done right. That means adapting or adopting Eliot's implementation.

<OT> <RANT> Numbered primitives are a plague and a pestilence. Please just stop doing that. </RANT> </OT>

Dave