[OpenJDK 2D-Dev] [9] JDK-8176287 :[macosx] The print test crashed with Nimbus L&F

[Prasanta Sadhukhan]

On 3/14/2017 9:51 PM, Sergey Bylokhov wrote:
>
>> 14 марта 2017 г., в 14:41, Philip Race <philip.race at oracle.com 
>> <mailto:philip.race at oracle.com>> написал(а):
>>
>> >Since this is mac specific code, I guess VS2010 will not play any 
>> part in building this.
>>
>> Ah, yes :-)
>>
>> Updated fix looks OK.
>
> Should we memset the data allocated via malloc(calloc was used before)?
>
Should be a good practice to do it, I guess .
Modified webrev adding memset:
http://cr.openjdk.java.net/~psadhukhan/8176287/webrev.02/

Regards
Prasanta
>>
>> -phil.
>>
>> On 3/14/17, 2:31 AM, Prasanta Sadhukhan wrote:
>>>
>>> JPRT 8u build resulted in failure, so I had to modify at 2 other places.
>>>
>>> QuartzSurfaceData.m:287 and QuartzSurfaceData.m:328
>>> Other things remains same.
>>> http://cr.openjdk.java.net/~psadhukhan/8176287/webrev.01/
>>>
>>> Regards
>>> Prasanta
>>> On 3/14/2017 10:47 AM, Philip Race wrote:
>>>>
>>>>
>>>> On 3/13/17, 10:14 PM, Prasanta Sadhukhan wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 3/14/2017 10:24 AM, Philip Race wrote:
>>>>>> The problem seems to have been that you were allocating zero 
>>>>>> bytes in the old code :
>>>>>> 950 CGFloat* colors= (CGFloat*)calloc(0, sizeof(CGFloat)*length);
>>>>>> 960 qsdo->gradientInfo->colordata = (CGFloat*)calloc(0, 
>>>>>> sizeof(CGFloat)*4*length); Regarding the new code, whilst it 
>>>>>> seems like it fixes the problem I have a nit
>>>>>> 937             int i;
>>>>>>  938             for (i=0; i<length; i++)
>>>>>>
>>>>>> Since this code appears at the start of a block I'd expect all 
>>>>>> compilers to be happy with
>>>>>>   for (int i=0; i<length; i++)
>>>>>>
>>>>>> is this not so ? Assuming yes, pls fix before push.
>>>>> Yes, it should be ok. I got a problem with jdk8u JPRT build 
>>>>> (during earlier backport) citing C99 compiler failure but I guess 
>>>>> that was because variable was declared not at blockstart.
>>>>> Will again do a JPRT and if its ok, I will push with this change.
>>>>
>>>> Testing the 8u backport via JPRT is good since that will use VS2010 
>>>> which
>>>> wins the "most likely to barf" award on such an issue.
>>>>
>>>> -phil
>>>>>>
>>>>>> Also I wonder if the regression test we created for LGP passes 
>>>>>> only because it is "short".
>>>>>> Perhaps later we can improve on that.
>>>>>>
>>>>>> The fix will also need to be backported since the original fix 
>>>>>> was backported.
>>>>>>
>>>>> ok.
>>>>>> So "+1" with those comments ..
>>>>>>
>>>>> Thanks
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>>> -phil.
>>>>>>
>>>>>> On 3/12/17, 11:49 PM, Prasanta Sadhukhan wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> Please review a jck print test crash fix for jdk9. The issue was 
>>>>>>> seen with only Nimbus L&F which seems to use Linear gradient path
>>>>>>> and not in other L&F (such as Aqua) .
>>>>>>>
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8176287
>>>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8176287/webrev.00/
>>>>>>>
>>>>>>> Linear Gradient path collects the gradient colors and fractions 
>>>>>>> values in native obtained from Java and allocates several arrays 
>>>>>>> to store the same in setupGradient() method.
>>>>>>> It seems even after being freed, in subsequent call to the same 
>>>>>>> gradient path routine, it may get the same allocated pointer the 
>>>>>>> next time the array is allocated causing it to crash citing 
>>>>>>> "memory being modified after freed".
>>>>>>>
>>>>>>> Optimise setupGradient() method to allocate fewer pointer. The 
>>>>>>> JCK test works now.
>>>>>>> Also, the JDK-8162796 testcase LinearGradientPrintingTest and 
>>>>>>> RadialGradientPrintingTest works with this optimisation.
>>>>>>>
>>>>>>> Regards
>>>>>>> Prasanta
>>>>>
>>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20170315/d48773b7/attachment.html>