From cf4bf4ceb1eee86197d51e77e640e59ca04739b8 Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Thu, 16 Mar 2017 01:02:59 +0800 Subject: [PATCH] fix stack issues with `AST_Node.evaluate()` (#1603) As patched in #1597, `make_node_from_constant()` makes inconsistent and sometimes incorrect calls to `optimize()` and `transform()`. Fix those issues properly by changing the semantics of `evaluate()` and `make_node_from_constant()`, with the side effect that `evaluate()` no longer eagerly converts constant to `AST_Node`. --- lib/compress.js | 261 ++++++++++++++++++++++++++---------------------- 1 file changed, 141 insertions(+), 120 deletions(-) diff --git a/lib/compress.js b/lib/compress.js index 59a96684..b3004fb5 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -98,10 +98,10 @@ function Compressor(options, false_by_default) { this.top_retain = function(def) { return top_retain.test(def.name); }; - } else if (typeof top_retain === "function") { + } else if (typeof top_retain == "function") { this.top_retain = top_retain; } else if (top_retain) { - if (typeof top_retain === "string") { + if (typeof top_retain == "string") { top_retain = top_retain.split(/,/); } this.top_retain = function(def) { @@ -151,7 +151,17 @@ merge(Compressor.prototype, { node = node.hoist_declarations(this); was_scope = true; } + // Before https://github.com/mishoo/UglifyJS2/pull/1602 AST_Node.optimize() + // would call AST_Node.transform() if a different instance of AST_Node is + // produced after OPT(). + // This corrupts TreeWalker.stack, which cause AST look-ups to malfunction. + // Migrate and defer all children's AST_Node.transform() to below, which + // will now happen after this parent AST_Node has been properly substituted + // thus gives a consistent AST snapshot. descend(node, this); + // Existing code relies on how AST_Node.optimize() worked, and omitting the + // following replacement call would result in degraded efficiency of both + // output and performance. descend(node, this); var opt = node.optimize(this); if (was_scope && opt instanceof AST_Scope) { @@ -384,7 +394,7 @@ merge(Compressor.prototype, { return new ctor(props); }; - function make_node_from_constant(compressor, val, orig) { + function make_node_from_constant(val, orig) { switch (typeof val) { case "string": return make_node(AST_String, orig, { @@ -404,9 +414,9 @@ merge(Compressor.prototype, { return make_node(AST_Number, orig, { value: val }); case "boolean": - return make_node(val ? AST_True : AST_False, orig).optimize(compressor); + return make_node(val ? AST_True : AST_False, orig); case "undefined": - return make_node(AST_Undefined, orig).transform(compressor); + return make_node(AST_Undefined, orig); default: if (val === null) { return make_node(AST_Null, orig, { value: null }); @@ -1198,11 +1208,11 @@ merge(Compressor.prototype, { } } }); - function to_node(compressor, value, orig) { + function to_node(value, orig) { if (value instanceof AST_Node) return make_node(value.CTOR, orig, value); if (Array.isArray(value)) return make_node(AST_Array, orig, { elements: value.map(function(value) { - return to_node(compressor, value, orig); + return to_node(value, orig); }) }); if (value && typeof value == "object") { @@ -1210,14 +1220,14 @@ merge(Compressor.prototype, { for (var key in value) { props.push(make_node(AST_ObjectKeyVal, orig, { key: key, - value: to_node(compressor, value[key], orig) + value: to_node(value[key], orig) })); } return make_node(AST_Object, orig, { properties: props }); } - return make_node_from_constant(compressor, value, orig); + return make_node_from_constant(value, orig); } def(AST_Node, noop); def(AST_Dot, function(compressor, suffix){ @@ -1228,7 +1238,7 @@ merge(Compressor.prototype, { var name; var defines = compressor.option("global_defs"); if (defines && HOP(defines, (name = this.name + suffix))) { - var node = to_node(compressor, defines[name], this); + var node = to_node(defines[name], this); var top = compressor.find_parent(AST_Toplevel); node.walk(new TreeWalker(function(node) { if (node instanceof AST_SymbolRef) { @@ -1243,45 +1253,40 @@ merge(Compressor.prototype, { node.DEFMETHOD("_find_defs", func); }); - function best_of(ast1, ast2) { + function best_of_expression(ast1, ast2) { return ast1.print_to_string().length > ast2.print_to_string().length ? ast2 : ast1; } function best_of_statement(ast1, ast2) { - return best_of(make_node(AST_SimpleStatement, ast1, { + return best_of_expression(make_node(AST_SimpleStatement, ast1, { body: ast1 }), make_node(AST_SimpleStatement, ast2, { body: ast2 })).body; } + function best_of(compressor, ast1, ast2) { + return (first_in_statement(compressor) ? best_of_statement : best_of_expression)(ast1, ast2); + } + // methods to evaluate a constant expression (function (def){ - // The evaluate method returns an array with one or two - // elements. If the node has been successfully reduced to a - // constant, then the second element tells us the value; - // otherwise the second element is missing. The first element - // of the array is always an AST_Node descendant; if - // evaluation was successful it's a node that represents the - // constant; otherwise it's the original or a replacement node. + // If the node has been successfully reduced to a constant, + // then its value is returned; otherwise the element itself + // is returned. + // They can be distinguished as constant value is never a + // descendant of AST_Node. AST_Node.DEFMETHOD("evaluate", function(compressor){ - if (!compressor.option("evaluate")) return [ this ]; - var val; + if (!compressor.option("evaluate")) return this; try { - val = this._eval(compressor); + var val = this._eval(compressor); + return !val || val instanceof RegExp || typeof val != "object" ? val : this; } catch(ex) { if (ex !== def) throw ex; - return [ this ]; - } - var node; - try { - node = make_node_from_constant(compressor, val, this); - } catch(ex) { - return [ this ]; + return this; } - return [ best_of(node, this), val ]; }); var unaryPrefix = makePredicate("! ~ - +"); AST_Node.DEFMETHOD("is_constant", function(){ @@ -1319,8 +1324,8 @@ merge(Compressor.prototype, { })); } var result = this.evaluate(compressor); - if (result.length > 1) { - return result[1]; + if (result !== this) { + return result; } throw new Error(string_template("Cannot evaluate constant [{file}:{line},{col}]", this.start)); }); @@ -1480,9 +1485,9 @@ merge(Compressor.prototype, { var stat = make_node(AST_SimpleStatement, alt, { body: alt }); - return best_of(negated, stat) === stat ? alt : negated; + return best_of_expression(negated, stat) === stat ? alt : negated; } - return best_of(negated, alt); + return best_of_expression(negated, alt); } def(AST_Node, function(){ return basic_negation(this); @@ -2233,27 +2238,24 @@ merge(Compressor.prototype, { }); OPT(AST_DWLoop, function(self, compressor){ - var cond = self.condition.evaluate(compressor); - self.condition = cond[0]; if (!compressor.option("loops")) return self; - if (cond.length > 1) { - if (cond[1]) { + var cond = self.condition.evaluate(compressor); + if (cond !== self.condition) { + if (cond) { return make_node(AST_For, self, { body: self.body }); - } else if (self instanceof AST_While) { - if (compressor.option("dead_code")) { - var a = []; - extract_declarations_from_unreachable_code(compressor, self.body, a); - return make_node(AST_BlockStatement, self, { body: a }); - } + } else if (compressor.option("dead_code") && self instanceof AST_While) { + var a = []; + extract_declarations_from_unreachable_code(compressor, self.body, a); + return make_node(AST_BlockStatement, self, { body: a }); } else { - // self instanceof AST_Do - return self; + cond = make_node_from_constant(cond, self.condition).transform(compressor); + self.condition = best_of_expression(cond, self.condition); } } if (self instanceof AST_While) { - return make_node(AST_For, self, self).transform(compressor); + return make_node(AST_For, self, self).optimize(compressor); } return self; }); @@ -2275,7 +2277,7 @@ merge(Compressor.prototype, { var first = self.body instanceof AST_BlockStatement ? self.body.body[0] : self.body; if (first instanceof AST_If) { if (first.body instanceof AST_Break - && compressor.loopcontrol_target(first.body.label) === self) { + && compressor.loopcontrol_target(first.body.label) === compressor.self()) { if (self.condition) { self.condition = make_node(AST_Binary, self.condition, { left: self.condition, @@ -2288,7 +2290,7 @@ merge(Compressor.prototype, { drop_it(first.alternative); } else if (first.alternative instanceof AST_Break - && compressor.loopcontrol_target(first.alternative.label) === self) { + && compressor.loopcontrol_target(first.alternative.label) === compressor.self()) { if (self.condition) { self.condition = make_node(AST_Binary, self.condition, { left: self.condition, @@ -2304,27 +2306,25 @@ merge(Compressor.prototype, { }; OPT(AST_For, function(self, compressor){ - var cond = self.condition; - if (cond) { - cond = cond.evaluate(compressor); - self.condition = cond[0]; - } if (!compressor.option("loops")) return self; - if (cond) { - if (cond.length > 1 && !cond[1]) { - if (compressor.option("dead_code")) { - var a = []; - if (self.init instanceof AST_Statement) { - a.push(self.init); - } - else if (self.init) { - a.push(make_node(AST_SimpleStatement, self.init, { - body: self.init - })); - } - extract_declarations_from_unreachable_code(compressor, self.body, a); - return make_node(AST_BlockStatement, self, { body: a }); + if (self.condition) { + var cond = self.condition.evaluate(compressor); + if (compressor.option("dead_code") && !cond) { + var a = []; + if (self.init instanceof AST_Statement) { + a.push(self.init); + } + else if (self.init) { + a.push(make_node(AST_SimpleStatement, self.init, { + body: self.init + })); } + extract_declarations_from_unreachable_code(compressor, self.body, a); + return make_node(AST_BlockStatement, self, { body: a }); + } + if (cond !== self.condition) { + cond = make_node_from_constant(cond, self.condition).transform(compressor); + self.condition = best_of_expression(cond, self.condition); } } if_break_in_loop(self, compressor); @@ -2340,9 +2340,8 @@ merge(Compressor.prototype, { // “has no side effects”; also it doesn't work for cases like // `x && true`, though it probably should. var cond = self.condition.evaluate(compressor); - self.condition = cond[0]; - if (cond.length > 1) { - if (cond[1]) { + if (cond !== self.condition) { + if (cond) { compressor.warn("Condition always true [{file}:{line},{col}]", self.condition.start); if (compressor.option("dead_code")) { var a = []; @@ -2361,6 +2360,8 @@ merge(Compressor.prototype, { return make_node(AST_BlockStatement, self, { body: a }).optimize(compressor); } } + cond = make_node_from_constant(cond, self.condition).transform(compressor); + self.condition = best_of_expression(cond, self.condition); } var negated = self.condition.negate(compressor); var self_condition_length = self.condition.print_to_string().length; @@ -2488,12 +2489,12 @@ merge(Compressor.prototype, { } break; } - var exp = self.expression.evaluate(compressor); - out: if (exp.length == 2) try { + var value = self.expression.evaluate(compressor); + out: if (value !== self.expression) try { // constant expression - self.expression = exp[0]; + var expression = make_node_from_constant(value, self.expression); + self.expression = best_of_expression(expression, self.expression); if (!compressor.option("dead_code")) break out; - var value = exp[1]; var in_if = false; var in_block = false; var started = false; @@ -2540,11 +2541,11 @@ merge(Compressor.prototype, { if (stopped) return MAP.skip; if (node instanceof AST_Case) { var exp = node.expression.evaluate(compressor); - if (exp.length < 2) { + if (exp === node.expression) { // got a case with non-constant expression, baling out throw self; } - if (exp[1] === value || started) { + if (exp === value || started) { started = true; if (aborts(node)) stopped = true; descend(node, this); @@ -2758,15 +2759,14 @@ merge(Compressor.prototype, { var separator; if (self.args.length > 0) { separator = self.args[0].evaluate(compressor); - if (separator.length < 2) break EXIT; // not a constant - separator = separator[1]; + if (separator === self.args[0]) break EXIT; // not a constant } var elements = []; var consts = []; exp.expression.elements.forEach(function(el) { - el = el.evaluate(compressor); - if (el.length > 1) { - consts.push(el[1]); + var value = el.evaluate(compressor); + if (value !== el) { + consts.push(value); } else { if (consts.length > 0) { elements.push(make_node(AST_String, self, { @@ -2774,7 +2774,7 @@ merge(Compressor.prototype, { })); consts.length = 0; } - elements.push(el[0]); + elements.push(el); } }); if (consts.length > 0) { @@ -2815,7 +2815,7 @@ merge(Compressor.prototype, { node.expression = node.expression.clone(); node.expression.expression = node.expression.expression.clone(); node.expression.expression.elements = elements; - return best_of(self, node); + return best_of(compressor, self, node); } } if (exp instanceof AST_Function) { @@ -2961,8 +2961,7 @@ merge(Compressor.prototype, { return e.expression; } if (e instanceof AST_Binary) { - var statement = first_in_statement(compressor); - self = (statement ? best_of_statement : best_of)(self, e.negate(compressor, statement)); + self = best_of(compressor, self, e.negate(compressor, first_in_statement(compressor))); } break; case "typeof": @@ -2975,7 +2974,15 @@ merge(Compressor.prototype, { }).optimize(compressor); } } - return self.evaluate(compressor)[0]; + // avoids infinite recursion of numerals + if (self.operator != "-" || !(self.expression instanceof AST_Number)) { + var ev = self.evaluate(compressor); + if (ev !== self) { + ev = make_node_from_constant(ev, self).optimize(compressor); + return best_of(compressor, ev, self); + } + } + return self; }); function has_side_effects_or_prop_access(node, compressor) { @@ -3097,48 +3104,48 @@ merge(Compressor.prototype, { case "&&": var ll = self.left.evaluate(compressor); var rr = self.right.evaluate(compressor); - if ((ll.length > 1 && !ll[1]) || (rr.length > 1 && !rr[1])) { + if (!ll || !rr) { compressor.warn("Boolean && always false [{file}:{line},{col}]", self.start); return make_node(AST_Seq, self, { car: self.left, cdr: make_node(AST_False, self) }).optimize(compressor); } - if (ll.length > 1 && ll[1]) { - return rr[0]; + if (ll !== self.left && ll) { + return self.right.optimize(compressor); } - if (rr.length > 1 && rr[1]) { - return ll[0]; + if (rr !== self.right && rr) { + return self.left.optimize(compressor); } break; case "||": var ll = self.left.evaluate(compressor); var rr = self.right.evaluate(compressor); - if ((ll.length > 1 && ll[1]) || (rr.length > 1 && rr[1])) { + if (ll !== self.left && ll || rr !== self.right && rr) { compressor.warn("Boolean || always true [{file}:{line},{col}]", self.start); return make_node(AST_Seq, self, { car: self.left, cdr: make_node(AST_True, self) }).optimize(compressor); } - if (ll.length > 1 && !ll[1]) { - return rr[0]; + if (!ll) { + return self.right.optimize(compressor); } - if (rr.length > 1 && !rr[1]) { - return ll[0]; + if (!rr) { + return self.left.optimize(compressor); } break; case "+": var ll = self.left.evaluate(compressor); var rr = self.right.evaluate(compressor); - if (ll.length > 1 && ll[0] instanceof AST_String && ll[1]) { + if (ll && typeof ll == "string") { compressor.warn("+ in boolean context always true [{file}:{line},{col}]", self.start); return make_node(AST_Seq, self, { car: self.right, cdr: make_node(AST_True, self) }).optimize(compressor); } - if (rr.length > 1 && rr[0] instanceof AST_String && rr[1]) { + if (rr && typeof rr == "string") { compressor.warn("+ in boolean context always true [{file}:{line},{col}]", self.start); return make_node(AST_Seq, self, { car: self.left, @@ -3150,12 +3157,11 @@ merge(Compressor.prototype, { if (compressor.option("comparisons") && self.is_boolean()) { if (!(compressor.parent() instanceof AST_Binary) || compressor.parent() instanceof AST_Assign) { - var statement = first_in_statement(compressor); var negated = make_node(AST_UnaryPrefix, self, { operator: "!", - expression: self.negate(compressor, statement) + expression: self.negate(compressor, first_in_statement(compressor)) }); - self = (statement ? best_of_statement : best_of)(self, negated); + self = best_of(compressor, self, negated); } if (compressor.option("unsafe_comps")) { switch (self.operator) { @@ -3307,9 +3313,9 @@ merge(Compressor.prototype, { }); if (self.right instanceof AST_Constant && !(self.left instanceof AST_Constant)) { - self = best_of(reversed, self); + self = best_of(compressor, reversed, self); } else { - self = best_of(self, reversed); + self = best_of(compressor, self, reversed); } } if (associative && self.is_number(compressor)) { @@ -3406,7 +3412,12 @@ merge(Compressor.prototype, { self.right = self.right.right; return self.transform(compressor); } - return self.evaluate(compressor)[0]; + var ev = self.evaluate(compressor); + if (ev !== self) { + ev = make_node_from_constant(ev, self).optimize(compressor); + return best_of(compressor, ev, self); + } + return self; }); OPT(AST_SymbolRef, function(self, compressor){ @@ -3421,11 +3432,11 @@ merge(Compressor.prototype, { && (!self.scope.uses_with || !compressor.find_parent(AST_With))) { switch (self.name) { case "undefined": - return make_node(AST_Undefined, self).transform(compressor); + return make_node(AST_Undefined, self).optimize(compressor); case "NaN": - return make_node(AST_NaN, self).transform(compressor); + return make_node(AST_NaN, self).optimize(compressor); case "Infinity": - return make_node(AST_Infinity, self).transform(compressor); + return make_node(AST_Infinity, self).optimize(compressor); } } if (compressor.option("evaluate") && compressor.option("reduce_vars")) { @@ -3433,12 +3444,14 @@ merge(Compressor.prototype, { if (d.fixed) { if (d.should_replace === undefined) { var init = d.fixed.evaluate(compressor); - if (init.length > 1) { - var value = init[0].print_to_string().length; + if (init !== d.fixed) { + init = make_node_from_constant(init, d.fixed).optimize(compressor); + init = best_of_expression(init, d.fixed); + var value = init.print_to_string().length; var name = d.name.length; var freq = d.references.length; var overhead = d.global || !freq ? 0 : (name + 2 + value) / freq; - d.should_replace = value <= name + overhead ? init[0] : false; + d.should_replace = value <= name + overhead ? init : false; } else { d.should_replace = false; } @@ -3509,8 +3522,8 @@ merge(Compressor.prototype, { return AST_Seq.cons(car, self); } var cond = self.condition.evaluate(compressor); - if (cond.length > 1) { - if (cond[1]) { + if (cond !== self.condition) { + if (cond) { compressor.warn("Condition always true [{file}:{line},{col}]", self.start); return maintain_this_binding(compressor.parent(), self, self.consequent); } else { @@ -3518,9 +3531,8 @@ merge(Compressor.prototype, { return maintain_this_binding(compressor.parent(), self, self.alternative); } } - var statement = first_in_statement(compressor); - var negated = cond[0].negate(compressor, statement); - if ((statement ? best_of_statement : best_of)(cond[0], negated) === negated) { + var negated = cond.negate(compressor, first_in_statement(compressor)); + if (best_of(compressor, cond, negated) === negated) { self = make_node(AST_Conditional, self, { condition: negated, consequent: self.alternative, @@ -3700,7 +3712,12 @@ merge(Compressor.prototype, { }); } } - return self.evaluate(compressor)[0]; + var ev = self.evaluate(compressor); + if (ev !== self) { + ev = make_node_from_constant(ev, self).optimize(compressor); + return best_of(compressor, ev, self); + } + return self; }); OPT(AST_Dot, function(self, compressor){ @@ -3739,13 +3756,17 @@ merge(Compressor.prototype, { break; } } - return self.evaluate(compressor)[0]; + var ev = self.evaluate(compressor); + if (ev !== self) { + ev = make_node_from_constant(ev, self).optimize(compressor); + return best_of(compressor, ev, self); + } + return self; }); function literals_in_boolean_context(self, compressor) { if (compressor.option("booleans") && compressor.in_boolean_context()) { - var best = first_in_statement(compressor) ? best_of_statement : best_of; - return best(self, make_node(AST_Seq, self, { + return best_of(compressor, self, make_node(AST_Seq, self, { car: self, cdr: make_node(AST_True, self) }).optimize(compressor)); -- 2.34.1