From 00342452e2e543452f91fcd7bd2b75e5d7c9ecd4 Mon Sep 17 00:00:00 2001 From: Benedikt Meurer Date: Mon, 30 Oct 2017 14:16:44 +0100 Subject: [PATCH] Fix OOB string character access in Printer#_maybeAddParen. (#6589) * Fix OOB string character access in Printer#_maybeAddParen. The `_maybeAddParen` method of the `Printer` class does ```js const chaPost = str[i + 1] ``` without checking that `i + 1` is still within the bounds of `str`. It seems like this triggers fairly often that the `str[i + 1]` access is out of bounds. The first out of bounds access will turn the KeyedLoadIC (in case of V8) into *MEGAMORPHIC* state, which is significantly slower for strings (there's a fix in flight for V8 to mitigate the cost a bit in that case). Even worse than that, the out of bounds access also pollutes the later comparisons, namely ```js chaPost === "/" ``` and ```js chaPost === "*" ``` which are now no longer monomorphic on strings, since `chaPost` was sometimes `undefined`. This is a non-breaking performance fix, which improves babel execution on the [web-tooling-benchmark](github.com/v8/web-tooling-benchmark) workload by around 6-9%. * Restructure and optimize the code a bit. --- packages/babel-generator/src/printer.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/babel-generator/src/printer.js b/packages/babel-generator/src/printer.js index d550e6635c..a1b7f82eca 100644 --- a/packages/babel-generator/src/printer.js +++ b/packages/babel-generator/src/printer.js @@ -245,15 +245,17 @@ export default class Printer { for (i = 0; i < str.length && str[i] === " "; i++) continue; if (i === str.length) return; + // Check for newline or comment. const cha = str[i]; - const chaPost = str[i + 1]; - - // Check for newline or comment - if (cha === "\n" || (cha === "/" && (chaPost === "/" || chaPost === "*"))) { - this.token("("); - this.indent(); - parenPushNewlineState.printed = true; + if (cha !== "\n") { + if (cha !== "/") return; + if (i + 1 === str.length) return; + const chaPost = str[i + 1]; + if (chaPost !== "/" && chaPost !== "*") return; } + this.token("("); + this.indent(); + parenPushNewlineState.printed = true; } _catchUp(prop: string, loc: Object) {