[Vm-dev] bugs in DeflatePlugin?

Florin Mateoc florin.mateoc at gmail.com
Tue Jan 11 12:27:30 UTC 2022


Hi Marcel,

Thank you for the quick response, but I am afraid the fix in
#primitiveZipSendBlock is still wrong.
While the method is working with multiple types of objects, in the rcvr
variable it clearly loads a zip encoder, and only a zip encoder:

  	(self loadZipEncoderFrom: rcvr)

And at the end of the method, it is in this ZipEncoder instance rcvr that
it is storing the modified values.
ZipEncoder is a subclass of WriteStream, not of ReadStream.

All the best,
Florin

On Tue, Jan 11, 2022 at 6:22 AM Marcel Taeumel <marcel.taeumel at hpi.de>
wrote:

>
> 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>
> 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 <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>
>> 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/76e444f2/attachment.html>


More information about the Vm-dev mailing list