Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strict mode in runtime #227

Merged
merged 10 commits into from
Sep 1, 2015
Merged

Strict mode in runtime #227

merged 10 commits into from
Sep 1, 2015

Conversation

tuchida
Copy link
Contributor

@tuchida tuchida commented Aug 7, 2015

Rhino implemented strict mode in parsing.

'use strict';
var eval = 1;
js: "test.js", line 2: "eval" is not a valid identifier for this use in strict mode.
js: var eval = 1;
js: ........^

But do not implement in runtime.

function a() {
  return arguments.caller;  // expected to throw exception, but not.
}

@tuchida
Copy link
Contributor Author

tuchida commented Aug 7, 2015

Other restriction and exceptions in runtime.

http://www.ecma-international.org/ecma-262/5.1/#sec-C

Arguments objects for strict mode functions do not dynamically share their array indexed property values with the corresponding formal parameter bindings of their functions. (10.6).

Strict mode eval code cannot instantiate variables or functions in the variable environment of the caller to eval. Instead, a new variable environment is created and that environment is used for declaration binding instantiation for the eval code (10.4.2).

If this is evaluated within strict mode code, then the this value is not coerced to an object. A this value of null or undefined is not converted to the global object and primitive values are not converted to wrapper objects. The this value passed via a function call (including calls made using Function.prototype.apply and Function.prototype.call) do not coerce the passed this value to an object (10.4.3, 11.1.1, 15.3.4.3, 15.3.4.4).

When a delete operator occurs within strict mode code, a TypeError is thrown if the property to be deleted has the attribute { [[Configurable]]:false } (11.4.1).

An implementation may not extend, beyond that defined in this specification, the meanings within strict mode functions of properties named caller or arguments of function instances. ECMAScript code may not create or modify properties with these names on function objects that correspond to strict mode functions (10.6, 13.2, 15.3.4.5.3).

@tuchida
Copy link
Contributor Author

tuchida commented Aug 7, 2015

May I delete the deprecated method?
https://github.com/mozilla/rhino/pull/227/files#diff-8f122a374db26c040ca0460f9c5b4541R3333
https://github.com/mozilla/rhino/pull/227/files#diff-8f122a374db26c040ca0460f9c5b4541R3449

For example, If a bytecode compiled with 1.7.7, do you need to be able to execute it in 1.7.8?

uchida_t added 9 commits August 10, 2015 12:11
ECMAScript 5.1
> The LeftHandSide also may not be a reference to a data property with
the attribute value {[[Writable]]:false}, to an accessor property with
the attribute value {[[Set]]:undefined}, nor to a non-existent property
of an object whose [[Extensible]] internal property has the value false.
In these cases a TypeError exception is thrown (11.13.1).
An accessor property do not have [[Writable]] attribute.

> 9.1.5.1 OrdinaryGetOwnProperty (O, P)
> 5. If X is a data property, then
>   a. Set D.[[Value]] to the value of X’s [[Value]] attribute.
>   b. Set D.[[Writable]] to the value of X’s [[Writable]] attribute
> 6. Else X is an accessor property, so
>   a. Set D.[[Get]] to the value of X’s [[Get]] attribute.
>   b. Set D.[[Set]] to the value of X’s [[Set]] attribute.
@gbrail
Copy link
Collaborator

gbrail commented Aug 25, 2015

Wow, let me test this out as best I can but in general and with the test262 test suite it looks basically good and long overdue -- so thanks!

AFAIK we have never said anything about what happens when you compile a class with one version of Rhino and use it with another. I also know that we have changed this internal interface in the past -- although we release a lot more often these days.

I wonder if we could put some version in the bytecode to make it easier to detect when this happens?

@gbrail
Copy link
Collaborator

gbrail commented Aug 27, 2015

I tried to build Trireme (https://github.com/apigee/trireme) with this patch and I immediately ran into a NullPointerException where one didn't exist before -- it seems that somewhere in the call chain, the pointer to "this" was getting lost.

I haven't been able to track this down, but perhaps you have some ideas...

Caused by: org.mozilla.javascript.WrappedException: Wrapped java.lang.NullPointerException (crypto.js#614)
at org.mozilla.javascript.Context.throwAsScriptRuntimeEx(Context.java:1896)
at org.mozilla.javascript.MemberBox.invoke(MemberBox.java:148)
at org.mozilla.javascript.FunctionObject.call(FunctionObject.java:387)
at org.mozilla.javascript.ScriptRuntime.applyOrCall(ScriptRuntime.java:2648)
at org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:287)
at org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:101)
at org.mozilla.javascript.optimizer.OptRuntime.call2(OptRuntime.java:42)
at io.apigee.trireme.node10.trireme.crypto._c_anonymous_44(crypto.js:614)
at io.apigee.trireme.node10.trireme.crypto.call(crypto.js)
at org.mozilla.javascript.optimizer.OptRuntime.callProp0(OptRuntime.java:85)
at org.mozilla.javascript.gen._Users_Apigee_src_noderunner_node10_node10src_target_test_classes_tests_cryptotests_js_2._c_anonymous_1(/Users/Apigee/src/noderunner/node10/node10src/target/test-classes/tests/cryptotests.js:68)
at org.mozilla.javascript.gen._Users_Apigee_src_noderunner_node10_node10src_target_test_classes_tests_cryptotests_js_2.call(/Users/Apigee/src/noderunner/node10/node10src/target/test-classes/tests/cryptotests.js)
at org.mozilla.javascript.ScriptRuntime.applyOrCall(ScriptRuntime.java:2648)
at org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:287)
at org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:101)
at org.mozilla.javascript.optimizer.OptRuntime.call2(OptRuntime.java:42)
at io.apigee.trireme.node10.node.module._c_anonymous_18(module.js:456)
at io.apigee.trireme.node10.node.module.call(module.js)
at org.mozilla.javascript.optimizer.OptRuntime.call2(OptRuntime.java:42)
at io.apigee.trireme.node10.node.module._c_anonymous_24(module.js:474)
at io.apigee.trireme.node10.node.module.call(module.js)
at org.mozilla.javascript.optimizer.OptRuntime.call2(OptRuntime.java:42)
at io.apigee.trireme.node10.node.module._c_anonymous_16(module.js:356)
at io.apigee.trireme.node10.node.module.call(module.js)
at org.mozilla.javascript.optimizer.OptRuntime.call1(OptRuntime.java:32)
at io.apigee.trireme.node10.node.module._c_anonymous_14(module.js:312)
at io.apigee.trireme.node10.node.module.call(module.js)
at org.mozilla.javascript.optimizer.OptRuntime.callN(OptRuntime.java:52)
at io.apigee.trireme.node10.node.module._c_anonymous_26(module.js:497)
at io.apigee.trireme.node10.node.module.call(module.js)
at org.mozilla.javascript.optimizer.OptRuntime.callProp0(OptRuntime.java:85)
at io.apigee.trireme.node10.main.trireme._c_startup_2(trireme.js:142)
at io.apigee.trireme.node10.main.trireme.call(trireme.js)
at org.mozilla.javascript.optimizer.OptRuntime.callName0(OptRuntime.java:74)
at io.apigee.trireme.node10.main.trireme._c_anonymous_1(trireme.js:923)
at io.apigee.trireme.node10.main.trireme.call(trireme.js)
at org.mozilla.javascript.ContextFactory.doTopCall(ContextFactory.java:395)
at org.mozilla.javascript.ScriptRuntime.doTopCall(ScriptRuntime.java:3358)
at io.apigee.trireme.node10.main.trireme.call(trireme.js)
at io.apigee.trireme.core.internal.ScriptRunner.runScript(ScriptRunner.java:762)
at io.apigee.trireme.core.internal.ScriptRunner$4.run(ScriptRunner.java:702)
at org.mozilla.javascript.Context.call(Context.java:504)
at org.mozilla.javascript.ContextFactory.call(ContextFactory.java:505)
at io.apigee.trireme.core.internal.ScriptRunner.call(ScriptRunner.java:697)
at io.apigee.trireme.core.ScriptFuture.run(ScriptFuture.java:183)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
at org.mozilla.javascript.ScriptableObject.getTopLevelScope(ScriptableObject.java:2196)
at org.mozilla.javascript.ScriptRuntime.setBuiltinProtoAndParent(ScriptRuntime.java:3762)
at org.mozilla.javascript.Context.newArray(Context.java:1678)
at io.apigee.trireme.core.modules.Crypto$CryptoImpl.getCiphers(Crypto.java:233)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:606)
at org.mozilla.javascript.MemberBox.invoke(MemberBox.java:126)
... 46 more

@tuchida
Copy link
Contributor Author

tuchida commented Aug 28, 2015

Thanks!

I tried to build Trireme. But I could not.

  1. git clone https://github.com/apigee/trireme.git

  2. Add to pom.xml

        <plugin>
          <artifactId>maven-compiler-plugin</artifactId>
          <configuration>
            <source>1.5</source>
            <target>1.5</target>
            <encoding>UTF-8</encoding>
          </configuration>
        </plugin>
  3. Use jdk1.7

    export JAVA_HOME='/path/to/jdk1.7.0_65'
  4. Use Rhino-trunk

    mvn install:install-file -Dfile=/path/to/rhino/buildGradle/libs/rhino-1.7.8-SNAPSHOT.jar -DgroupId=org.mozilla -DartifactId=rhino -Dversion=1.7.8 -Dpackaging=jar -DgeneratePom=true
         <dependency>
           <groupId>org.mozilla</groupId>
           <artifactId>rhino</artifactId>
    -        <version>1.7.7</version>
    +        <version>1.7.8</version>
       </dependency>
  5. mvn install

    org.mozilla.javascript.JavaScriptException: Error: Mismatch in valid_from: 4 8 17:22:11 2014 GMT !== Apr  8 17:22:11 2014 GMT (comparecerts.js#12)
    

    Would you share how to reproduce?

@tuchida
Copy link
Contributor Author

tuchida commented Aug 28, 2015

I found few deprecated method in ScriptRuntime.java. I thought they had been left for compatibility.

I also know that we have changed this internal interface in the past

In that case, I think it is no problem to deleted.

@gbrail
Copy link
Collaborator

gbrail commented Aug 28, 2015

Yeah, now that I re-test it looks like the regression was introduced by
some other change since 1.7.7. So this will take a little longer to fix I
am afraid.

On Thu, Aug 27, 2015 at 6:43 PM, tuchida [email protected] wrote:

I found few deprecated method in ScriptRuntime.java. I thought they had
been left for compatibility.

I also know that we have changed this internal interface in the past

In that case, I think it is no problem to deleted.


Reply to this email directly or view it on GitHub
#227 (comment).

Greg Brail | apigee https://apigee.com/ | twitter @gbrail
http://twitter.com/gbrail @apigee https://twitter.com/apigee
http://iloveapis.com/

@gbrail
Copy link
Collaborator

gbrail commented Aug 31, 2015

On the deprecation thing, I was thinking about the fact that this PR changes the format of compiled bytecode, so that bytecode compiled with this version won't work with older versions. But like I said, that has happened before.

I don't think we should remove additional "deprecated" functions from ScriptRuntime, however -- there is so much code out there that uses Rhino in unusual ways, I would be afraid of breaking it.

@gbrail gbrail merged commit e5a36df into mozilla:master Sep 1, 2015
@tuchida
Copy link
Contributor Author

tuchida commented Sep 1, 2015

Thanks for merge.

I don't think we should remove additional "deprecated" functions from ScriptRuntime

I see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants