Thank you, Dean, JC, Chris, and Serguei, for reviewing this change!
Best regards,
Daniil
From: Chris Plummer <[email protected]>
Date: Tuesday, April 30, 2019 at 11:39 AM
To: Daniil Titov <[email protected]>
Subject: Re: FW: 8222821: com/sun/jdi/ExceptionEvents.java failed
Looks good.
Chris
From: serviceability-dev <[email protected]> on
behalf of Jean Christophe Beyler <[email protected]>
Date: Thursday, April 25, 2019 at 7:38 PM
To: <[email protected]>
Cc: OpenJDK Serviceability <[email protected]>
Subject: Re: RFR: 8222821: com/sun/jdi/ExceptionEvents.java failed
Hi Daniil,
Looks good to me too, (thanks for the detailed explanation about the test
failure btw :-)),
Jc
On Thu, Apr 25, 2019 at 6:43 PM <[email protected]> wrote:
Looks good.
dl
On 4/25/19 6:33 PM, Daniil Titov wrote:
> Please review the change that fixes an intermittent failure of the test when
> running with Graal on.
>
> The test creates exception requests for different kinds of exceptions and
> errors, starts the debuggee that throws an exception, and listens for
> exception events. If the number of received exception events is not equal to
> 1 the test fails. For the case when the exception request is created for
> java.lang.Error class the test intermittently fails if Graal is on. It
> happens because, sometimes, in addition to StackOverflowError thrown by the
> test itself, jdk.vm.ci.common.JVMCIError is thrown from method getField() in
> class jdk.vm.ci.hotspot .HotSpotVMConfigAccess at line 252
> (src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotVMConfigAccess.java)
>
> 240 private VMField getField(String name, String cppType, boolean
> required) {
> 241 VMField entry = store.vmFields.get(name);
> 242 if (entry == null) {
> 243 if (!required) {
> 244 return null;
> 245 }
> 246 store.printConfig();
> 247 throw new JVMCIError("expected VM field not found
> in " + store + ": " + name);
> 248 }
> 249
> 250 // Make sure the native type is still the type we
> expect.
> 251 if (cppType != null && !cppType.equals(entry.type)) {
> 252 throw new JVMCIError("expected type " + cppType + "
> but VM field " + name + " is of type " + entry.type);
> 253 }
> 254 return entry;
> 255 }
>
> that in one case is caught at line 412 in
> src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java
>
> 404 public final int pendingFailedSpeculationOffset;
> 405 {
> 406 String name = "JavaThread::_pending_failed_speculation";
> 407 int offset = -1;
> 408 try {
> 409 offset = getFieldOffset(name, Integer.class,
> "jlong");
> 410 } catch (JVMCIError e) {
> 411 try {
> 412 offset = getFieldOffset(name, Integer.class,
> "long");
> 413 } catch (JVMCIError e2) {
> 414 }
> 415 }
> 416 if (offset == -1) {
> 417 throw new JVMCIError("cannot get offset of field "
> + name + " with type long or jlong");
> 418 }
> 419 pendingFailedSpeculationOffset = offset;
> 420 }
>
> and in other case at line 229 in the same class
> (src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java)
>
> 221 public final int classMirrorOffset;
> 222 {
> 223 String name = "Klass::_java_mirror";
> 224 int offset = -1;
> 225 boolean isHandle = false;
> 226 try {
> 227 offset = getFieldOffset(name, Integer.class, "oop");
> 228 } catch (JVMCIError e) {
> 229
> 230 }
> 231 if (offset == -1) {
> 232 try {
> 233 offset = getFieldOffset(name, Integer.class,
> "jobject");
> 234 isHandle = true;
> 235 } catch (JVMCIError e) {
> 236 try {
> 237 // JDK-8186777
> 238 offset = getFieldOffset(name,
> Integer.class, "OopHandle");
> 239 isHandle = true;
> 240 } catch (JVMCIError e2) {
> 241 }
> 242 }
> 243 }
> 244 if (offset == -1) {
> 245 throw new JVMCIError("cannot get offset of field "
> + name + " with type oop, jobject or OopHandle");
> 246 }
> 247 classMirrorOffset = offset;
> 248 classMirrorIsHandle = isHandle;
> 249 }
>
> That results in the number of received exception events exceeds 1 and the
> test fails.
>
> To ignore these unexpected events the fix adds "jdk.vm.ci.hotspot.*" class
> exclusion filter when it creates an exception request.
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8222821/webrev.01/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8222821
>
> Thanks!
> -Daniil
>
>
>
--
Thanks,
Jc
--- Begin Message ---
Hi Daniil,
Looks good to me.
Thanks,
Serguei
On 4/25/19 7:37 PM, Jean Christophe Beyler wrote:
Hi Daniil,
Looks good to me too, (thanks for the detailed explanation about the test
failure btw :-)),
Jc
On Thu, Apr 25, 2019 at 6:43 PM <[email protected]> wrote:
Looks good.
dl
On 4/25/19 6:33 PM, Daniil Titov wrote:
> Please review the change that fixes an intermittent failure of the test when
> running with Graal on.
>
> The test creates exception requests for different kinds of exceptions and
> errors, starts the debuggee that throws an exception, and listens for
> exception events. If the number of received exception events is not equal to
> 1 the test fails. For the case when the exception request is created for
> java.lang.Error class the test intermittently fails if Graal is on. It
> happens because, sometimes, in addition to StackOverflowError thrown by the
> test itself, jdk.vm.ci.common.JVMCIError is thrown from method getField() in
> class jdk.vm.ci.hotspot .HotSpotVMConfigAccess at line 252
> (src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotVMConfigAccess.java)
>
> 240 private VMField getField(String name, String cppType, boolean
> required) {
> 241 VMField entry = store.vmFields.get(name);
> 242 if (entry == null) {
> 243 if (!required) {
> 244 return null;
> 245 }
> 246 store.printConfig();
> 247 throw new JVMCIError("expected VM field not found
>in " + store + ": " + name);
> 248 }
> 249
> 250 // Make sure the native type is still the type we
>expect.
> 251 if (cppType != null && !cppType.equals(entry.type)) {
> 252 throw new JVMCIError("expected type " + cppType +
>" but VM field " + name + " is of type " + entry.type);
> 253 }
> 254 return entry;
> 255 }
>
> that in one case is caught at line 412 in
> src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java
>
> 404 public final int pendingFailedSpeculationOffset;
> 405 {
> 406 String name =
>"JavaThread::_pending_failed_speculation";
> 407 int offset = -1;
> 408 try {
> 409 offset = getFieldOffset(name, Integer.class,
>"jlong");
> 410 } catch (JVMCIError e) {
> 411 try {
> 412 offset = getFieldOffset(name, Integer.class,
>"long");
> 413 } catch (JVMCIError e2) {
> 414 }
> 415 }
> 416 if (offset == -1) {
> 417 throw new JVMCIError("cannot get offset of field "
>+ name + " with type long or jlong");
> 418 }
> 419 pendingFailedSpeculationOffset = offset;
> 420 }
>
> and in other case at line 229 in the same class
> (src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.hotspot/src/org/graalvm/compiler/hotspot/GraalHotSpotVMConfig.java)
>
> 221 public final int classMirrorOffset;
> 222 {
> 223 String name = "Klass::_java_mirror";
> 224 int offset = -1;
> 225 boolean isHandle = false;
> 226 try {
> 227 offset = getFieldOffset(name, Integer.class,
>"oop");
> 228 } catch (JVMCIError e) {
> 229
> 230 }
> 231 if (offset == -1) {
> 232 try {
> 233 offset = getFieldOffset(name, Integer.class,
>"jobject");
> 234 isHandle = true;
> 235 } catch (JVMCIError e) {
> 236 try {
> 237 // JDK-8186777
> 238 offset = getFieldOffset(name,
>Integer.class, "OopHandle");
> 239 isHandle = true;
> 240 } catch (JVMCIError e2) {
> 241 }
> 242 }
> 243 }
> 244 if (offset == -1) {
> 245 throw new JVMCIError("cannot get offset of field "
>+ name + " with type oop, jobject or OopHandle");
> 246 }
> 247 classMirrorOffset = offset;
> 248 classMirrorIsHandle = isHandle;
> 249 }
>
> That results in the number of received exception events exceeds 1 and the
> test fails.
>
> To ignore these unexpected events the fix adds "jdk.vm.ci.hotspot.*" class
> exclusion filter when it creates an exception request.
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8222821/webrev.01/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8222821
>
> Thanks!
> -Daniil
>
>
>
--
Thanks,
Jc
--- End Message ---
