From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 3DE433858C2D for ; Tue, 8 Nov 2022 22:54:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3DE433858C2D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1667948058; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=ebOkwifY+eh64etR4cyll4ZEiqdkKHj3pVOoJ+zbJ3Q=; b=JkakgWJns1/c9r2/wtJse+oCuKLpUIqCcGpRDSL1G7j7iMwRgLTc9Jc6Xtj8rp4hLh6D6/ jFpsrH8qFbEDzJrO53e5Aemxp+HqIyJz3imKmOMd8bBHkDS4w7r2svBUaPbgmaret9O02c jVW+DSQYoM0wH0T9W4g9qRRCyhDsN2U= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-94-bn8lyXxFMAWv0N1lyPK1jQ-1; Tue, 08 Nov 2022 17:54:16 -0500 X-MC-Unique: bn8lyXxFMAWv0N1lyPK1jQ-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 9188185A5A6 for ; Tue, 8 Nov 2022 22:54:15 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.17.189]) by smtp.corp.redhat.com (Postfix) with ESMTP id 657BF40C2140; Tue, 8 Nov 2022 22:54:15 +0000 (UTC) From: David Malcolm To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed] analyzer: eliminate region_model::eval_condition_without_cm [PR101962] Date: Tue, 8 Nov 2022 17:54:13 -0500 Message-Id: <20221108225413.2538404-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: In r12-3094-ge82e0f149b0aba I added the assumption that POINTER_PLUS_EXPR of non-NULL is non-NULL (for PR analyzer/101962). Whilst working on another bug, I noticed that this only works when the LHS is known to be non-NULL via region_model::eval_condition_without_cm, but not when it's known through a constraint. This distinction predates the original commit of the analyzer in GCC 10, but I believe it became irrelevant in the GCC 11 rewrite of the region model code (r11-2694-g808f4dfeb3a95f). Hence this patch eliminates region_model::eval_condition_without_cm in favor of all users simply calling region_model::eval_condition. Doing so enables the "POINTER_PLUS_EXPR of non-NULL is non-NULL" assumption to also be made when the LHS is known through a constraint (e.g. a conditional). Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-3819-g9bbcee450deb0f. gcc/analyzer/ChangeLog: PR analyzer/101962 * region-model-impl-calls.cc: Update comment. * region-model.cc (region_model::check_symbolic_bounds): Fix layout of "void" return. Replace usage of eval_condition_without_cm with eval_condition. (region_model::eval_condition): Take over body of... (region_model::eval_condition_without_cm): ...this subroutine, dropping the latter. Eliminating this distinction avoids issues where constraints were not considered when recursing. (region_model::compare_initial_and_pointer): Update comment. (region_model::symbolic_greater_than): Replace usage of eval_condition_without_cm with eval_condition. * region-model.h (region_model::eval_condition_without_cm): Delete decl. gcc/testsuite/ChangeLog: PR analyzer/101962 * gcc.dg/analyzer/data-model-23.c (test_3): New test. Signed-off-by: David Malcolm --- gcc/analyzer/region-model-impl-calls.cc | 2 +- gcc/analyzer/region-model.cc | 75 +++++++------------ gcc/analyzer/region-model.h | 3 - gcc/testsuite/gcc.dg/analyzer/data-model-23.c | 11 +++ 4 files changed, 38 insertions(+), 53 deletions(-) diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc index bc644f8f3ad..9ef31f6ab05 100644 --- a/gcc/analyzer/region-model-impl-calls.cc +++ b/gcc/analyzer/region-model-impl-calls.cc @@ -498,7 +498,7 @@ region_model::impl_call_fread (const call_details &cd) This has to be done here so that the sm-handling can use the fact that they point to the same region to establish that they are equal - (in region_model::eval_condition_without_cm), and thus transition + (in region_model::eval_condition), and thus transition all pointers to the region to the "freed" state together, regardless of casts. */ diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 0ca454a0f9c..5ffad64a9c5 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -1764,12 +1764,13 @@ public: /* Check whether an access is past the end of the BASE_REG. */ -void region_model::check_symbolic_bounds (const region *base_reg, - const svalue *sym_byte_offset, - const svalue *num_bytes_sval, - const svalue *capacity, - enum access_direction dir, - region_model_context *ctxt) const +void +region_model::check_symbolic_bounds (const region *base_reg, + const svalue *sym_byte_offset, + const svalue *num_bytes_sval, + const svalue *capacity, + enum access_direction dir, + region_model_context *ctxt) const { gcc_assert (ctxt); @@ -1777,7 +1778,7 @@ void region_model::check_symbolic_bounds (const region *base_reg, = m_mgr->get_or_create_binop (num_bytes_sval->get_type (), PLUS_EXPR, sym_byte_offset, num_bytes_sval); - if (eval_condition_without_cm (next_byte, GT_EXPR, capacity).is_true ()) + if (eval_condition (next_byte, GT_EXPR, capacity).is_true ()) { tree diag_arg = get_representative_tree (base_reg); tree offset_tree = get_representative_tree (sym_byte_offset); @@ -4161,44 +4162,18 @@ tristate region_model::eval_condition (const svalue *lhs, enum tree_code op, const svalue *rhs) const -{ - /* For now, make no attempt to capture constraints on floating-point - values. */ - if ((lhs->get_type () && FLOAT_TYPE_P (lhs->get_type ())) - || (rhs->get_type () && FLOAT_TYPE_P (rhs->get_type ()))) - return tristate::unknown (); - - tristate ts = eval_condition_without_cm (lhs, op, rhs); - if (ts.is_known ()) - return ts; - - /* Otherwise, try constraints. */ - return m_constraints->eval_condition (lhs, op, rhs); -} - -/* Determine what is known about the condition "LHS_SVAL OP RHS_SVAL" within - this model, without resorting to the constraint_manager. - - This is exposed so that impl_region_model_context::on_state_leak can - check for equality part-way through region_model::purge_unused_svalues - without risking creating new ECs. */ - -tristate -region_model::eval_condition_without_cm (const svalue *lhs, - enum tree_code op, - const svalue *rhs) const { gcc_assert (lhs); gcc_assert (rhs); - /* See what we know based on the values. */ - /* For now, make no attempt to capture constraints on floating-point values. */ if ((lhs->get_type () && FLOAT_TYPE_P (lhs->get_type ())) || (rhs->get_type () && FLOAT_TYPE_P (rhs->get_type ()))) return tristate::unknown (); + /* See what we know based on the values. */ + /* Unwrap any unmergeable values. */ lhs = lhs->unwrap_any_unmergeable (); rhs = rhs->unwrap_any_unmergeable (); @@ -4292,9 +4267,7 @@ region_model::eval_condition_without_cm (const svalue *lhs, shouldn't warn for. */ if (binop->get_op () == POINTER_PLUS_EXPR) { - tristate lhs_ts - = eval_condition_without_cm (binop->get_arg0 (), - op, rhs); + tristate lhs_ts = eval_condition (binop->get_arg0 (), op, rhs); if (lhs_ts.is_known ()) return lhs_ts; } @@ -4327,7 +4300,7 @@ region_model::eval_condition_without_cm (const svalue *lhs, } /* Handle comparisons between two svalues with more than one operand. */ - if (const binop_svalue *binop = lhs->dyn_cast_binop_svalue ()) + if (const binop_svalue *binop = lhs->dyn_cast_binop_svalue ()) { switch (op) { @@ -4369,10 +4342,14 @@ region_model::eval_condition_without_cm (const svalue *lhs, } } - return tristate::TS_UNKNOWN; + /* Otherwise, try constraints. + Cast to const to ensure we don't change the constraint_manager as we + do this (e.g. by creating equivalence classes). */ + const constraint_manager *constraints = m_constraints; + return constraints->eval_condition (lhs, op, rhs); } -/* Subroutine of region_model::eval_condition_without_cm, for rejecting +/* Subroutine of region_model::eval_condition, for rejecting equality of INIT_VAL(PARM) with &LOCAL. */ tristate @@ -4424,18 +4401,18 @@ region_model::symbolic_greater_than (const binop_svalue *bin_a, /* Eliminate the right-hand side of both svalues. */ if (const binop_svalue *bin_b = dyn_cast (b)) if (bin_a->get_op () == bin_b->get_op () - && eval_condition_without_cm (bin_a->get_arg1 (), - GT_EXPR, - bin_b->get_arg1 ()).is_true () - && eval_condition_without_cm (bin_a->get_arg0 (), - GE_EXPR, - bin_b->get_arg0 ()).is_true ()) + && eval_condition (bin_a->get_arg1 (), + GT_EXPR, + bin_b->get_arg1 ()).is_true () + && eval_condition (bin_a->get_arg0 (), + GE_EXPR, + bin_b->get_arg0 ()).is_true ()) return tristate (tristate::TS_TRUE); /* Otherwise, try to remove a positive offset or factor from BIN_A. */ if (is_positive_svalue (bin_a->get_arg1 ()) - && eval_condition_without_cm (bin_a->get_arg0 (), - GE_EXPR, b).is_true ()) + && eval_condition (bin_a->get_arg0 (), + GE_EXPR, b).is_true ()) return tristate (tristate::TS_TRUE); } return tristate::unknown (); diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 0caaf82936b..70c808f4973 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -450,9 +450,6 @@ class region_model tristate eval_condition (const svalue *lhs, enum tree_code op, const svalue *rhs) const; - tristate eval_condition_without_cm (const svalue *lhs, - enum tree_code op, - const svalue *rhs) const; tristate compare_initial_and_pointer (const initial_svalue *init, const region_svalue *ptr) const; tristate symbolic_greater_than (const binop_svalue *a, diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-23.c b/gcc/testsuite/gcc.dg/analyzer/data-model-23.c index c76dd4ed31e..d10dd057d96 100644 --- a/gcc/testsuite/gcc.dg/analyzer/data-model-23.c +++ b/gcc/testsuite/gcc.dg/analyzer/data-model-23.c @@ -24,3 +24,14 @@ void test_2 (void) __analyzer_eval (hide (NULL) - 1 == NULL); /* { dg-warning "FALSE" } */ __analyzer_eval (hide (NULL) + 1 == NULL); /* { dg-warning "FALSE" } */ } + +void test_3 (void *p) +{ + if (!p) + return; + __analyzer_eval (hide (p) == NULL); /* { dg-warning "FALSE" } */ + __analyzer_eval (hide (p) + 1 != NULL); /* { dg-warning "TRUE" } */ + __analyzer_eval (hide (p) + 1 == NULL); /* { dg-warning "FALSE" } */ + __analyzer_eval (hide (p) - 1 != NULL); /* { dg-warning "TRUE" } */ + __analyzer_eval (hide (p) - 1 == NULL); /* { dg-warning "FALSE" } */ +} -- 2.26.3