byteSizeOf bug?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

byteSizeOf bug?

johnmci

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
===========================================================================




Reply | Threaded
Open this post in threaded view
|

Re: byteSizeOf bug?

David T. Lewis
 
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
> ===========================================================================
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: byteSizeOf bug?

Andreas.Raab
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
Reply | Threaded
Open this post in threaded view
|

Re: byteSizeOf bug?

Eliot Miranda-2
 


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
Reply | Threaded
Open this post in threaded view
|

Re: byteSizeOf bug?

Eliot Miranda-2
 
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


Reply | Threaded
Open this post in threaded view
|

Re: byteSizeOf bug?

johnmci
 
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
===========================================================================




Reply | Threaded
Open this post in threaded view
|

Re: byteSizeOf bug?

johnmci
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
===========================================================================