Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8264142: Remove TRAPS/THREAD parameters for verifier related functions #3194

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter
Filter file types
Jump to
Jump to file
Failed to load files.

Always

Just for now

@@ -1240,8 +1240,7 @@ static void parse_annotations(const ConstantPool* const cp,
const u1* buffer, int limit,
AnnotationCollector* coll,
ClassLoaderData* loader_data,
const bool can_access_vm_annotations,
TRAPS) {
const bool can_access_vm_annotations) {

assert(cp != NULL, "invariant");
assert(buffer != NULL, "invariant");
@@ -1418,8 +1417,7 @@ void ClassFileParser::parse_field_attributes(const ClassFileStream* const cfs,
runtime_visible_annotations_length,
parsed_annotations,
_loader_data,
_can_access_vm_annotations,
CHECK);
_can_access_vm_annotations);
cfs->skip_u1_fast(runtime_visible_annotations_length);
} else if (attribute_name == vmSymbols::tag_runtime_invisible_annotations()) {
if (runtime_invisible_annotations_exists) {
@@ -2734,8 +2732,7 @@ Method* ClassFileParser::parse_method(const ClassFileStream* const cfs,
runtime_visible_annotations_length,
&parsed_annotations,
_loader_data,
_can_access_vm_annotations,
CHECK_NULL);
_can_access_vm_annotations);
cfs->skip_u1_fast(runtime_visible_annotations_length);
} else if (method_attribute_name == vmSymbols::tag_runtime_invisible_annotations()) {
if (runtime_invisible_annotations_exists) {
@@ -3563,7 +3560,7 @@ u2 ClassFileParser::parse_classfile_record_attribute(const ClassFileStream* cons
return calculate_attr_size;
}

void ClassFileParser::parse_classfile_synthetic_attribute(TRAPS) {
void ClassFileParser::parse_classfile_synthetic_attribute() {
set_class_synthetic_flag(true);
}

@@ -3763,7 +3760,7 @@ void ClassFileParser::parse_classfile_attributes(const ClassFileStream* const cf
attribute_length, THREAD);
return;
}
parse_classfile_synthetic_attribute(CHECK);
parse_classfile_synthetic_attribute();
} else if (tag == vmSymbols::tag_deprecated()) {
// Check for Deprecated tag - 4276120
if (attribute_length != 0) {
@@ -3801,8 +3798,7 @@ void ClassFileParser::parse_classfile_attributes(const ClassFileStream* const cf
runtime_visible_annotations_length,
parsed_annotations,
_loader_data,
_can_access_vm_annotations,
CHECK);
_can_access_vm_annotations);
cfs->skip_u1_fast(runtime_visible_annotations_length);
} else if (tag == vmSymbols::tag_runtime_invisible_annotations()) {
if (runtime_invisible_annotations_exists) {
@@ -4072,8 +4068,7 @@ void ClassFileParser::create_combined_annotations(TRAPS) {
// Transfer ownership of metadata allocated to the InstanceKlass.
void ClassFileParser::apply_parsed_class_metadata(
InstanceKlass* this_klass,
int java_fields_count,
TRAPS) {
int java_fields_count) {
assert(this_klass != NULL, "invariant");

_cp->set_pool_holder(this_klass);
@@ -5418,7 +5413,7 @@ void ClassFileParser::fill_instance_klass(InstanceKlass* ik,

// this transfers ownership of a lot of arrays from
// the parser onto the InstanceKlass*
apply_parsed_class_metadata(ik, _java_fields_count, CHECK);
apply_parsed_class_metadata(ik, _java_fields_count);

// can only set dynamic nest-host after static nest information is set
if (cl_inst_info.dynamic_nest_host() != NULL) {
@@ -5637,8 +5632,8 @@ void ClassFileParser::update_class_name(Symbol* new_class_name) {
// For an unsafe anonymous class that is in the unnamed package, move it to its host class's
// package by prepending its host class's package name to its class name and setting
// its _class_name field.
void ClassFileParser::prepend_host_package_name(const InstanceKlass* unsafe_anonymous_host, TRAPS) {
ResourceMark rm(THREAD);
void ClassFileParser::prepend_host_package_name(Thread* current, const InstanceKlass* unsafe_anonymous_host) {
ResourceMark rm(current);
assert(strrchr(_class_name->as_C_string(), JVM_SIGNATURE_SLASH) == NULL,
"Unsafe anonymous class should not be in a package");
TempNewSymbol host_pkg_name =
@@ -5674,7 +5669,7 @@ void ClassFileParser::fix_unsafe_anonymous_class_name(TRAPS) {
const jbyte* anon_last_slash = UTF8::strrchr((const jbyte*)_class_name->base(),
_class_name->utf8_length(), JVM_SIGNATURE_SLASH);
if (anon_last_slash == NULL) { // Unnamed package
prepend_host_package_name(_unsafe_anonymous_host, CHECK);
prepend_host_package_name(THREAD, _unsafe_anonymous_host);
} else {
if (!_unsafe_anonymous_host->is_same_class_package(_unsafe_anonymous_host->class_loader(), _class_name)) {
ResourceMark rm(THREAD);
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2021, 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
@@ -211,7 +211,7 @@ class ClassFileParser {
ConstantPool* cp,
TRAPS);

void prepend_host_package_name(const InstanceKlass* unsafe_anonymous_host, TRAPS);
void prepend_host_package_name(Thread* current, const InstanceKlass* unsafe_anonymous_host);
void fix_unsafe_anonymous_class_name(TRAPS);

void fill_instance_klass(InstanceKlass* ik, bool cf_changed_in_CFLH,
@@ -228,7 +228,7 @@ class ClassFileParser {

void create_combined_annotations(TRAPS);
void apply_parsed_class_attributes(InstanceKlass* k); // update k
void apply_parsed_class_metadata(InstanceKlass* k, int fields_count, TRAPS);
void apply_parsed_class_metadata(InstanceKlass* k, int fields_count);
void clear_class_metadata();

// Constant pool parsing
@@ -349,7 +349,7 @@ class ClassFileParser {
ClassAnnotationCollector* parsed_annotations,
TRAPS);

void parse_classfile_synthetic_attribute(TRAPS);
void parse_classfile_synthetic_attribute();
void parse_classfile_signature_attribute(const ClassFileStream* const cfs, TRAPS);
void parse_classfile_bootstrap_methods_attribute(const ClassFileStream* const cfs,
ConstantPool* cp,
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2021, 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
@@ -75,7 +75,7 @@ void StackMapFrame::initialize_object(
}

VerificationType StackMapFrame::set_locals_from_arg(
const methodHandle& m, VerificationType thisKlass, TRAPS) {
const methodHandle& m, VerificationType thisKlass) {
SignatureStream ss(m->signature());
int init_local_num = 0;
if (!m->is_static()) {
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2021, 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
@@ -153,7 +153,7 @@ class StackMapFrame : public ResourceObj {

// Set local variable type array based on m's signature.
VerificationType set_locals_from_arg(
const methodHandle& m, VerificationType thisKlass, TRAPS);
const methodHandle& m, VerificationType thisKlass);

// Search local variable type array and stack type array.
// Set every element with type of old_object to new_object.
@@ -122,8 +122,8 @@ bool VerificationType::is_reference_assignable_from(
return resolve_and_check_assignability(klass, name(), from.name(),
from_field_is_protected, from.is_array(), from.is_object(), THREAD);
} else if (is_array() && from.is_array()) {
VerificationType comp_this = get_component(context, CHECK_false);
VerificationType comp_from = from.get_component(context, CHECK_false);
VerificationType comp_this = get_component(context);
VerificationType comp_from = from.get_component(context);
if (!comp_this.is_bogus() && !comp_from.is_bogus()) {
return comp_this.is_component_assignable_from(comp_from, context,
from_field_is_protected, THREAD);
@@ -132,7 +132,7 @@ bool VerificationType::is_reference_assignable_from(
return false;
}

VerificationType VerificationType::get_component(ClassVerifier *context, TRAPS) const {
VerificationType VerificationType::get_component(ClassVerifier *context) const {
assert(is_array() && name()->utf8_length() >= 2, "Must be a valid array");
SignatureStream ss(name(), false);
ss.skip_array_prefix(1);
@@ -316,7 +316,7 @@ class VerificationType {
}
}

VerificationType get_component(ClassVerifier* context, TRAPS) const;
VerificationType get_component(ClassVerifier* context) const;

int dimensions() const {
assert(is_array(), "Must be an array");
@@ -134,17 +134,17 @@ void Verifier::trace_class_resolution(Klass* resolve_class, InstanceKlass* verif
}

// Prints the end-verification message to the appropriate output.
void Verifier::log_end_verification(outputStream* st, const char* klassName, Symbol* exception_name, TRAPS) {
if (HAS_PENDING_EXCEPTION) {
void Verifier::log_end_verification(outputStream* st, const char* klassName, Symbol* exception_name, oop pending_exception) {
if (pending_exception != NULL) {
st->print("Verification for %s has", klassName);
oop message = java_lang_Throwable::message(PENDING_EXCEPTION);
oop message = java_lang_Throwable::message(pending_exception);
if (message != NULL) {
char* ex_msg = java_lang_String::as_utf8_string(message);
st->print_cr(" exception pending '%s %s'",
PENDING_EXCEPTION->klass()->external_name(), ex_msg);
pending_exception->klass()->external_name(), ex_msg);
} else {
st->print_cr(" exception pending %s ",
PENDING_EXCEPTION->klass()->external_name());
pending_exception->klass()->external_name());
}
} else if (exception_name != NULL) {
st->print_cr("Verification for %s failed", klassName);
@@ -193,7 +193,8 @@ bool Verifier::verify(InstanceKlass* klass, bool should_verify_class, TRAPS) {

log_info(class, init)("Start class verification for: %s", klass->external_name());
if (klass->major_version() >= STACKMAP_ATTRIBUTE_MAJOR_VERSION) {
ClassVerifier split_verifier(klass, THREAD);
ClassVerifier split_verifier(jt, klass);
// We don't use CHECK here, or on inference_verify below, so that we can log any exception.
split_verifier.verify_class(THREAD);

This comment has been minimized.

@dholmes-ora

dholmes-ora Mar 26, 2021
Member

Can you add the following comment before verify_class please

// We don't use CHECK here, or on inference_verify below, so that we can log any exception.

Thanks

This comment has been minimized.

@hseigel

hseigel Mar 26, 2021
Author Member

done, thanks for suggesting this.

exception_name = split_verifier.result();

@@ -228,12 +229,12 @@ bool Verifier::verify(InstanceKlass* klass, bool should_verify_class, TRAPS) {
LogTarget(Info, class, init) lt1;
if (lt1.is_enabled()) {
LogStream ls(lt1);
log_end_verification(&ls, klass->external_name(), exception_name, THREAD);
log_end_verification(&ls, klass->external_name(), exception_name, PENDING_EXCEPTION);
}
LogTarget(Info, verification) lt2;
if (lt2.is_enabled()) {
LogStream ls(lt2);
log_end_verification(&ls, klass->external_name(), exception_name, THREAD);
log_end_verification(&ls, klass->external_name(), exception_name, PENDING_EXCEPTION);
}

if (HAS_PENDING_EXCEPTION) {
@@ -589,9 +590,8 @@ void ErrorContext::stackmap_details(outputStream* ss, const Method* method) cons

// Methods in ClassVerifier

ClassVerifier::ClassVerifier(
InstanceKlass* klass, TRAPS)
: _thread(THREAD), _previous_symbol(NULL), _symbols(NULL), _exception_type(NULL),
ClassVerifier::ClassVerifier(JavaThread* current, InstanceKlass* klass)
: _thread(current), _previous_symbol(NULL), _symbols(NULL), _exception_type(NULL),
_message(NULL), _method_signatures_table(NULL), _klass(klass) {
_this_type = VerificationType::reference_type(klass->name());
}
@@ -616,6 +616,7 @@ TypeOrigin ClassVerifier::ref_ctx(const char* sig) {
return TypeOrigin::implicit(vt);
}


void ClassVerifier::verify_class(TRAPS) {
log_info(verification)("Verifying class %s with new format", _klass->external_name());

@@ -653,8 +654,7 @@ void ClassVerifier::verify_class(TRAPS) {
// Translate the signature entries into verification types and save them in
// the growable array. Also, save the count of arguments.
void ClassVerifier::translate_signature(Symbol* const method_sig,
sig_as_verification_types* sig_verif_types,
TRAPS) {
sig_as_verification_types* sig_verif_types) {
SignatureStream sig_stream(method_sig);
VerificationType sig_type[2];
int sig_i = 0;
@@ -688,11 +688,11 @@ void ClassVerifier::translate_signature(Symbol* const method_sig,
}

void ClassVerifier::create_method_sig_entry(sig_as_verification_types* sig_verif_types,
int sig_index, TRAPS) {
int sig_index) {
// Translate the signature into verification types.
ConstantPool* cp = _klass->constants();
Symbol* const method_sig = cp->symbol_at(sig_index);
translate_signature(method_sig, sig_verif_types, CHECK_VERIFY(this));

This comment has been minimized.

@dholmes-ora

dholmes-ora Mar 26, 2021
Member

CHECK_VERIFY is not directly related to exceptions. Are there really no verification errors possible from all of these calls where CHECK_VERIFY has been removed?

This comment has been minimized.

@hseigel

hseigel Mar 26, 2021
Author Member

The calls where CHECK_VERIFY's were removed are to functions that deal with signatures. These methods do not check for errors because the signatures have already been verified by classFileParser.cpp in functions such as verify_legal_method_signature() and verify_legal_field_signature().

This comment has been minimized.

@dholmes-ora

dholmes-ora Mar 29, 2021
Member

Okay - thanks for confirming that.

translate_signature(method_sig, sig_verif_types);

// Add the list of this signature's verification types to the table.
bool is_unique = method_signatures_table()->put(sig_index, sig_verif_types);
@@ -718,8 +718,7 @@ void ClassVerifier::verify_method(const methodHandle& m, TRAPS) {
// Initial stack map frame: offset is 0, stack is initially empty.
StackMapFrame current_frame(max_locals, max_stack, this);
// Set initial locals
VerificationType return_type = current_frame.set_locals_from_arg(
m, current_type(), CHECK_VERIFY(this));
VerificationType return_type = current_frame.set_locals_from_arg( m, current_type());

int32_t stackmap_index = 0; // index to the stackmap array

@@ -1049,8 +1048,7 @@ void ClassVerifier::verify_method(const methodHandle& m, TRAPS) {
current_frame.push_stack(
VerificationType::null_type(), CHECK_VERIFY(this));
} else {
VerificationType component =
atype.get_component(this, CHECK_VERIFY(this));
VerificationType component = atype.get_component(this);
current_frame.push_stack(component, CHECK_VERIFY(this));
}
no_control_flow = false; break;
@@ -2827,7 +2825,7 @@ void ClassVerifier::verify_invoke_instructions(
// Not found, add the entry to the table.
GrowableArray<VerificationType>* verif_types = new GrowableArray<VerificationType>(10);
mth_sig_verif_types = new sig_as_verification_types(verif_types);
create_method_sig_entry(mth_sig_verif_types, sig_index, CHECK_VERIFY(this));
create_method_sig_entry(mth_sig_verif_types, sig_index);
}

// Get the number of arguments for this signature.
@@ -46,7 +46,8 @@ class Verifier : AllStatic {
// Verify the bytecodes for a class.
static bool verify(InstanceKlass* klass, bool should_verify_class, TRAPS);

static void log_end_verification(outputStream* st, const char* klassName, Symbol* exception_name, TRAPS);
static void log_end_verification(outputStream* st, const char* klassName, Symbol* exception_name,
oop pending_exception);

// Return false if the class is loaded by the bootstrap loader,
// or if defineClass was called requesting skipping verification
@@ -398,7 +399,7 @@ class ClassVerifier : public StackObj {
};

// constructor
ClassVerifier(InstanceKlass* klass, TRAPS);
ClassVerifier(JavaThread* current, InstanceKlass* klass);

// destructor
~ClassVerifier();
@@ -415,10 +416,10 @@ class ClassVerifier : public StackObj {

// Translates method signature entries into verificationTypes and saves them
// in the growable array.
void translate_signature(Symbol* const method_sig, sig_as_verification_types* sig_verif_types, TRAPS);
void translate_signature(Symbol* const method_sig, sig_as_verification_types* sig_verif_types);

// Initializes a sig_as_verification_types entry and puts it in the hash table.
void create_method_sig_entry(sig_as_verification_types* sig_verif_types, int sig_index, TRAPS);
void create_method_sig_entry(sig_as_verification_types* sig_verif_types, int sig_index);

// Return status modes
Symbol* result() const { return _exception_type; }
ProTip! Use n and p to navigate between commits in a pull request.