From: Dan Carpenter <dan.carpenter@oracle.com>
To: Harshvardhan Jha <harshvardhan.jha@oracle.com>
Cc: smatch@vger.kernel.org
Subject: Re: [PATCH] bits_clear and bits_set: Track bits cleared and set inside a function
Date: Wed, 21 Jul 2021 14:21:49 +0300 [thread overview]
Message-ID: <20210721112149.GO25548@kadam> (raw)
In-Reply-To: <20210721111436.107563-1-harshvardhan.jha@oracle.com>
On Wed, Jul 21, 2021 at 04:44:36PM +0530, Harshvardhan Jha wrote:
> Whenever a bit is set or cleared using bitwise operations inside a
> funciton body, the states are set using database entries.
>
> Signed-off-by: Harshvardhan Jha <harshvardhan.jha@oracle.com>
> ---
> Makefile | 2 +
> check_list.h | 2 +
> smatch.h | 12 +++++
> smatch_bits.c | 73 ++++++++++++++++++++++++---
> smatch_param_bits_clear.c | 102 ++++++++++++++++++++++++++++++++++++++
> smatch_param_bits_set.c | 87 ++++++++++++++++++++++++++++++++
> 6 files changed, 270 insertions(+), 8 deletions(-)
> create mode 100644 smatch_param_bits_clear.c
> create mode 100644 smatch_param_bits_set.c
>
> diff --git a/Makefile b/Makefile
> index 8855c974..3169b49e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -290,6 +290,8 @@ SMATCH_OBJS += smatch_mtag_data.o
> SMATCH_OBJS += smatch_mtag_map.o
> SMATCH_OBJS += smatch_mtag.o
> SMATCH_OBJS += smatch_nul_terminator.o
> +SMATCH_OBJS += smatch_param_bits_set.o
> +SMATCH_OBJS += smatch_param_bits_clear.o
> SMATCH_OBJS += smatch_param_cleared.o
> SMATCH_OBJS += smatch_param_compare_limit.o
> SMATCH_OBJS += smatch_parameter_names.o
> diff --git a/check_list.h b/check_list.h
> index fd205269..a57715ea 100644
> --- a/check_list.h
> +++ b/check_list.h
> @@ -169,6 +169,8 @@ CK(check_atomic_test)
> CK(check_checking_for_null_instead_of_err_ptr)
> CK(check_syscall_arg_type)
> CK(check_trinity_generator)
> +CK(register_param_bits_set)
> +CK(register_param_bits_clear)
>
> /* <- your test goes here */
> /* CK(register_template) */
> diff --git a/smatch.h b/smatch.h
> index 882e3927..a557f0cf 100644
> --- a/smatch.h
> +++ b/smatch.h
> @@ -848,6 +848,10 @@ enum info_type {
> FRESH_ALLOC = 1044,
> ALLOCATOR = 1045,
> FUNC_TIME = 1047,
> + BIT_SET = 1051,
> + BIT_CLEAR = 1052,
> + BIT_IS_SET = 1053,
> + BIT_IS_CLEAR = 1054,
>
> /* put random temporary stuff in the 7000-7999 range for testing */
> USER_DATA = 8017,
> @@ -1317,6 +1321,14 @@ struct bit_info *alloc_bit_info(unsigned long long set, unsigned long long possi
> struct smatch_state *alloc_bstate(unsigned long long set, unsigned long long possible);
> struct smatch_state *merge_bstates(struct smatch_state *one_state, struct smatch_state *two_state);
>
> +/* smatch_param_bits_set.c */
> +void __set_param_modified_helper(struct expression *expr, struct smatch_state *state);
> +void __set_param_modified_helper_sym(const char *name, struct symbol *sym, struct smatch_state *state);
> +
> +/* smatch_param_bits_clear.c */
> +void __set_param_modified_helper_clear(struct expression *expr, struct smatch_state *state);
> +void __set_param_modified_helper_sym_clear(const char *name, struct symbol *sym, struct smatch_state *state);
> +
>
> /* smatch_bit_info.c */
> struct bit_info *rl_to_binfo(struct range_list *rl);
> diff --git a/smatch_bits.c b/smatch_bits.c
> index 2f92ecd7..9cf60063 100644
> --- a/smatch_bits.c
> +++ b/smatch_bits.c
> @@ -41,6 +41,17 @@ struct bit_info *alloc_bit_info(unsigned long long set, unsigned long long possi
> return bit_info;
> }
>
> +void set_bits_modified_expr(struct expression *expr, struct smatch_state *state)
> +{
> + __set_param_modified_helper(expr, state);
> + set_state_expr(my_id, expr, state);
> +}
> +
> +void set_bits_modified_expr_sym(const char *name, struct symbol *sym, struct smatch_state *state)
> +{
> + __set_param_modified_helper_sym(name, sym, state);
> + set_state(my_id, name, sym, state);
> +}
> struct smatch_state *alloc_bstate(unsigned long long set, unsigned long long possible)
> {
> struct smatch_state *state;
> @@ -151,7 +162,8 @@ static void match_modify(struct sm_state *sm, struct expression *mod_expr)
>
> if (handled_by_assign_hook(mod_expr))
> return;
> - set_state(my_id, sm->name, sm->sym, alloc_bstate(0, -1ULL));
> + // set_state(my_id, sm->name, sm->sym, alloc_bstate(0, -1ULL));
Delete this stuff.
> + set_bits_modified_expr_sym(sm->name, sm->sym, alloc_bstate(0, -1ULL));
> }
>
> int binfo_equiv(struct bit_info *one, struct bit_info *two)
> @@ -169,6 +181,7 @@ struct smatch_state *merge_bstates(struct smatch_state *one_state, struct smatch
> one = one_state->data;
> two = two_state->data;
>
> +
Delete double blank line in a row.
> if (binfo_equiv(one, two))
> return one_state;
>
> @@ -247,9 +260,9 @@ struct bit_info *get_bit_info(struct expression *expr)
> sval_t known;
>
> expr = strip_parens(expr);
> -
Leave the blank line.
> - if (get_implied_value(expr, &known))
> + if (get_implied_value(expr, &known)) {
Don't add these parentheses.
> return alloc_bit_info(known.value, known.value);
> + }
>
> if (expr->type == EXPR_BINOP) {
> if (expr->op == '&')
> @@ -334,15 +347,20 @@ static void match_assign(struct expression *expr)
> if (is_unknown_binfo(get_type(expr->left), binfo))
> return;
>
> - set_state_expr(my_id, expr->left, alloc_bstate(binfo->set, binfo->possible));
> + // set_state_expr(my_id, expr->left, alloc_bstate(binfo->set, binfo->possible));
> + set_bits_modified_expr(expr->left, alloc_bstate(binfo->set, binfo->possible));
> } else if (expr->op == SPECIAL_OR_ASSIGN) {
> start = get_bit_info(expr->left);
> new = alloc_bstate(start->set | binfo->set, start->possible | binfo->possible);
> - set_state_expr(my_id, expr->left, new);
> + // set_state_expr(my_id, expr->left, new);
> + set_bits_modified_expr(expr->left, new);
> +
> } else if (expr->op == SPECIAL_AND_ASSIGN) {
> start = get_bit_info(expr->left);
> new = alloc_bstate(start->set & binfo->set, start->possible & binfo->possible);
> - set_state_expr(my_id, expr->left, new);
> + // set_state_expr(my_id, expr->left, new);
> + set_bits_modified_expr(expr->left, new);
> +
Delete extra blank line.
> }
> }
>
> @@ -369,7 +387,6 @@ static void match_condition(struct expression *expr)
>
> true_info.possible &= right.uvalue;
> false_info.possible &= ~right.uvalue;
> -
Unrelated whitespace change.
> set_true_false_states_expr(my_id, expr->left,
> alloc_bstate(true_info.set, true_info.possible),
> alloc_bstate(false_info.set, false_info.possible));
> @@ -454,9 +471,45 @@ static void set_param_bits(const char *name, struct symbol *sym, char *key, char
> value++;
> possible = strtoull(value, &value, 16);
>
> - set_state(my_id, fullname, sym, alloc_bstate(set, possible));
> + // set_state(my_id, fullname, sym, alloc_bstate(set, possible));
> + set_bits_modified_expr_sym(fullname, sym, alloc_bstate(set, possible));
> }
>
> +static void returns_bit_set(struct expression *expr, int param, char *key, char *value)
> +{
> + char *name;
> + struct symbol *sym;
> + unsigned long long set;
> + char *pEnd;
> +
> + name = get_name_sym_from_key(expr, param, key, &sym);
> +
> + if (!name)
> + return;
> +
> + set = strtoull(value, &pEnd, 16);
> + set_state(my_id, name, sym, alloc_bstate(set, -1ULL));
The indenting on this function is weird.
> +}
> +
> +static void returns_bit_clear(struct expression *expr, int param, char *key, char *value)
> +{
> + char *name;
> + struct symbol *sym;
> + unsigned long long possible;
> + char *pEnd;
> + struct bit_info *binfo;
> +
> + name = get_name_sym_from_key(expr, param, key, &sym);
> +
> + if (!name)
> + return;
> +
> + binfo = get_bit_info(expr);
> + possible = strtoull(value, &pEnd, 16);
> + set_state(my_id, name, sym, alloc_bstate(possible & binfo->set, possible & binfo->possible));
Fix indenting.
> +}
> +
> +
Double blank line.
> void register_bits(int id)
> {
> my_id = id;
> @@ -474,4 +527,8 @@ void register_bits(int id)
> add_hook(&match_call_info, FUNCTION_CALL_HOOK);
> add_member_info_callback(my_id, struct_member_callback);
> select_caller_info_hook(set_param_bits, BIT_INFO);
> +
> + select_return_states_hook(BIT_SET, &returns_bit_set);
> + select_return_states_hook(BIT_CLEAR, &returns_bit_clear);
> +
> }
> diff --git a/smatch_param_bits_clear.c b/smatch_param_bits_clear.c
> new file mode 100644
> index 00000000..403ead01
> --- /dev/null
> +++ b/smatch_param_bits_clear.c
> @@ -0,0 +1,102 @@
> +/*
> + * Copyright (C) 2021 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 "scope.h"
> +#include "smatch.h"
> +#include "smatch_slist.h"
> +#include "smatch_extra.h"
> +
> +static int my_id;
> +
> +// create helper functions for smatch_bits.c and use it to insert return state
> +static struct smatch_state *unmatched_state(struct sm_state *sm)
> +{
> + return alloc_bstate(-1ULL, -1ULL);
> +}
> +
> +static void match_assign(struct expression *expr)
> +{
> + sval_t sval;
> +
> + if (expr->type != EXPR_ASSIGNMENT)
> + return;
> +
> + if (expr->op != SPECIAL_AND_ASSIGN)
> + return;
> +
> + if(!get_implied_value(expr->right, &sval))
Add a space after if.
> + return;
> +
> + set_state_expr(my_id, expr->left, alloc_bstate(0, sval.uvalue));
> +}
> +
> +static void return_info_callback(int return_id, char *return_ranges,
> + struct expression *returned_expr,
> + int param,
> + const char *printed_name,
> + struct sm_state *sm)
> +{
> + unsigned long long bits_clear;
> + char buffer[64];
> + struct smatch_state *estate;
> + struct bit_info *binfo;
> + sval_t sval;
> +
> + estate = get_state(SMATCH_EXTRA, sm->name, sm->sym);
> + if (estate_get_single_value(estate, &sval))
> + return;
> +
> + binfo = sm->state->data;
> + bits_clear = binfo->possible;
> +
> + if (bits_clear == 0)
> + return;
> +
> + sprintf(buffer, "0x%llx", bits_clear);
> + sql_insert_return_states(return_id, return_ranges, BIT_CLEAR, param, printed_name, buffer);
> +}
> +
> +struct smatch_state *merge_bstates_clear(struct smatch_state *one_state, struct smatch_state *two_state)
> +{
> + struct bit_info *one, *two;
> +
> + one = one_state->data;
> + two = two_state->data;
> +
> +
Double blank line.
> + if (binfo_equiv(one, two))
> + return one_state;
> +
> + return alloc_bstate(0, one->possible | two->possible);
> +}
> +
> +// How to use smatch_bits.c states
> +// How to call the hook inside the function
> +
> +// BIT_IS_CLEAR AND BIT_IS_SET where to use the states
> +
> +void register_param_bits_clear(int id)
> +{
> + my_id = id;
> +
> + set_dynamic_states(my_id);
> + add_hook(&match_assign, ASSIGNMENT_HOOK);
> + add_unmatched_state_hook(my_id, &unmatched_state);
> + add_merge_hook(my_id, &merge_bstates);
> +
> + add_return_info_callback(my_id, return_info_callback);
> +}
> diff --git a/smatch_param_bits_set.c b/smatch_param_bits_set.c
> new file mode 100644
> index 00000000..d53de92a
> --- /dev/null
> +++ b/smatch_param_bits_set.c
> @@ -0,0 +1,87 @@
> +/*
> + * Copyright (C) 2021 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 "scope.h"
> +#include "smatch.h"
> +#include "smatch_slist.h"
> +#include "smatch_extra.h"
> +
> +static int my_id;
> +
> +void __set_param_modified_helper(struct expression *expr, struct smatch_state *state)
> +{
> + set_state_expr(my_id, expr, state);
> +}
> +
> +void __set_param_modified_helper_sym(const char *name, struct symbol *sym, struct smatch_state *state)
> +{
> + set_state(my_id, name, sym, state);
> +}
> +
> +// create helper functions for smatch_bits.c and use it to insert return state
> +static struct smatch_state *unmatched_state(struct sm_state *sm)
> +{
> + struct smatch_state *estate;
> +
> + estate = get_state(SMATCH_EXTRA, sm->name, sm->sym);
> +
> + return alloc_bstate(0, 0);
> +}
> +
> +static void return_info_callback(int return_id, char *return_ranges,
> + struct expression *returned_expr,
> + int param,
> + const char *printed_name,
> + struct sm_state *sm)
> +{
> + unsigned long long bits_set;
> + char buffer[64];
> + struct smatch_state *estate;
> + struct bit_info *binfo;
> + sval_t sval;
> +
> + estate = get_state(SMATCH_EXTRA, sm->name, sm->sym);
> + if (estate_get_single_value(estate, &sval))
> + return;
> +
> + binfo = sm->state->data;
> + bits_set = binfo->set;
> +
> + if (bits_set == 0)
> + return;
> +
> + sm_msg("%s: BIT_SET param=%d param_name='%s' set_bits='%llx'", __func__, param, printed_name, bits_set);
Delete this debug code.
> + sprintf(buffer, "0x%llx", bits_set);
> + sql_insert_return_states(return_id, return_ranges, BIT_SET, param, printed_name, buffer);
> +}
> +
regards,
dan carpenter
next prev parent reply other threads:[~2021-07-21 10:41 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-21 11:14 [PATCH] bits_clear and bits_set: Track bits cleared and set inside a function Harshvardhan Jha
2021-07-21 11:21 ` Dan Carpenter [this message]
-- strict thread matches above, loose matches on Subject: below --
2021-07-21 11:25 Harshvardhan Jha
2021-07-22 8:18 Harshvardhan Jha
2021-07-22 8:27 ` Dan Carpenter
2021-07-22 8:34 Harshvardhan Jha
2021-07-22 8:57 ` Dan Carpenter
2021-07-22 8:47 Harshvardhan Jha
2021-07-22 8:55 Harshvardhan Jha
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210721112149.GO25548@kadam \
--to=dan.carpenter@oracle.com \
--cc=harshvardhan.jha@oracle.com \
--cc=smatch@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).