search

eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.29k stars 722 forks source link

J9BCUtil will misinterpret some constant pool type codes in old core files #1653

Open keithc-ca opened 6 years ago

keithc-ca commented 6 years ago

The values of J9CPTYPE_INTERFACE_STATIC_METHOD and J9CPTYPE_INTERFACE_INSTANCE_METHOD changed from 17 & 18 to 18 & 19, respectively with the merge of #1426.

The method J9BCUtil.dumpCPShapeDescription() implicitly uses the new values via the local array, symbols, without regard to the vintage of the core file being examined.

Instead, it should be using the constants of the same names from J9ConstantPool to interpret a constant pool description. Note that the new type, J9CPTYPE_CONSTANT_DYNAMIC, will not be present in old core files; a fact that will need to be considered when designing a fix.

pshipton commented 6 years ago

Since J9CPTYPE_INTERFACE_STATIC_METHOD and J9CPTYPE_INTERFACE_INSTANCE_METHOD were just added a few weeks ago via #1167, and are only for Java9 and later (which isn't supported yet), I expect we can ignore this problem.

fengxue-IS commented 6 years ago

J9CPTYPE_INTERFACE_STATIC_METHOD and J9CPTYPE_INTERFACE_INSTANCE_METHOD currently are used for both Java8 and Java9 (as of #1167 merged)

for J9BCUtil, I think it would be better if we have a table to do the name mapping rather than an array to make it more clear

DanHeidinga commented 6 years ago

Mea culpa. I should have noticed this and called it out in the original review. Thanks for catching it Keith.

It does raise a bigger question though of when we want to introduce algorithm versions in DDR. Along with @pshipton and I's earlier conversation about testing, we may want to introduce a checklist of things committers should be watching for.

pshipton commented 6 years ago

@DanHeidinga from the previous comments, it seems the jdk9 label should be removed from #1167?

fengxue-IS commented 6 years ago

@pshipton #1167 is a jdk9 bug as openjdk introduced the fix for inconsistent constant pool and corresponding test cases in Java9.

Since OpenJ9 is sharing the VM for both Java8 and Java9, this fix will be affecting both 8 & 9. As I have commented in that PR, it follows the spec defined for Java9, and Java8 spec didn't specify it.

TheMarvelFan commented 1 year ago

Hi what is the latest info on this issue?

keithc-ca commented 1 year ago

No news to report here. I think the suggestion in https://github.com/eclipse-openj9/openj9/issues/1653#issuecomment-380275223 should form the basis of a fix.