Node: deps: V8 6.1 get one phase of the optimization pipeline disabled

[vsemozhetbyt]

Refs: https://v8project.blogspot.com/2017/09/disabling-escape-analysis.html

Does this mean that Node.js 8 LTS will stick with a bit slower V8?

[addaleax]

Does this mean that Node.js 8 LTS will stick with deoptimized V8?

I think that is a question for @nodejs/v8.

[ofrobots]

This means that Escape Analysis will/should be disabled in Node 8 LTS, if it ships with either V8 6.1 or 6.0. Some code patterns that benefit from Escape Analysis would no longer be as optimized; still it better than shipping with a optimization known to be buggy. It would be good to get some real world performance data from the upcoming RCs to check the actual impact, but I don't expect it to be large.

At this point it seems impractical to wait for V8 6.2 to ship with Node 8 LTS.

[YurySolovyov]

Is there a CLI flag to disable it for 8.x to see how much slower it will become?

[vsemozhetbyt]

@YurySolovyov FWIW:

> node.8.5.0.v8-6.0.exe --v8-options | grep --regexp=escape.*analysis
  --use_escape_analysis (use hydrogen escape analysis)
  --trace_escape_analysis (trace hydrogen escape analysis)
  --escape_analysis_iterations (maximum number of escape analysis fix-point iterations)
  --turbo_escape (enable escape analysis)

> node.9.0.0.v8-6.1.20170919.nightly.exe --v8-options | grep --regexp=escape.*analysis
  --turbo_escape (enable escape analysis)



md5-dad4c34a640cbe721978ae682ea36395



```console
> node.9.0.0.v8-6.3.20170919.v8-canary.exe --v8-options | grep --regexp=escape.*analysis
  --turbo_escape (enable escape analysis)

[bmeurer]

It's not deoptimized. It's just that one phase of the optimization pipeline is disabled, and this bug only affects Chrome. Node can still turn it on (with an appropriate bugfix in place).

[vsemozhetbyt]

@bmeurer Thank you! I did not mean completely deoptimized, with "deoptimized in some realms" I meant this statement:

Specifically, the following ES2015 features might suffer temporary slowdowns:

• destructuring
• for-of iteration
• array spread
• rest parameters

[bmeurer]

What I was saying is that the term deoptimization means something different. It's not about a missing/disabled optimization, but rather deoptimization is the process of transitioning from optimized code back to unoptimized code when some condition changes (one which the optimizing compiler speculated).

[vsemozhetbyt]

Yes, sorry, I used the word in not terminological sense, sorry for the confusing.

+Refs: https://twitter.com/mathias/status/911344535929794560

[vsemozhetbyt]

So can we close this issue or should it wait till the bugfix applied on the Node.js side?

[gibfahn]

should it wait till the bugfix applied on the Node.js side?

This sounds like a good idea.

[jokeyrhyme]

@bmeurer

this bug only affects Chrome

This bug affects anyone running untrusted code, so a Node.js web server that uses eval() on incoming request data would also be affected, right? It's possible I've misunderstood this, too

[bmeurer]

For a Node.js web server that uses eval() the bug in the escape analysis is probably the least of your problems.

[jokeyrhyme]

@bmeurer agreed, but given how deep our dependency-tree can be, and the varying levels of experience within our community, I figure it's worth aiming for accuracy and awareness

I've definitely done silly things with eval() in the past, it's a trial-by-fire for all JavaScript developers :)

[jspenguin2017]

What about VM2?
https://github.com/patriksimek/vm2

[vsemozhetbyt]

Refs: https://github.com/nodejs/node/pull/15762#issuecomment-335100061

[hashseed]

I think we need some sort of formal threat model so that in the future it becomes easier to judge how severe a vulnerability is.

[MylesBorins]

/cc @nodejs/security-wg regarding @hashseed's above request for a formal threat model

[vdeturckheim]

@hashseed you mean having a way to monitor and log threats on a specific topic even if no vulnerabilities has been identified yet? I love the idea.

[hashseed]

More a way to quickly identify whether a particular bug or behavior classifies as security issue. For example, hash flooding can cause DOS attacks, which are security issues for Node.js, but not Chrome.

Chrome's threat model does not include that since DOS is easier to achieve by running an infinite loop, and only causes a dead tab. The user does not really suffer from this. The web site does. However, if certain input patterns can cause a Node.js process to lock up, that would not be acceptable.

The V8 team has operated under the assumption that being able to provide arbitrary code is not part of Node.js' threat model, because if you could do that, you already can do worse (like file system access). But apparently that is not the entire truth.

Having such a threat model of course also helps proactively identify possible security issues, or avoid hotspots for issues. That's for example why Chrome uses sandboxing - without knowing which syscalls may be dangerous, simply blocking all of them already helps, because syscalls in general can be dangerous.

Conversely, a XSS attack where script of a domain can access information from another, for example via embedded iframes (V8 context), would be a severe issue in Chrome. Chrome is implementing out-of-process-iframes for this. In Node.js, cross-context only happens via vm.runInContext, and I have no idea how severe a leak would be.

It also serves a common language during discussions.

[MylesBorins]

@hashseed do you have a copy of the formal threat model from the V8 team? Maybe we could use that as a starting point

edit: we should move threat model conversation to https://github.com/nodejs/security-wg/issues/51

[hashseed]

While V8 does not have one, Chromium does. There may be a more up-to-date document, but I found this: https://seclab.stanford.edu/websec/chromium/chromium-security-architecture.pdf

[sam-github]

The V8 team has operated under the assumption that being able to provide arbitrary code is not part of Node.js' threat model, because if you could do that, you already can do worse (like file system access). But apparently that is not the entire truth.

I think it is the truth, but perhaps I'm forgetting a detail, what has caused you to doubt this?

Have you read https://github.com/nodejs/node/blob/master/README.md#security recently (it has changed)?

[MylesBorins]

Closing as 8.x is now shipping with 6.2