On Aug 29, 2012, at 4:16 PM, 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.
Sorry, I pasted the wrong url, the public one is available here: http://cr.openjdk.java.net/~rbackman/7093328/ Thanks for the review Dan! /R > > 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
