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- >>> >>> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
