[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
|Affects Version/s:||Release 1.8|
|Fix Version/s:||Release 1.8|
|Attachments:||0001-CLJ-1809-fix-off-by-one-error-in-direct-linking.patch clj-1809-2.patch clj-1809-3.patch|
|Patch:||Code and Test|
A variant of this issue showed up as it was preventing compilation in ClojureScript.
This is a simplified case (see original in comments):
See below for comparison bytecode.
Cause: In this code, there are locals with these indexes:
0 - this (if not static call)
The following block was added to FnExpr.parse() for static methods to adjust local binding stack indexes based on not having a "this":
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)
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).
Disabling the verifier, here's a dump of the emitted bytecode for inspection
Here's the bytecode emitted by clojure 1.7.0 for comparison
|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.
|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:
|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.