[OpenJDK 2D-Dev] [9] Review Request: 8080847 Copying of overlapping memory should be improved in java2d

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Tue May 26 11:34:30 UTC 2015


On 26.05.15 13:43, Jim Graham wrote:
> What crash in memcpy? 
Simply because behavior of this function is undefined if the two arrays 
"to" and "from" overlap. Plus this clears an output for the tools like 
valgrind and some other issues can be found easily.

> The issue you pointed to is about dealing with overlapping memory. 
> memcpy does not crash on overlapping memory copies, it just duplicates 
> data oddly in a way that most uses probably don't want.
>
> Also, the fix you gave only fixed the problem for the horizontal 
> direction, if the drawImage call were directed at 0,1 then we'd still 
> get all scan lines duplicated from the first...
Right, I can take a look to this bug too.
>
>         ...jim
>
> On 5/25/2015 12:32 PM, Sergey Bylokhov wrote:
>> Hi, Jim.
>> Actually there is a difference in support: it works but result is a
>> little bit wrong, and it does not work and crashes. This fix is not a
>> general solution for the incorrect result of the blit+aliasing, but for
>> the possible crash of memcpy especially after some improvements like in
>> glibc. I think this still an issue.
>>
>>> I don't recall if we ever promised that this case would work without
>>> aliasing issues.  I know that we went out of our way in the copyArea
>>> method to prevent the aliasing issue, doing the blits piecemeal so
>>> that they don't interfere with each other.  Further, while it may be
>>> easy enough to just call memmove to have the libraray do this for us
>>> in the IsoBlit case, other cases that don't fall into the IsoBlit
>>> macro will not be similarly protected.  In particular, if you specify
>>> an alpha value, you will not get this protection (at least not without
>>> a huge amount of work to overhaul the entire DrawImage pipeline).
>>>
>>> I would say that this would be OK if we planned to make this promise
>>> about drawImage across all image formats and composition modes, but
>>> that would be a far more complicated fix.  Until then, we should not
>>> open this can of worms by modifying this one specific Blit case...
>>>
>>>             ...jim
>>>
>>> On 5/25/2015 5:35 AM, Sergey Bylokhov wrote:
>>>> Hello.
>>>> Please review the fix forjdk9.
>>>>
>>>> I found this issue during code review of another task, related to
>>>> performance.
>>>>
>>>> The sample code below will call the IsomorphicCopy method which call
>>>> memcpy on the overlapping memory(this is the simplest example)
>>>>
>>>>       BufferedImage img = new BufferedImage(100, 100,
>>>> BufferedImage.TYPE_INT_ARGB_PRE);
>>>>       Graphics2D g = img.createGraphics();
>>>>       g.setComposite(AlphaComposite.Src);
>>>>       g.drawImage(img, 0, 0, null);
>>>>       g.dispose();
>>>>
>>>> http://linux.die.net/man/3/memcpy
>>>> "The memcpy() function copies n bytes from memory area src to memory
>>>> area dest. The memory areas must not overlap. Use memmove(3) if the
>>>> memory areas do overlap"
>>>>
>>>>
>>>> I can confirm this bug using valgrind and a program above:
>>>> command:
>>>> valgrind --smc-check=all --tool=memcheck --leak-check=full -v
>>>> ./9/client/build/linux-x86_64-normal-server-fastdebug/images/jdk/bin/java 
>>>>
>>>> -Xint
>>>> Main
>>>>
>>>> output:
>>>> ==60975== Source and destination overlap in memcpy(0xe1b8b4d8,
>>>> 0xe1b8b4d8, 400)
>>>> ==60975== at 0x4C2F71C: memcpy@@GLIBC_2.14 (in
>>>> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>> ==60975== by 0x1E0F504D: AnyIntIsomorphicCopy (in
>>>> /moe/workspaces/jdk/9/client-work/build/linux-x86_64-normal-server-fastdebug/images/jdk/lib/amd64/libawt.so) 
>>>>
>>>>
>>>>
>>>> ==60975== by 0x1E0F5DE8: Java_sun_java2d_loops_Blit_Blit (in
>>>> /moe/workspaces/jdk/9/client-work/build/linux-x86_64-normal-server-fastdebug/images/jdk/lib/amd64/libawt.so) 
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8080847
>>>> Webrev can be found at:
>>>> http://cr.openjdk.java.net/~serb/8080847/webrev.00
>>>>
>>
>>


-- 
Best regards, Sergey.




More information about the 2d-dev mailing list