Quantcast

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

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

prasanta sadhukhan
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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] JDK-8176287 :[macosx] The print test crashed with Nimbus L&F

Phil Race
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.

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.

So "+1" with those comments ..

-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
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] JDK-8176287 :[macosx] The print test crashed with Nimbus L&F

prasanta sadhukhan



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.

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] JDK-8176287 :[macosx] The print test crashed with Nimbus L&F

Phil Race


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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] JDK-8176287 :[macosx] The print test crashed with Nimbus L&F

prasanta sadhukhan

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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] JDK-8176287 :[macosx] The print test crashed with Nimbus L&F

Phil Race
>Since this is mac specific code, I guess VS2010 will not play any part in building this.

Ah, yes :-)

Updated fix looks OK.

-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


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] JDK-8176287 :[macosx] The print test crashed with Nimbus L&F

Sergey Bylokhov

14 марта 2017 г., в 14:41, Philip Race <[hidden email]> написал(а):

>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)?


-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



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [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 <[hidden email]> написал(а):

>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




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] JDK-8176287 :[macosx] The print test crashed with Nimbus L&F

Sergey Bylokhov

On 3/14/2017 9:51 PM, Sergey Bylokhov wrote:

14 марта 2017 г., в 14:41, Philip Race <[hidden email]> написал(а):

>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/

This looks fine to me, but I wonder why the calloc was replaced by malloc? 


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





Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] JDK-8176287 :[macosx] The print test crashed with Nimbus L&F

Phil Race
I wondered that too but since it appeared we then had a loop
that explicitly initialised all elements it should not matter.

But I supposed it was sort of an accidental byproduct of trying different things.

-phil.

On 3/15/17, 10:57 AM, Sergey Bylokhov wrote:

On 3/14/2017 9:51 PM, Sergey Bylokhov wrote:

14 марта 2017 г., в 14:41, Philip Race <[hidden email]> написал(а):

>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/

This looks fine to me, but I wonder why the calloc was replaced by malloc? 


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





Loading...