[Vm-dev] bugs in DeflatePlugin?

[Marcel Taeumel]

Hi Florin, hi Fabio --

Fixed via VMMaker.oscog-mt.3135.

Thanks for reporting this!

Best,
Marcel
Am 10.01.2022 19:39:46 schrieb Florin Mateoc <florin.mateoc at gmail.com>:
Also now MCSerializationTest>>#testMczSerialization is succeeding for me,
it was failing before (in JsSqueak)

On Mon, Jan 10, 2022 at 3:42 AM Fabio Niephaus wrote:

>
> Hi Florin,
>
> Happy new year and thanks for the bug report! PNGReadWriterTest seems
> to pass on TruffleSqueak (although I remember some bug fixing a while
> ago). Anyway, I looked into #primitiveZipSendBlock which seems to be
> the only sender of #loadZipEncoderFrom:.
>
> While #loadZipEncoderFrom: reads from writeStreamInstSize + 1 and + 2,
> which is off by one as you mentioned, #primitiveZipSendBlock stores at
> readStreamInstSize + 1 and + 2. And as it happens, readStreamInstSize
> seems to be 4 while writeStreamInstSize is 5, which compensates for
> the off-by-one error. However, I don't understand why reading from the
> wrong positions does not cause the tests to fail. Anyway, I cleaned
> this up in TruffleSqueak (see [1]).
>
> Fabio
>
> [1]
> https://github.com/hpi-swa/trufflesqueak/commit/5b6066b54e03e19ec4c4d2b22550ff081e46f6e6
>
> On Mon, Jan 10, 2022 at 4:33 AM Florin Mateoc
> wrote:
> >
> >
> > I should have also mentioned that PNGReadWriterTest also has 18 failures
> for SqueakJS, and I think I remember them failing in TruffleSqueak as well
> >
> > On Sun, Jan 9, 2022 at 10:18 PM Florin Mateoc
> wrote:
> >>
> >> Hi all, and Happy New Year!
> >>
> >> I think I just found a couple of bugs in the DeflatePlugin. They were
> causing errors in my JsSqueak, I don't fully understand why they don't seem
> to cause problems in Squeak. Anyway, after I fixed them, the primitives
> work and the PNGReadWriterTest tests are all succeeding for me.
> >>
> >> One is in #loadZipEncoderFrom:
> >> The method checks and then determines the writeStreamInstSize and then
> fetches the bitBuffer and bitPosition (the first two instvars of
> ZipEncoder, a direct subclass of WriteStream) from offsets
> writeStreamInstSize + 1 and writeStreamInstSize + 2, although the fetches
> are 0-based
> >>
> >> The other is in #primitiveZipSendBlock
> >> The method, still working on a ZipEncoder, at the end stores the
> modified values for bitBuffer and bitPosition at offset readStreamInstSize
> + 1 and readStreamInstSize + 2, which is just wrong.
> >>
> >> As I said, I don't quite get why this does not cause at least
> image-side errors if not VM crashes in Squeak, but they should probably be
> fixed anyway :)
> >>
> >> All the best,
> >> Florin
>

Also now MCSerializationTest>>#testMczSerialization is succeeding for me, it was failing before
(in JsSqueak)



On Mon, Jan 10, 2022 at 3:42 AM Fabio Niephaus <lists at fniephaus.com [mailto:lists at fniephaus.com]> wrote:

 

Hi Florin,



Happy new year and thanks for the bug report! PNGReadWriterTest seems

to pass on TruffleSqueak (although I remember some bug fixing a while

ago). Anyway, I looked into #primitiveZipSendBlock which seems to be

the only sender of #loadZipEncoderFrom:.



While #loadZipEncoderFrom: reads from writeStreamInstSize + 1 and + 2,

which is off by one as you mentioned, #primitiveZipSendBlock stores at

readStreamInstSize + 1 and + 2. And as it happens, readStreamInstSize

seems to be 4 while writeStreamInstSize is 5, which compensates for

the off-by-one error. However, I don't understand why reading from the

wrong positions does not cause the tests to fail. Anyway, I cleaned

this up in TruffleSqueak (see [1]).



Fabio



[1] https://github.com/hpi-swa/trufflesqueak/commit/5b6066b54e03e19ec4c4d2b22550ff081e46f6e6 [https://github.com/hpi-swa/trufflesqueak/commit/5b6066b54e03e19ec4c4d2b22550ff081e46f6e6]



On Mon, Jan 10, 2022 at 4:33 AM Florin Mateoc <florin.mateoc at gmail.com [mailto:florin.mateoc at gmail.com]> wrote:

>

>

> I should have also mentioned that PNGReadWriterTest also has 18 failures for SqueakJS, and I think I remember them failing in TruffleSqueak as well

>

> On Sun, Jan 9, 2022 at 10:18 PM Florin Mateoc <florin.mateoc at gmail.com [mailto:florin.mateoc at gmail.com]> wrote:

>>

>> Hi all, and Happy New Year!

>>

>> I think I just found a couple of bugs in the DeflatePlugin. They were causing errors in my JsSqueak, I don't fully understand why they don't seem to cause problems in Squeak. Anyway, after I fixed them, the primitives work and the PNGReadWriterTest tests are all succeeding for me.

>>

>> One is in #loadZipEncoderFrom:

>> The method checks and then determines the writeStreamInstSize and then fetches the bitBuffer and bitPosition (the first two instvars of ZipEncoder, a direct subclass of WriteStream) from offsets writeStreamInstSize + 1 and writeStreamInstSize + 2, although the fetches are 0-based

>>

>> The other is in #primitiveZipSendBlock

>> The method, still working on a ZipEncoder, at the end stores the modified values for bitBuffer and bitPosition at offset readStreamInstSize + 1 and readStreamInstSize + 2, which is just wrong.

>>

>> As I said, I don't quite get why this does not cause at least image-side errors if not VM crashes in Squeak, but they should probably be fixed anyway :)

>>

>> All the best,

>> Florin


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20220111/458aaa69/attachment-0001.html>