[PATCH][asmtools] Fix incorrect handling of some CPX2 constant pool entries by JDIS
Maxim Degtyarev
mdegtyarev at gmail.com
Sat Mar 17 03:48:37 UTC 2018
I think in such case we should at least get rid of `switch (cns.tag)`
in `ConstantPool::ConstantStrValue()` by replacing it with `if (cns
instanceof CPX2)`
Updated patch attached.
BTW, I saw my original message was forwarded to the asmtools-dev, but
that mailing list looks dead. Which mailing list should be used for
asmtools-related issues?
2018-03-14 22:04 GMT+03:00 Leonid Kuskov <Leonid.Kuskov at oracle.com>:
> Hi Maxim,
>
> Thank you for the patch. Indeed it does fix a bug - the fix is correct but
> it seems to be overloaded with extra code.
> CX2 class contains enough information to split structures into 2 groups:
> referencing to CLASS_info
> (Fieldref, Methodref and InterfaceMethodref) and all others, hence the
> marker class CPX2_ClassMember is not necessary.
> There you can just add one method to CX2 as follows:
> boolean refersClassMember() {
> return tag == CONSTANT_FIELD || tag == CONSTANT_METHOD || tag ==
> CONSTANT_INTERFACEMETHOD;
> }
> and change only 1 line:
> -if (cns2.value1 == cd.this_cpx) {
>
> +if ( cns2.value1 == cd.this_cpx && cns2.refersClassMember() ) {
>
> What do you think if I use this shorter fix as your contribution?
>
> Thanks,
> Leonid
>
>
> On 3/12/18 16:52, Maxim Degtyarev wrote:
>>
>> The proposed patch (see attached file jdis-cpx2-bug.diff.txt) address
>> incorrect handling of some constant pool entries represented with CPX2
>> class by ASMTOOLS JDIS.
>>
>> JDIS incorrectly assume that field CPX2::value1 is always used to hold
>> index of constant pool entry of type CONSTANT_CLASS.
>> This assumption is valid only for entries of type CONSTANT_FIELD,
>> CONSTANT_METHOD and CONSTANT_INTERFACEMETHOD.
>> For CONSTANT_NAMEANDTYPE, CONSTANT_METHODHANDLE, CONSTANT_DYNAMIC and
>> CONSTANT_INVOKEDYNAMIC this assumption is wrong.
>> As a result asmtools jdis may produce malformed output for valid input.
>>
>> Example:
>>
>> public class Test {
>>
>> // Force `class Test` entry appear in the constant pool before
>> InvokeDynamic entries.
>> public Class<?> CLASS = Test.class;
>>
>> private static Runnable[] workers = {
>> () -> System.out.println("Test"),
>> () -> System.out.println("Test"),
>> () -> System.out.println("Test"),
>> () -> System.out.println("Test"),
>> () -> System.out.println("Test"),
>> () -> System.out.println("Test"),
>> () -> System.out.println("Test"),
>> () -> System.out.println("Test"),
>> () -> System.out.println("Test"),
>> () -> System.out.println("Test")
>> };
>>
>> public static void main(String... args) {
>>
>> for(Runnable worker : workers) {
>> worker.run();
>> }
>>
>> }
>>
>> }
>>
>> One of invokedynamic instructions may be rendered by jdis as
>>
>> invokedynamic InvokeDynamic run:"()Ljava/lang/Runnable;";
>>
>> instead of
>>
>> invokedynamic InvokeDynamic
>>
>> REF_invokeStatic:java/lang/invoke/LambdaMetafactory.metafactory:"(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;":run:"()Ljava/lang/Runnable;"
>> MethodType "()V", MethodHandle
>> REF_invokeStatic:Test.lambda$static$1:"()V", MethodType "()V";
>
>
-------------- next part --------------
# HG changeset patch
# User maccimo
# Date 1521257710 -10800
# Sat Mar 17 06:35:10 2018 +0300
# Node ID 9bb02bff6c94157521bd91a8a923d4d6820b1b9f
# Parent 2904229ed97ec1d90a055ad86759f867cdc5296b
Fix incorrect handling of constant pool entries of type CONSTANT_NAMEANDTYPE, CONSTANT_METHODHANDLE, CONSTANT_DYNAMIC and CONSTANT_INVOKEDYNAMIC.
diff --git a/src/org/openjdk/asmtools/jdis/ConstantPool.java b/src/org/openjdk/asmtools/jdis/ConstantPool.java
--- a/src/org/openjdk/asmtools/jdis/ConstantPool.java
+++ b/src/org/openjdk/asmtools/jdis/ConstantPool.java
@@ -598,6 +598,10 @@
break;
}
}
+
+ public boolean refersClassMember() {
+ return tag == TAG.CONSTANT_FIELD || tag == TAG.CONSTANT_METHOD || tag == TAG.CONSTANT_INTERFACEMETHOD;
+ }
}
/* -------------------------------------------------------- */
@@ -998,17 +1002,10 @@
if (cns == null) {
return "#" + cpx;
}
- switch (cns.tag) {
- case CONSTANT_METHODHANDLE:
- case CONSTANT_DYNAMIC:
- case CONSTANT_INVOKEDYNAMIC:
- case CONSTANT_METHOD:
- case CONSTANT_INTERFACEMETHOD:
- case CONSTANT_FIELD: {
- CPX2 cns2 = (CPX2) cns;
- if (cns2.value1 == cd.this_cpx) {
- cpx = cns2.value2;
- }
+ if (cns instanceof CPX2) {
+ CPX2 cns2 = (CPX2) cns;
+ if (cns2.value1 == cd.this_cpx && cns2.refersClassMember()) {
+ cpx = cns2.value2;
}
}
return cns.tag.tagname + " " + StringValue(cpx);
More information about the code-tools-dev
mailing list