[OpenJDK 2D-Dev] RFR 8144828: Marlin renderer causes unaligned write accesses

[Jim Graham]

I noticed that too, but they are equivalent.  I think it is much more 
common to use ~3, but I didn't wan to hold this up for a syntax nit...

			...jim

On 12/9/15 10:34 AM, Philip Race wrote:
> looks fine although I wonder why you use & -4 instead of & ~3 ?
>
> -phil.
>
>
> On 12/8/15, 3:17 PM, Jim Graham wrote:
>> With the submission of JDK-8144938 as a follow-on bug, this fix looks
>> fine now...
>>
>>             ...jim
>>
>> On 12/7/15 5:13 PM, Jim Graham wrote:
>>> Do we make sure that the subpixel coords don't overflow an integer
>>> somewhere?
>>>
>>>              ...jim
>>>
>>> On 12/7/15 1:52 PM, Laurent Bourgès wrote:
>>>> Moreover, it is concretely impossible as subpixel coords implies the
>>>> maximum largest width is MAX_INT / 8 or 16 ...
>>>>
>>>> I should defintely add the same point filter as done in the
>>>> DuctusRenderingEngine to filter both NaN and huge coords !
>>>>
>>>> Laurent
>>>>
>>>> Le 7 déc. 2015 22:26, "Laurent Bourgès" <bourges.laurent at gmail.com
>>>> <mailto:bourges.laurent at gmail.com>> a écrit :
>>>>>
>>>>> Jim,
>>>>>
>>>>> You are theoretical right !
>>>>> However it needs a 2Gb row = px_bbox1 - px0 ! That's only possible if
>>>>> you allocate an image with a single row (MAX_INT) and the shape
>>>>> covers that full range !
>>>>>
>>>>> Do you really want me to fix that impossible case ? using 3L (and
>>>>> -4L).
>>>>>
>>>>> Laurent
>>>>>
>>>>> Le 7 déc. 2015 22:15, "Jim Graham" <james.graham at oracle.com
>>>>> <mailto:james.graham at oracle.com>> a écrit :
>>>>> >
>>>>> > pos and needSize are longs.  px_bbox1 and px0 are positive intswith px_bbox1 known to be greater than px0 so the subtraction
>>>>> should not be able to overflow an int.  But, now that we are adding
>>>>> 3, is it possible for that calculation to overflow?  Would it be
>>>>> better to protect against that by using 3L (and -4L) even though
>>>>> these are theoretical issues? (The result of that calculation gets
>>>>> cast to long before adding it to pos anyway.)
>>>>> >
>>>>> >                         ...jim
>>>>> >
>>>>> >
>>>>> > On 12/7/15 8:34 AM, Laurent Bourgès wrote:
>>>>> >>
>>>>> >> Hi,
>>>>> >>
>>>>> >> Please review the following (critical) bug fix that ensures (4bytes)
>>>>> >> aligned memory accesses (Unsafe) in MarlinCache:
>>>>> >>
>>>>> >> bug:https://bugs.openjdk.java.net/browse/JDK-8144828
>>>>> >>webrev:http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144828.0/
>>>>> >>
>>>>> >> Changes:
>>>>> >> - added MarlinConst.doCheckUnsafe (false by default) to manuallycheck
>>>>> >> addresses
>>>>> >> - MarlinCache: align needed bytes in copyAARowNoRLE() as the same
>>>>> >> off-heap block is also used by copyAARowRLE_WithBlockFlags()which store
>>>>> >> RLE entries as integer:
>>>>> >>
>>>>> >> + // determine need array size:
>>>>> >> + // for RLE encoding, position must be aligned to 4 bytes (int):
>>>>> >> + // align - 1 = 3 so add +3 and round-off by mask ~3 = -4
>>>>> >> + final long needSize = pos + ((px_bbox1 - px0 + 3) & -4);
>>>>> >>
>>>>> >> I did not tested on the sparc platform as I do not have one !
>>>>> >> But it tested the patch on my linux 64 (intel).
>>>>> >>
>>>>> >> Best regards,
>>>>> >> Laurent
>>>>