572819 - more 4.5 esp fixes + analysis suite updates

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Ehren Metcalfe]

Attachment: treehydra fixes

The patches in bug 496418 needed fixing to handle real passes.

Comment 1

[Ehren Metcalfe]

Attachment: analysis suite updates

Updates for outparams.js, flow.js, and mayreturn.js.

Comment 2

[(dormant account)]

Comment on attachment 452019 [details] [diff] [review]
treehydra fixes

>diff --git a/libs/gcc_util.js b/libs/gcc_util.js
>--- a/libs/gcc_util.js
>+++ b/libs/gcc_util.js
>@@ -70,16 +70,35 @@ let function_decl_name = decl_name_strin
> 
> /** Return a string representation of the return type of the given
>  *  FUNCTION_DECL. */
> function function_decl_return_type_name(tree) {
>   let type = TREE_TYPE(TREE_TYPE(tree));
>   return type_string(type);
> }
> 
>+/** Return the expression returned by a given GIMPLE_RETURN or
>+ *  RETURN_EXPR. Return undefined with void returns. */
>+function return_expr(exp) {
>+  TREE_CHECK(exp, RETURN_EXPR, GIMPLE_RETURN);
>+
>+  let ret = exp.operands()[0];
>+  if (!ret)
>+    return undefined;
>+  switch (ret.tree_code()) {
>+  case INIT_EXPR:
>+  case GIMPLE_MODIFY_STMT:{
>+    let rhs = ret.operands()[1];
>+    return rhs;
>+  }
>+  default:
>+    return ret;
>+  }
>+}
>+
> /** Iterate over the basic blocks in the CFG. */
> function cfg_bb_iterator(cfg) {
>   let bb_entry = cfg.x_entry_block_ptr;
>   let bb_exit = cfg.x_exit_block_ptr;
>   let bb = bb_entry;
>   while (true) {
>     yield bb;
>     if (bb === bb_exit) break;
>@@ -397,31 +416,44 @@ function call_function_decl(expr) {
> /** The argument must be a CALL_EXPR. If it represents a call to a named
>  * function (not a method or function pointer), then return the name.
>  * Otherwise return undefined. */
> function call_function_name(expr) {
>   let decl = call_function_decl(expr)
>   return decl ? decl_name_string(decl) : undefined;
> }
> 
>+/** The argument must be a GIMPLE_CALL. If it represents a call to a named
>+ * function (not a method or function pointer), then return the name.
>+ * Otherwise return undefined. */
>+function gimple_call_function_name(gs) {
>+  let decl = gimple_call_fndecl(gs);
>+  return decl ? decl_name_string(decl) : undefined;
>+}
>+
> /** Return the ith argument of the function, counting from zero and including
>  * 'this' in the list if it is present. */
> function call_arg(expr, i) {
>   let arg_count = call_expr_nargs(expr);
>   if (i < 0 || i >= arg_count)
>     throw new Error("arg index out of range: call has " + arg_count + " args");
>   return CALL_EXPR_ARG(expr, i);
> }
> 
> /** Iterator over the argument list of the function call. */
> var call_arg_iterator = call_expr_arg_iterator;
> 
>-/** Return an array of the arguments of a function call. */
>-function call_args(expr) {
>-  return [ a for (a in call_arg_iterator(expr)) ];
>+/** Return an array of the arguments of a CALL_EXPR. */
>+function call_expr_args(call_expr) {
>+  return [ a for (a in call_arg_iterator(call_expr)) ];
>+}
>+
>+/** Return an array of the arguments of a GIMPLE_CALL. */
>+function gimple_call_args(gimple_call) {
>+  return [ a for (a in gimple_call_arg_iterator(gimple_call)) ];
> }
> 
> /** Iterate over the base hierarchy of the given RECORD_TYPE in DFS order,
>  * including the type itself. */
> function dfs_base_iterator(record_type) {
>   // Worklist of BINFOs.
>   let work = [ TYPE_BINFO(record_type) ];
>   // Set of already-visited types -- We don't use decl_name_string here because the <anon> return
>diff --git a/libs/treehydra.js b/libs/treehydra.js
>--- a/libs/treehydra.js
>+++ b/libs/treehydra.js
>@@ -51,21 +51,29 @@ if (isGCC42) {
> 
> if (!isUsingGCCTuples) {
>   this.gimple_code = TREE_CODE;
>   this.GIMPLE_CALL = CALL_EXPR;
>   this.GIMPLE_ASSIGN = GIMPLE_MODIFY_STMT;
>   this.GIMPLE_COND = COND_EXPR;
>   this.GIMPLE_SWITCH = SWITCH_EXPR;
>   this.GIMPLE_RETURN = RETURN_EXPR;
>+  this.GIMPLE_LABEL = LABEL_EXPR;
>+  this.gimple_call_fn = CALL_EXPR_FN;
>   this.gimple_call_fndecl = call_function_decl;
>+  this.gimple_call_function_name = call_function_name;
>+  this.gimple_call_args = call_expr_args;
>   this.gimple_call_arg = call_arg;
>   this.gimple_call_arg_iterator = call_arg_iterator;
>   this.gs_display = isn_display;
>   this.gimple_op = function(isn, i) { return isn.operands()[i]; }
>+} else {
>+  this.FILTER_EXPR = undefined;
>+  this.EXC_PTR_EXPR = undefined;
>+  this.GIMPLE_MODIFY_STMT = undefined;
> }
> 
> // Class that the lazyness builds itself around of
> function GCCNode () {
> }
> 
> GCCNode.prototype.toString = function () {
>   if (!this._struct_name) {
>diff --git a/libs/unstable/analysis.js b/libs/unstable/analysis.js
>--- a/libs/unstable/analysis.js
>+++ b/libs/unstable/analysis.js
>@@ -54,35 +54,45 @@ function decl_get_key(d) {
> 
> /** Label function suitable for printing DECLs. */
> function decl_get_label(d) {
>   return expr_display(d);
> }
> 
> /* Analysis-related accessors */
> 
>-function ensure_decl_p(decl) {
>-  if (!DECL_P(decl))
>-    throw new Error("Fix code to only put decls here. Got :"+decl.tree_code());
>-  return decl
>+function defs_filter(ls) {
>+  for each(let d in ls) {
>+    if (!d)
>+      continue;
>+
>+    // ignore |this|
>+    if (d.tree_code() == COMPONENT_REF)
>+      d = TREE_OPERAND(d, 1);
>+    
>+    try {
>+      walk_tree(d, function (t) {
>+        if (DECL_P(t))
>+          throw t;
>+      });
>+    } catch (t) { yield t; }
>+  }
> }
> 
>-function filter_crap(ls) {
>-  for each(var d in ls) {
>+function uses_filter(ls) {
>+  for each(let d in ls) {
>     if (!d)
>       continue;
>-    
>-    switch(d.tree_code()) {
>-    case INTEGER_CST:
>-      break;
>-    case ADDR_EXPR:
>-      d = TREE_OPERAND(d, 0);
>-    default:
>-      yield ensure_decl_p(d);
>-    }
>+
>+    try {
>+      walk_tree(d, function (t) {
>+        if (DECL_P(t))
>+          throw t;
>+      });
>+    } catch (t) { yield t; }
>   }
> }
> 
> /** Iterate over variables defined by the instruction. 
>     If kind == 'strong', iterate over only strong defs. */
> function isn_defs(isn, kind) {
>   let retls = []
>   switch (gimple_code(isn)) {
>@@ -90,37 +100,37 @@ function isn_defs(isn, kind) {
>     retls.push(isn.gimple_ops[0]);
>     if (kind != 'strong') {
>       retls.push(isn.gimple_ops[1]);
>       retls.push(isn.gimple_ops[2]);
>     }
>     break;
>   case GIMPLE_CALL:
>     {
>-      retls.push(isn.gimple_ops[1]);
>+      retls.push(gimple_call_lhs(isn));
>       if (kind != 'strong') {
>         for (let d in gimple_call_arg_iterator(isn))
>           retls.push(d);
>       }
>     }
>     break;
>-  case GIMPLE_RETURN:{
>-    let operand = isn.gimple_ops[0];
>-    if (kind != 'strong')
>+  case GIMPLE_RETURN:
>+    if (kind != 'strong') {
>+      let operand = isn.gimple_ops[0];
>       retls.push(operand);
>     }
>     break;
>   case GIMPLE_COND:
>   case GIMPLE_SWITCH:
>     break;
>   default:
>     throw new Error("isn_defs ni " + gimple_code(isn));
>   }
>   
>-  for each(let d in filter_crap(retls))
>+  for each(let d in defs_filter(retls))
>     yield d;
> };
> 
> /** Iterate over variables used by the instruction.  */
> function isn_uses(isn) {
>   let retls = [];
>   switch (gimple_code(isn)) {
>   case GIMPLE_ASSIGN:
>@@ -139,17 +149,17 @@ function isn_uses(isn) {
>     retls.push(isn.gimple_ops[1]);
>     break;
>   case GIMPLE_SWITCH:
>     retls.push(isn.gimple_ops[0]);
>     break;
>   default:
>     throw new Error("ni " + gimple_code(isn));
>   }
>-  for each(let d in filter_crap(retls))
>+  for each(let d in uses_filter(retls))
>     yield d;
> };
> 
> if (!isUsingGCCTuples) {
> isn_defs = function (isn, kind) {
>   switch (TREE_CODE(isn)) {
>   case GIMPLE_MODIFY_STMT:
>     yield GENERIC_TREE_OPERAND(isn, 0);
>diff --git a/libs/unstable/zero_nonzero.js b/libs/unstable/zero_nonzero.js
>--- a/libs/unstable/zero_nonzero.js
>+++ b/libs/unstable/zero_nonzero.js
>@@ -47,20 +47,50 @@ function negate (v) {
> function Zero_NonZero() {
> }
> 
> Zero_NonZero.prototype.flowState = function(isn, state) {
>   switch (gimple_code(isn)) {
>   case GIMPLE_ASSIGN:
>     this.processAssign(isn, state);
>     break;
>-  case GIMPLE_RETURN:
>-    let op = gimple_op(isn, 0);
>+  case RETURN_EXPR:
>+    // 4.3 embeds assignments in returns
>+    let op = isn.operands()[0];
>     if (op) this.processAssign(op, state);
>     break;
>+  case GIMPLE_CALL:
>+    this.processCall(isn, state);
>+  }
>+}
>+
>+Zero_NonZero.prototype.processCall = function (isn, state) {
>+  // these are handled by processAssign (4.3 issue)
>+  if (isn.tree_code() == CALL_EXPR)
>+    return;
>+
>+  for (let arg in gimple_call_arg_iterator(isn)) {
>+    if (arg.tree_code() == ADDR_EXPR) {
>+      let vbl = arg.operands()[0];
>+      if (DECL_P(vbl)) {
>+        state.assignValue(vbl, ESP.TOP, isn);
>+      }
>+    }
>+  }
>+  
>+  let lhs = gimple_call_lhs(isn);
>+  if (!lhs)
>+    return;
>+
>+  let fname = gimple_call_function_name(isn);
>+  if (fname == '__builtin_expect') {
>+    // Same as an assign from arg 0 to lhs
>+    state.assign(lhs, gimple_call_args(isn, 0), isn);
>+  } else {
>+    state.assignValue(lhs, ESP.TOP, isn);
>   }
> }
> 
> // State transition for assignments. We'll just handle constants and copies.
> // For everything else, the result is unknown (i.e., TOP).
> Zero_NonZero.prototype.processAssign = function(isn, state) {
>   let lhs = gimple_op(isn, 0);
>   let rhs = gimple_op(isn, 1);
>@@ -85,19 +115,20 @@ Zero_NonZero.prototype.processAssign = f
>       state.assignValue(lhs, value, isn);
>       break;
>     case NE_EXPR: {
>       // We only care about gcc-generated x != 0 for conversions and such.
>       let [op1, op2] = rhs.operands();
>       if (DECL_P(op1) && expr_literal_int(op2) == 0) {
>         state.assign(lhs, op1, isn);
>       }
>+      break;
>     }
>-      break;
>     case CALL_EXPR:
>+      /* only relevant with GCC 4.3 */
>       let fname = call_function_name(rhs);
>       if (fname == '__builtin_expect') {
>         // Same as an assign from arg 0 to lhs
>         state.assign(lhs, call_args(rhs)[0], isn);
>       } else {
>         state.assignValue(lhs, ESP.TOP, isn);
>         for (let arg in call_arg_iterator(rhs)) {
>           if (arg.tree_code() == ADDR_EXPR) {
>@@ -183,20 +214,19 @@ Zero_NonZero.prototype.flowStateIf = fun
>       [op1,op2] = [op2,op1];
>     }
>     if (!DECL_P(op1)) break;
>     if (expr_literal_int(op2) != 0) break;
>     let val = comparison == EQ_EXPR ? Lattice.ZERO : Lattice.NONZERO;
>     this.filter(state, op1, val, truth, isn);
>     break;
>   default:
>-    let exp = GENERIC_TREE_OPERAND(isn, 0);
>+    let exp = isn.operands()[0];
>     if (DECL_P(exp)) {
>       this.filter(state, exp, Lattice.NONZERO, truth, isn);
>-      return;
>     }
>   }
> }
> 
> // State transition for switch cases.
> Zero_NonZero.prototype.flowStateSwitch = function(isn, truth, state) {
>   let exp = gimple_op(isn, 0);
>   
>diff --git a/test/return_expr.cc b/test/return_expr.cc
>new file mode 100644
>--- /dev/null
>+++ b/test/return_expr.cc
>@@ -0,0 +1,4 @@
>+int foo() {
>+  int x = 0;
>+  return x;
>+}
>diff --git a/test/test_return_expr.js b/test/test_return_expr.js
>new file mode 100644
>--- /dev/null
>+++ b/test/test_return_expr.js
>@@ -0,0 +1,32 @@
>+// { 'test': 'treehydra', 'input': 'return_expr.cc', 'output': 'unit_test', 'lang': 'c,c++' }
>+
>+require({ after_gcc_pass: "cfg" });
>+
>+include('unit_test.js');
>+
>+var test = new TestCase();
>+
>+function process_tree(fn) {
>+  let cfg = function_decl_cfg(fn);
>+  for (let bb in cfg_bb_iterator(cfg)) {
>+    for (let isn in bb_isn_iterator(bb)) {
>+      if (isn.tree_code() == GIMPLE_RETURN) {
>+        let ret = return_expr(isn);
>+        test.assertTrue(ret.tree_code() == VAR_DECL);
>+      }
>+    }
>+  }
>+}
>+
>+function process_cp_pre_genericize(fn) {
>+  walk_tree(DECL_SAVED_TREE(fn), function (t) {
>+    if (t.tree_code() == RETURN_EXPR) {
>+      let ret = return_expr(t);
>+      test.assertTrue(ret.tree_code() == VAR_DECL);
>+    }
>+  });
>+}
>+
>+function input_end() {
>+  print("OK")
>+}

Comment 3

[Ehren Metcalfe]

http://hg.mozilla.org/rewriting-and-analysis/dehydra/rev/b8f81c7ed170

Taras, when you have a sec can you push my analysis suite changes?

Comment 4

[Ehren Metcalfe]

Attachment: m-c patch

added commit info

Comment 5

[Ehren Metcalfe]

Attachment: m-c patch

added last minute fix to handle boolean_type outparams

Comment 6

[[e-mail]]

Comment on attachment 466603 [details] [diff] [review]
m-c patch

http://hg.mozilla.org/mozilla-central/rev/b7daee6451bd