RFR(s): 8187547: PPC64: icache invalidation is incorrect in some places

[Anton Kozlov]

Martin,

thanks for review!

I tried to keep knowledge of where relocation is pointing to in macroAssembler.
But if simplier code is preferable, of course I'm agree with this.

Webrev with suggestion applied:
http://cr.openjdk.java.net/~akozlov/8187547/webrev.02

I'm not very confident with the process for now (when repo closed), so please do what should be done, I'll try to assist as much as I can.

Backport should be easy, as the patch applies to jdk8u as well

Thanks,
Anton

On 14.09.2017 18:35, Doerr, Martin wrote:
> Hi Anton,
> 
> thank you very much for providing a fix. Looks correct.
> 
> In your current version, other_insn_offset is always negative.
> I'd prefer to make it always positive and simplify the usage like:
> assert(other_insn_offset > 0, "first instruction must be found");
> start = addr - other_insn_offset;
> range = BytesPerInstWord + other_insn_offset;
> This would be better readable. Would you agree?
> 
> At the moment, jdk10 repos are temporarily closed, but we can sponsor the change when it's open again and after a 2nd review.
> Backports will also need to get addressed.
> 
> Please note that ppc-aix-port-dev is not appropriate for reviews because the PPC64 port is part of the main repos. Therefore, I've added hotspot-runtime-dev.
> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-bounces at openjdk.java.net] On Behalf Of Anton Kozlov
> Sent: Donnerstag, 14. September 2017 15:06
> To: ppc-aix-port-dev at openjdk.java.net
> Subject: RFR(s): 8187547: PPC64: icache invalidation is incorrect in some places
> 
> Hi, All!
> 
> Icache invalidation range calculation in NativeMovConstReg::set_data_plain and NativeMovConstReg::set_narrow_oop is incorrect and could cause VM crash:
> 
> https://bugs.openjdk.java.net/browse/JDK-8187547
> 
> I suppose the root is in mismatch of supposed and actual return values of MacroAssembler::patch_set_narrow_oop and MacroAssembler::patch_calculate_address_from_global_toc_at.
> These functions takes address of the middle of sequence and expected to return first instruction offset (negative by current implementation). Instead of this, they return `-offset == abs(offset)` and offset to `data` respectively.
> 
> Supposed fix: http://cr.openjdk.java.net/~akozlov/8187547/webrev.01/
> 
> Thanks,
> Anton
>