RFR 8202913: loader constraint message for fields specifies incorrect referring class
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Wed May 30 15:40:46 UTC 2018
Hi Harold,
> ... AppClassLoader) for the class|abstractclass|interface test.Parent defining
I meant you should print the proper identifier, not the text with '|' by using
"... %s ..." , ..., k->external_kind(), ...
As this replaces 'type' by one or two (abstract class) words I don't think
it makes the message more wordy, just more precise. See also patch below
(just edited, not tested). Feel free to ignore my proposal, and else I don't
need a new webrev in case you adapt this.
The rest looks good. Great it's all with '.' and you moved the test!
Best regards,
Goetz.
diff -r 6b4a85ad6bea src/hotspot/share/interpreter/linkResolver.cpp
--- a/src/hotspot/share/interpreter/linkResolver.cpp Wed May 30 17:32:28 2018 +0200
+++ b/src/hotspot/share/interpreter/linkResolver.cpp Wed May 30 17:37:04 2018 +0200
@@ -690,19 +690,20 @@
const char* msg = "loader constraint violation: when resolving field"
" \"%s\" of type %s, the class loader %s of the current class, "
"%s, and the class loader %s for the field's defining "
- "type, %s, have different Class objects for type %s";
+ "%s, %s, have different Class objects for type %s";
const char* field_name = field->as_C_string();
const char* loader1_name = java_lang_ClassLoader::describe_external(ref_loader());
const char* sel = sel_klass->external_name();
+ const char* sel_type = sel_klass->external_kind();
const char* loader2_name = java_lang_ClassLoader::describe_external(sel_loader());
const char* failed_type_name = failed_type_symbol->as_klass_external_name();
const char* curr_klass_name = current_klass->external_name();
size_t buflen = strlen(msg) + strlen(field_name) + 2 * strlen(failed_type_name) +
strlen(loader1_name) + strlen(curr_klass_name) +
- strlen(loader2_name) + strlen(sel) + 1;
+ strlen(loader2_name) + strlen(sel) + strlen(sel_type) + 1;
char* buf = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, char, buflen);
jio_snprintf(buf, buflen, msg, field_name, failed_type_name, loader1_name,
- curr_klass_name, loader2_name, sel, failed_type_name);
+ curr_klass_name, loader2_name, sel_type, sel, failed_type_name);
THROW_MSG(vmSymbols::java_lang_LinkageError(), buf);
}
}
> -----Original Message-----
> From: Harold David Seigel [mailto:harold.seigel at oracle.com]
> Sent: Mittwoch, 30. Mai 2018 15:07
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
> dev at openjdk.java.net
> Subject: Re: RFR 8202913: loader constraint message for fields specifies
> incorrect referring class
>
> Hi Goetz,
>
> Thanks for reviewing this change!
>
>
> Please review this updated webrev for this bug:
>
> http://cr.openjdk.java.net/~hseigel/bug_8202913.2/webrev/index.h
> tml
>
> The new exception message for the included test looks like this:
>
> loader constraint violation: when resolving field "_field1" of type
> pkg.Foo, the class loader "<unnamed>" (instance of
> pkg.ClassLoaderForChildGrandFoo, child of "app"
> jdk.internal.loader.ClassLoaders$AppClassLoader) of the current class,
> pkg.Child, and the class loader "<unnamed>" (instance of
> pkg.ClassLoaderForParentFoo, child of "app"
> jdk.internal.loader.ClassLoaders$AppClassLoader) for the field's defining
> type, pkg.Parent, have different Class objects for type pkg.Foo
>
>
> Please also see comments in-line.
>
>
>
> On 5/25/2018 10:23 AM, Lindenmaier, Goetz wrote:
>
>
> Hi,
>
>
> I think this would read better if you replace 'type' by what is
> returned by
> Klass::external_kind().
>
> correcting myself, you can just say 'class' ... interfaces don't define
> fields ...
>
> Karen pointed out that interfaces can have public static fields. So, I'll keep it
> as 'type', not 'class'.
>
>
>
>
> Best regards,
> Goetz.
>
>
>
> -----Original Message-----
> From: Lindenmaier, Goetz
> Sent: Freitag, 25. Mai 2018 16:18
> To: 'Harold David Seigel' <harold.seigel at oracle.com>
> <mailto:harold.seigel at oracle.com> ; hotspot-runtime-
> dev at openjdk.java.net <mailto:dev at openjdk.java.net>
> Subject: RE: RFR 8202913: loader constraint message for fields
> specifies
> incorrect referring class
>
> Hi Harold,
>
> Thanks for adding this further improvement over my change.
>
>
> ... AppClassLoader) for the field's
> defining type, Parent, have different Class objects for
> type Foo
>
> I think this would read better if you replace 'type' by what is
> returned by
> Klass::external_kind().
>
> Actually I would prefer to read
> ... AppClassLoader) for the class|abstractclass|interface
> test.Parent defining
> this field,
> have different Class objects for type Foo
>
> The message is already too wordy. I'd prefer to not add this additional text
> because it is not related to the reason for the exception.
>
>
>
>
> Could you please add the test files into some package so that
> you
> can assure class names are printed as test.Parent and not
> test/Parent?
> There is external_name() to get the proper names from
> classes, I don't
> know for symbols.
> Related tests are in runtime/LoaderConstraint. Please move
> your test
> there.
>
> Done.
>
> Thanks, Harold
>
>
>
>
> Best regards,
> Goetz.
>
>
>
>
>
> -----Original Message-----
> From: hotspot-runtime-dev [mailto:hotspot-runtime-
> dev-
> bounces at openjdk.java.net
> <mailto:bounces at openjdk.java.net> ] On Behalf Of Harold David Seigel
> Sent: Freitag, 25. Mai 2018 15:29
> To: hotspot-runtime-dev at openjdk.java.net
> <mailto:hotspot-runtime-dev at openjdk.java.net>
> Subject: RFR 8202913: loader constraint message for
> fields specifies
>
> incorrect
>
> referring class
>
> Hi,
>
> Please review this change to correct and simplify the
> error message
> displayed when a loader constraint check fails when
> trying to access a
> field.
>
> The old message (for this test case):
>
> loader constraint violation: when resolving field
> "_field1" the
> class loader "<unnamed>" (instance of
> ClassLoaderForChildGrandFoo,
> child of "app"
> jdk.internal.loader.ClassLoaders$AppClassLoader) of
> the referring class, Parent, and the class loader
> "<unnamed>"
> (instance of ClassLoaderForParentFoo, child of
> "app"
> jdk.internal.loader.ClassLoaders$AppClassLoader)
> for the field's
> resolved type, Foo, have different Class objects for
> that type
>
> The new message:
>
> loader constraint violation: when resolving field
> "_field1" of type
> Foo, the class loader "<unnamed>" (instance of
> ClassLoaderForChildGrandFoo, child of "app"
> jdk.internal.loader.ClassLoaders$AppClassLoader)
> of the current
> class, Child, and the class loader "<unnamed>"
> (instance of
> ClassLoaderForParentFoo, child of "app"
> jdk.internal.loader.ClassLoaders$AppClassLoader)
> for the field's
> defining type, Parent, have different Class objects
> for type Foo
>
> Open Webrev:
> http://cr.openjdk.java.net/~hseigel/bug_8202913/webrev/
>
> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-
> 8202913
>
> This fix was tested with Mach5 tiers 1 and 2 tests and
> builds on
> Linux-X64, Windows, Solaris Sparc, and Mac OS X, with
> tiers 3-5 tests on
> Linux-x64, and with JCK-11 Lang and VM tests.
>
> Thanks, Harold
>
>
>
More information about the hotspot-runtime-dev
mailing list