On Thu, Jul 31, 2008 at 02:01:08PM -0700, Rich Hickey wrote: > > > On Jul 31, 11:43 am, Stefan Ring <[email protected]> wrote: > > Thanks a lot. Your change makes the CACAO verifier happy but I think > > you should revert it. > > > > The instruction in question is actually invokeinterface, not > > invokevirtual. After reading the specification for invokeinterface > > (esp. "Runtime Exceptions"), I have the strong impression that clojure > > is right and CACAO is wrong. > > > > Yes, invokeinterface, sorry. > > Ok, I've backed it out.
Although the current version of clojure does generate technically valid bytecode with respect to invokeinterface, I would like to recommend that it be modified to emit a checkcast. The ability of invokeinterface to accept a target of type Object is the result of a relaxation in the strictness of the verifier which was presumed to be necessary in certain special cases, as described further in [1]. The paper also explains why this relaxation is unnecessary, and proposes a stricter verifier. Even with the specification's (and hotspot's) less strict verifier, the use of invokeinterface without a checkcast means that runtime type errors in interface calls result in an IncompatibleClassChangeError rather than the more conventional ClassCastException, which may lead to confusion. Background: I am packaging clojure for Debian, which uses gij as its default JVM. gij uses the stricter verifier, and rejects clojure generated code with a stack trace similar to that shown in [2]. I am attaching a patch which modifies clojure to emit a checkcast wherever it is required by a strict verifier. Thanks, -- Peter [1] http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.22.1261&rep=rep1&type=pdf [2] http://server.complang.tuwien.ac.at/cgi-bin/bugzilla/show_bug.cgi?id=82#c1
From 9c991e251cbc27947737445847a4103b371737ae Mon Sep 17 00:00:00 2001 From: Peter Collingbourne <[email protected]> Date: Mon, 29 Dec 2008 01:40:19 +0000 Subject: [PATCH] Generate bytecode compatible with stricter JVMs (e.g. gij) --- src/clj/clojure/core_proxy.clj | 2 ++ src/clj/clojure/genclass.clj | 3 +++ src/jvm/clojure/lang/Compiler.java | 6 ++---- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/clj/clojure/core_proxy.clj b/src/clj/clojure/core_proxy.clj index 8a77734..fc20cf7 100644 --- a/src/clj/clojure/core_proxy.clj +++ b/src/clj/clojure/core_proxy.clj @@ -62,6 +62,7 @@ (. gen (dup)) (. gen (ifNull else-label)) ;if found + (. gen (checkCast ifn-type)) (. gen (loadThis)) ;box args (dotimes [i (count ptypes)] @@ -125,6 +126,7 @@ (. gen (loadThis)) (. gen (dup)) (. gen (getField ctype fmap imap-type)) + (. gen (checkCast (totype clojure.lang.IPersistentCollection))) (. gen (loadArgs)) (. gen (invokeInterface (totype clojure.lang.IPersistentCollection) (. Method (getMethod "clojure.lang.IPersistentCollection cons(Object)")))) diff --git a/src/clj/clojure/genclass.clj b/src/clj/clojure/genclass.clj index e293fe0..e8f55b6 100644 --- a/src/clj/clojure/genclass.clj +++ b/src/clj/clojure/genclass.clj @@ -181,6 +181,7 @@ (when is-overload (. gen (mark found-label))) ;if found + (. gen (checkCast ifn-type)) (when-not as-static (. gen (loadThis))) ;box args @@ -274,6 +275,7 @@ (emit-get-var gen init-name) (. gen dup) (. gen ifNull no-init-label) + (. gen (checkCast ifn-type)) ;box init args (dotimes [i (count pclasses)] (. gen (loadArg i)) @@ -386,6 +388,7 @@ (emit-get-var gen main-name) (. gen dup) (. gen ifNull no-main-label) + (. gen (checkCast ifn-type)) (. gen loadArgs) (. gen (invokeStatic rt-type (. Method (getMethod "clojure.lang.ISeq seq(Object)")))) (. gen (invokeInterface ifn-type (new Method "applyTo" obj-type diff --git a/src/jvm/clojure/lang/Compiler.java b/src/jvm/clojure/lang/Compiler.java index bb62536..961e16a 100644 --- a/src/jvm/clojure/lang/Compiler.java +++ b/src/jvm/clojure/lang/Compiler.java @@ -1155,8 +1155,7 @@ static class InstanceMethodExpr extends MethodExpr{ { Type type = Type.getType(method.getDeclaringClass()); target.emit(C.EXPRESSION, fn, gen); - if(!method.getDeclaringClass().isInterface()) - gen.checkCast(type); + gen.checkCast(type); MethodExpr.emitTypedArgs(fn, gen, method.getParameterTypes(), args); if(context == C.RETURN) { @@ -1179,8 +1178,7 @@ static class InstanceMethodExpr extends MethodExpr{ { Type type = Type.getType(method.getDeclaringClass()); target.emit(C.EXPRESSION, fn, gen); - if(!method.getDeclaringClass().isInterface()) - gen.checkCast(type); + gen.checkCast(type); MethodExpr.emitTypedArgs(fn, gen, method.getParameterTypes(), args); if(context == C.RETURN) { -- 1.5.6.5
signature.asc
Description: Digital signature
