Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1669,7 +1669,7 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
//
// Generic interpreted method entry to (asm) interpreter
//
address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized) {
address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized, bool object_init) {
// determine code generation flags
bool inc_counter = UseCompiler || CountCompiledCalls;

Expand Down Expand Up @@ -1796,6 +1796,12 @@ address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized) {
#endif
}

// Issue a StoreStore barrier on entry to Object_init if the
// class has strict field fields. Be lazy, always do it.
if (object_init) {
__ membar(MacroAssembler::StoreStore);
}

// start execution
#ifdef ASSERT
{
Expand Down
8 changes: 7 additions & 1 deletion src/hotspot/cpu/arm/templateInterpreterGenerator_arm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,7 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
//
// Generic interpreted method entry to (asm) interpreter
//
address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized) {
address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized, bool object_init) {
// determine code generation flags
bool inc_counter = UseCompiler || CountCompiledCalls;

Expand Down Expand Up @@ -1254,6 +1254,12 @@ address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized) {
#endif
}

// Issue a StoreStore barrier on entry to Object_init if the
// class has strict field fields. Be lazy, always do it.
if (object_init) {
__ membar(MacroAssembler::StoreStore, R1_tmp);
}

// start execution
#ifdef ASSERT
{ Label L;
Expand Down
9 changes: 8 additions & 1 deletion src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1706,7 +1706,7 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {

// Generic interpreted method entry to (asm) interpreter.
//
address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized) {
address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized, bool object_init) {
bool inc_counter = UseCompiler || CountCompiledCalls;
address entry = __ pc();
// Generate the code to allocate the interpreter stack frame.
Expand Down Expand Up @@ -1801,6 +1801,13 @@ address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized) {
// JVMTI support
__ notify_method_entry();

// --------------------------------------------------------------------------
// Issue a StoreStore barrier on entry to Object_init if the
// class has strict field fields. Be lazy, always do it.
if (object_init) {
__ membar(Assembler::StoreStore);
}

// --------------------------------------------------------------------------
// Start executing instructions.
__ dispatch_next(vtos);
Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/cpu/riscv/templateInterpreterGenerator_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1545,6 +1545,12 @@ address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized) {
#endif
}

// Issue a StoreStore barrier on entry to Object_init if the
// class has strict field fields. Be lazy, always do it.
if (object_init) {
__ membar(MacroAssembler::StoreStore);
}

// start execution
#ifdef ASSERT
__ verify_frame_setup();
Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1797,6 +1797,12 @@ address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized) {
#endif // ASSERT
}

// If object_init == true, we should insert a StoreStore barrier here to
// prevent strict fields initial default values from being observable.
// However, s390 is a TSO platform, so if `this` escapes, strict fields
// initialized values are guaranteed to be the ones observed, so the
// barrier can be elided.

// start execution

#ifdef ASSERT
Expand Down
8 changes: 7 additions & 1 deletion src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1239,7 +1239,7 @@ address TemplateInterpreterGenerator::generate_abstract_entry(void) {
//
// Generic interpreted method entry to (asm) interpreter
//
address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized) {
address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized, bool object_init) {
// determine code generation flags
bool inc_counter = UseCompiler || CountCompiledCalls;

Expand Down Expand Up @@ -1360,6 +1360,12 @@ address TemplateInterpreterGenerator::generate_normal_entry(bool synchronized) {
#endif
}

// If object_init == true, we should insert a StoreStore barrier here to
// prevent strict fields initial default values from being observable.
// However, x86 is a TSO platform, so if `this` escapes, strict fields
// initialized values are guaranteed to be the ones observed, so the
// barrier can be elided.

// start execution
#ifdef ASSERT
{
Expand Down
7 changes: 7 additions & 0 deletions src/hotspot/share/interpreter/abstractInterpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,13 @@ AbstractInterpreter::MethodKind AbstractInterpreter::method_kind(const methodHan
if (m->code_size() == 1) {
// We need to execute the special return bytecode to check for
// finalizer registration so create a normal frame.
// No need to use the method kind with a memory barrier on entry
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is correct because the return bytecode will be rewritten:

if (method->intrinsic_id() == vmIntrinsics::_Object_init) {
// rewrite the return bytecodes of Object.<init> to register the
// object for finalization if needed.
methodHandle m(THREAD, method);
rewrite_Object_init(m, CHECK);

->
case Bytecodes::_return: *bcs.bcp() = Bytecodes::_return_register_finalizer; break;

And thus, this code will never be executed:

// Issue a StoreStore barrier after all stores but before return
// from any constructor for any class with a final field. We don't
// know if this is a finalizer, so we always do so.
if (_desc->bytecode() == Bytecodes::_return)
__ membar(MacroAssembler::StoreStore);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed JDK-8369044.

// because the method is empty and already has a memory barrier on return
return zerolocals;
} else if (EnableValhalla) {
// For non-empty Object constructors, we need a memory barrier
// when entering the method to ensure correctness of strict fields
return object_init;
}
break;
default: break;
Expand Down Expand Up @@ -303,6 +309,7 @@ void AbstractInterpreter::print_method_kind(MethodKind kind) {
case getter : tty->print("getter" ); break;
case setter : tty->print("setter" ); break;
case abstract : tty->print("abstract" ); break;
case object_init : tty->print("object_init" ); break;
case java_lang_math_sin : tty->print("java_lang_math_sin" ); break;
case java_lang_math_cos : tty->print("java_lang_math_cos" ); break;
case java_lang_math_tan : tty->print("java_lang_math_tan" ); break;
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/interpreter/abstractInterpreter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class AbstractInterpreter: AllStatic {
getter, // getter method
setter, // setter method
abstract, // abstract method (throws an AbstractMethodException)
object_init, // special barrier on entry
method_handle_invoke_FIRST, // java.lang.invoke.MethodHandles::invokeExact, etc.
method_handle_invoke_LAST = (method_handle_invoke_FIRST
+ (static_cast<int>(vmIntrinsics::LAST_MH_SIG_POLY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ void TemplateInterpreterGenerator::generate_all() {
method_entry(getter)
method_entry(setter)
method_entry(abstract)
method_entry(object_init)
method_entry(java_lang_math_sin )
method_entry(java_lang_math_cos )
method_entry(java_lang_math_tan )
Expand Down Expand Up @@ -417,6 +418,7 @@ address TemplateInterpreterGenerator::generate_method_entry(
case Interpreter::empty : break;
case Interpreter::getter : break;
case Interpreter::setter : break;
case Interpreter::object_init : break;
case Interpreter::abstract : entry_point = generate_abstract_entry(); break;
default:
entry_point = generate_intrinsic_entry(kind); // process the rest
Expand All @@ -429,14 +431,17 @@ address TemplateInterpreterGenerator::generate_method_entry(

// We expect the normal and native entry points to be generated first so we can reuse them.
if (native) {
assert(kind != Interpreter::object_init, "Not supported");
entry_point = Interpreter::entry_for_kind(synchronized ? Interpreter::native_synchronized : Interpreter::native);
if (entry_point == nullptr) {
entry_point = generate_native_entry(synchronized);
}
} else {
entry_point = Interpreter::entry_for_kind(synchronized ? Interpreter::zerolocals_synchronized : Interpreter::zerolocals);
entry_point = kind == Interpreter::object_init ?
Interpreter::entry_for_kind(Interpreter::object_init) :
Interpreter::entry_for_kind(synchronized ? Interpreter::zerolocals_synchronized : Interpreter::zerolocals);
if (entry_point == nullptr) {
entry_point = generate_normal_entry(synchronized);
entry_point = generate_normal_entry(synchronized, kind == Interpreter::object_init);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -90,7 +90,7 @@ class TemplateInterpreterGenerator: public AbstractInterpreterGenerator {
// generate intrinsic method entries
address generate_intrinsic_entry(AbstractInterpreter::MethodKind kind);

address generate_normal_entry(bool synchronized);
address generate_normal_entry(bool synchronized, bool object_init);
address generate_native_entry(bool synchronized);
address generate_abstract_entry(void);
address generate_math_entry(AbstractInterpreter::MethodKind kind);
Expand Down