[Vm-dev] bugs in DeflatePlugin?

[Florin Mateoc]

Yes, sorry if I was not clear, but you got it :)

Thank you,
Florin

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

>
> Hi Florin --
>
> Not sure whether you suggested a fix but ... I just have to replace
> "readStreamInstSize" with "writeStreamInstSize", too. I will do that.
>
> Best,
> Marcel
>
> Am 11.01.2022 13:27:50 schrieb Florin Mateoc <florin.mateoc at gmail.com>:
> 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
> 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 :
> > 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
> > 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
> >>
> >>
> >
> 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/ef634dac/attachment-0001.html>