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

[Sergey Bylokhov]

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.