From 7565171e48037cbbdd748a1308cc41f95a1f335b Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Fri, 25 Apr 2014 13:25:48 +0300 Subject: [PATCH] unreachable: move it out of smatch_flow.c and smatch_states.c The unreachable code in smatch_states.c seemed important because it would let people know when they call set_state() but nothing happened. The problem was that it duplicated code in smatch_flow.c so when smatch_flow.c was enabled I turned off the warning in smatch_states.c. which meant that I only ever saw the warnings from smatch_flow.c and normal smatch users only ever saw the warnings from smatch_states.c. The code in smatch_states.c eventually became buggy and I didn't notice. I have deleted it. The code in check_unreachable.c is almost the same as the old smatch_flow.c code except that I use a different method to find the last statement in a function. Signed-off-by: Dan Carpenter --- check_list.h | 1 + check_unreachable.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++ smatch_flow.c | 70 --------------------------------- smatch_states.c | 30 +-------------- 4 files changed, 112 insertions(+), 98 deletions(-) create mode 100644 check_unreachable.c diff --git a/check_list.h b/check_list.h index 6ad3872b..960567a2 100644 --- a/check_list.h +++ b/check_list.h @@ -99,6 +99,7 @@ CK(check_struct_type) CK(check_cast_assign) CK(check_readl_infinite_loops) CK(check_double_checking) +CK(check_unreachable) /* <- your test goes here */ /* CK(register_template) */ diff --git a/check_unreachable.c b/check_unreachable.c new file mode 100644 index 00000000..693b3a21 --- /dev/null +++ b/check_unreachable.c @@ -0,0 +1,109 @@ +/* + * Copyright (C) 2014 Oracle. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt + */ + +#include "smatch.h" + +static int my_id; + +static int empty_statement(struct statement *stmt) +{ + if (!stmt) + return 0; + if (stmt->type == STMT_EXPRESSION && !stmt->expression) + return 1; + return 0; +} + +static int is_last_stmt(struct statement *cur_stmt) +{ + struct symbol *fn = get_base_type(cur_func_sym); + struct statement *stmt; + + if (!fn) + return 0; + stmt = fn->stmt; + if (!stmt) + stmt = fn->inline_stmt; + if (!stmt || stmt->type != STMT_COMPOUND) + return 0; + stmt = last_ptr_list((struct ptr_list *)stmt->stmts); + if (stmt == cur_stmt) + return 1; + return 0; +} + +static void print_unreached_initializers(struct symbol_list *sym_list) +{ + struct symbol *sym; + + FOR_EACH_PTR(sym_list, sym) { + if (sym->initializer) + sm_msg("info: '%s' is not actually initialized (unreached code).", + (sym->ident ? sym->ident->name : "this variable")); + } END_FOR_EACH_PTR(sym); +} + +static void print_unreached(struct statement *stmt) +{ + static int print = 1; + + if (__inline_fn) + return; + + if (!__path_is_null()) { + print = 1; + return; + } + if (!print) + return; + + switch (stmt->type) { + case STMT_COMPOUND: /* after a switch before a case stmt */ + case STMT_RANGE: + case STMT_CASE: + case STMT_LABEL: + return; + case STMT_DECLARATION: /* switch (x) { int a; case foo: ... */ + print_unreached_initializers(stmt->declaration); + return; + case STMT_RETURN: /* gcc complains if you don't have a return statement */ + if (is_last_stmt(stmt)) + return; + break; + case STMT_GOTO: + /* people put extra breaks inside switch statements */ + if (stmt->goto_label && stmt->goto_label->type == SYM_NODE && + strcmp(stmt->goto_label->ident->name, "break") == 0) + return; + break; + default: + break; + } + if (empty_statement(stmt)) + return; + if (!option_spammy) + return; + sm_msg("info: ignoring unreachable code."); + print = 0; +} + +void check_unreachable(int id) +{ + my_id = id; + + add_hook(&print_unreached, STMT_HOOK); +} diff --git a/smatch_flow.c b/smatch_flow.c index cbdc53d4..3ff4a407 100644 --- a/smatch_flow.c +++ b/smatch_flow.c @@ -486,69 +486,6 @@ static int last_stmt_on_same_line() return 0; } -static struct statement *last_stmt; -static int is_last_stmt(struct statement *stmt) -{ - if (stmt == last_stmt) - return 1; - return 0; -} - -static void print_unreached_initializers(struct symbol_list *sym_list) -{ - struct symbol *sym; - - FOR_EACH_PTR(sym_list, sym) { - if (sym->initializer) - sm_msg("info: '%s' is not actually initialized (unreached code).", - (sym->ident ? sym->ident->name : "this variable")); - } END_FOR_EACH_PTR(sym); -} - -static void print_unreached(struct statement *stmt) -{ - static int print = 1; - - if (__inline_fn) - return; - - if (!__path_is_null()) { - print = 1; - return; - } - if (!print) - return; - - switch (stmt->type) { - case STMT_COMPOUND: /* after a switch before a case stmt */ - case STMT_RANGE: - case STMT_CASE: - case STMT_LABEL: - return; - case STMT_DECLARATION: /* switch (x) { int a; case foo: ... */ - print_unreached_initializers(stmt->declaration); - return; - case STMT_RETURN: /* gcc complains if you don't have a return statement */ - if (is_last_stmt(stmt)) - return; - break; - case STMT_GOTO: - /* people put extra breaks inside switch statements */ - if (stmt->goto_label && stmt->goto_label->type == SYM_NODE && - strcmp(stmt->goto_label->ident->name, "break") == 0) - return; - break; - default: - break; - } - if (empty_statement(stmt)) - return; - if (!option_spammy) - return; - sm_msg("info: ignoring unreachable code."); - print = 0; -} - static void split_asm_constraints(struct expression_list *expr_list) { struct expression *expr; @@ -599,9 +536,6 @@ static void split_known_switch(struct statement *stmt, sval_t sval) stmt = stmt->switch_statement; - if (!last_stmt) - last_stmt = last_ptr_list((struct ptr_list *)stmt->stmts); - __push_scope_hooks(); FOR_EACH_PTR(stmt->stmts, tmp) { __smatch_lineno = tmp->pos.line; @@ -660,7 +594,6 @@ void __split_stmt(struct statement *stmt) add_ptr_list(&big_statement_stack, stmt); free_expression_stack(&big_expression_stack); set_position(stmt->pos); - print_unreached(stmt); __pass_to_client(stmt, STMT_HOOK); switch (stmt->type) { @@ -678,8 +611,6 @@ void __split_stmt(struct statement *stmt) case STMT_COMPOUND: { struct statement *tmp; - if (!last_stmt) - last_stmt = last_ptr_list((struct ptr_list *)stmt->stmts); __push_scope_hooks(); FOR_EACH_PTR(stmt->stmts, tmp) { __split_stmt(tmp); @@ -1137,7 +1068,6 @@ static void split_function(struct symbol *sym) if (sym->ident) cur_func = sym->ident->name; __smatch_lineno = sym->pos.line; - last_stmt = NULL; loop_count = 0; sm_debug("new function: %s\n", cur_func); __stree_id = 0; diff --git a/smatch_states.c b/smatch_states.c index 1d4991ed..4494ddc1 100644 --- a/smatch_states.c +++ b/smatch_states.c @@ -71,37 +71,11 @@ void __print_cur_stree(void) __print_stree(cur_stree); } -static int in_declarations_bit(void) -{ - struct statement *tmp; - - FOR_EACH_PTR_REVERSE(big_statement_stack, tmp) { - if (tmp->type == STMT_DECLARATION) - return 1; - return 0; - } END_FOR_EACH_PTR_REVERSE(tmp); - return 0; -} - int unreachable(void) { - static int reset_warnings = 1; - - if (cur_stree) { - if (!__inline_fn) - reset_warnings = 1; - return 0; - } - - if (in_declarations_bit()) + if (!cur_stree) return 1; - - /* option spammy turns on a noisier version of this */ - if (reset_warnings && !option_spammy) - sm_msg("info: ignoring unreachable code."); - if (!__inline_fn) - reset_warnings = 0; - return 1; + return 0; } struct sm_state *set_state(int owner, const char *name, struct symbol *sym, struct smatch_state *state) -- 2.11.4.GIT