From 2820f348791c23ce43ff481ffe44d5227fb7b6ef Mon Sep 17 00:00:00 2001 From: hseigel Date: Fri, 9 May 2014 15:21:20 -0400 Subject: [PATCH] 8036533: Method for correct defaults 8036156: Limit default method hierarchy Summary: Fix protected access checks Reviewed-by: coleenp, lfoltan, acorn, ahgross --- src/share/vm/classfile/stackMapFrame.cpp | 12 ++++++------ src/share/vm/classfile/stackMapFrame.hpp | 8 ++++---- src/share/vm/classfile/verificationType.cpp | 16 ++++++++++------ src/share/vm/classfile/verificationType.hpp | 12 ++++++++---- src/share/vm/classfile/verifier.cpp | 18 +++++++++--------- src/share/vm/runtime/reflection.cpp | 7 ++++--- 6 files changed, 41 insertions(+), 32 deletions(-) diff --git a/src/share/vm/classfile/stackMapFrame.cpp b/src/share/vm/classfile/stackMapFrame.cpp index c3f69463c..a06ef2c66 100644 --- a/src/share/vm/classfile/stackMapFrame.cpp +++ b/src/share/vm/classfile/stackMapFrame.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2014, 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 @@ -163,7 +163,7 @@ int StackMapFrame::is_assignable_to( VerificationType* from, VerificationType* to, int32_t len, TRAPS) const { int32_t i = 0; for (i = 0; i < len; i++) { - if (!to[i].is_assignable_from(from[i], verifier(), THREAD)) { + if (!to[i].is_assignable_from(from[i], verifier(), false, THREAD)) { break; } } @@ -260,7 +260,7 @@ VerificationType StackMapFrame::pop_stack_ex(VerificationType type, TRAPS) { } VerificationType top = _stack[--_stack_size]; bool subtype = type.is_assignable_from( - top, verifier(), CHECK_(VerificationType::bogus_type())); + top, verifier(), false, CHECK_(VerificationType::bogus_type())); if (!subtype) { verifier()->verify_error( ErrorContext::bad_type(_offset, stack_top_ctx(), @@ -280,7 +280,7 @@ VerificationType StackMapFrame::get_local( return VerificationType::bogus_type(); } bool subtype = type.is_assignable_from(_locals[index], - verifier(), CHECK_(VerificationType::bogus_type())); + verifier(), false, CHECK_(VerificationType::bogus_type())); if (!subtype) { verifier()->verify_error( ErrorContext::bad_type(_offset, @@ -303,14 +303,14 @@ void StackMapFrame::get_local_2( "get long/double overflows locals"); return; } - bool subtype = type1.is_assignable_from(_locals[index], verifier(), CHECK); + bool subtype = type1.is_assignable_from(_locals[index], verifier(), false, CHECK); if (!subtype) { verifier()->verify_error( ErrorContext::bad_type(_offset, TypeOrigin::local(index, this), TypeOrigin::implicit(type1)), "Bad local variable type"); } else { - subtype = type2.is_assignable_from(_locals[index + 1], verifier(), CHECK); + subtype = type2.is_assignable_from(_locals[index + 1], verifier(), false, CHECK); if (!subtype) { /* Unreachable? All local store routines convert a split long or double * into a TOP during the store. So we should never end up seeing an diff --git a/src/share/vm/classfile/stackMapFrame.hpp b/src/share/vm/classfile/stackMapFrame.hpp index 237accec0..52e1558b8 100644 --- a/src/share/vm/classfile/stackMapFrame.hpp +++ b/src/share/vm/classfile/stackMapFrame.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2014, 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 @@ -238,7 +238,7 @@ class StackMapFrame : public ResourceObj { if (_stack_size != 0) { VerificationType top = _stack[_stack_size - 1]; bool subtype = type.is_assignable_from( - top, verifier(), CHECK_(VerificationType::bogus_type())); + top, verifier(), false, CHECK_(VerificationType::bogus_type())); if (subtype) { --_stack_size; return top; @@ -253,9 +253,9 @@ class StackMapFrame : public ResourceObj { assert(type2.is_long() || type2.is_double(), "must be long/double_2"); if (_stack_size >= 2) { VerificationType top1 = _stack[_stack_size - 1]; - bool subtype1 = type1.is_assignable_from(top1, verifier(), CHECK); + bool subtype1 = type1.is_assignable_from(top1, verifier(), false, CHECK); VerificationType top2 = _stack[_stack_size - 2]; - bool subtype2 = type2.is_assignable_from(top2, verifier(), CHECK); + bool subtype2 = type2.is_assignable_from(top2, verifier(), false, CHECK); if (subtype1 && subtype2) { _stack_size -= 2; return; diff --git a/src/share/vm/classfile/verificationType.cpp b/src/share/vm/classfile/verificationType.cpp index 561618bde..120d9a37d 100644 --- a/src/share/vm/classfile/verificationType.cpp +++ b/src/share/vm/classfile/verificationType.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2014, 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 @@ -42,7 +42,8 @@ VerificationType VerificationType::from_tag(u1 tag) { } bool VerificationType::is_reference_assignable_from( - const VerificationType& from, ClassVerifier* context, TRAPS) const { + const VerificationType& from, ClassVerifier* context, + bool from_field_is_protected, TRAPS) const { instanceKlassHandle klass = context->current_class(); if (from.is_null()) { // null is assignable to any reference @@ -62,9 +63,11 @@ bool VerificationType::is_reference_assignable_from( Handle(THREAD, klass->protection_domain()), true, CHECK_false); KlassHandle this_class(THREAD, obj); - if (this_class->is_interface()) { - // We treat interfaces as java.lang.Object, including - // java.lang.Cloneable and java.io.Serializable + if (this_class->is_interface() && (!from_field_is_protected || + from.name() != vmSymbols::java_lang_Object())) { + // If we are not trying to access a protected field or method in + // java.lang.Object then we treat interfaces as java.lang.Object, + // including java.lang.Cloneable and java.io.Serializable. return true; } else if (from.is_object()) { Klass* from_class = SystemDictionary::resolve_or_fail( @@ -76,7 +79,8 @@ bool VerificationType::is_reference_assignable_from( VerificationType comp_this = get_component(context, CHECK_false); VerificationType comp_from = from.get_component(context, CHECK_false); if (!comp_this.is_bogus() && !comp_from.is_bogus()) { - return comp_this.is_assignable_from(comp_from, context, CHECK_false); + return comp_this.is_assignable_from(comp_from, context, + from_field_is_protected, CHECK_false); } } return false; diff --git a/src/share/vm/classfile/verificationType.hpp b/src/share/vm/classfile/verificationType.hpp index eec0bf3f5..29ccf4d2c 100644 --- a/src/share/vm/classfile/verificationType.hpp +++ b/src/share/vm/classfile/verificationType.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2014, 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 @@ -265,7 +265,8 @@ class VerificationType VALUE_OBJ_CLASS_SPEC { // is assignable to another. Returns true if one can assign 'from' to // this. bool is_assignable_from( - const VerificationType& from, ClassVerifier* context, TRAPS) const { + const VerificationType& from, ClassVerifier* context, + bool from_field_is_protected, TRAPS) const { if (equals(from) || is_bogus()) { return true; } else { @@ -286,7 +287,9 @@ class VerificationType VALUE_OBJ_CLASS_SPEC { return from.is_integer(); default: if (is_reference() && from.is_reference()) { - return is_reference_assignable_from(from, context, CHECK_false); + return is_reference_assignable_from(from, context, + from_field_is_protected, + CHECK_false); } else { return false; } @@ -308,7 +311,8 @@ class VerificationType VALUE_OBJ_CLASS_SPEC { private: bool is_reference_assignable_from( - const VerificationType&, ClassVerifier*, TRAPS) const; + const VerificationType&, ClassVerifier*, bool from_field_is_protected, + TRAPS) const; }; #endif // SHARE_VM_CLASSFILE_VERIFICATIONTYPE_HPP diff --git a/src/share/vm/classfile/verifier.cpp b/src/share/vm/classfile/verifier.cpp index 9d9420889..c2ee1938e 100644 --- a/src/share/vm/classfile/verifier.cpp +++ b/src/share/vm/classfile/verifier.cpp @@ -1733,7 +1733,7 @@ void ClassVerifier::verify_exception_handler_table(u4 code_length, char* code_da VerificationType throwable = VerificationType::reference_type(vmSymbols::java_lang_Throwable()); bool is_subclass = throwable.is_assignable_from( - catch_type, this, CHECK_VERIFY(this)); + catch_type, this, false, CHECK_VERIFY(this)); if (!is_subclass) { // 4286534: should throw VerifyError according to recent spec change verify_error(ErrorContext::bad_type(handler_pc, @@ -2188,7 +2188,7 @@ void ClassVerifier::verify_field_instructions(RawBytecodeStream* bcs, stack_object_type = current_type(); } is_assignable = target_class_type.is_assignable_from( - stack_object_type, this, CHECK_VERIFY(this)); + stack_object_type, this, false, CHECK_VERIFY(this)); if (!is_assignable) { verify_error(ErrorContext::bad_type(bci, current_frame->stack_top_ctx(), @@ -2215,7 +2215,7 @@ void ClassVerifier::verify_field_instructions(RawBytecodeStream* bcs, // It's protected access, check if stack object is assignable to // current class. is_assignable = current_type().is_assignable_from( - stack_object_type, this, CHECK_VERIFY(this)); + stack_object_type, this, true, CHECK_VERIFY(this)); if (!is_assignable) { verify_error(ErrorContext::bad_type(bci, current_frame->stack_top_ctx(), @@ -2315,7 +2315,7 @@ void ClassVerifier::verify_invoke_init( instanceKlassHandle mh(THREAD, m->method_holder()); if (m->is_protected() && !mh->is_same_class_package(_klass())) { bool assignable = current_type().is_assignable_from( - objectref_type, this, CHECK_VERIFY(this)); + objectref_type, this, true, CHECK_VERIFY(this)); if (!assignable) { verify_error(ErrorContext::bad_type(bci, TypeOrigin::cp(new_class_index, objectref_type), @@ -2488,11 +2488,11 @@ void ClassVerifier::verify_invoke_instructions( bool have_imr_indirect = cp->tag_at(index).value() == JVM_CONSTANT_InterfaceMethodref; if (!current_class()->is_anonymous()) { subtype = ref_class_type.is_assignable_from( - current_type(), this, CHECK_VERIFY(this)); + current_type(), this, false, CHECK_VERIFY(this)); } else { VerificationType host_klass_type = VerificationType::reference_type(current_class()->host_klass()->name()); - subtype = ref_class_type.is_assignable_from(host_klass_type, this, CHECK_VERIFY(this)); + subtype = ref_class_type.is_assignable_from(host_klass_type, this, false, CHECK_VERIFY(this)); // If invokespecial of IMR, need to recheck for same or // direct interface relative to the host class @@ -2536,7 +2536,7 @@ void ClassVerifier::verify_invoke_instructions( VerificationType top = current_frame->pop_stack(CHECK_VERIFY(this)); VerificationType hosttype = VerificationType::reference_type(current_class()->host_klass()->name()); - bool subtype = hosttype.is_assignable_from(top, this, CHECK_VERIFY(this)); + bool subtype = hosttype.is_assignable_from(top, this, false, CHECK_VERIFY(this)); if (!subtype) { verify_error( ErrorContext::bad_type(current_frame->offset(), current_frame->stack_top_ctx(), @@ -2561,7 +2561,7 @@ void ClassVerifier::verify_invoke_instructions( // It's protected access, check if stack object is // assignable to current class. bool is_assignable = current_type().is_assignable_from( - stack_object_type, this, CHECK_VERIFY(this)); + stack_object_type, this, true, CHECK_VERIFY(this)); if (!is_assignable) { if (ref_class_type.name() == vmSymbols::java_lang_Object() && stack_object_type.is_array() @@ -2744,7 +2744,7 @@ void ClassVerifier::verify_return_value( "Method expects a return value"); return; } - bool match = return_type.is_assignable_from(type, this, CHECK_VERIFY(this)); + bool match = return_type.is_assignable_from(type, this, false, CHECK_VERIFY(this)); if (!match) { verify_error(ErrorContext::bad_type(bci, current_frame->stack_top_ctx(), TypeOrigin::signature(return_type)), diff --git a/src/share/vm/runtime/reflection.cpp b/src/share/vm/runtime/reflection.cpp index a8d24fa6d..f0df88c7e 100644 --- a/src/share/vm/runtime/reflection.cpp +++ b/src/share/vm/runtime/reflection.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2014, 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 @@ -482,7 +482,7 @@ static bool under_host_klass(InstanceKlass* ik, Klass* host_klass) { ik = InstanceKlass::cast(hc); // There's no way to make a host class loop short of patching memory. - // Therefore there cannot be a loop here unles there's another bug. + // Therefore there cannot be a loop here unless there's another bug. // Still, let's check for it. assert(--inf_loop_check > 0, "no host_klass loop"); } @@ -551,7 +551,8 @@ bool Reflection::verify_field_access(Klass* current_class, if (access.is_protected()) { if (!protected_restriction) { // See if current_class (or outermost host class) is a subclass of field_class - if (host_class->is_subclass_of(field_class)) { + // An interface may not access protected members of j.l.Object + if (!host_class->is_interface() && host_class->is_subclass_of(field_class)) { if (access.is_static() || // static fields are ok, see 6622385 current_class == resolved_class || field_class == resolved_class || -- GitLab