Compiler produces VerifyError when compiling simple let expression inside a finally block

Description

A variant of this issue showed up as it was preventing compilation in ClojureScript.

This is a simplified case (see original in comments):

1 2 3 4 (defn x [y] (try (finally (let [z y]))))

produces

1 VerifyError (class: user$x, method: invokeStatic signature: (Ljava/lang/Object;)Ljava/lang/Object;) Can only throw Throwable objects java.lang.Class.getDeclaredConstructors0 (Class.java:-2)

See below for comparison bytecode.

Cause: In this code, there are locals with these indexes:

0 - this (if not static call)
1 - y (arg)
2 - retLocal (created by compiler to hold result of try body)
3 - finalLocal (created by compiler to hold result of finally body)
4 - z (let local)

The following block was added to FnExpr.parse() for static methods to adjust local binding stack indexes based on not having a "this":

1 2 3 4 5 6 7 8 9 10 11 12 13 if(fn.canBeDirect){ for(FnMethod fm : (Collection<FnMethod>)methods) { if(fm.locals != null) { for(LocalBinding lb : (Collection<LocalBinding>)RT.keys(fm.locals)) { lb.idx -= 1; } fm.maxLocal -= 1; } } }

However, in this example locals 2 and 3 are never registered with fn (registerLocal is not called). This (doesn't) happen in TryExpr.parse() where retLocal and finallyLocal call getAndIncLocalNum() but not via registerLocal(). From a search, there are several other places where this happens as well.

The result in the example above is that we end up with the following indexes:

0 - y (arg)
2 - retLocal (created by compiler to hold result of try body)
3 - finalLocal (created by compiler to hold result of finally body)
3 - z (let local)

The overlap in the last 2 indices leads ultimately to the verifier error.

Approach: Make the lb.index adjustment only happen when lb.isArg - these should always be at the beginning of the locals table and therefore reducing their indexes will not affect any other added locals. Also, do not adjust fm.maxlocal (fyi, maxlocal is never used for anything).

Patch: clj-1809-3.patch

Disabling the verifier, here's a dump of the emitted bytecode for inspection

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 // 1.8 public static java.lang.Object invokeStatic(java.lang.Object); Code: 0: aconst_null 1: astore_1 2: aload_0 3: aconst_null 4: astore_0 5: astore_2 6: aconst_null 7: pop 8: goto 20 11: astore_2 12: aload_0 13: aconst_null 14: astore_0 15: astore_2 16: aconst_null 17: pop 18: aload_2 19: athrow 20: aload_1 21: areturn Exception table: from to target type 0 2 11 any

Here's the bytecode emitted by clojure 1.7.0 for comparison

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 // 1.7 public java.lang.Object invoke(java.lang.Object); Code: 0: aconst_null 1: astore_2 2: aload_1 3: aconst_null 4: astore_1 5: astore_3 6: aconst_null 7: pop 8: goto 22 11: astore 4 13: aload_1 14: aconst_null 15: astore_1 16: astore_3 17: aconst_null 18: pop 19: aload 4 21: athrow 22: aload_2 23: areturn Exception table: from to target type 0 2 11 any

Environment

None

Status

Assignee

Fogus

Reporter

Nicola Mometto

Labels

Approval

Ok

Patch

Code and Test

Fix versions

Affects versions

Release 1.8

Priority

Critical
Configure