JDK-7122939 : TraceBytecodes broken with UseCompressedOops
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 7
  • Priority: P4
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2011-12-19
  • Updated: 2012-03-22
  • Resolved: 2012-01-20
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7 JDK 8 Other
7u4Fixed 8Fixed hs23Fixed
Description
Hi,

I've just realized that -XX:+TraceBytecodes does not work anymore if
-XX:+UseCompressedOops is set.

This is because TemplateInterpreterGenerator::trace_bytecode() uses
'r12' which holds the heap base for temporary saving the 'sp' before
calling the Interpreter::trace_code() template:

  __ mov(r12, rsp); // remember sp (can only use r12 if not using call_VM)
  __ andptr(rsp, -16); // align stack as required by ABI
  __ call(RuntimeAddress(Interpreter::trace_code(t->tos_in())));
  __ mov(rsp, r12); // restore sp
  __ reinit_heapbase();

Notice that the comment explicitly mentions that 'r12' can only be
used if 'call_VM' will not be used subsequently.

But this is exactly what Interpreter::trace_code() does:


address TemplateInterpreterGenerator::generate_trace_code(TosState state) {
...
  __ call_VM(noreg,...

void MacroAssembler::call_VM(Register oop_result,
...
  call_VM_helper(oop_result, entry_point, 3, check_exceptions);

void MacroAssembler::call_VM_helper(Register oop_result, address
entry_point, int number_of_arguments, bool check_exceptions) {
...
  call_VM_base(oop_result, noreg, rax, entry_point,
number_of_arguments, check_exceptions);

void MacroAssembler::call_VM_base(Register oop_result,
...
  LP64_ONLY(if (UseCompressedOops) verify_heapbase("call_VM_base");)


This finally leads to a call to 'verify_heapbase()' which will fail
because 'r12' has been overwritten.

The proposed fix is trivial: don't do the check if we are running with
-XX:+TraceBytecodes:

diff -r 86cbe939f0c7 src/cpu/x86/vm/assembler_x86.cpp
--- a/src/cpu/x86/vm/assembler_x86.cpp  Mon Sep 19 12:18:46 2011 -0700
+++ b/src/cpu/x86/vm/assembler_x86.cpp  Fri Dec 16 18:01:31 2011 +0100
@@ -5967,7 +5967,7 @@
   assert(number_of_arguments >= 0   , "cannot have negative number of
arguments");
   LP64_ONLY(assert(java_thread == r15_thread, "unexpected register"));
 #ifdef ASSERT
-  LP64_ONLY(if (UseCompressedOops) verify_heapbase("call_VM_base");)
+  LP64_ONLY(if (UseCompressedOops && !TraceBytecodes)
verify_heapbase("call_VM_base");)
 #endif // ASSERT

   assert(java_thread != oop_result  , "cannot use the same register
for java_thread & oop_result");

For your convenience I've also attached the patch as a file to this message.

Regards,
Volker

Comments
EVALUATION http://hg.openjdk.java.net/lambda/lambda/hotspot/rev/96ce4c27112f
22-03-2012

EVALUATION http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/96ce4c27112f
20-12-2011