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