Fix lambda deserialization in classes with 252+ lambdas #5857

retronym · 2017-04-19T01:38:07Z

Create a lambda deserializer per group of target methods,
and call these sequentially trapping the particular pattern
of exception that is thrown when a target method is absent
from the map.

Fixes scala/bug#10232

  // access flags 0x100A
  private static synthetic $deserializeLambda$(Ljava/lang/invoke/SerializedLambda;)Ljava/lang/Object;
    TRYCATCHBLOCK L0 L1 L1 java/lang/IllegalArgumentException
   L0
    ALOAD 0
    INVOKEDYNAMIC lambdaDeserialize(Ljava/lang/invoke/SerializedLambda;)Ljava/lang/Object; [
      // handle kind 0x6 : INVOKESTATIC
      scala/runtime/LambdaDeserialize.bootstrap(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/invoke/MethodHandle;)Ljava/lang/invoke/CallSite;
      // arguments:
      // handle kind 0x6 : INVOKESTATIC
      Test$.$anonfun$main$1$adapted(Lscala/Function1;)Ljava/lang/Object;,
      // handle kind 0x6 : INVOKESTATIC
      Test$.$anonfun$lambdas$1(Ljava/lang/Object;)Ljava/lang/String;,

      ...

      Test$.$anonfun$lambdas$249(Ljava/lang/Object;)Ljava/lang/String;,
      // handle kind 0x6 : INVOKESTATIC
      Test$.$anonfun$lambdas$250(Ljava/lang/Object;)Ljava/lang/String;
    ]
    ARETURN
   L1
    ALOAD 0
    INVOKEDYNAMIC lambdaDeserialize(Ljava/lang/invoke/SerializedLambda;)Ljava/lang/Object; [
      // handle kind 0x6 : INVOKESTATIC
      scala/runtime/LambdaDeserialize.bootstrap(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/invoke/MethodHandle;)Ljava/lang/invoke/CallSite;
      // arguments:
      // handle kind 0x6 : INVOKESTATIC
      Test$.$anonfun$lambdas$251(Ljava/lang/Object;)Ljava/lang/String;,
      // handle kind 0x6 : INVOKESTATIC
      Test$.$anonfun$lambdas$252(Ljava/lang/Object;)Ljava/lang/String;,

      ...

      // handle kind 0x6 : INVOKESTATIC
      Test$.$anonfun$lambdas$256(Ljava/lang/Object;)Ljava/lang/String;
    ]
    ARETURN
    MAXSTACK = 2
    MAXLOCALS = 1

retronym · 2017-04-19T01:45:51Z

Unfortunately, this change uses exceptions as control flow for lambda#252 and above, but that seems to be the best way around the compatibility constraint that the newly generated code for $lambdaDeserialize$ should work when run with older standard libraries.

I've prototyped a change to avoid the overhead of filling in the stack trace of the IllegalArgumentException used for control flow (2.12.x...retronym:ticket/10232-stackless), but I'm reluctant to merge that it also relies on hacks to avoid requiring API changes to the runtime support.

lrytz · 2017-04-19T13:35:14Z

src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala

+      mv.visitVarInsn(ASTORE, 1)
+      mv.visitVarInsn(ALOAD, 1)
+      mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/IllegalArgumentException", "getCause", "()Ljava/lang/Throwable;", false)
+      mv.visitJumpInsn(IFNULL, nextLabel(i))


should there be an ALOAD 1; ATHROW here?

Actually, I've just noticed that the second throw IllegalArgumentException in LambdaDeserializer is detritus that should have been removed in 131402f.

I'll remove that throw and then simplify this PR (no need to check getCause == null here, just swallow any IAE unconditionally in all but the terminal indy call.

lrytz · 2017-04-19T13:49:35Z

Looks good otherwise. I guess for 2.13 we can have a solution that doesn't use exceptional control flow? You could mark the commit [nomerge].

I should have removed the try/catch when I removed the code path that could throw that exception in 131402f.

retronym · 2017-04-20T06:24:49Z

Pushed the discussed simplification.

I guess for 2.13 we can have a solution that doesn't use exceptional control flow? You could mark the commit [nomerge].

I'd rather merge this into 2.13.x and then adapt the result, I've opened a ticket to track that: https://github.com/scala/scala-dev/issues/373

lrytz · 2017-04-20T06:42:35Z

Sure, that's fine too.

lrytz · 2017-04-20T06:58:10Z

src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala

+    def nextLabel(i: Int) = if (i == numGroups - 2) terminalLabel else initialLabels(i + 1)
+
+    for ((label, i) <- initialLabels.iterator.zipWithIndex) {
+      mv.visitTryCatchBlock(label, nextLabel(i), nextLabel(i), "java/lang/IllegalArgumentException")


Can you add jlIllegalArgumentExceptionRef to CoreBTypes, like jlClassCastExceptionRef, then use .internalName? This ensures that the ClassBType is part of the classBTypeFromInternalName map, which is excpected for computing stack map frames during class writing.

lrytz · 2017-04-20T07:07:49Z

src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala

+   *     case i: IllegalArgumentException => // swallow
+   *   }
+   *   </FOR>
+   *   return indy[scala.runtime.LambdaDeserialize.bootstrap, targetMethodGroup${NUM_GROUPS-1}](l)


the generated structure is more like

try return indy[scala.runtime.LambdaDeserialize.bootstrap, targetMethodGroup$0](l) catch { case i: IllegalArgumentException => try return indy[scala.runtime.LambdaDeserialize.bootstrap, targetMethodGroup$1](l) catch { case i: IllegalArgumentException => ... return indy[scala.runtime.LambdaDeserialize.bootstrap, targetMethodGroup${NUM_GROUPS-1}](l) } }

Is there a difference? In any case, your version is clearer, so I'll use that.

Just a detail to make the comment align better with the implementation.

lrytz · 2017-04-20T07:28:27Z

src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala

@@ -103,6 +103,7 @@ class CoreBTypes[BTFS <: BTypesFromSymbols[_ <: Global]](val bTypes: BTFS) {
  lazy val jlCloneableRef            : ClassBType = classBTypeFromSymbol(JavaCloneableClass)        // java/lang/Cloneable
  lazy val jiSerializableRef         : ClassBType = classBTypeFromSymbol(JavaSerializableClass)     // java/io/Serializable
  lazy val jlClassCastExceptionRef   : ClassBType = classBTypeFromSymbol(ClassCastExceptionClass)   // java/lang/ClassCastException
+  lazy val jlIllegalArgExceptionRef  : ClassBType = classBTypeFromSymbol(IllegalArgExceptionClass)  // java/lang/ClassCastException


Comment is out of synch..

lrytz · 2017-04-20T07:28:30Z

src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala

+    def nextLabel(i: Int) = if (i == numGroups - 2) terminalLabel else initialLabels(i + 1)
+
+    for ((label, i) <- initialLabels.iterator.zipWithIndex) {
+      mv.visitTryCatchBlock(label, nextLabel(i), nextLabel(i), coreBTypes.jlIllegalArgExceptionRef.internalName)


coreBTypes._ is already imported

Create a lambda deserializer per group of target methods, and call these sequentially trapping the particular pattern of exception that is thrown when a target method is absent from the map. Fixes scala/bug#10232 ``` // access flags 0x100A private static synthetic $deserializeLambda$(Ljava/lang/invoke/SerializedLambda;)Ljava/lang/Object; TRYCATCHBLOCK L0 L1 L1 java/lang/IllegalArgumentException L0 ALOAD 0 INVOKEDYNAMIC lambdaDeserialize(Ljava/lang/invoke/SerializedLambda;)Ljava/lang/Object; [ // handle kind 0x6 : INVOKESTATIC scala/runtime/LambdaDeserialize.bootstrap(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/invoke/MethodHandle;)Ljava/lang/invoke/CallSite; // arguments: // handle kind 0x6 : INVOKESTATIC Test$.$anonfun$main$1$adapted(Lscala/Function1;)Ljava/lang/Object;, // handle kind 0x6 : INVOKESTATIC Test$.$anonfun$lambdas$1(Ljava/lang/Object;)Ljava/lang/String;, ... Test$.$anonfun$lambdas$249(Ljava/lang/Object;)Ljava/lang/String;, // handle kind 0x6 : INVOKESTATIC Test$.$anonfun$lambdas$250(Ljava/lang/Object;)Ljava/lang/String; ] ARETURN L1 ALOAD 0 INVOKEDYNAMIC lambdaDeserialize(Ljava/lang/invoke/SerializedLambda;)Ljava/lang/Object; [ // handle kind 0x6 : INVOKESTATIC scala/runtime/LambdaDeserialize.bootstrap(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;[Ljava/lang/invoke/MethodHandle;)Ljava/lang/invoke/CallSite; // arguments: // handle kind 0x6 : INVOKESTATIC Test$.$anonfun$lambdas$251(Ljava/lang/Object;)Ljava/lang/String;, // handle kind 0x6 : INVOKESTATIC Test$.$anonfun$lambdas$252(Ljava/lang/Object;)Ljava/lang/String;, ... // handle kind 0x6 : INVOKESTATIC Test$.$anonfun$lambdas$256(Ljava/lang/Object;)Ljava/lang/String; ] ARETURN MAXSTACK = 2 MAXLOCALS = 1 ```

scala-jenkins added this to the 2.12.3 milestone Apr 19, 2017

retronym requested a review from lrytz April 19, 2017 01:45

retronym changed the title ~~Fix lambda serialization in classes with 252+ lambdas~~ Fix lambda deserialization in classes with 252+ lambdas Apr 19, 2017

retronym force-pushed the ticket/10232 branch from 4e09eb1 to be630ef Compare April 19, 2017 01:51

lrytz reviewed Apr 19, 2017

View reviewed changes

retronym force-pushed the ticket/10232 branch from be630ef to 7c5d1da Compare April 20, 2017 06:20

Remove dead code catch in LambdaDeserializer

d9343a7

I should have removed the try/catch when I removed the code path that could throw that exception in 131402f.

retronym force-pushed the ticket/10232 branch from 7c5d1da to 20587e9 Compare April 20, 2017 06:22

lrytz reviewed Apr 20, 2017

View reviewed changes

retronym force-pushed the ticket/10232 branch from 20587e9 to 043e31d Compare April 20, 2017 07:11

lrytz approved these changes Apr 20, 2017

View reviewed changes

retronym force-pushed the ticket/10232 branch from 043e31d to 9f522c6 Compare April 24, 2017 00:34

retronym merged commit 7cc1f86 into scala:2.12.x Apr 24, 2017

SethTisue mentioned this pull request Apr 24, 2017

Merge 2.12.x to 2.13.x [ci: last-only] #5862

Merged

retronym mentioned this pull request Jan 9, 2019

Adapt lambda deserialization fix in 2.13.x to avoid exceptions as control flow scala/bug#11345

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix lambda deserialization in classes with 252+ lambdas #5857

Fix lambda deserialization in classes with 252+ lambdas #5857

retronym commented Apr 19, 2017 •

edited

Loading

retronym commented Apr 19, 2017

lrytz Apr 19, 2017

retronym Apr 20, 2017

lrytz commented Apr 19, 2017

retronym commented Apr 20, 2017 •

edited

Loading

lrytz commented Apr 20, 2017

lrytz Apr 20, 2017 •

edited

Loading

retronym Apr 20, 2017

lrytz Apr 20, 2017

retronym Apr 20, 2017

lrytz Apr 20, 2017

lrytz Apr 20, 2017

lrytz Apr 20, 2017

Fix lambda deserialization in classes with 252+ lambdas #5857

Fix lambda deserialization in classes with 252+ lambdas #5857

Conversation

retronym commented Apr 19, 2017 • edited Loading

retronym commented Apr 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrytz commented Apr 19, 2017

retronym commented Apr 20, 2017 • edited Loading

lrytz commented Apr 20, 2017

lrytz Apr 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

retronym commented Apr 19, 2017 •

edited

Loading

retronym commented Apr 20, 2017 •

edited

Loading

lrytz Apr 20, 2017 •

edited

Loading