Request for review (S) 4926272: methodOopDesc::method_from_bcp is unsafe

[Tom Rodriguez]

On Jan 3, 2011, at 1:38 PM, Coleen Phillimore wrote:

> Ramki and Keith pointed out that there is a non-zero probability that constMethodKlass's address could look like a set of bytecodes so we could fail to search up to the top of the constMethodOop type.  A new cleaner webrev which solves the problem for when there is no permgen is available here.   This was tested with 32 and 64 bit with the failing testcases and -XX:+UseConcMarkSweepGC.  Please review.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/4926272_2/

That seems like a hack to support an existing hack.  Couldn't we remove the underlying assumption that requires converting from bcp to method?  If we were writing this from scratch and it was proposed to do it this way, I think everyone would rightfully gag.  All uses of a bcp should have the methodOop itself nearby, so it's only a matter of passing it in as needed.  All the calls in Bytecodes can trivially have a methodOop argument added.  The harder/uglier piece is Bytecode flyweight objects in bytecode.hpp.  These would have to be converted into ResourceObj or StackObj and keep around a pointer to the methodOop. There's also the oddity that when the CI uses these routines the bcp doesn't actually point into the underlying methodOop but in those cases the conversion back to methodOop will never be required because we don't have any breakpoint bytecodes in the CI copy.

tom

> 
> Thanks,
> Coleen
> 
> On 12/29/2010 6:18 PM, Coleen Phillimore wrote:
>> Summary: Instead of using GC to find the block start, scan backward in memory to known contents in the constMethodOop
>> 
>> open webrev at http://cr.openjdk.java.net/~coleenp/4926272/
>> bug link at http://bugs.sun.com/view_bug.do?bug_id=4926272
>> 
>> Tested on 32 and 64 bit platforms with the test cases that reliably failed.
>> 
>> Thanks,
>> Coleen
>> 
>