Skip to content

Commit

Permalink
8282218: C1: Missing side effects of dynamic class loading during con…
Browse files Browse the repository at this point in the history
…stant linkage

Reviewed-by: thartmann, kvn
  • Loading branch information
Vladimir Ivanov committed May 6, 2022
1 parent d8f9686 commit 5212535
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 7 deletions.
6 changes: 4 additions & 2 deletions src/hotspot/share/c1/c1_GraphBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ void GraphBuilder::load_constant() {
case T_ARRAY : // fall-through
case T_OBJECT : {
ciObject* obj = con.as_object();
if (!obj->is_loaded() || (PatchALot && (obj->is_null_object() || obj->klass() != ciEnv::current()->String_klass()))) {
if (!obj->is_loaded() || (PatchALot && !stream()->is_string_constant())) {
// A Class, MethodType, MethodHandle, Dynamic, or String.
patch_state = copy_state_before();
t = new ObjectConstant(obj);
Expand All @@ -973,7 +973,9 @@ void GraphBuilder::load_constant() {
}
Value x;
if (patch_state != NULL) {
bool kills_memory = stream()->is_dynamic_constant(); // arbitrary memory effects from running BSM during linkage
// Arbitrary memory effects from running BSM or class loading (using custom loader) during linkage.
bool kills_memory = stream()->is_dynamic_constant() ||
(!stream()->is_string_constant() && !method()->holder()->has_trusted_loader());
x = new Constant(t, patch_state, kills_memory);
} else {
x = new Constant(t);
Expand Down
12 changes: 12 additions & 0 deletions src/hotspot/share/ci/ciInstanceKlass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ ciInstanceKlass::ciInstanceKlass(Klass* k) :
_is_shared = true;
}

_has_trusted_loader = compute_has_trusted_loader();

// Lazy fields get filled in only upon request.
_super = NULL;
_java_mirror = NULL;
Expand Down Expand Up @@ -132,6 +134,7 @@ ciInstanceKlass::ciInstanceKlass(ciSymbol* name,
_super = NULL;
_java_mirror = NULL;
_field_cache = NULL;
_has_trusted_loader = compute_has_trusted_loader();
}


Expand Down Expand Up @@ -568,6 +571,15 @@ bool ciInstanceKlass::has_object_fields() const {
);
}

bool ciInstanceKlass::compute_has_trusted_loader() {
ASSERT_IN_VM;
oop loader_oop = loader();
if (loader_oop == NULL) {
return true; // bootstrap class loader
}
return java_lang_ClassLoader::is_trusted_loader(loader_oop);
}

// ------------------------------------------------------------------
// ciInstanceKlass::find_method
//
Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/share/ci/ciInstanceKlass.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class ciInstanceKlass : public ciKlass {
bool _has_nonstatic_concrete_methods;
bool _is_hidden;
bool _is_record;
bool _has_trusted_loader;

ciFlags _flags;

Expand Down Expand Up @@ -107,6 +108,7 @@ class ciInstanceKlass : public ciKlass {
bool compute_shared_has_subklass();
int compute_nonstatic_fields();
GrowableArray<ciField*>* compute_nonstatic_fields_impl(GrowableArray<ciField*>* super_fields);
bool compute_has_trusted_loader();

// Update the init_state for shared klasses
void update_if_shared(InstanceKlass::ClassState expected) {
Expand Down Expand Up @@ -287,6 +289,10 @@ class ciInstanceKlass : public ciKlass {
return !is_interface() && !is_abstract();
}

bool has_trusted_loader() const {
return _has_trusted_loader;
}

// Replay support

// Dump the current state of this klass for compilation replay.
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/ci/ciStreams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ constantTag ciBytecodeStream::get_constant_pool_tag(int index) const {
// ------------------------------------------------------------------
// ciBytecodeStream::get_raw_pool_tag
//
constantTag ciBytecodeStream::get_raw_pool_tag(int index) const {
constantTag ciBytecodeStream::get_raw_pool_tag_at(int index) const {
VM_ENTRY_MARK;
return _method->get_Method()->constants()->tag_at(index);
}
Expand Down
21 changes: 17 additions & 4 deletions src/hotspot/share/ci/ciStreams.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,14 @@ class ciBytecodeStream : StackObj {
constantTag get_constant_pool_tag(int index) const;
BasicType get_basic_type_for_constant_at(int index) const;

constantTag get_raw_pool_tag(int index) const;
constantTag get_raw_pool_tag_at(int index) const;

// True if the klass-using bytecode points to an unresolved klass
constantTag get_raw_pool_tag() const {
int index = get_constant_pool_index();
return get_raw_pool_tag_at(index);
}

// True if the klass-using bytecode points to an unresolved klass
bool is_unresolved_klass() const {
constantTag tag = get_constant_pool_tag(get_klass_index());
return tag.is_unresolved_klass();
Expand All @@ -243,12 +248,20 @@ class ciBytecodeStream : StackObj {
cur_bc() == Bytecodes::_ldc_w ||
cur_bc() == Bytecodes::_ldc2_w, "not supported: %s", Bytecodes::name(cur_bc()));

int index = get_constant_pool_index();
constantTag tag = get_raw_pool_tag(index);
constantTag tag = get_raw_pool_tag();
return tag.is_dynamic_constant() ||
tag.is_dynamic_constant_in_error();
}

bool is_string_constant() const {
assert(cur_bc() == Bytecodes::_ldc ||
cur_bc() == Bytecodes::_ldc_w ||
cur_bc() == Bytecodes::_ldc2_w, "not supported: %s", Bytecodes::name(cur_bc()));

constantTag tag = get_raw_pool_tag();
return tag.is_string();
}

bool is_in_error() const {
assert(cur_bc() == Bytecodes::_ldc ||
cur_bc() == Bytecodes::_ldc_w ||
Expand Down
96 changes: 96 additions & 0 deletions test/hotspot/jtreg/compiler/c1/TestClassConstantPatching.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
* Copyright (c) 2022, 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
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/**
* @test
* @bug 8282218
*
* @run main/othervm -Xcomp -XX:TieredStopAtLevel=1
* -XX:CompileCommand=compileonly,compiler.c1.TestClassConstantPatching$Test::run
* compiler.c1.TestClassConstantPatching
*/
package compiler.c1;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Method;

public class TestClassConstantPatching {
public static int COUNTER = 0;

static class CustomLoader extends ClassLoader {
@Override
protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
if (name.startsWith(Test.class.getName())) {
Class<?> c = findLoadedClass(name);
if (c != null) {
return c;
}
try {
COUNTER++; // update counter on loading

InputStream in = getSystemResourceAsStream(name.replace('.', File.separatorChar) + ".class");
byte[] buf = in.readAllBytes();
return defineClass(name, buf, 0, buf.length);
} catch (IOException e) {
throw new ClassNotFoundException(name);
}
}
return super.loadClass(name, resolve);
}
}

public static class Test {
static class T {}

public static Class<?> run(boolean cond) {
int before = COUNTER;
Class<?> c = null;
if (cond) {
c = T.class;
}
int after = COUNTER;
if (cond && before == after) {
throw new AssertionError("missed update");
}
return c;
}
}

public static void main(String[] args) throws ReflectiveOperationException {
ClassLoader cl = new CustomLoader();

Class.forName(TestClassConstantPatching.class.getName(), true, cl); // preload counter holder class

Class<?> test = Class.forName(Test.class.getName(), true, cl);

Method m = test.getDeclaredMethod("run", boolean.class);

m.invoke(null, false);
m.invoke(null, true);

System.out.println("TEST PASSED");
}
}

1 comment on commit 5212535

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.