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

[Laurent Bourgès]

Phil,

To be short: ~3= -4.

You are right, javac could have done that computation for me !

Laurent

Le 9 déc. 2015 19:34, "Philip Race" <philip.race at oracle.com> a écrit :
>
> 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
>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20151209/4845694b/attachment.html>