search

OpenSmalltalk / opensmalltalk-vm

Cross-platform virtual machine for Squeak, Pharo, Cuis, and Newspeak.
http://opensmalltalk.org/
Other
557 stars 111 forks source link

primitiveImageFormatVersion may be not correct #638

Open marceltaeumel opened 2 years ago

marceltaeumel commented 2 years ago

The return value of primitiveImageFormatVersion is hard-coded and does not consider the value of multipleBytecodeSetsActive, which is indeed important when trying to open a particular image with a particular VM.

See:

And also:

Note that this can be fixed on the image-side by checking primitiveMultipleBytecodeSetsActive. Yet, we have a discrepancy between the definition of "image format" at time of snapshotting and during run-time.

dtlewis290 commented 2 years ago

It is a VM bug, sorry for overlooking this. Proposed fix in VMMaker inbox VMMaker.oscog-dtl.3185

dtlewis290 commented 2 years ago

Fix is in VMMaker (VMMaker.oscog-dtl.3185), still need to commit generated code and build VM.

dtlewis290 commented 2 years ago

@marceltaeumel the fix inVMMaker.oscog-dtl.3185 is trivial, but I need someone else (you or Eliot) to do the code generation this time. The reason is that my generated code has differences that I cannot account for (specifically one value in the primitiveMetadataTable array is generated as 0x100 but should be 0x200, and in several places there are differences in integer type declarations). I have not be able to track down the reason for the differences, so I cannot commit the generated code.

marceltaeumel commented 2 years ago

Wow. This is not over. Next issue is in imageSegmentVersion. I will try to fix this on the image side for now. But this is still a bug in the VM primitve 98 primitiveStoreImageSegment... :-(

marceltaeumel commented 2 years ago

The biggest issue for an image-side discrimination is that the VMMaker-specific constant 512 ... MultipleBytecodeSetsBitmask is not part of the regular image code. Which is a good thing, I suppose. Still ...

dtlewis290 commented 2 years ago

For my understanding - is the concern with imageSegmentVersion a problem for practical reasons (something does not work as expected)? It certainly makes sense that an image segment should have a format number that matches the image from which it was saved, but as a practical matter does anything bad happen if this is not the case?

OpenSmalltalk-Bot commented 2 years ago

In the extreme case, surely it could mean that the data was in a completely non-acceptable format?

On 2022-06-07, at 4:26 PM, David T Lewis @.***> wrote:

For my understanding - is the concern with imageSegmentVersion a problem for practical reasons (something does not work as expected)? It certainly makes sense that an image segment should have a format number that matches the image from which it was saved, but as a practical matter does anything bad happen if this is not the case?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

tim

tim Rowledge; @.***; http://www.rowledge.org/tim Useful random insult:- A prime candidate for natural deselection.

dtlewis290 commented 2 years ago

I am trying to understand the real-world scenario in which this matters. I have an image running Sista bytecodes and my image format is 68533. I save an image segment to disk, and that image segment contains objects that contain compiled code that contain Sista bytecodes. The saved image segment records itself with format number 68021 (not 68533). Later on, someone loads that image segment using a VM that is a year or two old, so the VM does not know about 68533. But the VM probably does know how to run Sista bytecodes because it is only a year or two old. Everything still works, so as a practical matter what is the problem?

dtlewis290 commented 2 years ago

Regarding image side code and MultipleBytecodeSetsBitmask in VMMaker, on the image side you would use this (only if necessary, and hopefully it is not necessary):

(ImageFormat fromInteger: 68021) requiresMultipleBytecodeSupport ==> false (ImageFormat fromInteger: 68533) requiresMultipleBytecodeSupport ==> true

marceltaeumel commented 2 years ago

[...] is the concern with imageSegmentVersion a problem for practical reasons [...] I am trying to understand the real-world scenario in which this matters. [...]

Well, loads of tests fail or err at the moment. Maybe we should just fix the image-side load code for image segments to also accept 68021, even if the imageFormatForSnapshot is 68533.

marceltaeumel commented 2 years ago

I committed a quickfix via System-mt.1355. Yet, this topic might need more discussion.

OpenSmalltalk-Bot commented 1 year ago

My apologies, please disregard. Marcel, thank you for doing the code generation :-)

Dave

On Thu, Jun 02, 2022 at 01:21:29PM -0700, David T Lewis wrote:

@marceltaeumel the fix inVMMaker.oscog-dtl.3185 is trivial, but I need someone else (you or Eliot) to do the code generation this time. The reason is that my generated code has differences that I cannot account for (specifically one value in the primitiveMetadataTable array is generated as 0x100 but should be 0x200, and in several places there are differences in integer type declarations). I have not be able to track down the reason for the differences, so I cannot commit the generated code.

-- Reply to this email directly or view it on GitHub: https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/638#issuecomment-1145304546 You are receiving this because you are subscribed to this thread.

Message ID: @.***>