RFR(M): 8140594: Various minor code improvements (compiler)

Tobias Hartmann tobias.hartmann at oracle.com
Mon May 9 08:40:33 UTC 2016


Hi Goetz,

thanks for the explanation, this looks good to me!

I'm running the hs-comp PIT now and will push the fix afterwards, if there are no objections.

Thanks,
Tobias

On 09.05.2016 10:30, Lindenmaier, Goetz wrote:
> Hi Tobias, 
> 
>> I can sponsor your fix but since we are close to JDK 9 FC, we are not allowed
>> to push enhancements to hs-comp. Shouldn't this be a "bug" anyway?
> Thanks for reviewing and the offer to sponsor!
> I changed the Jira issue to bug.  Sorry for being that close to FC with my RFR.
> 
>>> - size of pointer passed to jio_snprintf()
>> Please also fix the indentation in line 6004.
> Fixed.
> 
>>> classLoader.cpp
>>> - jio_snprintf does null termination. But it might return -1 if truncated,
>>>   in this case array access at -1.
>> You can remove "int n;"
> Fixed.
> 
>>> generateOopMap.cpp
>>> - Remaining fields not initialized.
>> I would put the loop bodies in a new line.
> Fixed.
> 
>>> relocator.cpp
>>> - delta might be -4 ... assert returns. 
>> I don't understand this change. If delta is -4, the assert in the baseline version
>> is triggered. With your fix, the assert is triggered as well.
> 
> Sorry, that's a bad comment.  I meant that the assert does not 
> protect against getting -4 in that line, because coverity knows 
> about -XX:SuppressErrorAt.  But that's pointless, as assert goes
> away in product builds anyways.  
> 
> The assert shall be triggered, therefore I moved it out of the if().
> But the if() now protects against oob with -4 or smaller.
> 
> I also rebased the change, and added you as reviewer.  Find the new
> Webrev here:
> http://cr.openjdk.java.net/~goetz/wr16/8140594-covCo/webrev.01/
> And the incremental patch:
> http://cr.openjdk.java.net/~goetz/wr16/8140594-covCo/webrev.01/incremental_diff.patch
> 
> Best regards,
>   Goetz.
> 


More information about the hotspot-compiler-dev mailing list