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):

(defn x [y] (try (finally (let [z y]))))

produces

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":

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.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.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

Attachments

3
  • 21 Sep 2015, 04:43 PM
  • 18 Sep 2015, 04:41 PM
  • 31 Aug 2015, 05:30 AM

Activity

Show:

Rich Hickey October 26, 2015 at 9:00 PM

Thanks for chasing this down Nicola.

Fogus October 9, 2015 at 6:35 PM

With some digging I was able to determine the problem and how the solution works to fix that problem. In the future, whenever reporting bytecode verification errors it might help to show the equivalent Java code pertaining to the problemmatic bytecode.

Alex Miller September 18, 2015 at 4:41 PM

clj-1809-2.patch is identical to prior patch, just updated to apply to current master.

Nicola Mometto September 11, 2015 at 6:50 PM

yeah that's how the bug originally got reported and that's the function I used to find a minimal reproducible example

import September 11, 2015 at 6:43 PM

Comment made by: zentrope

Here's where the error is thrown:

https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/util.cljc#L142-L155

(defn last-modified [src] (cond (file? src) (.lastModified ^File src) (url? src) (let [conn (.openConnection ^URL src)] (try (.getLastModified conn) (finally (let [ins (.getInputStream conn)] (when ins (.close ins)))))) :else (throw (IllegalArgumentException. (str "Cannot get last modified for " src)))))
Completed

Details

Assignee

Reporter

Approval

Patch

Priority

Affects versions

Fix versions

Created August 31, 2015 at 4:18 AM
Updated October 27, 2015 at 7:06 PM
Resolved October 27, 2015 at 7:06 PM