<< Back to previous view

[CLJ-1809] Compiler produces VerifyError when compiling simple let expression inside a finally block Created: 30/Aug/15  Updated: 27/Oct/15  Resolved: 27/Oct/15

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.8
Fix Version/s: Release 1.8

Type: Defect Priority: Critical
Reporter: Nicola Mometto Assignee: Fogus
Resolution: Completed Votes: 0
Labels: compiler, regression

Attachments: Text File 0001-CLJ-1809-fix-off-by-one-error-in-direct-linking.patch     Text File clj-1809-2.patch     Text File clj-1809-3.patch    
Patch: Code and Test
Approval: Ok

 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


 Comments   
Comment by Nicola Mometto [ 30/Aug/15 11:30 PM ]

The attached patch fixes the issue however I'm unfamiliar with the direct linking support in the compiler so I'm not sure this is the right fix.

Comment by Keith Irwin [ 11/Sep/15 12:25 PM ]

I discovered this issue compiling ClojureScript applications using lein-figwheel, which invokes cljsbuild. Using just lein-cljsbuild produces the same issue.

Stack Trace:

Exception in thread "main" java.lang.VerifyError: (class: cljs/util$last_modified, method: invokeStatic signature: (Ljava/lang/Object;)Ljava/lang/Object;) Can only throw Throwable objects, compiling:(util.cljc:142:1)
	at clojure.lang.Compiler$DefExpr.eval(Compiler.java:463)
	at clojure.lang.Compiler.eval(Compiler.java:6939)
	at clojure.lang.Compiler.load(Compiler.java:7381)
	at clojure.lang.RT.loadResourceScript(RT.java:372)
	at clojure.lang.RT.loadResourceScript(RT.java:363)
	at clojure.lang.RT.load(RT.java:453)
	at clojure.lang.RT.load(RT.java:419)
	at clojure.core$load$fn__5661.invoke(core.clj:5883)
	at clojure.core$load.invokeStatic(core.clj:5882)
	at clojure.core$load_one.invokeStatic(core.clj:5683)
	at clojure.core$load_one.invoke(core.clj)
	at clojure.core$load_lib$fn__5610.invoke(core.clj:5728)
	at clojure.core$load_lib.invokeStatic(core.clj:5727)
	at clojure.core$load_lib.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$load_libs.invokeStatic(core.clj:5765)
	at clojure.core$load_libs.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$require.invokeStatic(core.clj:5787)
	at clojure.core$require.doInvoke(core.clj)
	at clojure.lang.RestFn.invoke(RestFn.java:2088)
	at cljs.repl$eval15$loading__5553__auto____16.invoke(repl.cljc:9)
	at cljs.repl$eval15.invokeStatic(repl.cljc:9)
	at cljs.repl$eval15.invoke(repl.cljc)
	at clojure.lang.Compiler.eval(Compiler.java:6934)
	at clojure.lang.Compiler.eval(Compiler.java:6923)
	at clojure.lang.Compiler.load(Compiler.java:7381)
	at clojure.lang.RT.loadResourceScript(RT.java:372)
	at clojure.lang.RT.loadResourceScript(RT.java:363)
	at clojure.lang.RT.load(RT.java:453)
	at clojure.lang.RT.load(RT.java:419)
	at clojure.core$load$fn__5661.invoke(core.clj:5883)
	at clojure.core$load.invokeStatic(core.clj:5882)
	at clojure.core$load_one.invokeStatic(core.clj:5683)
	at clojure.core$load_one.invoke(core.clj)
	at clojure.core$load_lib$fn__5610.invoke(core.clj:5728)
	at clojure.core$load_lib.invokeStatic(core.clj:5727)
	at clojure.core$load_lib.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$load_libs.invokeStatic(core.clj:5765)
	at clojure.core$load_libs.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$require.invokeStatic(core.clj:5787)
	at clojure.core$require.doInvoke(core.clj)
	at clojure.lang.RestFn.invoke(RestFn.java:3204)
	at figwheel_sidecar.repl$eval9$loading__5553__auto____10.invoke(repl.clj:1)
	at figwheel_sidecar.repl$eval9.invokeStatic(repl.clj:1)
	at figwheel_sidecar.repl$eval9.invoke(repl.clj)
	at clojure.lang.Compiler.eval(Compiler.java:6934)
	at clojure.lang.Compiler.eval(Compiler.java:6923)
	at clojure.lang.Compiler.load(Compiler.java:7381)
	at clojure.lang.RT.loadResourceScript(RT.java:372)
	at clojure.lang.RT.loadResourceScript(RT.java:363)
	at clojure.lang.RT.load(RT.java:453)
	at clojure.lang.RT.load(RT.java:419)
	at clojure.core$load$fn__5661.invoke(core.clj:5883)
	at clojure.core$load.invokeStatic(core.clj:5882)
	at clojure.core$load_one.invokeStatic(core.clj:5683)
	at clojure.core$load_one.invoke(core.clj)
	at clojure.core$load_lib$fn__5610.invoke(core.clj:5728)
	at clojure.core$load_lib.invokeStatic(core.clj:5727)
	at clojure.core$load_lib.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$load_libs.invokeStatic(core.clj:5765)
	at clojure.core$load_libs.doInvoke(core.clj)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invokeStatic(core.clj:647)
	at clojure.core$require.invokeStatic(core.clj:5787)
	at clojure.core$require.doInvoke(core.clj)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at user$eval5.invokeStatic(form-init6950918879748180251.clj:1)
	at user$eval5.invoke(form-init6950918879748180251.clj)
	at clojure.lang.Compiler.eval(Compiler.java:6934)
	at clojure.lang.Compiler.eval(Compiler.java:6923)
	at clojure.lang.Compiler.load(Compiler.java:7381)
	at clojure.lang.Compiler.loadFile(Compiler.java:7319)
	at clojure.main$load_script.invokeStatic(main.clj:275)
	at clojure.main$init_opt.invokeStatic(main.clj:277)
	at clojure.main$init_opt.invoke(main.clj)
	at clojure.main$initialize.invokeStatic(main.clj:308)
	at clojure.main$null_opt.invokeStatic(main.clj:342)
	at clojure.main$null_opt.invoke(main.clj)
	at clojure.main$main.invokeStatic(main.clj:421)
	at clojure.main$main.doInvoke(main.clj)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:383)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.lang.VerifyError: (class: cljs/util$last_modified, method: invokeStatic signature: (Ljava/lang/Object;)Ljava/lang/Object;) Can only throw Throwable objects
	at java.lang.Class.getDeclaredConstructors0(Native Method)
	at java.lang.Class.privateGetDeclaredConstructors(Class.java:2671)
	at java.lang.Class.getConstructor0(Class.java:3075)
	at java.lang.Class.newInstance(Class.java:412)
	at clojure.lang.Compiler$ObjExpr.eval(Compiler.java:4923)
	at clojure.lang.Compiler$DefExpr.eval(Compiler.java:450)
	... 93 more
Comment by Keith Irwin [ 11/Sep/15 12:39 PM ]

Reproducible using ClojureScript 1.7.122, but not 1.7.48 or 1.7.58.

Comment by Keith Irwin [ 11/Sep/15 12:43 PM ]

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)))))
Comment by Nicola Mometto [ 11/Sep/15 12:50 PM ]

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

Comment by Alex Miller [ 18/Sep/15 10:41 AM ]

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

Comment by Fogus [ 09/Oct/15 12: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.

Comment by Rich Hickey [ 26/Oct/15 3:00 PM ]

Thanks for chasing this down Nicola.

Generated at Sun May 19 15:34:03 CDT 2019 using JIRA 4.4#649-r158309.