299539 - Bad bytecode for function assignments

Status: VERIFIED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Igor Bukanov]

From a message that Mike C. posted to netscape.public.mozilla.jseng:

Hi,
I am trying to use the new refGet/refSet features in 1_6R1, but I get an
error when trying to load the generated class.

Example:
// file rassign.js
Session("foo") = "bar";

-------------------------
java -cp js.jar org.mozilla.javascript.tools.jsc.Main rassign.js

java -cp .;js.jar rassign

Exception in thread "main" java.lang.VerifyError: (class: rassign, method:
_c0 s
ignature:
(Lrassign;Lorg/mozilla/javascript/Context;Lorg/mozilla/javascript/Scri
ptable;Lorg/mozilla/javascript/Scriptable;[Ljava/lang/Object;)Ljava/lang/Obj
ect;
) Incompatible argument to function
-------------------------

If I decompile the generated class and I recompile it with Sun's javac
(after converting the DUP instructions in the function exec() to a Java
accepted call), the class gets loaded correctly.

Tested with JRE 1.4.2 and 1.5.

Comment 1

[Igor Bukanov]

Attachment: Fix

Comment 2

[Igor Bukanov]

I committed the fix

Comment 3

[Mike C.]

(In reply to comment #2)
> I committed the fix

Thanks for your quick response.

There's one more problem that leaves the Context.scratchRefTarget unpopped, I 
think in 
    ScriptRuntime.callRef() 
the call to 
    storeScriptable(cx, thisObj); 
should be changed to 
    ref.pushTarget(cx, ref, thisObj);
if later on in the generated class you'll use ref.popTarget().

Also you might want to mark this bug as related to 243057.

Comment 4

[Igor Bukanov]

(In reply to comment #3)
> (In reply to comment #2)
> > I committed the fix
> 
> Thanks for your quick response.
> 
> There's one more problem that leaves the Context.scratchRefTarget unpopped, I 
> think in 
>     ScriptRuntime.callRef() 
> the call to 
>     storeScriptable(cx, thisObj); 
> should be changed to 
>     ref.pushTarget(cx, ref, thisObj);
> if later on in the generated class you'll use ref.popTarget().

This is not applicable to the CVS head. After 1.6R1 the Ref was changed not to
pass the target argument there. It simplified the implementation for the price
of forcing Ref objects to store the target themselves.

Comment 5

[Igor Bukanov]

*** Bug 301488 has been marked as a duplicate of this bug. ***