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

[Philip Race]

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 ints 
>>>> with 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 (4 
>>>> bytes)
>>>> >> 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 manually 
>>>> check
>>>> >> 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
>>>