Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8293044: C1: Missing access check on non-accessible class
Reviewed-by: thartmann, dlong
  • Loading branch information
Vladimir Ivanov committed Sep 9, 2022
1 parent 91d00b3 commit 005b49b
Show file tree
Hide file tree
Showing 8 changed files with 244 additions and 17 deletions.
15 changes: 5 additions & 10 deletions src/hotspot/share/c1/c1_GraphBuilder.cpp
Expand Up @@ -2232,8 +2232,7 @@ void GraphBuilder::invoke(Bytecodes::Code code) {

void GraphBuilder::new_instance(int klass_index) {
ValueStack* state_before = copy_state_exhandling();
bool will_link;
ciKlass* klass = stream()->get_klass(will_link);
ciKlass* klass = stream()->get_klass();
assert(klass->is_instance_klass(), "must be an instance klass");
NewInstance* new_instance = new NewInstance(klass->as_instance_klass(), state_before, stream()->is_unresolved_klass());
_memory->new_instance(new_instance);
Expand All @@ -2248,8 +2247,7 @@ void GraphBuilder::new_type_array() {


void GraphBuilder::new_object_array() {
bool will_link;
ciKlass* klass = stream()->get_klass(will_link);
ciKlass* klass = stream()->get_klass();
ValueStack* state_before = !klass->is_loaded() || PatchALot ? copy_state_before() : copy_state_exhandling();
NewArray* n = new NewObjectArray(klass, ipop(), state_before);
apush(append_split(n));
Expand All @@ -2274,8 +2272,7 @@ bool GraphBuilder::direct_compare(ciKlass* k) {


void GraphBuilder::check_cast(int klass_index) {
bool will_link;
ciKlass* klass = stream()->get_klass(will_link);
ciKlass* klass = stream()->get_klass();
ValueStack* state_before = !klass->is_loaded() || PatchALot ? copy_state_before() : copy_state_for_exception();
CheckCast* c = new CheckCast(klass, apop(), state_before);
apush(append_split(c));
Expand All @@ -2295,8 +2292,7 @@ void GraphBuilder::check_cast(int klass_index) {


void GraphBuilder::instance_of(int klass_index) {
bool will_link;
ciKlass* klass = stream()->get_klass(will_link);
ciKlass* klass = stream()->get_klass();
ValueStack* state_before = !klass->is_loaded() || PatchALot ? copy_state_before() : copy_state_exhandling();
InstanceOf* i = new InstanceOf(klass, apop(), state_before);
ipush(append_split(i));
Expand Down Expand Up @@ -2331,8 +2327,7 @@ void GraphBuilder::monitorexit(Value x, int bci) {


void GraphBuilder::new_multi_array(int dimensions) {
bool will_link;
ciKlass* klass = stream()->get_klass(will_link);
ciKlass* klass = stream()->get_klass();
ValueStack* state_before = !klass->is_loaded() || PatchALot ? copy_state_before() : copy_state_exhandling();

Values* dims = new Values(dimensions, dimensions, NULL);
Expand Down
41 changes: 37 additions & 4 deletions src/hotspot/share/c1/c1_Runtime1.cpp
Expand Up @@ -1282,6 +1282,37 @@ JRT_END

#else // DEOPTIMIZE_WHEN_PATCHING

static bool is_patching_needed(JavaThread* current, Runtime1::StubID stub_id) {
if (stub_id == Runtime1::load_klass_patching_id ||
stub_id == Runtime1::load_mirror_patching_id) {
// last java frame on stack
vframeStream vfst(current, true);
assert(!vfst.at_end(), "Java frame must exist");

methodHandle caller_method(current, vfst.method());
int bci = vfst.bci();
Bytecodes::Code code = caller_method()->java_code_at(bci);

switch (code) {
case Bytecodes::_new:
case Bytecodes::_anewarray:
case Bytecodes::_multianewarray:
case Bytecodes::_instanceof:
case Bytecodes::_checkcast: {
Bytecode bc(caller_method(), caller_method->bcp_from(bci));
constantTag tag = caller_method->constants()->tag_at(bc.get_index_u2(code));
if (tag.is_unresolved_klass_in_error()) {
return false; // throws resolution error
}
break;
}

default: break;
}
}
return true;
}

void Runtime1::patch_code(JavaThread* current, Runtime1::StubID stub_id) {
#ifndef PRODUCT
if (PrintC1Statistics) {
Expand All @@ -1306,10 +1337,12 @@ void Runtime1::patch_code(JavaThread* current, Runtime1::StubID stub_id) {
frame caller_frame = runtime_frame.sender(&reg_map);
assert(caller_frame.is_compiled_frame(), "Wrong frame type");

// Make sure the nmethod is invalidated, i.e. made not entrant.
nmethod* nm = CodeCache::find_nmethod(caller_frame.pc());
if (nm != NULL) {
nm->make_not_entrant();
if (is_patching_needed(current, stub_id)) {
// Make sure the nmethod is invalidated, i.e. made not entrant.
nmethod* nm = CodeCache::find_nmethod(caller_frame.pc());
if (nm != NULL) {
nm->make_not_entrant();
}
}

Deoptimization::deoptimize_frame(current, caller_frame.id());
Expand Down
22 changes: 21 additions & 1 deletion src/hotspot/share/ci/ciStreams.cpp
Expand Up @@ -23,9 +23,10 @@
*/

#include "precompiled.hpp"
#include "ci/ciCallSite.hpp"
#include "ci/ciConstant.hpp"
#include "ci/ciField.hpp"
#include "ci/ciKlass.hpp"
#include "ci/ciObjArrayKlass.hpp"
#include "ci/ciStreams.hpp"
#include "ci/ciSymbols.hpp"
#include "ci/ciUtilities.inline.hpp"
Expand Down Expand Up @@ -191,6 +192,25 @@ ciKlass* ciBytecodeStream::get_klass(bool& will_link) {
return CURRENT_ENV->get_klass_by_index(cpool, get_klass_index(), will_link, _holder);
}

// ciBytecodeStream::get_klass
//
// If this bytecode is a new, newarray, multianewarray, instanceof,
// or checkcast, get the referenced klass. Retuns an unloaded ciKlass
// if the referenced klass is not accessible.
ciKlass* ciBytecodeStream::get_klass() {
bool will_link;
ciKlass* klass = get_klass(will_link);
if (!will_link && klass->is_loaded()) { // klass not accessible
if (klass->is_array_klass()) {
assert(!klass->is_type_array_klass(), "");
klass = ciEnv::unloaded_ciobjarrayklass();
} else {
klass = ciEnv::unloaded_ciinstance_klass();
}
}
return klass;
}

// ------------------------------------------------------------------
// ciBytecodeStream::get_constant_raw_index
//
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/ci/ciStreams.hpp
Expand Up @@ -219,6 +219,7 @@ class ciBytecodeStream : StackObj {

// If this bytecode is a new, newarray, multianewarray, instanceof,
// or checkcast, get the referenced klass.
ciKlass* get_klass();
ciKlass* get_klass(bool& will_link);
int get_klass_index() const;

Expand Down
6 changes: 4 additions & 2 deletions src/hotspot/share/interpreter/bytecode.hpp
Expand Up @@ -77,9 +77,11 @@ class Bytecode: public StackObj {
int get_index_u2(Bytecodes::Code bc, bool is_wide = false) const {
assert_same_format_as(bc, is_wide); assert_index_size(2, bc, is_wide);
address p = addr_at(is_wide ? 2 : 1);
if (can_use_native_byte_order(bc, is_wide))
if (can_use_native_byte_order(bc, is_wide)) {
return Bytes::get_native_u2(p);
else return Bytes::get_Java_u2(p);
} else {
return Bytes::get_Java_u2(p);
}
}
int get_index_u1_cpcache(Bytecodes::Code bc) const {
assert_same_format_as(bc); assert_index_size(1, bc);
Expand Down
86 changes: 86 additions & 0 deletions test/hotspot/jtreg/compiler/c1/KlassAccessCheck.jasm
@@ -0,0 +1,86 @@
/*
* 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.
*/

package compiler/c1;

super public class KlassAccessCheck
version 51:0
{

public static Method testNewInstance:"()V"
stack 2 locals 0
{
new class compiler/c1/types/PackagePrivateClass;
return;
}


public static Method testNewArray:"()[Ljava/lang/Object;"
stack 1 locals 0
{
iconst_1;
anewarray class compiler/c1/types/PackagePrivateClass;
areturn;
}

public static Method testMultiNewArray:"()[[Ljava/lang/Object;"
stack 2 locals 1
{
iconst_1;
iconst_1;
multianewarray class "[[Lcompiler/c1/types/PackagePrivateClass;", 2;
areturn;
}

public static Method testCheckCast:"(Ljava/lang/Object;)Ljava/lang/Object;"
stack 1 locals 2
{
aload_0;
checkcast class compiler/c1/types/PackagePrivateClass;
areturn;
}

public static Method testCheckCastArr:"(Ljava/lang/Object;)Ljava/lang/Object;"
stack 1 locals 2
{
aload_0;
checkcast class "[Lcompiler/c1/types/PackagePrivateClass;";
areturn;
}

public static Method testInstanceOf:"(Ljava/lang/Object;)Z"
stack 1 locals 2
{
aload_0;
instanceof class compiler/c1/types/PackagePrivateClass;
ireturn;
}

public static Method testInstanceOfArr:"(Ljava/lang/Object;)Z"
stack 1 locals 2
{
aload_0;
instanceof class "[Lcompiler/c1/types/PackagePrivateClass;";
ireturn;
}
} // end Class KlassAccessCheck
29 changes: 29 additions & 0 deletions test/hotspot/jtreg/compiler/c1/KlassAccessCheckPackagePrivate.jasm
@@ -0,0 +1,29 @@
/*
* 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.
*/

package compiler/c1/types;

super class PackagePrivateClass
version 51:0
{}

61 changes: 61 additions & 0 deletions test/hotspot/jtreg/compiler/c1/KlassAccessCheckTest.java
@@ -0,0 +1,61 @@
/*
* 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 8293044
* @requires vm.compiler1.enabled
* @compile KlassAccessCheckPackagePrivate.jasm
* @compile KlassAccessCheck.jasm
* @run main/othervm -Xbatch -XX:TieredStopAtLevel=1 compiler.c1.KlassAccessCheckTest
*/

package compiler.c1;

public class KlassAccessCheckTest {
static void test(Runnable r) {
for (int i = 0; i < 1000; ++i) {
try {
r.run();
throw new AssertionError("No IllegalAccessError thrown");
} catch (IllegalAccessError e) {
// Expected
} catch (AssertionError e) {
throw e; // rethrow
} catch (Throwable e) {
throw new AssertionError("Wrong exception thrown", e);
}
}
}

public static void main(String[] args) {
test(() -> KlassAccessCheck.testNewInstance());
test(() -> KlassAccessCheck.testNewArray());
test(() -> KlassAccessCheck.testMultiNewArray());
test(() -> KlassAccessCheck.testCheckCast(42));
test(() -> KlassAccessCheck.testCheckCastArr(new Integer[0]));
test(() -> KlassAccessCheck.testInstanceOf(42));
test(() -> KlassAccessCheck.testInstanceOfArr(new Integer[0]));
System.out.println("TEST PASSED");
}
}

3 comments on commit 005b49b

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on 005b49b Oct 3, 2022

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 005b49b Oct 3, 2022

Choose a reason for hiding this comment

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

@GoeLin the backport was successfully created on the branch GoeLin-backport-005b49bb in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 005b49bb from the openjdk/jdk repository.

The commit being backported was authored by Vladimir Ivanov on 9 Sep 2022 and was reviewed by Tobias Hartmann and Dean Long.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u-dev:

$ git fetch https://github.com/openjdk-bots/jdk17u-dev GoeLin-backport-005b49bb:GoeLin-backport-005b49bb
$ git checkout GoeLin-backport-005b49bb
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u-dev GoeLin-backport-005b49bb

Please sign in to comment.