Re: [PATCH 2/2] [PATCH 2/2] Fix handle_bit_test so that null set condition is taken care of

On Mon, Jul 12, 2021 at 06:24:28PM +0530, Harshvardhan Jha wrote:
> ---
>  smatch_extra.c        | 17 +++++++++--------
>  validation/sm_bits2.c | 28 ++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 8 deletions(-)
>  create mode 100644 validation/sm_bits2.c
> 
> diff --git a/smatch_extra.c b/smatch_extra.c
> index e864646a..27bf5a02 100644
> --- a/smatch_extra.c
> +++ b/smatch_extra.c
> @@ -2061,7 +2061,7 @@ static void match_comparison(struct expression *expr)
>  
>  static bool handle_bit_test(struct expression *expr)
>  {
> -	struct range_list *orig_rl, *rl;
> +	struct range_list *orig_rl, *rl, *true_rl, *false_rl;
>  	struct expression *shift, *mask, *var;
>  	struct bit_info *bit_info;
>  	sval_t sval;
> @@ -2083,23 +2083,24 @@ static bool handle_bit_test(struct expression *expr)
>  	bit_info = get_bit_info(mask);
>  	if (!bit_info)
>  		return false;
> -	if (!bit_info->possible)
> +	if (!bit_info->possible){
> +		set_true_false_states_expr(my_id, var, alloc_estate_empty(), NULL);
>  		return false;
> +	}
>  
>  	get_absolute_rl(var, &orig_rl);
>  	if (sval_is_negative(rl_min(orig_rl)) ||
>  	    rl_max(orig_rl).uvalue > type_bits(get_type(shift->left)))
>  		return false;
>  
> -	low.value = ffsll(bit_info->possible);
> -	high.value = sm_fls64(bit_info->possible);
> +	low.value = ffsll(bit_info->possible) - 1;
> +	high.value = sm_fls64(bit_info->possible) - 1;
>  	rl = alloc_rl(low, high);
>  	rl = cast_rl(get_type(var), rl);
> -	rl = rl_intersection(orig_rl, rl);
> -	if (!rl)
> -		return false;
> +	true_rl = rl_intersection(orig_rl, rl);
> +	false_rl = rl_filter(orig_rl, rl);

The false_rl isn't correct.  For the false_side we should use
bit_info->set instead of bit_info->possible.  It doesn't matter in the
test code because the value of mask is known so the possibly set and
the definitely set are the same.

In other words there can be some overlap between the potential values
on the true path and the false path.

regards,
dan carpenter