RFR: 8264142: Remove TRAPS/THREAD parameters for verifier related functions [v3]

David Holmes dholmes at openjdk.java.net
Fri Mar 26 05:22:26 UTC 2021


On Thu, 25 Mar 2021 21:05:45 GMT, Harold Seigel <hseigel at openjdk.org> wrote:

>> Please review this small change to remove unneeded TRAPS and THREAD parameters from functions in verifier related files.  This change was tested with Mach5 tiers 1 and 2 on Linux, Mac OS, and Windows, and tiers 3-5 on Linux x64.
>> 
>> Thanks, Harold
>
> Harold Seigel has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review comment changes

Hi Harold,

Some good cleanup here, but I have a couple of things that need adjusting - see comments inline.

Thanks,
David

src/hotspot/share/classfile/verifier.cpp line 138:

> 136: // Prints the end-verification message to the appropriate output.
> 137: void Verifier::log_end_verification(outputStream* st, const char* klassName, Symbol* exception_name, oop pending_exception) {
> 138:    Thread* THREAD = Thread::current();

This is not needed.

src/hotspot/share/classfile/verifier.cpp line 142:

> 140:   if (pending_exception != NULL) {
> 141:     st->print("Verification for %s has", klassName);
> 142:     oop message = java_lang_Throwable::message(PENDING_EXCEPTION);

This should be pending_exception

src/hotspot/share/classfile/verifier.cpp line 198:

> 196:   log_info(class, init)("Start class verification for: %s", klass->external_name());
> 197:   if (klass->major_version() >= STACKMAP_ATTRIBUTE_MAJOR_VERSION) {
> 198:     ClassVerifier split_verifier(THREAD->as_Java_thread(), klass);

You already have the "jt" JavaThread for this.

src/hotspot/share/classfile/verifier.cpp line 199:

> 197:   if (klass->major_version() >= STACKMAP_ATTRIBUTE_MAJOR_VERSION) {
> 198:     ClassVerifier split_verifier(THREAD->as_Java_thread(), klass);
> 199:     split_verifier.verify_class(THREAD);

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

src/hotspot/share/classfile/verifier.cpp line 695:

> 693:   ConstantPool* cp = _klass->constants();
> 694:   Symbol* const method_sig = cp->symbol_at(sig_index);
> 695:   translate_signature(method_sig, sig_verif_types, CHECK_VERIFY(this));

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?

-------------

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3194


More information about the hotspot-runtime-dev mailing list