RFR 8062116: JVMTI GetClassMethods is Slow

[Jeremy Manson]

On Wed, Nov 5, 2014 at 3:40 PM, Coleen Phillimore <
coleen.phillimore at oracle.com> wrote:

>
> On 11/5/14, 6:13 PM, Jeremy Manson wrote:
>
>
>
> On Tue, Nov 4, 2014 at 8:56 PM, serguei.spitsyn at oracle.com <
> serguei.spitsyn at oracle.com> wrote:
>
>>  The fix looks good in general.
>>
>> src/share/vm/oops/method.cpp
>>
>> 1785   bool contains(Method** m) {1786     if (m == NULL) return false;1787     for (JNIMethodBlockNode* b = &_head; b != NULL; b = b->_next) {1788       if (b->_methods <= m && m < b->_methods + b->_number_of_methods) {*1789         ptrdiff_t idx = m - b->_methods;**1790         if (b->_methods + idx == m) {**
>> 1791           return true;
>> 1792         }*
>> 1793       }
>> 1794     }
>> 1795     return false;  // not found
>> 1796   }
>>
>>
>> Just noticed that the lines 1789-1792 can be replaced with one liner:
>>  *         return true;*
>>
>
>  Ah, you have found our crappy workaround for wild pointers to
> non-aligned places in the middle of _methods.
>
>
> Can you explain this?  Why are there wild pointers?
>

My belief was that end user code could pass any old garbage to this
function.  It's called by Method::is_method_id, which is called
by jniCheck::validate_jmethod_id.  The idea was that this would help check
jni deliver useful information in the case of the end user inputting
garbage that happened to be in the right memory range.

Having said that, at a second glance, it looks as if it that call is
protected by a call to is_method() (in checked_resolve_jmethod_id), so the
program will probably crash before it gets to this check.

The other point about it was that the result of >= and < is technically
unspecified; if it were ever implemented as anything other than a binary
comparison between integers (which it never is, now that no one has a
segmented architecture), the comparison could pass spuriously, so checking
would be a good thing.  Of course, the comparison could fail spuriously,
too.

Anyway, I'm happy to leave it in as belt-and-suspenders (and add a comment,
obviously, since it has caused confusion), or take it out.  Your call.

Jeremy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20141105/606fd1b0/attachment-0001.html>