8231757: [ppc] Fix VerifyOops. Errors show since 8231058.
authorgoetz
Mon, 11 Nov 2019 16:24:35 +0100
changeset 59014 36fde8064316
parent 59011 e4d7fcab43d7
child 59015 b9a85aa023b1
8231757: [ppc] Fix VerifyOops. Errors show since 8231058. Summary: Also make the checks print the wrong value and where a failure occurred. Reviewed-by: dholmes, mdoerr
src/hotspot/cpu/ppc/c1_CodeStubs_ppc.cpp
src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp
src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp
src/hotspot/cpu/ppc/gc/g1/g1BarrierSetAssembler_ppc.cpp
src/hotspot/cpu/ppc/gc/shared/barrierSetAssembler_ppc.cpp
src/hotspot/cpu/ppc/globalDefinitions_ppc.hpp
src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp
src/hotspot/cpu/ppc/macroAssembler_ppc.cpp
src/hotspot/cpu/ppc/macroAssembler_ppc.hpp
src/hotspot/cpu/ppc/methodHandles_ppc.cpp
src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp
src/hotspot/cpu/ppc/stubGenerator_ppc.cpp
test/hotspot/jtreg/runtime/CheckUnhandledOops/TestVerifyOops.java
--- a/src/hotspot/cpu/ppc/c1_CodeStubs_ppc.cpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/c1_CodeStubs_ppc.cpp	Mon Nov 11 16:24:35 2019 +0100
@@ -322,7 +322,7 @@
 void PatchingStub::emit_code(LIR_Assembler* ce) {
   // copy original code here
   assert(NativeGeneralJump::instruction_size <= _bytes_to_copy && _bytes_to_copy <= 0xFF,
-         "not enough room for call");
+         "not enough room for call, need %d", _bytes_to_copy);
   assert((_bytes_to_copy & 0x3) == 0, "must copy a multiple of four bytes");
 
   Label call_patch;
@@ -340,7 +340,7 @@
     __ load_const(_obj, addrlit, R0);
     DEBUG_ONLY( compare_with_patch_site(__ code_section()->start() + being_initialized_entry, _pc_start, _bytes_to_copy); )
   } else {
-    // Make a copy the code which is going to be patched.
+    // Make a copy of the code which is going to be patched.
     for (int i = 0; i < _bytes_to_copy; i++) {
       address ptr = (address)(_pc_start + i);
       int a_byte = (*ptr) & 0xFF;
--- a/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp	Mon Nov 11 16:24:35 2019 +0100
@@ -743,10 +743,11 @@
           if (UseCompressedOops && !wide) {
             // Encoding done in caller
             __ stw(from_reg->as_register(), offset, base);
+            __ verify_coop(from_reg->as_register(), FILE_AND_LINE);
           } else {
             __ std(from_reg->as_register(), offset, base);
+            __ verify_oop(from_reg->as_register(), FILE_AND_LINE);
           }
-          __ verify_oop(from_reg->as_register());
           break;
         }
       case T_FLOAT : __ stfs(from_reg->as_float_reg(), offset, base); break;
@@ -783,10 +784,11 @@
         if (UseCompressedOops && !wide) {
           // Encoding done in caller.
           __ stwx(from_reg->as_register(), base, disp);
+          __ verify_coop(from_reg->as_register(), FILE_AND_LINE); // kills R0
         } else {
           __ stdx(from_reg->as_register(), base, disp);
+          __ verify_oop(from_reg->as_register(), FILE_AND_LINE); // kills R0
         }
-        __ verify_oop(from_reg->as_register()); // kills R0
         break;
       }
     case T_FLOAT : __ stfsx(from_reg->as_float_reg(), base, disp); break;
@@ -831,7 +833,7 @@
           } else {
             __ ld(to_reg->as_register(), offset, base);
           }
-          __ verify_oop(to_reg->as_register());
+          __ verify_oop(to_reg->as_register(), FILE_AND_LINE);
           break;
         }
       case T_FLOAT:  __ lfs(to_reg->as_float_reg(), offset, base); break;
@@ -862,7 +864,7 @@
         } else {
           __ ldx(to_reg->as_register(), base, disp);
         }
-        __ verify_oop(to_reg->as_register());
+        __ verify_oop(to_reg->as_register(), FILE_AND_LINE);
         break;
       }
     case T_FLOAT:  __ lfsx(to_reg->as_float_reg() , base, disp); break;
@@ -1141,7 +1143,7 @@
   }
 
   if (addr->base()->type() == T_OBJECT) {
-    __ verify_oop(src);
+    __ verify_oop(src, FILE_AND_LINE);
   }
 
   PatchingStub* patch = NULL;
@@ -1238,7 +1240,7 @@
     ShouldNotReachHere();
   }
   if (is_reference_type(to_reg->type())) {
-    __ verify_oop(to_reg->as_register());
+    __ verify_oop(to_reg->as_register(), FILE_AND_LINE);
   }
 }
 
@@ -1265,7 +1267,7 @@
   }
 
   if (addr->base()->is_oop_register()) {
-    __ verify_oop(src);
+    __ verify_oop(src, FILE_AND_LINE);
   }
 
   PatchingStub* patch = NULL;
@@ -2321,7 +2323,7 @@
                      *op->stub()->entry());
 
   __ bind(*op->stub()->continuation());
-  __ verify_oop(op->obj()->as_register());
+  __ verify_oop(op->obj()->as_register(), FILE_AND_LINE);
 }
 
 
@@ -2546,7 +2548,7 @@
     Register Rtmp1 = op->tmp3()->as_register();
     bool should_profile = op->should_profile();
 
-    __ verify_oop(value);
+    __ verify_oop(value, FILE_AND_LINE);
     CodeStub* stub = op->stub();
     // Check if it needs to be profiled.
     ciMethodData* md = NULL;
@@ -3099,7 +3101,7 @@
   assert(do_null || do_update, "why are we here?");
   assert(!TypeEntries::was_null_seen(current_klass) || do_update, "why are we here?");
 
-  __ verify_oop(obj);
+  __ verify_oop(obj, FILE_AND_LINE);
 
   if (do_null) {
     if (!TypeEntries::was_null_seen(current_klass)) {
--- a/src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/c1_MacroAssembler_ppc.cpp	Mon Nov 11 16:24:35 2019 +0100
@@ -44,7 +44,7 @@
   const Register temp_reg = R12_scratch2;
   Label Lmiss;
 
-  verify_oop(receiver);
+  verify_oop(receiver, FILE_AND_LINE);
   MacroAssembler::null_check(receiver, oopDesc::klass_offset_in_bytes(), &Lmiss);
   load_klass(temp_reg, receiver);
 
@@ -100,7 +100,7 @@
   // Load object header.
   ld(Rmark, oopDesc::mark_offset_in_bytes(), Roop);
 
-  verify_oop(Roop);
+  verify_oop(Roop, FILE_AND_LINE);
 
   // Save object being locked into the BasicObjectLock...
   std(Roop, BasicObjectLock::obj_offset_in_bytes(), Rbox);
@@ -157,7 +157,7 @@
   if (UseBiasedLocking) {
     // Load the object out of the BasicObjectLock.
     ld(Roop, BasicObjectLock::obj_offset_in_bytes(), Rbox);
-    verify_oop(Roop);
+    verify_oop(Roop, FILE_AND_LINE);
     biased_locking_exit(CCR0, Roop, R0, done);
   }
   // Test first it it is a fast recursive unlock.
@@ -167,7 +167,7 @@
   if (!UseBiasedLocking) {
     // Load object.
     ld(Roop, BasicObjectLock::obj_offset_in_bytes(), Rbox);
-    verify_oop(Roop);
+    verify_oop(Roop, FILE_AND_LINE);
   }
 
   // Check if it is still a light weight lock, this is is true if we see
@@ -316,7 +316,7 @@
 //         relocInfo::runtime_call_type);
   }
 
-  verify_oop(obj);
+  verify_oop(obj, FILE_AND_LINE);
 }
 
 
@@ -383,7 +383,7 @@
     //     relocInfo::runtime_call_type);
   }
 
-  verify_oop(obj);
+  verify_oop(obj, FILE_AND_LINE);
 }
 
 
@@ -399,8 +399,7 @@
   bne(CCR0, not_null);
   stop("non-null oop required");
   bind(not_null);
-  if (!VerifyOops) return;
-  verify_oop(r);
+  verify_oop(r, FILE_AND_LINE);
 }
 
 #endif // PRODUCT
--- a/src/hotspot/cpu/ppc/gc/g1/g1BarrierSetAssembler_ppc.cpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/gc/g1/g1BarrierSetAssembler_ppc.cpp	Mon Nov 11 16:24:35 2019 +0100
@@ -335,12 +335,12 @@
   __ ld(value, 0, tmp1);      // Resolve (untagged) jobject.
 
   __ beq(CCR0, not_weak);     // Test for jweak tag.
-  __ verify_oop(value);
+  __ verify_oop(value, FILE_AND_LINE);
   g1_write_barrier_pre(masm, IN_NATIVE | ON_PHANTOM_OOP_REF,
                        noreg, noreg, value,
                        tmp1, tmp2, needs_frame);
   __ bind(not_weak);
-  __ verify_oop(value);
+  __ verify_oop(value, FILE_AND_LINE);
   __ bind(done);
 }
 
--- a/src/hotspot/cpu/ppc/gc/shared/barrierSetAssembler_ppc.cpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/gc/shared/barrierSetAssembler_ppc.cpp	Mon Nov 11 16:24:35 2019 +0100
@@ -113,7 +113,7 @@
   __ clrrdi(tmp1, value, JNIHandles::weak_tag_size);
   __ ld(value, 0, tmp1);      // Resolve (untagged) jobject.
 
-  __ verify_oop(value);
+  __ verify_oop(value, FILE_AND_LINE);
   __ bind(done);
 }
 
--- a/src/hotspot/cpu/ppc/globalDefinitions_ppc.hpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/globalDefinitions_ppc.hpp	Mon Nov 11 16:24:35 2019 +0100
@@ -30,6 +30,10 @@
 #error "CC_INTERP is no longer supported. Removed in change 8145117."
 #endif
 
+#ifndef FILE_AND_LINE
+#define FILE_AND_LINE __FILE__ ":" XSTR(__LINE__)
+#endif
+
 // Size of PPC Instructions
 const int BytesPerInstWord = 4;
 
--- a/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp	Mon Nov 11 16:24:35 2019 +0100
@@ -2313,7 +2313,7 @@
 }
 
 void InterpreterMacroAssembler::verify_oop(Register reg, TosState state) {
-  if (state == atos) { MacroAssembler::verify_oop(reg); }
+  if (state == atos) { MacroAssembler::verify_oop(reg, FILE_AND_LINE); }
 }
 
 // Local helper function for the verify_oop_or_return_address macro.
--- a/src/hotspot/cpu/ppc/macroAssembler_ppc.cpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/macroAssembler_ppc.cpp	Mon Nov 11 16:24:35 2019 +0100
@@ -3120,7 +3120,7 @@
   li(R0, 0);
   std(R0, in_bytes(JavaThread::vm_result_offset()), R16_thread);
 
-  verify_oop(oop_result);
+  verify_oop(oop_result, FILE_AND_LINE);
 }
 
 void MacroAssembler::get_vm_result_2(Register metadata_result) {
@@ -4917,6 +4917,13 @@
   }
 }
 
+void MacroAssembler::verify_coop(Register coop, const char* msg) {
+  if (!VerifyOops) { return; }
+  if (UseCompressedOops) { decode_heap_oop(coop); }
+  verify_oop(coop, msg);
+  if (UseCompressedOops) { encode_heap_oop(coop, coop); }
+}
+
 // READ: oop. KILL: R0. Volatile floats perhaps.
 void MacroAssembler::verify_oop(Register oop, const char* msg) {
   if (!VerifyOops) {
@@ -4926,6 +4933,9 @@
   address/* FunctionDescriptor** */fd = StubRoutines::verify_oop_subroutine_entry_address();
   const Register tmp = R11; // Will be preserved.
   const int nbytes_save = MacroAssembler::num_volatile_regs * 8;
+
+  BLOCK_COMMENT("verify_oop {");
+
   save_volatile_gprs(R1_SP, -nbytes_save); // except R0
 
   mr_if_needed(R4_ARG2, oop);
@@ -4942,6 +4952,8 @@
   pop_frame();
   restore_LR_CR(tmp);
   restore_volatile_gprs(R1_SP, -nbytes_save); // except R0
+
+  BLOCK_COMMENT("} verify_oop");
 }
 
 void MacroAssembler::verify_oop_addr(RegisterOrConstant offs, Register base, const char* msg) {
--- a/src/hotspot/cpu/ppc/macroAssembler_ppc.hpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/macroAssembler_ppc.hpp	Mon Nov 11 16:24:35 2019 +0100
@@ -914,6 +914,9 @@
   // Verify R16_thread contents.
   void verify_thread();
 
+  // Calls verify_oop. If UseCompressedOops is on, decodes the oop.
+  // Preserves reg.
+  void verify_coop(Register reg, const char*);
   // Emit code to verify that reg contains a valid oop if +VerifyOops is set.
   void verify_oop(Register reg, const char* s = "broken oop");
   void verify_oop_addr(RegisterOrConstant offs, Register base, const char* s = "contains broken oop");
--- a/src/hotspot/cpu/ppc/methodHandles_ppc.cpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/methodHandles_ppc.cpp	Mon Nov 11 16:24:35 2019 +0100
@@ -77,7 +77,7 @@
   Klass* klass = SystemDictionary::well_known_klass(klass_id);
   Label L_ok, L_bad;
   BLOCK_COMMENT("verify_klass {");
-  __ verify_oop(obj_reg);
+  __ verify_oop(obj_reg, FILE_AND_LINE);
   __ cmpdi(CCR0, obj_reg, 0);
   __ beq(CCR0, L_bad);
   __ load_klass(temp_reg, obj_reg);
@@ -172,16 +172,16 @@
   assert(method_temp == R19_method, "required register for loading method");
 
   // Load the invoker, as MH -> MH.form -> LF.vmentry
-  __ verify_oop(recv);
+  __ verify_oop(recv, FILE_AND_LINE);
   __ load_heap_oop(method_temp, NONZERO(java_lang_invoke_MethodHandle::form_offset_in_bytes()), recv,
                    temp2, noreg, false, IS_NOT_NULL);
-  __ verify_oop(method_temp);
+  __ verify_oop(method_temp, FILE_AND_LINE);
   __ load_heap_oop(method_temp, NONZERO(java_lang_invoke_LambdaForm::vmentry_offset_in_bytes()), method_temp,
                    temp2, noreg, false, IS_NOT_NULL);
-  __ verify_oop(method_temp);
+  __ verify_oop(method_temp, FILE_AND_LINE);
   __ load_heap_oop(method_temp, NONZERO(java_lang_invoke_MemberName::method_offset_in_bytes()), method_temp,
                    temp2, noreg, false, IS_NOT_NULL);
-  __ verify_oop(method_temp);
+  __ verify_oop(method_temp, FILE_AND_LINE);
   __ ld(method_temp, NONZERO(java_lang_invoke_ResolvedMethodName::vmtarget_offset_in_bytes()), method_temp);
 
   if (VerifyMethodHandles && !for_compiler_entry) {
@@ -318,7 +318,7 @@
 
     Register temp1_recv_klass = temp1;
     if (iid != vmIntrinsics::_linkToStatic) {
-      __ verify_oop(receiver_reg);
+      __ verify_oop(receiver_reg, FILE_AND_LINE);
       if (iid == vmIntrinsics::_linkToSpecial) {
         // Don't actually load the klass; just null-check the receiver.
         __ null_check_throw(receiver_reg, -1, temp1,
--- a/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp	Mon Nov 11 16:24:35 2019 +0100
@@ -1742,9 +1742,9 @@
         assert(r->is_valid(), "bad oop arg");
         if (r->is_stack()) {
           __ ld(temp_reg, reg2offset(r), R1_SP);
-          __ verify_oop(temp_reg);
+          __ verify_oop(temp_reg, FILE_AND_LINE);
         } else {
-          __ verify_oop(r->as_Register());
+          __ verify_oop(r->as_Register(), FILE_AND_LINE);
         }
       }
     }
@@ -2107,7 +2107,7 @@
 
   __ cmpdi(CCR0, R3_ARG1, 0);
   __ beq(CCR0, ic_miss);
-  __ verify_oop(R3_ARG1);
+  __ verify_oop(R3_ARG1, FILE_AND_LINE);
   __ load_klass(receiver_klass, R3_ARG1);
 
   __ cmpd(CCR0, receiver_klass, ic);
--- a/src/hotspot/cpu/ppc/stubGenerator_ppc.cpp	Tue Apr 24 13:59:02 2018 +0200
+++ b/src/hotspot/cpu/ppc/stubGenerator_ppc.cpp	Mon Nov 11 16:24:35 2019 +0100
@@ -440,7 +440,6 @@
     StubCodeMark mark(this, "StubRoutines", "forward_exception");
     address start = __ pc();
 
-#if !defined(PRODUCT)
     if (VerifyOops) {
       // Get pending exception oop.
       __ ld(R3_ARG1,
@@ -456,7 +455,6 @@
       }
       __ verify_oop(R3_ARG1, "StubRoutines::forward exception: not an oop");
     }
-#endif
 
     // Save LR/CR and copy exception pc (LR) into R4_ARG2.
     __ save_LR_CR(R4_ARG2);
@@ -702,9 +700,9 @@
 #if !defined(PRODUCT)
   // Wrapper which calls oopDesc::is_oop_or_null()
   // Only called by MacroAssembler::verify_oop
-  static void verify_oop_helper(const char* message, oop o) {
+  static void verify_oop_helper(const char* message, oopDesc* o) {
     if (!oopDesc::is_oop_or_null(o)) {
-      fatal("%s", message);
+      fatal("%s. oop: " PTR_FORMAT, message, p2i(o));
     }
     ++ StubRoutines::_verify_oop_count;
   }
@@ -725,7 +723,6 @@
     return start;
   }
 
-
   // -XX:+OptimizeFill : convert fill/copy loops into intrinsic
   //
   // The code is implemented(ported from sparc) as we believe it benefits JVM98, however
--- a/test/hotspot/jtreg/runtime/CheckUnhandledOops/TestVerifyOops.java	Tue Apr 24 13:59:02 2018 +0200
+++ b/test/hotspot/jtreg/runtime/CheckUnhandledOops/TestVerifyOops.java	Mon Nov 11 16:24:35 2019 +0100
@@ -21,10 +21,19 @@
  * questions.
  */
 
+// The test fails on sparc because there are errors in VerifyOops.
 /*
  * @test
  * @bug 8231058
- * @requires vm.debug & (os.arch != "sparc") & (os.arch != "sparcv9")
+ * @requires vm.debug & vm.bits == "64"
+ * @requires (os.arch != "sparcv9")
+ * @run main/othervm -XX:+VerifyOops -XX:+UseCompressedOops TestVerifyOops
+ * @run main/othervm -XX:+VerifyOops -XX:-UseCompressedOops TestVerifyOops
+ */
+/*
+ * @test
+ * @bug 8231058
+ * @requires vm.debug & vm.bits == "32"
  * @run main/othervm -XX:+VerifyOops TestVerifyOops
  */