<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi JC,<br>
<br>
So assuming the change to move the assignment outside of the
conditional is already in place, you are changing:<br>
<br>
debugeeClass =
jni->FindClass(DEBUGEE_CLASS_NAME);<br>
<br>
to<br>
<br>
ExceptionCheckingJniEnvPtr jni_wrapper(jni);<br>
...<br>
debugeeClass =
jni_wrapper->FindClass(DEBUGEE_CLASS_NAME, TRACE_JNI_CALL);<br>
<br>
But none of your ExceptionCheckingJni changes are pushed yet,
right?<br>
<br>
thanks,<br>
<br>
Chris<br>
<br>
On 11/2/18 10:44 AM, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF9BGBzEizNAo558SX+2TMVWoiBCF0M_5zhv=***@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=utf-8">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">Hi Chris,
<div><br>
</div>
<div>Here is what it would "look like",
there is a bit more clean up to make it
true for all methods, handling the
"verbose" flag, etc but it should help
see what I'm talking about:</div>
<div><a
href="http://cr.openjdk.java.net/%7Ejcbeyler/exception/"
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/exception/</a><br>
</div>
<div><br>
</div>
<div>Basically:</div>
<div> - The test now no longer needs the
NSK_JNI_VERIFY but requires a
TRACE_JNI_CALL as last argument to the
JNI calls if you want </div>
<div> <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/exception/test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetTag/settag001/settag001.cpp.udiff.html"
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/exception/test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetTag/settag001/settag001.cpp.udiff.html</a></div>
<div> Note: I left the
nsk_setVerboseMode call though this
version does not care about that flag
but I would do it to be conforming of
course</div>
<div><br>
</div>
<div> - The wrapper class would have a
new macro TRACE_JNI_CALL and the methods
would have new parameters for the line
and file that are defaulted to -1 and 0
so that we can know if they are used or
not:</div>
<div> <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/exception/test/hotspot/jtreg/vmTestbase/nsk/share/jni/ExceptionCheckingJniEnv.hpp.udiff.html"
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/exception/test/hotspot/jtreg/vmTestbase/nsk/share/jni/ExceptionCheckingJniEnv.hpp.udiff.html</a></div>
<div><br>
</div>
<div> - Finally, the real change comes
from here:</div>
<div> <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/exception/test/hotspot/jtreg/vmTestbase/nsk/share/jni/ExceptionCheckingJniEnv.cpp.udiff.html"
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/exception/test/hotspot/jtreg/vmTestbase/nsk/share/jni/ExceptionCheckingJniEnv.cpp.udiff.html</a></div>
<div><br>
</div>
<div> Where we do this:</div>
<div> a) add a new constructor for
the JNIVerifier class for having the
parameter of the JNI call be passed in
for printing the value of, this will
sadly have to be duplicated by the
number of different argument numbers
since variadic templates is C++11
onwards but that is not a huge problem
since it is contained to this file</div>
<div> b) at
JNIVerifier construction, we print it
out the "before" call and this should be
protected by the verbose flag like the
nsk_ltrace/nsk_verify methods do it for
compatibility</div>
<div> c) at JNIVerifier
construction, we now can call the print
parameter methods to pass the values of
the call to be printed out</div>
<div> - again sadly without c++11
we will have to have a few of these here
but that is limited to this part of the
code so should be fine</div>
<div> d) the JNI wrapper methods
get augmented by the line/file
parameters which default to -1 and 0</div>
<div> e) the constructor is now
also passing in the JNI method
parameters</div>
<div><br>
</div>
<div>It's mostly boiler plate code but
allows us to go from:</div>
<div>
<div>- if
(!NSK_JNI_VERIFY(jni, (debugeeClass =</div>
<div>-
jni->FindClass(DEBUGEE_CLASS_NAME))
!= NULL)) {</div>
<div>+ debugeeClass =
jni_wrapper->FindClass(DEBUGEE_CLASS_NAME,
TRACE_JNI_CALL);</div>
<div>+ if (debugeeClass !=
NULL) {</div> </div> <div><br> </div> <div>And print out:</div> <div> <div>>> Calling JNI method
FindClass from
/usr/local/google/home/jcbeyler/mercurial/jdk12-verify/test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetTag/settag001/settag001.cpp:69</div> <div>>> Calling with these
parameter(s):</div>
<div> nsk/jvmti/SetTag/settag001</div>
<div><< Called JNI method
FindClass from
/usr/local/google/home/jcbeyler/mercurial/jdk12-verify/test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetTag/settag001/settag001.cpp:69</div>
<div>FATAL ERROR in native method: JNI
method FindClass : Return is NULL from
/usr/local/google/home/jcbeyler/mercurial/jdk12-verify/test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetTag/settag001/settag001.cpp:69</div> </div> <div><br> </div> <div>With the >> and << lines
being the "equivalent" to the trace
methods and the "FATAL ERROR being the
equivalent to the NSK_COMPLAIN done in
the verify method.</div>
<div><br>
</div>
<div>Let me know what you think,</div>
<div>Jc</div>
<div><br>
</div>
<div> d) at JNIVerifier
destruction, we can print out the
"called the method"</div>
<div><br>
</div>
<div><br>
</div>
<div> </div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Thu, Nov 1, 2018 at 1:24 PM Chris Plummer <<a
href="mailto:***@oracle.com"
moz-do-not-send="true">***@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div class="m_1232422551998170429moz-cite-prefix">Hi JC,<br>
<br>
I would need to see a webrev with the ExceptionCheckingJni
support, new macros, and a few example uses. You don't
need to fix everything. I just want to get a better feel
for the changes and what the implementation would look
like.<br>
<br>
thanks,<br>
<br>
Chris<br>
<br>
On 11/1/18 1:03 PM, JC Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">Hi Chris,
<div><br>
</div>
<div>Thanks for going over my email :)</div>
<div><br>
</div>
<div>So I skipped over the tracing part for
clarity and striving to be brief. But if we
can accept:</div>
<div>
<div> debugeeClass =
jni->FindClass(DEBUGEE_CLASS_NAME,
TRACE_JNI_CALL);</div>
</div>
<div><br>
</div>
<div>and use a mechanism such as </div>
<div> debugeeClass =
jni->FindClass(DEBUGEE_CLASS_NAME,
TRACE_JNI_CALL);</div>
<div><br>
</div>
<div>it is actually easy to print out things
like:</div> <div>>> JNI FindClass with the following
parameters from
test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetTag/settag001/settag001.cpp:69:</div> <div>>> <span style="white-space:pre-wrap"> </span>"nsk/jvmti/SetTag/settag001"<br>
</div>
<div>...</div>
<div>
<div><< JNI FindClass from
test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetTag/settag001/settag001.cpp:69:</div>
<div><br>
</div>
</div>
<div>Before the actual call to the method,
allowing to have the full NSK_*_VERIFY
system. The hardest question is do we accept
to go from:</div>
<div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr"><span
class="m_1232422551998170429gmail-im"
style="color:rgb(80,0,80)">
<div> if
(!NSK_JNI_VERIFY(jni,
(debugeeClass =<br>
</div>
<div>
<div>
jni->FindClass(DEBUGEE_CLASS_NAME))
!= NULL)) {</div>
<div>
nsk_jvmti_setFailStatus();</div>
<div>
return;</div>
<div> }</div>
</div>
</span></div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<div><br>
</div>
<div>to:</div>
<div>
<div><br>
</div>
<div>#define TRACE_JNI_CALL __LINE__,
__FILE__</div>
<div>...</div>
<div><br>
</div>
<div> ExceptionCheckingJniEnvPtr
jni_wrapper(jni);</div>
<div>...</div>
<div> debugeeClass =
jni->FindClass(DEBUGEE_CLASS_NAME,
TRACE_JNI_CALL);</div>
</div>
<div><br>
</div>
<div>What I would do if this is accepted is
postpone the assignment extraction and move
the code to migrating the NSK*VERIFY macros
to using the exception wrapper and then
moving the assignments will be easier and a
nop from a debug printout point of view.</div>
<div><br>
</div>
<div>Thanks,</div>
<div>Jc</div>
<div><br>
</div>
<div><br>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Thu, Nov 1, 2018 at 12:01 PM Chris
Plummer <<a href="mailto:***@oracle.com"
target="_blank" moz-do-not-send="true">***@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div
class="m_1232422551998170429m_5548951279341903837moz-cite-prefix">On
11/1/18 9:53 AM, JC Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>Sorry this is a bit of a
long reply to myself but
I've tried to see what we
have, what we could have, is
it better. Here is my
analysis of how to keep the
information we currently
provide when a test fails at
a NSK*VERIFY point and how
we could maintain the level
of detail (and even expand)
while still cleaning up the
macros/assignments.</div>
<div><br>
</div>
<div>I've been looking at this
and am going to go through
examples to provide
alternatives :). For this,
I'll use the SetTag test and
will be forcing a failure on
the line:</div>
<div><br>
</div>
<div> if
(!NSK_JNI_VERIFY(jni,
(debugeeClass =<br>
</div>
<div>
<div>
jni->FindClass(DEBUGEE_CLASS_NAME))
!= NULL)) {</div>
<div>
nsk_jvmti_setFailStatus();</div>
<div> return;</div>
<div> }</div>
</div>
<div><br>
</div>
<div>Without a change and with
the verbose system set up
for the test, the error
message is:</div>
<div><br>
</div>
<div>
<div>The following fake
exception stacktrace is
for failuire analysis. </div>
<div>nsk.share.Fake_Exception_for_RULE_Creation:
(settag001.cpp:65)
(debugeeClass =
jni->FindClass(DEBUGEE_CLASS_NAME)) != NULL</div>
<div><span style="white-space:pre-wrap"> </span>at
nsk_lvcomplain(nsk_tools.cpp:172)</div>
<div># ERROR: settag001.cpp,
65: (debugeeClass =
jni->FindClass(DEBUGEE_CLASS_NAME))
!= NULL</div>
<div># verified JNI
assertion is FALSE</div>
<div># ERROR:
agent_tools.cpp, 282:
Debuggee status sync
aborted because agent
thread has finished</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
I think you are missing something here. This appears
to be the output from nsk_jni_lverify() when the
condition indicates failure. However, I don't see
the verbose tracing from nsk_ltrace(), which I think
is more relevant because that's were you are likely
to want to see the actual JNI or JVMTI call being
made.<br>
<br>
#define NSK_JNI_VERIFY(jni, action) \<br>
(nsk_ltrace(NSK_TRACE_BEFORE,__FILE__,__LINE__,"%s\n",#action), \<br>
nsk_jni_lverify(NSK_TRUE,jni,action,__FILE__,__LINE__,"%s\n",#action))<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div>
<div><br>
</div>
</div>
<div>(Note the typo for
failuire, I
created JDK-8213246 to fix
it).</div>
<div><br>
</div>
<div>With my proposed change
to remove the assignment,
the code becomes:</div>
<div> debugeeClass
=
jni->FindClass(DEBUGEE_CLASS_NAME);</div>
<div>
<div> if
(!NSK_JNI_VERIFY(jni,
debuggeeClass != NULL)) {</div>
<div>
<div>
nsk_jvmti_setFailStatus();</div>
<div>
return;</div>
<div> }</div>
</div>
</div>
<div><br>
</div>
<div>
<div>The following fake
exception stacktrace is
for failuire analysis. </div>
<div>nsk.share.Fake_Exception_for_RULE_Creation:
(settag001.cpp:66)
debugeeClass != NULL</div>
<div><span style="white-space:pre-wrap"> </span>at
nsk_lvcomplain(nsk_tools.cpp:172)</div>
<div># ERROR: settag001.cpp,
66: debugeeClass != NULL</div>
<div># verified JNI
assertion is FALSE</div>
<div># ERROR:
agent_tools.cpp, 282:
Debuggee status sync
aborted because agent
thread has finished</div>
<div>STDERR:</div>
</div>
<div><br>
</div>
<div>In this case, we do lose
that the failure seems to
have happened in FindClass,
we need to look at the code.</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
I think losing the actual call info in the trace for
failures is fine. All we really need is the line
number info. I don't think anyone is going to be
trying to debug the root cause based on trace
message, which is no different than the source that
can be viewed given the line number info.<br>
<br>
But back to my point above, if you enable tracing,
seeing which JVMTI and JNI calls are being made in
the trace output could be useful. You could scan all
logs and see what JVMTI and JNI calls were made, and
how often. Possibly useful for coverage analysis of
the tests. However, I don't consider this critical,
and am not opposed to losing it in order to make the
code more readable.<br>
<br>
In any case, a solution would be to have a wrapper
script like NSK_JNI_VERIFY, but just for the purpose
of tracing, not error checking:<br>
<br>
<div> debugeeClass =
NSK_JNI_WRAPPER(jni->FindClass(DEBUGEE_CLASS_NAME));</div>
<div>
<div> if (!NSK_JNI_VERIFY(jni,
debuggeeClass != NULL)) {</div>
<div>
<div> nsk_jvmti_setFailStatus();</div>
<div> return;</div>
<div> }<br>
<br>
And NSK_JNI_WRAPPER would just to the tracing
and execute the passed in action.<br>
</div>
</div>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div><br>
</div>
<div>However, the
ExceptionJNIWrapper that I
implemented a little while
ago will provide this
information:</div>
<div>FATAL ERROR in native
method: FindClass : Return
is NULL<br>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
As I mentioned above, I don't consider the API that
failed to be all that useful in error output as long
as we have the line number info. At best maybe it
helps identify two separate failures as being the
same if the line numbers were to change.<br>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div><br>
</div>
<div>which is not sufficient
to really figure out where
this happened and what were
the arguments. But we could
extend the
ExceptionJNIWrapper to have
each JNI call</div>
<div>allow two optional
arguments and do this:</div>
<div><br>
</div>
<div>...</div>
<div>// For tracing purposes.<br>
</div>
<div>#define TRACE_JNI_CALL
__LINE__, __FILE__</div>
<div>...</div>
<div><br>
</div>
<div>
ExceptionCheckingJniEnvPtr
jni_wrapper(jni);</div>
<div>...</div>
<div>
<div> debugeeClass
=
jni->FindClass(DEBUGEE_CLASS_NAME,
TRACE_JNI_CALL);</div>
<div><br>
</div>
</div>
<div>Where now the JNI call
looks "almost" like non-test
code and a real JNI call but
with just that optional
macro added. Then the
ExceptionCheckingJniEnvPtr
can be modified to print
out:</div>
<div><br>
</div>
<div>
<div>FATAL ERROR in native
method: JNI method
FindClass : Return is NULL
from
test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetTag/settag001/settag001.cpp:69</div>
</div>
<div><br>
</div>
<div>Which now contains all
the same information except
what the line looks like.
I'm not sure that is useful,
instead, if we wanted to go
one step further, we could
add prints to all parameters
that are passed in the JNI
methods via the wrapper,
which would</div>
<div>then look like something
like:</div>
<div><br>
</div>
<div>
<div>JNI method FindClass
was called from
test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetTag/settag001/settag001.cpp:69
with these parameters:</div>
</div>
<div>
<div><span style="white-space:pre-wrap"> </span>"nsk/jvmti/SetTag/settag001"</div>
<div>FATAL ERROR in native
method: JNI method
FindClass : Return is NULL
from
test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetTag/settag001/settag001.cpp:69</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
I'm not familiar with you ExceptionJNIWrapper
changes. The error output above looks fine, but as I
mentioned earlier, I'm ok with just the source line
number info.<br>
<br>
thanks,<br>
<br>
Chris<br>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div><br>
</div>
<div>Let me know what you
think.</div>
<div><br>
</div>
<div>Thanks,</div>
<div>Jc</div>
<div><br>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Wed, Oct 31, 2018 at 1:54 PM
JC Beyler <<a
href="mailto:***@google.com"
target="_blank" moz-do-not-send="true">***@google.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0
0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
<div dir="ltr">
<div dir="ltr">@Claes: you are right, I did
not do a grep such as "if.* =$"; by adding
the space instead of the $, I missed a few
:)</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">I've been meaning to ask if
we could deprecate these anyway. Are these
really being used? And if so, are we
saying everything here is useful:
<div> - Line/File + JNI call?<br>
<div><br>
</div>
<div>
<div>I ask because I'd like to
deprecate the NSK_VERIFY macros but
the LINE/FILE is a bit annoying to
deprecate.</div>
<div><br>
</div>
<div>I'd rather be able to remove them
entirely but we could do an
alternative and migrate the
NSK_VERIFY to:</div>
<div><br>
</div>
<div> testedFieldID
= jni->GetStaticFieldID(testedClass,
FIELD_NAME, FIELD_SIGNATURE);</div>
<div> if (!testedFieldID) {<br>
</div>
<div><br>
</div>
<div>where the print out of the jni
method and argument values can still
be done in the JNI wrapper I added
(ExceptionCheckingJniEnv.cpp) so
we'd have the printout of the calls
but not the line and filename from
where the call was done.</div>
<div><br>
</div>
<div>If lines and filenames are really
important, then we could do
something like:</div>
<div>
<div> testedFieldID =
NSK_TRACE(jni->GetStaticFieldID(testedClass,
FIELD_NAME, FIELD_SIGNATURE));</div>
<div> if (!testedFieldID) {<br>
</div>
<br
class="m_1232422551998170429m_5548951279341903837m_-9088134372540873412m_-823222977695390093gmail-Apple-interchange-newline">
</div>
<div>Which would add a line for which
line/file is doing the JNI call. The
verify part can go into the wrapper
I was talking about
(ExceptionCheckingJniEnv.cpp). </div>
<div><br>
</div>
<div>
<div>Thanks,<br>
</div>
</div>
<div>Jc</div>
<div><br>
</div>
</div>
</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Wed, Oct 31, 2018 at 11:52
AM Chris Plummer <<a
href="mailto:***@oracle.com"
target="_blank" moz-do-not-send="true">***@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote"
style="margin:0 0 0 .8ex;border-left:1px
#ccc solid;padding-left:1ex">On 10/31/18
11:30 AM, <a
href="mailto:***@oracle.com"
target="_blank" moz-do-not-send="true">***@oracle.com</a>
wrote:<br>
> It prints the FILE/LINE which is
enough to identify the error point.<br>
Yes, but it also prints the JNI calls.<br>
> But I agree that doing the
assignments within the NSK_JNI_VERIFY was
<br>
> intentional as it gives more context.<br>
> From the other hand it make the code
ugly and less readable.<br>
> Not sure, what direction is better to
go.<br>
Agreed. Somewhat of a tossup now as to
whether or not this change should <br>
be done. I'm fine either way.<br>
<br>
Chris<br>
><br>
> Thanks,<br>
> Serguei<br>
><br>
><br>
> On 10/31/18 11:21, Chris Plummer
wrote:<br>
>> Hi Claes,<br>
>><br>
>> It's good that you brought this
up. NSK_JNI_VERIFY is always "on", <br>
>> but moving the assignment outside
of it does slightly change the <br>
>> behavior of the macro (although
the code still executes "correctly"):<br>
>><br>
>> /**<br>
>> * Execute action with JNI call,
check result for true and<br>
>> * pending exception and complain
error if required.<br>
>> * Also trace action execution if
tracing mode enabled.<br>
>> */<br>
>> #define NSK_JNI_VERIFY(jni,
action) \<br>
>>
(nsk_ltrace(NSK_TRACE_BEFORE,__FILE__,__LINE__,"%s\n",#action),
\<br>
>>
nsk_jni_lverify(NSK_TRUE,jni,action,__FILE__,__LINE__,"%s\n",#action))<br>
>><br>
>> So you will no longer see the JNI
call in the trace or error output. <br>
>> You will just see the comparison
to the JNI result, which gives no <br>
>> context as to the source of the
result. So I'm now starting to think <br>
>> that doing the assignments within
the NSK_JNI_VERIFY was intentional <br>
>> so we would see the JNI call in
the trace output.<br>
>><br>
>> Chris<br>
>><br>
>> On 10/31/18 3:16 AM, Claes
Redestad wrote:<br>
>>> Hi,<br>
>>><br>
>>> here's a case that your
script seems to miss:<br>
>>><br>
>>> <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8213003/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassLoaderClasses/clsldrclss002/clsldrclss002.cpp.udiff.html"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://cr.openjdk.java.net/%7Ejcbeyler/8213003/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassLoaderClasses/clsldrclss002/clsldrclss002.cpp.udiff.html</a>
<br>
>>><br>
>>><br>
>>> if (!NSK_JNI_VERIFY(jni,
(testedFieldID =<br>
>>>
jni->GetStaticFieldID(testedClass,
FIELD_NAME, <br>
>>> FIELD_SIGNATURE)) != NULL))<br>
>>><br>
>>> I'd note that with some of
your changes here you're unconditionally <br>
>>> executing logic outside of
NSK_JNI_VERIFY macros. Does care need be <br>
>>> taken to wrap the code blocks
in equivalent macros/ifdefs? Or are <br>
>>> the NSK_JNI_VERIFY macros
always-on in these tests (and essentially
<br>
>>> pointless)?<br>
>>><br>
>>> /Claes<br>
>>><br>
>>> On 2018-10-31 05:42, JC
Beyler wrote:<br>
>>>> Hi all,<br>
>>>><br>
>>>> I worked on updating my
script and handling more assignments so I
<br>
>>>> redid a second pass on
the same files to get all the cases. Now,
on <br>
>>>> those files the regex
"if.* = " no longer shows any cases we
would <br>
>>>> want to fix.<br>
>>>><br>
>>>> Could I get a review for
this webrev:<br>
>>>> Webrev: <a
href="http://cr.openjdk.java.net/%7Ejcbeyler/8213003/webrev.00/"
rel="noreferrer" target="_blank"
moz-do-not-send="true">http://cr.openjdk.java.net/~jcbeyler/8213003/webrev.00/</a><br>
>>>> Bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8213003"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8213003</a><br>
>>>><br>
>>>> I tested this on my dev
machine by passing all the tests that were
<br>
>>>> modified.<br>
>>>><br>
>>>> Thanks!<br>
>>>> Jc<br>
>>><br>
>><br>
>><br>
><br>
<br>
<br>
</blockquote>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr"
class="m_1232422551998170429m_5548951279341903837m_-9088134372540873412m_-823222977695390093gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr"
class="m_1232422551998170429m_5548951279341903837m_-9088134372540873412gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<p><br>
</p>
</div>
</blockquote>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr"
class="m_1232422551998170429gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<p><br>
</p>
</div>
</blockquote>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr" class="gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<p><br>
</p>
</body>
</html>