RFR (S): 7195816: NPG: Crash in c1_ValueType (original) (raw)

RFR (S): 7195816: NPG: Crash in c1_ValueType - ShouldNotReachHere

Coleen Phillimore coleen.phillimore at oracle.com
Mon Sep 10 08:32:23 PDT 2012

Roland, I think the cleanups are good. Additional question about my favorite snippet of code in this webrev:

1038 case T_ARRAY: // fall through 1039 case T_OBJECT: // fall through 1040 #ifndef _LP64 1041 // We get here to store a method pointer to the stack to pass to 1042 // a dtrace runtime call. This can't work on 64 bit with 1043 // compressed klass ptrs: T_METADATA can be a compressed klass 1044 // ptr or a 64 bit method pointer. 1045 case T_METADATA: 1046 #endif 1047 if (UseCompressedOops&& !wide) { 1048 __ movl(as_Address(to_addr), compressed_src); 1049 } else { 1050 __ movptr(as_Address(to_addr), src->as_register()); 1051 } 1052 break;

So is this correct also?

1038 case T_ARRAY: // fall through 1039 case T_OBJECT: // fall through 1047 if (UseCompressedOops&& !wide) { 1048 __ movl(as_Address(to_addr), compressed_src); 1049 } else { 1050 __ movptr(as_Address(to_addr), src->as_register()); 1051 } 1052 break; case T_METADATA: 1041 // We get here to store a method pointer to the stack to pass to 1042 // a dtrace runtime call. This can't work on 64 bit with 1043 // compressed klass ptrs: T_METADATA can be a compressed klass 1044 // ptr or a 64 bit method pointer. 1045 LP64_ONLY(ShouldNotReachHere); 1050 __ movptr(as_Address(to_addr), src->as_register()); break;

Thanks, Coleen On 9/10/2012 9:45 AM, Roland Westrelin wrote:

Thanks for taking a look at this.

Thank you for fixing c1. We didn't realize there was this much left out. I took the opportunity to clean things up a bit. The minimal fix for this particular problem could maybe have been smaller but I think the code is better that way and could help catch problems in the future. In c1LIRAssemblerx86.cpp, this is sort of confusing. I would prefer a separate case for C1METADATA, because it would use UseCompressedKlassPointers rather than UseCompressedOops so even without the ifdef, part of this case doesn't make sense. Do you have fixes for this in your UseCompressedKlassPointer changes? A metadata ptr manipulated by c1 can be either a klass ptr or a method ptr. When the code is emitted in c1LIRAssembler*.cpp, we know something is a metadata ptr but the extra bit of information that tells whether it's a method or klass ptr is lost. The code below is the single place where the fact that we don't know whether the ptr is a klass or a method could be a problem: a metadata ptr is written to memory but on 64 bit with compressed klass ptrs it could be either a 32 bit store or a 64 bit store. Anyway, this code path is only taken on 32 bit because the code that C1 generates and the 32 bit x86 C calling convention are such that the only case where a metadata ptr is written to memory is to write it to the stack to pass as an argument to a dtrace runtime call. This code doesn't change with the UseCompressedKlassPointer changes. In short it's a corner case that only happens on x86/32 bit. Why isn't there the same sort of change from c1LIRAssemblersparc.cpp? All arguments to dtrace runtime calls are passed in registers so we won't try to write one of them to memory. Roland.

More information about the hotspot-compiler-dev mailing list