facebook / hermes

A JavaScript engine optimized for running React Native.
https://hermesengine.dev/
MIT License
9.91k stars 641 forks source link

Feature Request: local `eval` for SES (Secure EcmaScript) support #957

Open leotm opened 1 year ago

leotm commented 1 year ago

Problem

Hey there, Leo here from team LavaMoat and MetaMask πŸ‘‹

i'm working with the folks at EndoJS bringing SES (formerly Agoric/SES) support to RN and metamask-mobile

which runs on JSC and V8, but not Hermes as we know

Excluded From Support

  • Local mode eval() (use and introduce local variables)
  • with statements
# RN: Android/iOS (Hermes) with SES
ERROR  TypeError: SES cannot initialize unless 'eval' is the original intrinsic 'eval',
suitable for direct-eval (dynamically scoped eval) (SES_DIRECT_EVAL), js engine: hermes
# https://user-images.githubusercontent.com/1881059/232474795-666c83e9-53aa-4df0-a8cd-a5e45277392d.png
# https://user-images.githubusercontent.com/1881059/232477719-56c92715-b72d-4c5d-9756-a972cd88c6b1.png

stacktraces ^ from the SES lockdown shim (e.g. curl -O https://npmfs.com/download/ses/0.18.4/dist/lockdown.umd.js) also importable via yarn add ses, but needing additional Metro and Babel config for .cjs

i note potentially supporting this feature's been discussed in the past (cc @kumavis) and past convo's

is this a feature you guys looking to make happen anytime soon? or still only accepting community contributions

Solution

Describe the solution you'd like to happen: local eval() support πŸ™ Or with statements support (can be split into separate ft req)

Alternatives considered

Additional Context

SES

https://github.com/endojs/endo/blob/master/packages/ses/error-codes/SES_DIRECT_EVAL.md

The SES Hardened JavaScript shim captures the eval function when it is initialized. The eval function it finds must be the original eval because SES uses its dynamic scope to implement its isolated eval.

If you see this error, something running before ses initialized, most likely another instance of ses, has replaced eval with something else.

React Native LavaMoat tracker

Hermes docs

hermesengine.dev/playground

# eval(1)

/tmp/hermes-input.js:1:1: warning: Direct call to eval(), but lexical scope is not supported.

Function<global>(1 params, 1 registers, 0 symbols):
Offset in debug table: source 0x0000, lexical 0x0000
    LoadConstUInt8    r0, 1
    DirectEval        r0, r0
    Ret               r0
tmikov commented 1 year ago

We have planned support for local eval() in one of the next major releases of Hermes. However please note that the performance will suffer greatly.

leotm commented 1 year ago

that's great appreciate the update ^ and noted RE the perf consideration then once released will monitor when lands in future RN core version (unless by then settled on alt solution that doesn't require eval or with)

leotm commented 1 year ago

sry @tmikov couple more questions, are there any plans on Module Harmony / Compartments / Lockdown / Harden? thought better asking here than spamming multiple ft requests

since we're only using/wanting direct eval to build Compartments if you'd be happier supporting Compartments rather than direct eval? (hopefully w/o the perf hit) or alternatively to Compartment the constituent elements? like import source, virtual Module and confined Evaluators

tmikov commented 1 year ago

@leotm to my embarrassment, I have to profess my ignorance and admit that I haven't really followed SES closely. Frankly, at this point Hermes has gaps in EcmaScript compliance that have to be dealt with first, so Compartments and others that are still in the proposal stage haven't been on our radar.

We need local eval, so we will implement it in any case, but only the strict mode version (to be precise: local eval will work in non-strict mode, but it can't introduce variables into the surrounding scope).

We have no plans to support non-strict local eval or with - unfortunately they are incompatible with our code generation strategy and bytecode instructions. It is theoretically possible to implement them on top of JS objects with the existing bytecode, but the effort would be significant and the performance truly appalling. Is with required by your use case?

Fundamentally, Hermes was designed to be a minimalistic and efficient AOT compiler. We work best when we can access all source ahead of time, when we have static visibility into the module system (for cross-module inlining), etc. Very dynamic runtime behaviors are not really compatible with our charter. Early on we decided that we can't support every use case, because then we would just be re-implementing v8, badly. In order to offer a meaningful alternative to mainstream engines, we have to focus on the use cases where we can provide an improvement (fast startup, small binary, less memory, etc). In the next major version we will also offer a dramatic perf improvement for a certain set of use cases, but not all.

I took a quick look at https://github.com/tc39/proposal-compartments. A very cursory read makes me believe that compartments require a great deal of runtime dynamic behavior. Is that really the case?

leotm commented 1 year ago

πŸ˜„ gotcha makes sense RE the EcmaScript compliance priorities ^ still good news for local eval and thanks for clarifying the non-strict mode behaviour, yes with is required

but we have an idea (proposal of a proposal?) without with

leotm commented 1 year ago

I took a quick look at https://github.com/tc39/proposal-compartments. A very cursory read makes me believe that compartments require a great deal of runtime dynamic behavior. Is that really the case?

cc @erights @kriskowal will be able to answer better than i can

tmikov commented 1 year ago

FWIW, I think we might be able to add support for with. It feels wrong to completely ignore a language feature that has been with us for many years. It also feels good to pass most of test262. Plus, the implementation is interesting and will not slow everything down. But I can't promise it is high priority.

leotm commented 1 year ago

many years in the deprecation πŸ˜… but yes covering most of test262 would :feelsgood: indeed priority noted ^ and great the mid/low effort won't slow the roadmap down too much thx again for the earlier Hermes history/design insight, the RN EU doc was helpful too