[qfcc] Use flow analysis for dealloc check

I decided that the check for whether control reaches the end of the
function without performing some necessary action (eg, invoking
[super dealoc] in a derived -dealoc) is conceptually the return
statement using a pseudo operand and the necessary action defining that
pseudo operand and thus is the same as checking for uninitialised
variables. Thus, add a pseudo operand type and use one to represent the
invocation of [super alloc], with a special function to call when the
"used" pseudo operand is "uninitialised".

While I currently don't know what else pseudo operands could be used
for, the system should be flexible enough to add any check.

Fixes #24
This commit is contained in:
Bill Currie 2021-12-25 12:42:15 +09:00
parent f903211362
commit 22c67fc268
5 changed files with 173 additions and 18 deletions

View file

@ -97,6 +97,7 @@ typedef struct function_s {
struct statement_s **statements; struct statement_s **statements;
int num_statements; int num_statements;
int pseudo_addr;///< pseudo address space for flow analysis int pseudo_addr;///< pseudo address space for flow analysis
struct pseudoop_s *pseudo_ops;///< pseudo operands used by this function
} function_t; } function_t;
extern function_t *current_func; extern function_t *current_func;

View file

@ -39,8 +39,11 @@ typedef enum {
op_temp, op_temp,
op_alias, op_alias,
op_nil, op_nil,
op_pseudo,
} op_type_e; } op_type_e;
struct expr_s;
typedef struct tempop_s { typedef struct tempop_s {
struct def_s *def; struct def_s *def;
int offset; int offset;
@ -53,6 +56,13 @@ typedef struct tempop_s {
int flowaddr; ///< "address" of temp in flow analysis, != 0 int flowaddr; ///< "address" of temp in flow analysis, != 0
} tempop_t; } tempop_t;
typedef struct pseudoop_s {
struct pseudoop_s *next;
const char *name;
struct flowvar_s *flowvar;
void (*uninitialized) (struct expr_s *expr, struct pseudoop_s *op);
} pseudoop_t;
typedef struct operand_s { typedef struct operand_s {
struct operand_s *next; struct operand_s *next;
op_type_e op_type; op_type_e op_type;
@ -66,6 +76,7 @@ typedef struct operand_s {
struct ex_label_s *label; struct ex_label_s *label;
tempop_t tempop; tempop_t tempop;
struct operand_s *alias; struct operand_s *alias;
pseudoop_t *pseudoop;
}; };
} operand_t; } operand_t;
@ -101,6 +112,9 @@ typedef struct statement_s {
operand_t *opc; operand_t *opc;
struct expr_s *expr; ///< source expression for this statement struct expr_s *expr; ///< source expression for this statement
int number; ///< number of this statement in function int number; ///< number of this statement in function
operand_t *use; ///< list of pseudo operands used
operand_t *def; ///< list of pseudo operands defined
operand_t *kill; ///< list of pseudo operands killed
} statement_t; } statement_t;
typedef struct sblock_s { typedef struct sblock_s {

View file

@ -121,6 +121,8 @@ get_operand_def (expr_t *expr, operand_t *op)
return get_operand_def (expr, op->alias); return get_operand_def (expr, op->alias);
case op_nil: case op_nil:
internal_error (expr, "unexpected nil operand"); internal_error (expr, "unexpected nil operand");
case op_pseudo:
internal_error (expr, "unexpected pseudo operand");
} }
internal_error (expr, "unexpected operand"); internal_error (expr, "unexpected operand");
return 0; return 0;

View file

@ -283,6 +283,8 @@ flowvar_get_def (flowvar_t *var)
internal_error (op->expr, "unexpected alias operand"); internal_error (op->expr, "unexpected alias operand");
case op_nil: case op_nil:
internal_error (op->expr, "unexpected nil operand"); internal_error (op->expr, "unexpected nil operand");
case op_pseudo:
internal_error (op->expr, "unexpected pseudo operand");
} }
internal_error (op->expr, "oops, blue pill"); internal_error (op->expr, "oops, blue pill");
return 0; return 0;
@ -311,6 +313,11 @@ flow_get_var (operand_t *op)
op->def->flowvar = new_flowvar (); op->def->flowvar = new_flowvar ();
return op->def->flowvar; return op->def->flowvar;
} }
if (op->op_type == op_pseudo) {
if (!op->pseudoop->flowvar)
op->pseudoop->flowvar = new_flowvar ();
return op->pseudoop->flowvar;
}
return 0; return 0;
} }
@ -349,6 +356,27 @@ count_operand (operand_t *op)
return 0; return 0;
} }
static int
count_operand_chain (operand_t *op)
{
int count = 0;
while (op) {
count += count_operand (op);
op = op->next;
}
return count;
}
/** Allocate flow analysis pseudo address space.
*/
static int
get_pseudo_address (function_t *func, int size)
{
int addr = func->pseudo_addr;
func->pseudo_addr += size;
return addr;
}
/** Allocate flow analysis pseudo address space to a temporary variable. /** Allocate flow analysis pseudo address space to a temporary variable.
* *
* If the operand already has an address allocated (flowvar_t::flowaddr is * If the operand already has an address allocated (flowvar_t::flowaddr is
@ -376,8 +404,7 @@ get_temp_address (function_t *func, operand_t *op)
top = top->tempop.alias; top = top->tempop.alias;
} }
if (!top->tempop.flowaddr) { if (!top->tempop.flowaddr) {
top->tempop.flowaddr = func->pseudo_addr; top->tempop.flowaddr = get_pseudo_address (func, top->size);
func->pseudo_addr += top->size;
} }
if (top->tempop.offset) { if (top->tempop.offset) {
internal_error (top->expr, "real tempop with a non-zero offset"); internal_error (top->expr, "real tempop with a non-zero offset");
@ -412,7 +439,9 @@ add_operand (function_t *func, operand_t *op)
var->number = func->num_vars++; var->number = func->num_vars++;
var->op = op; var->op = op;
func->vars[var->number] = var; func->vars[var->number] = var;
if (op->op_type == op_temp) { if (op->op_type == op_pseudo) {
var->flowaddr = get_pseudo_address (func, 1);
} else if (op->op_type == op_temp) {
var->flowaddr = get_temp_address (func, op); var->flowaddr = get_temp_address (func, op);
} else if (flowvar_is_local (var)) { } else if (flowvar_is_local (var)) {
var->flowaddr = func->num_statements + def_offset (var->op->def); var->flowaddr = func->num_statements + def_offset (var->op->def);
@ -420,6 +449,15 @@ add_operand (function_t *func, operand_t *op)
} }
} }
static void
add_operand_chain (function_t *func, operand_t *op)
{
while (op) {
add_operand (func, op);
op = op->next;
}
}
/** Create symbols and defs for params/return if not already available. /** Create symbols and defs for params/return if not already available.
*/ */
static symbol_t * static symbol_t *
@ -528,7 +566,7 @@ flow_build_vars (function_t *func)
// First, run through the statements making sure any accessed variables // First, run through the statements making sure any accessed variables
// have their flowvars reset. Local variables will be fine, but global // have their flowvars reset. Local variables will be fine, but global
// variables make have had flowvars added in a previous function, and it's // variables may have had flowvars added in a previous function, and it's
// easier to just clear them all. // easier to just clear them all.
// This is done before .return and .param so they won't get reset just // This is done before .return and .param so they won't get reset just
// after being counted // after being counted
@ -556,6 +594,10 @@ flow_build_vars (function_t *func)
flow_analyze_statement (s, 0, 0, 0, operands); flow_analyze_statement (s, 0, 0, 0, operands);
for (j = 0; j < 4; j++) for (j = 0; j < 4; j++)
num_vars += count_operand (operands[j]); num_vars += count_operand (operands[j]);
// count any pseudo operands referenced by the statement
num_vars += count_operand_chain (s->use);
num_vars += count_operand_chain (s->def);
num_vars += count_operand_chain (s->kill);
} }
if (!num_vars) if (!num_vars)
return; return;
@ -580,6 +622,9 @@ flow_build_vars (function_t *func)
flow_analyze_statement (s, 0, 0, 0, operands); flow_analyze_statement (s, 0, 0, 0, operands);
for (j = 0; j < 4; j++) for (j = 0; j < 4; j++)
add_operand (func, operands[j]); add_operand (func, operands[j]);
add_operand_chain (func, s->use);
add_operand_chain (func, s->def);
add_operand_chain (func, s->kill);
} }
// and set the use/def sets for the vars (has to be a separate pass // and set the use/def sets for the vars (has to be a separate pass
// because the allias handling reqruires the flow address to be valid // because the allias handling reqruires the flow address to be valid
@ -892,15 +937,24 @@ flow_uninit_scan_statements (flownode_t *node, set_t *defs, set_t *uninit)
for (var_i = set_first (stuse); var_i; var_i = set_next (var_i)) { for (var_i = set_first (stuse); var_i; var_i = set_next (var_i)) {
var = node->graph->func->vars[var_i->element]; var = node->graph->func->vars[var_i->element];
if (set_is_intersecting (defs, var->define)) { if (set_is_intersecting (defs, var->define)) {
def_t *def = flowvar_get_def (var); if (var->op->op_type == op_pseudo) {
if (def) { pseudoop_t *op = var->op->pseudoop;
if (options.warnings.uninited_variable) { if (op->uninitialized) {
warning (st->expr, "%s may be used uninitialized", op->uninitialized (st->expr, op);
def->name); } else {
internal_error (0, "pseudoop uninitialized not set");
} }
} else { } else {
bug (st->expr, "st %d, uninitialized temp %s", def_t *def = flowvar_get_def (var);
st->number, operand_string (var->op)); if (def) {
if (options.warnings.uninited_variable) {
warning (st->expr, "%s may be used uninitialized",
def->name);
}
} else {
bug (st->expr, "st %d, uninitialized temp %s",
st->number, operand_string (var->op));
}
} }
} }
// avoid repeat warnings in this node // avoid repeat warnings in this node
@ -1063,7 +1117,7 @@ flow_add_op_var (set_t *set, operand_t *op, int is_use)
// defs properly, but I am as yet uncertain of how. // defs properly, but I am as yet uncertain of how.
if (op->op_type == op_temp) { if (op->op_type == op_temp) {
tempop_visit_all (&op->tempop, ol, flow_tempop_add_aliases, set); tempop_visit_all (&op->tempop, ol, flow_tempop_add_aliases, set);
} else { } else if (op->op_type == op_def) {
def_visit_all (op->def, ol, flow_def_add_aliases, set); def_visit_all (op->def, ol, flow_def_add_aliases, set);
} }
} }
@ -1099,12 +1153,24 @@ flow_analyze_statement (statement_t *s, set_t *use, set_t *def, set_t *kill,
operand_t *aux_op1 = 0; operand_t *aux_op1 = 0;
operand_t *aux_op2 = 0; operand_t *aux_op2 = 0;
if (use) if (use) {
set_empty (use); set_empty (use);
if (def) for (operand_t *op = s->use; op; op = op->next) {
flow_add_op_var (use, op, 1);
}
}
if (def) {
set_empty (def); set_empty (def);
if (kill) for (operand_t *op = s->def; op; op = op->next) {
flow_add_op_var (def, op, 0);
}
}
if (kill) {
set_empty (kill); set_empty (kill);
for (operand_t *op = s->kill; op; op = op->next) {
flow_add_op_var (kill, op, 0);
}
}
if (operands) { if (operands) {
for (i = 0; i < FLOW_OPERANDS; i++) for (i = 0; i < FLOW_OPERANDS; i++)
operands[i] = 0; operands[i] = 0;

View file

@ -70,6 +70,7 @@ const char *op_type_names[] = {
"op_temp", "op_temp",
"op_alias", "op_alias",
"op_nil", "op_nil",
"op_pseudo",
}; };
const char *st_type_names[] = { const char *st_type_names[] = {
@ -183,7 +184,9 @@ operand_string (operand_t *op)
buf); buf);
} }
case op_nil: case op_nil:
return va (0, "nil"); return "nil";
case op_pseudo:
return va (0, "pseudo: %s", op->pseudoop->name);
} }
return ("??"); return ("??");
} }
@ -260,6 +263,9 @@ _print_operand (operand_t *op)
case op_nil: case op_nil:
printf ("nil"); printf ("nil");
break; break;
case op_pseudo:
printf ("pseudo: %s", op->pseudoop->name);
break;
} }
} }
@ -270,6 +276,20 @@ print_operand (operand_t *op)
puts (""); puts ("");
} }
static void
print_operand_chain (const char *name, operand_t *op)
{
if (op) {
printf (" %s:", name);
while (op) {
printf (" ");
_print_operand (op);
op = op->next;
}
printf ("\n");
}
}
void void
print_statement (statement_t *s) print_statement (statement_t *s)
{ {
@ -283,8 +303,12 @@ print_statement (statement_t *s)
if (s->opc) if (s->opc)
_print_operand (s->opc); _print_operand (s->opc);
printf (")\n"); printf (")\n");
print_operand_chain ("use", s->use);
print_operand_chain ("def", s->def);
print_operand_chain ("kill", s->kill);
} }
static pseudoop_t *pseudoops_freelist;
static sblock_t *sblocks_freelist; static sblock_t *sblocks_freelist;
static statement_t *statements_freelist; static statement_t *statements_freelist;
static operand_t *operands_freelist; static operand_t *operands_freelist;
@ -319,6 +343,16 @@ new_statement (st_type_t type, const char *opcode, expr_t *expr)
return statement; return statement;
} }
static pseudoop_t *
new_pseudoop (const char *name)
{
pseudoop_t *pseudoop;
ALLOC (256, pseudoop_t, pseudoops, pseudoop);
pseudoop->name = save_string (name);
return pseudoop;
}
static operand_t * static operand_t *
new_operand (op_type_e op, expr_t *expr, void *return_addr) new_operand (op_type_e op, expr_t *expr, void *return_addr)
{ {
@ -359,6 +393,16 @@ free_sblock (sblock_t *sblock)
FREE (sblocks, sblock); FREE (sblocks, sblock);
} }
static operand_t *
pseudo_operand (pseudoop_t *pseudoop, expr_t *expr)
{
operand_t *op;
op = new_operand (op_pseudo, expr, __builtin_return_address (0));
op->pseudoop = pseudoop;
op->size = 1;
return op;
}
operand_t * operand_t *
nil_operand (type_t *type, expr_t *expr) nil_operand (type_t *type, expr_t *expr)
{ {
@ -729,6 +773,7 @@ operand_address (operand_t *reference, expr_t *e)
case op_value: case op_value:
case op_label: case op_label:
case op_nil: case op_nil:
case op_pseudo:
break; break;
} }
internal_error (e, "invalid operand type for operand address: %s", internal_error (e, "invalid operand type for operand address: %s",
@ -2029,11 +2074,31 @@ remove_dead_blocks (sblock_t *blocks)
return did_anything; return did_anything;
} }
static void
super_dealloc_warning (expr_t *expr, pseudoop_t *op)
{
warning (expr,
"control may reach end of derived -dealloc without invoking %s",
op->name);
}
static void static void
search_for_super_dealloc (sblock_t *sblock) search_for_super_dealloc (sblock_t *sblock)
{ {
operand_t *op;
pseudoop_t *super_dealloc = new_pseudoop ("[super dealloc]");
int super_dealloc_found = 0;
super_dealloc->next = current_func->pseudo_ops;
current_func->pseudo_ops = super_dealloc;
super_dealloc->uninitialized = super_dealloc_warning;
while (sblock) { while (sblock) {
for (statement_t *st = sblock->statements; st; st = st->next) { for (statement_t *st = sblock->statements; st; st = st->next) {
if (statement_is_return (st)) {
op = pseudo_operand (super_dealloc, st->expr);
op->next = st->use;
st->use = op;
continue;
}
if (!statement_is_call (st)) { if (!statement_is_call (st)) {
continue; continue;
} }
@ -2056,13 +2121,20 @@ search_for_super_dealloc (sblock_t *sblock)
if (arg && arg->next && is_selector (arg)) { if (arg && arg->next && is_selector (arg)) {
selector_t *sel = get_selector (st->expr->e.expr.e2); selector_t *sel = get_selector (st->expr->e.expr.e2);
if (sel && strcmp (sel->name, "dealloc") == 0) { if (sel && strcmp (sel->name, "dealloc") == 0) {
return; op = pseudo_operand (super_dealloc, st->expr);
op->next = st->use;
st->def = op;
super_dealloc_found++;
} }
} }
} }
sblock = sblock->next; sblock = sblock->next;
} }
warning (0, "Derived class -dealloc does not call [super dealloc]"); // warn only when NOT optimizing because flow analysis will catch the
// missed invokation
if (!super_dealloc_found && !options.code.optimize) {
warning (0, "Derived class -dealloc does not call [super dealloc]");
}
} }
static void static void