From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from aserp2130.oracle.com ([141.146.126.79]:59546 "EHLO aserp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726187AbfANKSX (ORCPT ); Mon, 14 Jan 2019 05:18:23 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=corp-2018-07-02; bh=OOhhWyn0Wh7jRLvJe4602MeSUrBpQc+zA+yC2UHeO7g=; b=C7HC240S7iPBNpR3Tyj6lwVWrcvIF0MRtyi4BKg8zN0EiQbpfFi/rrEnfh/Nd9mlDOP+ rnQFr/QUnBlPDarIJ2AHcFN9fJsWEFHHXTKmFuuNPeg0sRG75o4lsGsG2Yogfy3Fzu2O /knd7O8yQHmY8HAcKinnnoGF40MpirRwfa2kDnaaQYSbQQbeHdVzcfQ6hHUpsIg36s7U Xr3JiZmes7zisAYXBC+/cixRo6lB88R5lMLVgrHs2oJGP73BlmURpiplNl8qFtmKQvEe D5VvHcU5GA+bSwGD2dqwJeBgwkoyqPN+vh4cKMIBj8FArhKb1GJFlw831ZTVPXnpUyXv WQ== Date: Mon, 14 Jan 2019 13:18:04 +0300 From: Dan Carpenter Subject: Re: Odd smatch issue? Message-ID: <20190114101804.GA4482@kadam> References: <20190111123241.GA11408@movementarian.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190111123241.GA11408@movementarian.org> Sender: smatch-owner@vger.kernel.org List-ID: To: John Levon Cc: smatch@vger.kernel.org On Fri, Jan 11, 2019 at 12:32:41PM +0000, John Levon wrote: > > > static long > lx_cap_update_priv(void) > { > const int lx_cap_mapping[4] = { 0, 0, 0 }; > int i = 63; > /* enabling the below line disables the warning */ > //int cap_set = i == 0; > lx_cap_mapping[i]; > } > Thanks John, I am testing this patch: [PATCH] extra: preserve hard_max after comparisons to zero John Levon reported that if you had code like: int lx_cap_mapping[4]; int i = 63; if (i) ; lx_cap_mapping[i] = 0; The code should trigger a warning but it doesn't. I initially thought the hard_max was getting lost in merge_estates(). I changed that to say if we were merging an impossible state with a possible one then we should keep the hard max from the possible one. It turns out that wasn't the issue, but I sort of think it's the correct thing to do anyway. The real problem is that we don't preserve the hard_max after comparisons to zero, whether it's impossible or not. That is handled in match_condition(). I changed that function to re-use handle_comparison(). After that because the original "i" had a hard_max then handle_comparison() sets the hard_max flag for both the possible and the impossible sides. Reported-by: John Levon Signed-off-by: Dan Carpenter --- smatch_estate.c | 4 +++- smatch_extra.c | 27 ++------------------------- 2 files changed, 5 insertions(+), 26 deletions(-) diff --git a/smatch_estate.c b/smatch_estate.c index 61a8fd34..69b84d91 100644 --- a/smatch_estate.c +++ b/smatch_estate.c @@ -43,7 +43,9 @@ struct smatch_state *merge_estates(struct smatch_state *s1, struct smatch_state tmp = alloc_estate_rl(value_ranges); rlist = get_shared_relations(estate_related(s1), estate_related(s2)); set_related(tmp, rlist); - if (estate_has_hard_max(s1) && estate_has_hard_max(s2)) + + if ((estate_has_hard_max(s1) && (!estate_rl(s2) || estate_has_hard_max(s2))) || + (estate_has_hard_max(s2) && (!estate_rl(s1) || estate_has_hard_max(s1)))) estate_set_hard_max(tmp); estate_set_fuzzy_max(tmp, sval_max(estate_get_fuzzy_max(s1), estate_get_fuzzy_max(s2))); diff --git a/smatch_extra.c b/smatch_extra.c index c2d97a88..e781c80f 100644 --- a/smatch_extra.c +++ b/smatch_extra.c @@ -1989,11 +1989,6 @@ static void handle_MOD_condition(struct expression *expr) /* this is actually hooked from smatch_implied.c... it's hacky, yes */ void __extra_match_condition(struct expression *expr) { - struct smatch_state *pre_state; - struct smatch_state *true_state; - struct smatch_state *false_state; - struct range_list *pre_rl; - expr = strip_expr(expr); switch (expr->type) { case EXPR_CALL: @@ -2001,27 +1996,9 @@ void __extra_match_condition(struct expression *expr) return; case EXPR_PREOP: case EXPR_SYMBOL: - case EXPR_DEREF: { - sval_t zero; - - zero = sval_blank(expr); - zero.value = 0; - - pre_state = get_extra_state(expr); - if (estate_is_empty(pre_state)) - return; - if (pre_state) - pre_rl = estate_rl(pre_state); - else - get_absolute_rl(expr, &pre_rl); - if (possibly_true_rl(pre_rl, SPECIAL_EQUAL, rl_zero())) - false_state = alloc_estate_sval(zero); - else - false_state = alloc_estate_empty(); - true_state = alloc_estate_rl(remove_range(pre_rl, zero, zero)); - set_extra_expr_true_false(expr, true_state, false_state); + case EXPR_DEREF: + handle_comparison(get_type(expr), expr, SPECIAL_NOTEQUAL, zero_expr()); return; - } case EXPR_COMPARE: match_comparison(expr); return; -- 2.17.1