Dan, Thank you again for the quick code review.
On 7/22/15 8:57 PM, Daniel D. Daugherty wrote:
On 7/22/15 11:22 AM, Coleen Phillimore wrote:
Summary: Need to get source_file_name from the_class's constant pool
not previous version constant pool
open webrev at http://cr.openjdk.java.net/~coleenp/8087315.01/
src/share/vm/classfile/javaClasses.cpp
So in the new code, you avoid the use of holder->source_file_name()
(where holder has been reset to a previous version ik) to fetch the
Symbol because you can't trust that the source file name CP index
saved in the previous version ik will work with that ik's constant
pool. Interesting problem with previous versions...
So you added a helper that grabs the source file name CP index out
of the previous version ik and sanity checks it before using it
with the merged CP.
And you had to do this in two places so yet another reason for
the helper...
Sorry for verbiage... I had to remind myself how this stuff works...
Yes, you are exactly right. I'm so glad you remember this. I don't
know how, but I'm glad you do. :)
test/runtime/RedefineTests/RedefineRunningMethodsWithBacktrace.java
Nice test.
So what does the output look like when the test is
run without your fix? Something like what is in the
bug report?
Yes, it crashes like the bug report. I was really happy to get a small
test to reproduce this.
Thanks!
Coleen
Thumbs up.
Dan
bug link https://bugs.openjdk.java.net/browse/JDK-8087315
Tested with added test (yay!), RBT (remote build and test),
vm.redefine.testlist, jdk/test/java/lang/instrument and failing
testcase 1000 times (reproduced <400).
Thanks,
Coleen
