From patchwork Tue Nov 8 22:54:13 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 1701505 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=cLU+x/SQ; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4N6Nhn1nyyz23lT for ; Wed, 9 Nov 2022 09:54:45 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8A9A9385842F for ; Tue, 8 Nov 2022 22:54:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8A9A9385842F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1667948081; bh=ebOkwifY+eh64etR4cyll4ZEiqdkKHj3pVOoJ+zbJ3Q=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=cLU+x/SQTEOIBVxW58q5OJ1dM/iZdtOGh22f4yn9+dYc+U2LzJIjGHFyw62pehGI+ AC4TY59IH5208va3w5B7c/rHbiqjHCMBRIq1qc5LIEfmsCIPA8mlLFH2Bg3nB6a2lk srV7EPCaWWfqTDP2viQm/BCyK428iuAcnWApkSMg= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org 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 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) 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 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" 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" } */ +}