I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8.
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
From: Bert Freudenberg Sent: Monday, September 22, 2014 4:00 PM
I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8.
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
That's funky! Hope it wasn't my code! Seems wrong to not copy the array before modifying it (Like your buggy VM did).
Ron
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
On 22.09.2014, at 22:21, Ron Teitelbaum ron@usmedrec.com wrote:
From: Bert Freudenberg Sent: Monday, September 22, 2014 4:00 PM
I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8.
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
That's funky! Hope it wasn't my code! Seems wrong to not copy the array before modifying it (Like your buggy VM did).
It's not a copy. It moved the numbers 8-17 to index 11. Which is what I erroneously had assumed to be the desired behavior.
- Bert -
Ron
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
2014-09-22 22:34 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:21, Ron Teitelbaum ron@usmedrec.com wrote:
From: Bert Freudenberg Sent: Monday, September 22, 2014 4:00 PM
I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8. ==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
That's funky! Hope it wasn't my code! Seems wrong to not copy the array before modifying it (Like your buggy VM did).
It's not a copy. It moved the numbers 8-17 to index 11. Which is what I erroneously had assumed to be the desired behavior.
- Bert -
It's a known problem. The primitive should use memmove rather than memcpy. So the solution is quite simple, it's just that it's waiting for a good soul to integrate it.
Ron
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
2014-09-22 22:47 GMT+02:00 Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com>:
2014-09-22 22:34 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:21, Ron Teitelbaum ron@usmedrec.com wrote:
From: Bert Freudenberg Sent: Monday, September 22, 2014 4:00 PM
I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8. ==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
That's funky! Hope it wasn't my code! Seems wrong to not copy the
array
before modifying it (Like your buggy VM did).
It's not a copy. It moved the numbers 8-17 to index 11. Which is what I erroneously had assumed to be the desired behavior.
- Bert -
It's a known problem. The primitive should use memmove rather than memcpy. So the solution is quite simple, it's just that it's waiting for a good soul to integrate it.
Well, if memcpy was used, it seems not last time I inquired:
https://lists.gforge.inria.fr/pipermail/pharo-bugtracker/2014-April/048910.h...
Ron
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
2014-09-22 22:52 GMT+02:00 Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com>:
2014-09-22 22:47 GMT+02:00 Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com>:
2014-09-22 22:34 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:21, Ron Teitelbaum ron@usmedrec.com wrote:
From: Bert Freudenberg Sent: Monday, September 22, 2014 4:00 PM
I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8. ==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
That's funky! Hope it wasn't my code! Seems wrong to not copy the
array
before modifying it (Like your buggy VM did).
It's not a copy. It moved the numbers 8-17 to index 11. Which is what I erroneously had assumed to be the desired behavior.
- Bert -
It's a known problem. The primitive should use memmove rather than memcpy. So the solution is quite simple, it's just that it's waiting for a good soul to integrate it.
Well, if memcpy was used, it seems not last time I inquired:
https://lists.gforge.inria.fr/pipermail/pharo-bugtracker/2014-April/048910.h...
And another link if you don't have access to fogbugz
https://lists.gforge.inria.fr/pipermail/pharo-bugtracker/2014-April/048812.h...
Ron
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
On 22.09.2014, at 22:47, Nicolas Cellier nicolas.cellier.aka.nice@gmail.com wrote:
2014-09-22 22:34 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:21, Ron Teitelbaum ron@usmedrec.com wrote:
From: Bert Freudenberg Sent: Monday, September 22, 2014 4:00 PM
I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8. ==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
That's funky! Hope it wasn't my code! Seems wrong to not copy the array before modifying it (Like your buggy VM did).
It's not a copy. It moved the numbers 8-17 to index 11. Which is what I erroneously had assumed to be the desired behavior.
- Bert -
It's a known problem. The primitive should use memmove rather than memcpy. So the solution is quite simple, it's just that it's waiting for a good soul to integrate it.
What I'm saying is that there is code which will actually fail in subtle ways if we change that behavior. The LZW decode *relies* on this. If we change the primitive it would fail.
So rather than changing what the "stringReplace" primitive does we might rather want to add a "stringMove" primitive.
Btw, BitBlt has "move" semantics and I believe this is even used in some places (via #hackBits:).
- Bert -
Ron
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
2014-09-22 23:16 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:47, Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com> wrote:
2014-09-22 22:34 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:21, Ron Teitelbaum ron@usmedrec.com wrote:
From: Bert Freudenberg Sent: Monday, September 22, 2014 4:00 PM
I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8. ==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
That's funky! Hope it wasn't my code! Seems wrong to not copy the
array
before modifying it (Like your buggy VM did).
It's not a copy. It moved the numbers 8-17 to index 11. Which is what I erroneously had assumed to be the desired behavior.
- Bert -
It's a known problem. The primitive should use memmove rather than memcpy. So the solution is quite simple, it's just that it's waiting for a good soul to integrate it.
What I'm saying is that there is code which will actually fail in subtle ways if we change that behavior. The LZW decode *relies* on this. If we change the primitive it would fail.
Ah, oh ! I missread your post! Relying on an undefined behavior is not a clever thing... OK, since we never changed the implementation, it's a stable behavior... but it is for sure unspecified. There is no limit to the hackish nature of some Squeak code, really... Hang it in the hall of shame.
So rather than changing what the "stringReplace" primitive does we might rather want to add a "stringMove" primitive.
Btw, BitBlt has "move" semantics and I believe this is even used in some places (via #hackBits:).
- Bert -
Ron
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
2014-09-23 21:20 GMT+02:00 Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com>:
2014-09-22 23:16 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:47, Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com> wrote:
2014-09-22 22:34 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:21, Ron Teitelbaum ron@usmedrec.com wrote:
From: Bert Freudenberg Sent: Monday, September 22, 2014 4:00 PM
I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8. ==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
That's funky! Hope it wasn't my code! Seems wrong to not copy the
array
before modifying it (Like your buggy VM did).
It's not a copy. It moved the numbers 8-17 to index 11. Which is what I erroneously had assumed to be the desired behavior.
- Bert -
It's a known problem. The primitive should use memmove rather than memcpy. So the solution is quite simple, it's just that it's waiting for a good soul to integrate it.
What I'm saying is that there is code which will actually fail in subtle ways if we change that behavior. The LZW decode *relies* on this. If we change the primitive it would fail.
Ah, oh ! I missread your post! Relying on an undefined behavior is not a clever thing... OK, since we never changed the implementation, it's a stable behavior... but it is for sure unspecified. There is no limit to the hackish nature of some Squeak code, really... Hang it in the hall of shame.
Is it decompressBlock:with: ?
So rather than changing what the "stringReplace" primitive does we might rather want to add a "stringMove" primitive.
Btw, BitBlt has "move" semantics and I believe this is even used in some places (via #hackBits:).
- Bert -
Ron
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
On 23.09.2014, at 21:51, Nicolas Cellier nicolas.cellier.aka.nice@gmail.com wrote:
2014-09-23 21:20 GMT+02:00 Nicolas Cellier nicolas.cellier.aka.nice@gmail.com:
2014-09-22 23:16 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:47, Nicolas Cellier nicolas.cellier.aka.nice@gmail.com wrote:
2014-09-22 22:34 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:21, Ron Teitelbaum ron@usmedrec.com wrote:
From: Bert Freudenberg Sent: Monday, September 22, 2014 4:00 PM
I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8. ==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
That's funky! Hope it wasn't my code! Seems wrong to not copy the array before modifying it (Like your buggy VM did).
It's not a copy. It moved the numbers 8-17 to index 11. Which is what I erroneously had assumed to be the desired behavior.
- Bert -
It's a known problem. The primitive should use memmove rather than memcpy. So the solution is quite simple, it's just that it's waiting for a good soul to integrate it.
What I'm saying is that there is code which will actually fail in subtle ways if we change that behavior. The LZW decode *relies* on this. If we change the primitive it would fail.
Ah, oh ! I missread your post! Relying on an undefined behavior is not a clever thing... OK, since we never changed the implementation, it's a stable behavior... but it is for sure unspecified. There is no limit to the hackish nature of some Squeak code, really... Hang it in the hall of shame.
Is it decompressBlock:with: ?
Yep.
- Bert -
So rather than changing what the "stringReplace" primitive does we might rather want to add a "stringMove" primitive.
Btw, BitBlt has "move" semantics and I believe this is even used in some places (via #hackBits:).
- Bert -
Ron
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
On 23-09-2014, at 12:20 PM, Nicolas Cellier nicolas.cellier.aka.nice@gmail.com wrote:
Ah, oh ! I missread your post! Relying on an undefined behavior is not a clever thing... OK, since we never changed the implementation, it's a stable behavior... but it is for sure unspecified. There is no limit to the hackish nature of some Squeak code, really... Hang it in the hall of shame.
It’s not like the PNG code doesn’t need some fixes anyway.
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Useful random insult:- Calling her stupid would be an insult to stupid people.
On Mon, Sep 22, 2014 at 2:16 PM, Bert Freudenberg bert@freudenbergs.de wrote:
On 22.09.2014, at 22:47, Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com> wrote:
2014-09-22 22:34 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:21, Ron Teitelbaum ron@usmedrec.com wrote:
From: Bert Freudenberg Sent: Monday, September 22, 2014 4:00 PM
I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8. ==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
That's funky! Hope it wasn't my code! Seems wrong to not copy the
array
before modifying it (Like your buggy VM did).
It's not a copy. It moved the numbers 8-17 to index 11. Which is what I erroneously had assumed to be the desired behavior.
- Bert -
It's a known problem. The primitive should use memmove rather than memcpy. So the solution is quite simple, it's just that it's waiting for a good soul to integrate it.
What I'm saying is that there is code which will actually fail in subtle ways if we change that behavior. The LZW decode *relies* on this. If we change the primitive it would fail.
So rather than changing what the "stringReplace" primitive does we might rather want to add a "stringMove" primitive.
+1. An implement a move family, e.g.
SequenceableCollection>>moveFrom: start to: stop with: replacement startingAt: repStart | index repOff | repOff := repStart - start. start > repStart ifTrue: [index := stop + 1. [(index := index - 1) >= start] whileTrue: [self at: index put: (replacement at: repOff + index)]] ifFalse: [index := start - 1. [(index := index + 1) <= stop] whileTrue: [self at: index put: (replacement at: repOff + index)]]
But more reasonably
SequenceableCollection>>moveFrom: start to: stop to: repStart | index repOff | repOff := repStart - start. start > repStart ifTrue: [index := stop + 1. [(index := index - 1) >= start] whileTrue: [self at: index put: (self at: repOff + index)]] ifFalse: [index := start - 1. [(index := index + 1) <= stop] whileTrue: [self at: index put: (self at: repOff + index)]]
Btw, BitBlt has "move" semantics and I believe this is even used in some
places (via #hackBits:).
Yes, you're right. But I think there was a time when you could turn that off and get interesting effects :-).
- Bert -
Ron
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
On Mon, Sep 22, 2014 at 1:47 PM, Nicolas Cellier < nicolas.cellier.aka.nice@gmail.com> wrote:
2014-09-22 22:34 GMT+02:00 Bert Freudenberg bert@freudenbergs.de:
On 22.09.2014, at 22:21, Ron Teitelbaum ron@usmedrec.com wrote:
From: Bert Freudenberg Sent: Monday, September 22, 2014 4:00 PM
I just found out why PNG decoding was broken in SqueakJS [*]. Wouldn't have believed that any code actually relied on the following behavior:
| a | a := (1 to: 20) asArray. a replaceFrom: 11 to: 20 with: a startingAt: 8. ==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
That's funky! Hope it wasn't my code! Seems wrong to not copy the
array
before modifying it (Like your buggy VM did).
It's not a copy. It moved the numbers 8-17 to index 11. Which is what I erroneously had assumed to be the desired behavior.
- Bert -
It's a known problem. The primitive should use memmove rather than memcpy. So the solution is quite simple, it's just that it's waiting for a good soul to integrate it.
Um, no. It has ever been so and must therefore remain. This is the method (as in the Smalltalk-80 V2 sources) that the primitive optimizes:
replaceFrom: start to: stop with: replacement startingAt: repStart "This destructively replaces elements from start to stop in the receiver starting at index, repStart, in the collection, replacement. Answer the receiver. No range checks are performed - this may be primitively implemented."
| index repOff | repOff _ repStart - start. index _ start - 1. [(index _ index + 1) <= stop] whileTrue: [self at: index put: (replacement at: repOff + index)]
So the primitive is correct. This is memcpy behaviour. IIRC BitBlt is the same. There were some BitBlt ticks played with image fill that depended on this.
| oc | oc := OrderedCollection withAll: (1 to: 20). oc replaceFrom: 11 to: 20 with: oc startingAt: 8. oc => an OrderedCollection(1 2 3 4 5 6 7 8 9 10 8 9 10 8 9 10 8 9 10 8)
Ron
In my (buggy) VM it answered
==> #(1 2 3 4 5 6 7 8 9 10 8 9 10 11 12 13 14 15 16 17)
which apparently did not at all match what The Creators intended ;)
- Bert -
[*] specifically, #decompressBlock:with:
squeak-dev@lists.squeakfoundation.org