Skip to content

Commit

Permalink
fix a bug in selecting loop control variables.
Browse files Browse the repository at this point in the history
No union integer fields should be selected as loop control variable if another field is pointer because stepping could potentially invalidate the pointer analysis
  • Loading branch information
Xuejun Yang committed Jul 23, 2011
1 parent 11305f8 commit 524f6b5
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 11 deletions.
4 changes: 0 additions & 4 deletions src/StatementFor.cpp
Expand Up @@ -147,10 +147,6 @@ StatementFor::make_iteration(CGContext& cg_context, StatementAssign*& init, Expr
var = VariableSelector::SelectLoopCtrlVar(cg_context, invalid_vars);
ERROR_GUARD(NULL);
if (var->is_volatile() ||
// disallow an integer field of an union, which also has a pointer field, to be IV because
// increment IV would cause the pointer to be invalid, and the current points-to analysis
// simply assumes loop stepping has no pointer effect
(var->is_inside_union_field() && var->get_top_container()->type->contain_pointer_field()) ) {
invalid_vars.push_back(var);
} else {
break;
Expand Down
3 changes: 1 addition & 2 deletions src/Type.cpp
Expand Up @@ -439,10 +439,9 @@ Type::is_volatile_struct_union() const
bool
Type::has_int_field() const
{
if (!is_aggregate()) return false;
if (is_int()) return true;
for (size_t i=0; i<fields.size(); ++i) {
const Type* t = fields[i];
if (t->is_int()) return true;
if (t->has_int_field()) return true;
}
return false;
Expand Down
21 changes: 16 additions & 5 deletions src/VariableSelector.cpp
Expand Up @@ -1097,18 +1097,29 @@ Variable *
VariableSelector::SelectLoopCtrlVar(const CGContext &cg_context, const vector<const Variable*>& invalid_vars)
{
// Note that many of the functions that select `var' can return null, if
const Type* type = get_int_type();
//eVariableScope scope = VariableSelectionProbability(eNewValue);
//ERROR_GUARD(NULL);
const Type* type = get_int_type();
vector<Variable*> vars;
find_all_non_array_visible_vars(cg_context.get_current_block(), vars);
find_all_non_array_visible_vars(cg_context.get_current_block(), vars);
// remove union variables that have both integer field(s) and pointer field(s)
// because incrementing the integer field causes the pointer to be invalid, and the current
// points-to analysis simply assumes loop stepping has no pointer effect
size_t len = vars.size();
for (size_t i=0; i<len; i++) {
if (vars[i]->type &&
(!vars[i]->type->has_int_field() || // remove variables isn't (or doesn't contain) integers
(vars[i]->type->eType == eUnion &&
vars[i]->type->contain_pointer_field()))) {
vars.erase(vars.begin() + i);
i--;
len--;
}
}
Variable* var = choose_var(vars, Effect::WRITE, cg_context, type, 0, eConvert, invalid_vars, true);
ERROR_GUARD(NULL);
if (var == NULL) {
var = GenerateNewGlobal(Effect::WRITE, cg_context, type, 0);
}
return var;
//return select(Effect::WRITE, cg_context, type, 0, invalid_vars, eConvert, scope);
}

// --------------------------------------------------------------
Expand Down

0 comments on commit 524f6b5

Please sign in to comment.