RFR 8214572: nsk/jvmti/unit/ForceEarlyReturn/earlyretbase should not suspend the thread when the top frame executes JVMCI code

[JC Beyler]

Hi Daniil,

The webrev looks good but I have a few comments and questions (if you don't
mind :-)):

Comments:
  - You say that normally the test will be removed from the problem list
once the two fixes are done but in this webrev, you've already removed it
(I can't see the other case so I can't see if it is resolved :-))
  - now that we are in C++ for the tests, could we not declare the
variables at their first use instead of doing the pedantic top of the block
C style?
  - I feel that this sort of "wait until we are not in JVMCI frames" might
happen a lot, maybe we could move that code into a helper method (+ it
cleans the method a bit to just concentrate on the rest) and then if needed
we can make it public to other tests?

Questions because I'm not familiar with JVMCI consequences so not really
comments on the webrev but so that I know:
  - Is it normaly that you can suspend when you are in a JVMCI frame?
will/is there not a better way that we could detect that we are in a JVMCI
frame? Is it always safe to suspend a JVMCI frame?

Thanks!
Jc

On Fri, Nov 30, 2018 at 7:11 PM Daniil Titov <daniil.x.titov at oracle.com>
wrote:

> Please review the change for nsk/jvmti/unit/ForceEarlyReturn/earlyretbase
> test. The problem here is that before doing an early force return the test
> doesn't check that the test thread is suspended at a right place where the
> top frame executes the test method rather than JVMCI code triggered by
> invocation counter overflow. That results in the early return happens for a
> wrong method and the test fails.
>
> The fix changes the test to do resume/suspend in the loop until the target
> method is executed in the top frame or the loop counter exceeds the limit.
>
> There is another problem with this test that requires changes on VM side
> and is currently covered by JDK-8195635:" [Graal]
> nsk/jvmti/unit/ForceEarlyReturn/earlyretbase crashes with assertion
> "compilation level out of bounds"".  The test will have to be removed from
> test/hotspot/jtreg/ProblemList-graal.txt only after both these issues are
> fixed.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8214572
> Webrev: http://cr.openjdk.java.net/~dtitov/8214572/webrev.01/
>
> Thanks,
> Daniil
>
>
>
>
>
>
>

-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181130/3d33b134/attachment-0001.html>