RFR: 7093328: JVMTI: jvmtiPrimitiveFieldCallback always report 0's for static primitives
Daniel D. Daugherty
daniel.daugherty at oracle.com
Wed Aug 29 07:48:06 PDT 2012
Adding serviceability-dev at openjdk.java.net back into the thread...
"Reply list" seems to only pick one list...
The JCK team has opened a bug for adding a test:
7194847 4/3 Need JCK JVM TI tests for IterateThroughHeap to
cover issue reported by 7093328
I don't know if the VM/SQE team will be adding a test to the VM/NSK
testbase for this. I don't think a JavaTest/JTREG test is a good
fit for this since it has to be a native test.
On 8/29/12 8:42 AM, Coleen Phillimore wrote:
> This change looks good. Is there a test for this? Is it possible to
> add a jtreg test for this and add it to test/serviceability directory?
> This code is different in the permgen elimination repository because
> we don't pass klasses as oops. I'll start with the test in the bug
> but it would be great to have a jtreg test for it so we know if we
> broke it.
> On 8/29/2012 10:25 AM, Daniel D. Daugherty wrote:
>> On 8/29/12 8:16 AM, Daniel D. Daugherty wrote:
>>> Logistics issue: This review request went out to OpenJDK alias, but
>>> I think this webrev is not accessible outside of Oracle. The webrev
>>> should be publicly accessible when the review is public. However,
>>> in this case, this is a one line fix so IMHO you could get away with
>>> a context diff in the suggested fix of the bug report.
>>> On 8/29/12 1:24 AM, Rickard Bäckman wrote:
>>>> Hi all,
>>>> can I get a couple of reviews for the following bug fix:
>>>> webrev: http://rbackman.se.oracle.com/~rbackman/7093328/
>>> Thumbs up.
>>> No content comments.
>>> This is a perfect example of how casting can bite you. :-)
>>> The buggy line:
>>> 1165 address addr = (address)k + offset;
>>> dates back to the original implementation of this function:
>>> D 1.47 05/08/22 20:30:26 ab23780 77 76 01476/01330/01852
>>> 6195957: further clean-up and caching of field maps
>>> In the buggy code, a value is copied from the instanceKlass
>>> at the offset instead of from the Java mirror. If the value
>>> copied from the instanceKlass happened to match the expected
>>> value, then that was pure luck.
>>> The bug report claims that this last "worked" in 6u26. I suspect
>>> that "working" is defined as returning a non-zero value.
>> Very nice find:
>>> I did some research (archeology), the change that introduced this:
>>> changeset: 2223:c7f3d0b4570f
>>> user: never
>>> date: Fri Mar 18 16:00:34 2011 -0700
>>> summary: 7017732: move static fields into Class to prepare for
>>> perm ten removal
>> I had forgotten that static fields moved from the instanceKlass
>> into Class. The original code from 2005 did indeed work as expected
>> way back then. In fact, there were three such uses in jvmtiTagMap.cpp
>> back then and it looks like the other two uses were updated to use
>> the Java mirror and one was missed.
More information about the hotspot-runtime-dev