Thanks, Dmitry!
I reviewed it again. Now I think the fix is good.
Sorry for the wrong opinion and confusion.
One nit is about the line:
205 noErrors &= readProviderData(env, provider, p);
The ampersand '&' is not necessary here.
The loop is stopped as soon as the noErrors is equal to zero, so there
is no need to accumulate it.
Thanks,
Serguei
On 7/25/14 4:16 PM, Dmitry Samersoff wrote:
Serguei,
Previous fix get offset to pointer passed as parameter without bounds
check. It's safe here, but not the best solution in general and cause a
warning.
Also readProviderData checks for exceptions, so I see no reason to check
it again.
*In this version:*
we don't need to count errors and can abort after first one.
CHECK_(0)L at 204 cause the function to return, but left allocated memory.
-Dmitry
On 2014-07-26 01:34, [email protected] wrote:
Previous fix was more elegant.
In fact, I do not like new one.
Thanks,
Serguei
On 7/25/14 6:04 AM, Jaroslav Bachorik wrote:
On 07/25/2014 01:40 PM, Dmitry Samersoff wrote:
Jaroslav,
readProviderData already use CHECK
So it might be better to:
1. change readProviderData to return 0 on error and 1 on success.
2. add check for exception to ll 201 and abort the loop if at least
one of providers is not read.
Ok, here is another attempt, now without introducing a new function.
http://cr.openjdk.java.net/~jbachorik/8030115/webrev.01
-JB-
-Dmitry
On 2014-07-24 15:07, Jaroslav Bachorik wrote:
Please, review the change for the fix of the following problem
In jdk/src/share/native/sun/tracing/dtrace/JVM.c the following code is
invoked in loop
198 for (i = 0; i < num_providers; ++i) {
199 JVM_DTraceProvider* p = &(jvm_providers[i]);
200 jobject provider = (*env)->GetObjectArrayElement(
201 env, providers, i);
202 readProviderData(env, provider, p);
203 }
The first problem is that GetGetObjectArrayElement() call on L200 may
throw an exception which is not checked subsequently. On L202 the call
to readProviderData() can also raise a Java exception without
appropriate after-check. When getting back to the beginning of the loop
GetObjectArrayElement() may be called with a pending exception which is
deemed unsafe.
The fix extracts the inner part of the loop into a separate function
where the result of each step is properly checked for pending
exceptions. If there is a pending exception the loop will be
interrupted, resources cleaned and
Java_sun_tracing_dtrace_JVM_activate0() function will return 0 with the
pending exception recorded in env.
Webrev: http://cr.openjdk.java.net/~jbachorik/8030115/webrev.00
Thanks,
-JB-
