[Pkg-javascript-commits] [uglifyjs] 110/228: fix stack issues with `AST_Node.evaluate()` (#1603)
Jonas Smedegaard
dr at jones.dk
Sat Apr 15 14:25:21 UTC 2017
This is an automated email from the git hooks/post-receive script.
js pushed a commit to branch master
in repository uglifyjs.
commit cf4bf4ceb1eee86197d51e77e640e59ca04739b8
Author: Alex Lam S.L <alexlamsl at gmail.com>
Date: Thu Mar 16 01:02:59 2017 +0800
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 59a9668..b3004fb 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));
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/uglifyjs.git
More information about the Pkg-javascript-commits
mailing list