Adding [email protected] 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.
Dan
On 8/29/12 8:42 AM, Coleen Phillimore wrote:
Rickard,
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.
thanks,
Coleen
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:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7093328
webrev: http://rbackman.se.oracle.com/~rbackman/7093328/
Thanks
/R
Thumbs up.
src/share/vm/prims/jvmtiTagMap.cpp
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:
src/share/vm/prims/SCCS/s.jvmtiTagMap.cpp
D 1.47 05/08/22 20:30:26 ab23780 77 76 01476/01330/01852
MRs:
COMMENTS:
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.
Dan
Rickard,
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.
Dan
