Fix a multitude of crashes and exceptions during Kotlin decompilation (…

…#339) * Fix crashes caused by copying KFunctionExprents * Fix many smaller crashes from unexpected types * Improve logging if missing metadata * Fix mistake in handler for JvmThrows annotation generation * Work around odd constructor oddities * Fix method name lookups and overly greedy package replacements * Update kotlin test results
Vineflower · Jan 25, 2024 · ffebfcd · ffebfcd
1 parent fea0353
commit ffebfcd
Show file tree

Hide file tree

Showing 10 changed files with 646 additions and 602 deletions.
diff --git a/plugins/kotlin/src/main/java/org/vineflower/kotlin/KotlinChooser.java b/plugins/kotlin/src/main/java/org/vineflower/kotlin/KotlinChooser.java
@@ -72,12 +72,29 @@ public static void setContextVariables(StructClass cl) {
 
     try {
       int kIndex = anno.getParNames().indexOf("k");
-      int k = (int) ((ConstExprent) anno.getParValues().get(kIndex)).getValue();
-
       int d1Index = anno.getParNames().indexOf("d1");
-      Exprent d1 = anno.getParValues().get(d1Index);
-
       int d2Index = anno.getParNames().indexOf("d2");
+
+      if (kIndex == -1) {
+        DecompilerContext.getLogger().writeMessage("No k attribute in class metadata for class " + cl.qualifiedName, IFernflowerLogger.Severity.WARN);
+        DecompilerContext.setProperty(KotlinDecompilationContext.CURRENT_TYPE, null);
+        return;
+      }
+
+      if (d1Index == -1) {
+        DecompilerContext.getLogger().writeMessage("No d1 attribute in class metadata for class " + cl.qualifiedName, IFernflowerLogger.Severity.WARN);
+        DecompilerContext.setProperty(KotlinDecompilationContext.CURRENT_TYPE, null);
+        return;
+      }
+
+      if (d2Index == -1) {
+        DecompilerContext.getLogger().writeMessage("No d2 attribute in class metadata for class " + cl.qualifiedName, IFernflowerLogger.Severity.WARN);
+        DecompilerContext.setProperty(KotlinDecompilationContext.CURRENT_TYPE, null);
+        return;
+      }
+
+      int k = (int) ((ConstExprent) anno.getParValues().get(kIndex)).getValue();
+      Exprent d1 = anno.getParValues().get(d1Index);
       Exprent d2 = anno.getParValues().get(d2Index);
 
       String[] data1 = getDataFromExpr((NewExprent) d1);

diff --git a/plugins/kotlin/src/main/java/org/vineflower/kotlin/KotlinWriter.java b/plugins/kotlin/src/main/java/org/vineflower/kotlin/KotlinWriter.java
@@ -1553,7 +1553,7 @@ public static void appendJvmAnnotations(TextBuffer buffer, int indent, StructMem
               buffer.append(",").appendPossibleNewline(" ");
             }
             first = false;
-            String name = attrib.getExcClassname(i, pool);
+            String name = pool.getPrimitiveConstant(i).getString();
             buffer.append(name).append("::class");
           }
           buffer.popNewlineGroup();

diff --git a/plugins/kotlin/src/main/java/org/vineflower/kotlin/expr/KConstExprent.java b/plugins/kotlin/src/main/java/org/vineflower/kotlin/expr/KConstExprent.java
@@ -18,6 +18,12 @@ public TextBuffer toJava(int indent) {
 
     TextBuffer buf = new TextBuffer();
     buf.addBytecodeMapping(bytecode);
+
+    if (getValue() == null) {
+      //TODO figure out why this happens here instead of elsewhere
+      return buf.append("Class<*>");
+    }
+
     String value = getValue().toString();
     VarType type = new VarType(value, !value.startsWith("["));
     buf.appendCastTypeName(type).append("::class.java");

diff --git a/plugins/kotlin/src/main/java/org/vineflower/kotlin/expr/KFunctionExprent.java b/plugins/kotlin/src/main/java/org/vineflower/kotlin/expr/KFunctionExprent.java
@@ -1,11 +1,6 @@
 package org.vineflower.kotlin.expr;
 
-import org.jetbrains.java.decompiler.code.CodeConstants;
-import org.jetbrains.java.decompiler.main.plugins.PluginImplementationException;
-import org.jetbrains.java.decompiler.modules.decompiler.exps.ConstExprent;
-import org.jetbrains.java.decompiler.modules.decompiler.exps.Exprent;
-import org.jetbrains.java.decompiler.modules.decompiler.exps.FieldExprent;
-import org.jetbrains.java.decompiler.modules.decompiler.exps.FunctionExprent;
+import org.jetbrains.java.decompiler.modules.decompiler.exps.*;
 import org.jetbrains.java.decompiler.modules.decompiler.vars.CheckTypesResult;
 import org.jetbrains.java.decompiler.struct.gen.CodeType;
 import org.jetbrains.java.decompiler.struct.gen.TypeFamily;
@@ -41,15 +36,14 @@ public KFunctionExprent(FunctionType funcType, List<Exprent> operands, BitSet by
   }
 
   public KFunctionExprent(FunctionExprent func) {
-    super(func.getFuncType(), new ArrayList<>(KUtils.replaceExprents(func.getLstOperands())), func.bytecode);
+    this(func, KFunctionType.NONE, func.getExprType());
+  }
 
-    if (func instanceof KFunctionExprent) {
-      KFunctionExprent kFunc = (KFunctionExprent) func;
-      this.kType = kFunc.kType;
-    } else {
-      setImplicitType(func.getExprType());
-    }
+  private KFunctionExprent(FunctionExprent func, KFunctionType kType, VarType exprType) {
+    super(func.getFuncType(), new ArrayList<>(KUtils.replaceExprents(func.getLstOperands())), func.bytecode);
 
+    this.kType = kType;
+    setImplicitType(exprType);
     setNeedsCast(func.doesCast());
 
     if (getFuncType() == FunctionType.EQ) {
@@ -87,7 +81,13 @@ public TextBuffer toJava(int indent) {
             return buf;
           case GET_KCLASS:
             Exprent operand = lstOperands.get(0);
-            if (operand instanceof ConstExprent) {
+            if (operand instanceof VarExprent) {
+              VarExprent varExprent = ((VarExprent) operand);
+              if (!varExprent.getVarType().equals(VarType.VARTYPE_CLASS)) {
+                throw new IllegalArgumentException("Variable accessing KClass is not a Class");
+              }
+              return buf.append(varExprent.toJava()).append(".kotlin");
+            } else if (operand instanceof ConstExprent) {
               ConstExprent constExprent = (ConstExprent) operand;
               String value = constExprent.getValue().toString();
               VarType type = new VarType(value, !value.startsWith("["));
@@ -322,6 +322,6 @@ public int getPrecedence() {
 
   @Override
   public Exprent copy() {
-    return new KFunctionExprent((FunctionExprent) super.copy());
+    return new KFunctionExprent((FunctionExprent) super.copy(), kType, getExprType());
   }
 }
diff --git a/plugins/kotlin/src/main/java/org/vineflower/kotlin/struct/KConstructor.java b/plugins/kotlin/src/main/java/org/vineflower/kotlin/struct/KConstructor.java
@@ -145,17 +145,31 @@ public boolean stringify(TextBuffer buffer, int indent) {
         parameter.stringify(indent + 1, buf);
       }
 
-      buf.appendPossibleNewline("", true).popNewlineGroup().append(") : ");
+      buf.appendPossibleNewline("", true).popNewlineGroup();
 
-      Exprent firstExpr = method.getOrBuildGraph().first.exprents.get(0);
-      if (!(firstExpr instanceof InvocationExprent)) {
-        throw new IllegalStateException("First expression of constructor is not InvocationExprent");
+      String methodDescriptor = method.methodStruct.getName() + method.methodStruct.getDescriptor();
+      String containingClass = node.classStruct.qualifiedName;
+
+      List<Exprent> exprents = method.getOrBuildGraph().first.exprents;
+      if (exprents.isEmpty()) {
+        DecompilerContext.getLogger().writeMessage("Unexpected empty constructor body in " + containingClass + " " + methodDescriptor, IFernflowerLogger.Severity.WARN);
+        return true;
       }
 
-      InvocationExprent invocation = (InvocationExprent) firstExpr;
-      buf.append(invocation.toJava(indent + 1), node.classStruct.qualifiedName, InterpreterUtil.makeUniqueKey(method.methodStruct.getName(), method.methodStruct.getDescriptor()));
+      buf.append(") ");
+
+      Exprent firstExpr = exprents.get(0);
+      if (!(firstExpr instanceof InvocationExprent)) {
+        // no detected super / this constructor call (something isn't right)
+        DecompilerContext.getLogger().writeMessage("Unexpected missing super/this constructor call in " + containingClass + " " + methodDescriptor, IFernflowerLogger.Severity.WARN);
+      } else {
+        buf.append(": ");
+
+        InvocationExprent invocation = (InvocationExprent) firstExpr;
+        buf.append(invocation.toJava(indent + 1), node.classStruct.qualifiedName, InterpreterUtil.makeUniqueKey(method.methodStruct.getName(), method.methodStruct.getDescriptor()));
 
-      method.getOrBuildGraph().first.exprents.remove(0);
+        method.getOrBuildGraph().first.exprents.remove(0);
+      }
     }
 
     if (method.getOrBuildGraph().first.exprents.isEmpty()) {

diff --git a/plugins/kotlin/src/main/java/org/vineflower/kotlin/struct/KFunction.java b/plugins/kotlin/src/main/java/org/vineflower/kotlin/struct/KFunction.java
@@ -130,8 +130,8 @@ public static Map<StructMethod, KFunction> parse(ClassesProcessor.ClassNode node
 
       MethodWrapper method = null;
 
+      String lookupName = jvmData.hasName() ? resolver.resolve(jvmData.getName()) : name;
       if (jvmData.hasDesc()) {
-        String lookupName = jvmData.hasName() ? resolver.resolve(jvmData.getName()) : name;
         method = wrapper.getMethodWrapper(lookupName, resolver.resolve(jvmData.getDesc()));
       }
 
@@ -148,7 +148,7 @@ public static Map<StructMethod, KFunction> parse(ClassesProcessor.ClassNode node
         int endOfParams = desc.length();
         desc.append(")").append(returnType);
 
-        method = wrapper.getMethodWrapper(name, desc.toString());
+        method = wrapper.getMethodWrapper(lookupName, desc.toString());
 
         if (method == null) {
           throw new IllegalStateException("Couldn't find method " + name + " " + desc + " in class " + struct.qualifiedName);

diff --git a/plugins/kotlin/src/main/java/org/vineflower/kotlin/struct/KProperty.java b/plugins/kotlin/src/main/java/org/vineflower/kotlin/struct/KProperty.java
@@ -263,7 +263,7 @@ private static void appendVisibility(TextBuffer buf, ProtoBuf.Visibility visibil
 
       String propDesc = null;
       KType type = null;
-      if (property.hasReturnType() && property.getReturnType().hasClassName()) {
+      if (property.hasReturnType()) {
         type = KType.from(property.getReturnType(), nameResolver);
         propDesc = KTypes.getJavaSignature(type.kotlinType, property.getReturnType().getNullable());
       }

diff --git a/plugins/kotlin/src/main/java/org/vineflower/kotlin/util/KTypes.java b/plugins/kotlin/src/main/java/org/vineflower/kotlin/util/KTypes.java
@@ -53,14 +53,23 @@ public final class KTypes {
     Map.entry("kotlin/collections/Iterable", "java/lang/Iterable")
   );
 
-  public static final Map<String, String> KOTLIN_TO_JAVA_UTIL = Map.of(
-    "kotlin/collections/MutableMap", "java/util/Map",
-    "kotlin/collections/MutableList", "java/util/List",
-    "kotlin/collections/MutableSet", "java/util/Set",
-    "kotlin/collections/MutableIterator", "java/util/Iterator",
-    "kotlin/collections/MutableMap$MutableEntry", "java/util/Map$Entry"
+  public static final Map<String, String> KOTLIN_TO_JAVA_UTIL = Map.ofEntries(
+    Map.entry("kotlin/collections/MutableCollection", "java/util/Collection"),
+    Map.entry("kotlin/collections/MutableMap", "java/util/Map"),
+    Map.entry("kotlin/collections/MutableList", "java/util/List"),
+    Map.entry("kotlin/collections/MutableSet", "java/util/Set"),
+    Map.entry("kotlin/collections/MutableIterator", "java/util/Iterator"),
+    Map.entry("kotlin/collections/MutableListIterator", "java/util/ListIterator"),
+    Map.entry("kotlin/collections/MutableMap$MutableEntry", "java/util/Map$Entry"),
+    Map.entry("kotlin/collections/Collection", "java/util/Collection"),
+    Map.entry("kotlin/collections/Map", "java/util/Map"),
+    Map.entry("kotlin/collections/List", "java/util/List"),
+    Map.entry("kotlin/collections/ListIterator", "java/util/ListIterator"),
+    Map.entry("kotlin/collections/Set", "java/util/Set"),
+    Map.entry("kotlin/collections/Iterator", "java/util/Iterator"),
+    Map.entry("kotlin/collections/Map$Entry", "java/util/Map$Entry")
   );
-  
+
   private static final Map<String, String> KOTLIN_PRIMITIVE_TYPES = Map.of(
     "kotlin/Int", "I",
     "kotlin/Long", "J",
@@ -94,15 +103,15 @@ public static String getJavaSignature(String kotlinType, boolean isNullable) {
         return "L" + KOTLIN_TO_JAVA_LANG.get(kotlinType) + ";";
       } else if (KOTLIN_TO_JAVA_UTIL.containsKey(kotlinType)) {
         return "L" + KOTLIN_TO_JAVA_UTIL.get(kotlinType) + ";";
-      } else if (kotlinType.startsWith("kotlin/collections/")) {
-        String javaType = kotlinType.substring("kotlin/collections/".length());
-        javaType = javaType.startsWith("Mutable") ? javaType.substring("Mutable".length()) : javaType;
-        return "Ljava/util/" + javaType + ";";
       } else if (kotlinType.startsWith("kotlin/Function")) {
-        if (Integer.parseInt(kotlinType.substring("kotlin/Function".length())) > MAX_KOTLIN_FUNCTION_ARITY) {
-          return "Lkotlin/jvm/functions/FunctionN;";
+        try {
+          if (Integer.parseInt(kotlinType.substring("kotlin/Function".length())) > MAX_KOTLIN_FUNCTION_ARITY) {
+            return "Lkotlin/jvm/functions/FunctionN;";
+          }
+          return "Lkotlin/jvm/functions" + kotlinType.substring("kotlin".length()) + ";";
+        } catch (NumberFormatException e) {
+          // Not a function type with arity
         }
-        return "Lkotlin/jvm/functions" + kotlinType.substring("kotlin".length()) + ";";
       }
     }
     return "L" + kotlinType + ";";