Smalltalk › Squeak › Squeak VM

byteSizeOf bug?

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

7 messages Options

johnmci

byteSizeOf bug?

I noted in the current VMMaker dtl 12/20/2009 21:19 that

byteSizeOf: oop
| slots |
self flag: #Dan.
(self isIntegerObject: oop) ifTrue:[^0].
slots := self slotSizeOf: oop.
(self isBytesNonInt: oop)
ifTrue:[^slots]
ifFalse:[^slots * 4]

changed to

byteSizeOf: oop
| slots |
self flag: #Dan.
(self isIntegerObject: oop) ifTrue:[^0].
slots := self slotSizeOf: oop.
(self isBytesNonInt: oop)
ifTrue:[^slots]
ifFalse:[^slots * self bytesPerWord]

So I was trying to build a 64bit VM this evening and it would die in loadBitBltFromWarping
where it check to see if the destination bit form has a byteSizeOf() that is equal to pitch * height
this causes a prim failure before any drawing can occur.

In the past if the form was 1,521,520 bytes this altered call returns 3,043,040.

I assume this is a bug since with the previous code I could build runnable 64bit VMs...?

Or then again is the bitBitPlugin wrong in it's assumption of slot size?
--
===========================================================================
John M. McIntosh <[hidden email]> Twitter: squeaker68882
Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com
===========================================================================

David T. Lewis

Re: byteSizeOf bug?

This is the change that we discussed in this thread:

http://lists.squeakfoundation.org/pipermail/vm-dev/2009-December/003563.html

It was applied in SqS/VMMaker-dtl.155 on December 20, 2009.

Apparently I did not test the resulting VM on a 64 bit image :-/

I don't have time to follow up right now, but it's likely that this
change should be reverted, at least until we can sort out the side
effects.

Dave

On Wed, Jan 13, 2010 at 12:58:22AM -0800, John M McIntosh wrote:

>
> I noted in the current VMMaker dtl 12/20/2009 21:19 that
>
> byteSizeOf: oop
> | slots |
> self flag: #Dan.
> (self isIntegerObject: oop) ifTrue:[^0].
> slots := self slotSizeOf: oop.
> (self isBytesNonInt: oop)
> ifTrue:[^slots]
> ifFalse:[^slots * 4]
>
> changed to
>
> byteSizeOf: oop
> | slots |
> self flag: #Dan.
> (self isIntegerObject: oop) ifTrue:[^0].
> slots := self slotSizeOf: oop.
> (self isBytesNonInt: oop)
> ifTrue:[^slots]
> ifFalse:[^slots * self bytesPerWord]
>
> So I was trying to build a 64bit VM this evening and it would die in loadBitBltFromWarping
> where it check to see if the destination bit form has a byteSizeOf() that is equal to pitch * height
> this causes a prim failure before any drawing can occur.
>
> In the past if the form was 1,521,520 bytes this altered call returns 3,043,040.
>
> I assume this is a bug since with the previous code I could build runnable 64bit VMs...?
>
> Or then again is the bitBitPlugin wrong in it's assumption of slot size?
> --
> ===========================================================================
> John M. McIntosh <[hidden email]> Twitter: squeaker68882
> Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com
> ===========================================================================
>
>
>

Andreas.Raab

Re: byteSizeOf bug?

In reply to this post by johnmci

John M McIntosh wrote:
> So I was trying to build a 64bit VM this evening and it would die in loadBitBltFromWarping
> where it check to see if the destination bit form has a byteSizeOf() that is equal to pitch * height
> this causes a prim failure before any drawing can occur.
>
> In the past if the form was 1,521,520 bytes this altered call returns 3,043,040.
>
> I assume this is a bug since with the previous code I could build runnable 64bit VMs...?
>
> Or then again is the bitBitPlugin wrong in it's assumption of slot size?

Depends. In a 64bit image does a Bitmap contain 32 bit entities or 64
bit entities? If the former, then byteSizeOf is wrong for Bitmap (and
possibly other 32 bit containing entities). If the latter, then the
pitch computation in BitBlt is wrong.

Cheers,
- Andreas

Eliot Miranda-2

Re: byteSizeOf bug?

On Wed, Jan 13, 2010 at 9:13 AM, Andreas Raab <[hidden email]> wrote:

John M McIntosh wrote:

So I was trying to build a 64bit VM this evening and it would die in loadBitBltFromWarping
where it check to see if the destination bit form has a byteSizeOf() that is equal to pitch * height
this causes a prim failure before any drawing can occur.
In the past if the form was 1,521,520 bytes this altered call returns 3,043,040.

I assume this is a bug since with the previous code I could build runnable 64bit VMs...?

Or then again is the bitBitPlugin wrong in it's assumption of slot size?

Depends. In a 64bit image does a Bitmap contain 32 bit entities or 64 bit entities? If the former, then byteSizeOf is wrong for Bitmap (and possibly other 32 bit containing entities). If the latter, then the pitch computation in BitBlt is wrong.

The former. See lengthOf:baseHeader:format:. There is a distinction between pointer objects (format <= 4) whose slots are BytesPerWord long, 32-bit objects (fmt between: 5 and: 7) whose slots are 32-bits in size, and byte objects whose slots are 8 bits in size. byteSizeOf: clearly discards this distinction. Why not reimplement it as

byteSizeOf: oop

| header format size |

(self isIntegerObject: oop) ifTrue:[^0].

header := self baseHeader: oop.

format := self formatOfHeader: header.

(header bitAnd: TypeMask) = HeaderTypeSizeAndClass

ifTrue: [ size := (self sizeHeader: oop) bitAnd: LongSizeMask ]

ifFalse: [ size := (header bitAnd: SizeMask)].

size := size - (header bitAnd: Size4Bit).

format <= 4

ifTrue: [ ^ size - BaseHeaderSize "words"].

format < 8

ifTrue: [ ^ size - BaseHeaderSize "32-bit longs"]

ifFalse: [ ^ (size - BaseHeaderSize) - (format bitAnd: 3) "bytes"]

which is lengthOf:baseHeader:format: rewritten not to reduce the byte size to slot count, and is of course equal to

byteSizeOf: oop

| header format size |

(self isIntegerObject: oop) ifTrue:[^0].

header := self baseHeader: oop.

format := self formatOfHeader: header.

(header bitAnd: TypeMask) = HeaderTypeSizeAndClass

ifTrue: [ size := (self sizeHeader: oop) bitAnd: LongSizeMask ]

ifFalse: [ size := (header bitAnd: SizeMask)].

size := size - (header bitAnd: Size4Bit).

^format < 8

ifTrue: [ size - BaseHeaderSize "32-bit longs"]

ifFalse: [ (size - BaseHeaderSize) - (format bitAnd: 3) "bytes"]

with the implementation for formatOfHeader: attached and used everywhere where ((format >> 8) bitAnd: 16rF) is used?

Cheers,
- Andreas

ObjectMemory-formatOfHeader.st (1K) Download Attachment

ObjectMemory-byteSizeOf.st (910 bytes) Download Attachment

Eliot Miranda-2

Re: byteSizeOf bug?

BTW, there's clearly an exception for format = 7 which is supposed to be 64-bit longs. So lengthOf:baseheader:format: looks wrong for 64-bit images. It needs to say something like

fmt <= 4

ifTrue: [ ^ (sz - BaseHeaderSize) >> ShiftForWord "words"].

fmt = 7

ifTrue: [ ^ (sz - BaseHeaderSize) >> 3 "64-bit longs"].

fmt < 8

ifTrue: [ ^ (sz - BaseHeaderSize) >> 2 "32-bit longs"]

ifFalse: [ ^ (sz - BaseHeaderSize) - (fmt bitAnd: 3) "bytes"]

or format = 7 needs to be documented as being unused.

On Wed, Jan 13, 2010 at 10:16 AM, Eliot Miranda <[hidden email]> wrote:

On Wed, Jan 13, 2010 at 9:13 AM, Andreas Raab <[hidden email]> wrote:

John M McIntosh wrote:

So I was trying to build a 64bit VM this evening and it would die in loadBitBltFromWarping
where it check to see if the destination bit form has a byteSizeOf() that is equal to pitch * height
this causes a prim failure before any drawing can occur.
In the past if the form was 1,521,520 bytes this altered call returns 3,043,040.

I assume this is a bug since with the previous code I could build runnable 64bit VMs...?

Or then again is the bitBitPlugin wrong in it's assumption of slot size?

Depends. In a 64bit image does a Bitmap contain 32 bit entities or 64 bit entities? If the former, then byteSizeOf is wrong for Bitmap (and possibly other 32 bit containing entities). If the latter, then the pitch computation in BitBlt is wrong.

The former. See lengthOf:baseHeader:format:. There is a distinction between pointer objects (format <= 4) whose slots are BytesPerWord long, 32-bit objects (fmt between: 5 and: 7) whose slots are 32-bits in size, and byte objects whose slots are 8 bits in size. byteSizeOf: clearly discards this distinction. Why not reimplement it as

byteSizeOf: oop
| header format size |
(self isIntegerObject: oop) ifTrue:[^0].

header := self baseHeader: oop.
format := self formatOfHeader: header.
(header bitAnd: TypeMask) = HeaderTypeSizeAndClass

ifTrue: [ size := (self sizeHeader: oop) bitAnd: LongSizeMask ]
ifFalse: [ size := (header bitAnd: SizeMask)].

size := size - (header bitAnd: Size4Bit).
format <= 4
ifTrue: [ ^ size - BaseHeaderSize "words"].

format < 8
ifTrue: [ ^ size - BaseHeaderSize "32-bit longs"]
ifFalse: [ ^ (size - BaseHeaderSize) - (format bitAnd: 3) "bytes"]

which is lengthOf:baseHeader:format: rewritten not to reduce the byte size to slot count, and is of course equal to

byteSizeOf: oop
| header format size |
(self isIntegerObject: oop) ifTrue:[^0].

header := self baseHeader: oop.
format := self formatOfHeader: header.
(header bitAnd: TypeMask) = HeaderTypeSizeAndClass

ifTrue: [ size := (self sizeHeader: oop) bitAnd: LongSizeMask ]
ifFalse: [ size := (header bitAnd: SizeMask)].

size := size - (header bitAnd: Size4Bit).
^format < 8
ifTrue: [ size - BaseHeaderSize "32-bit longs"]

ifFalse: [ (size - BaseHeaderSize) - (format bitAnd: 3) "bytes"]

with the implementation for formatOfHeader: attached and used everywhere where ((format >> 8) bitAnd: 16rF) is used?

Cheers,
- Andreas

johnmci

Re: byteSizeOf bug?

I made Mantis http://bugs.squeak.org/view.php?id=7447

to track this issue

On 2010-01-13, at 10:20 AM, Eliot Miranda wrote:

BT

===========================================================================

John M. McIntosh <[hidden email]> Twitter: squeaker68882

Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com

===========================================================================

johnmci

Re: byteSizeOf bug?

In reply to this post by Eliot Miranda-2

On 2010-01-13, at 10:20 AM, Eliot Miranda wrote:

BTW, there's clearly an exception for format = 7 which is supposed to be 64-bit longs. So lengthOf:baseheader:format: looks wrong for 64-bit images. It needs to say something like

fmt <= 4

ifTrue: [ ^ (sz - BaseHeaderSize) >> ShiftForWord "words"].
fmt = 7

ifTrue: [ ^ (sz - BaseHeaderSize) >> 3 "64-bit longs"].
fmt < 8

ifTrue: [ ^ (sz - BaseHeaderSize) >> 2 "32-bit longs"]
ifFalse: [ ^ (sz - BaseHeaderSize) - (fmt bitAnd: 3) "bytes"]

or format = 7 needs to be documented as being unused.

The work to add longer immediate integer types, floats, wasn't done so no-doubt that fmt = 7 *would* be used for something.

===========================================================================

John M. McIntosh <[hidden email]> Twitter: squeaker68882

Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com

===========================================================================