From patchwork Tue Dec 6 23:31:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 61616 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5DEE4382D3F0 for ; Tue, 6 Dec 2022 23:32:26 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5DEE4382D3F0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1670369546; bh=1q4y5tR8VKLkdBRZvPukHxBprujPHT90kqg4IPDODfM=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=rMLC9yNH+yXQWAJphJig9sIAfD9kDoHywh5djKZQ/HX9jgikS3ciQdxAnPdLQtqrm PEcsb7dxv204YWh7NerZ5zG+5bTUEudlU/5I1nybh9q6pbX1CO8KgSl+WOoB5P1iEd 1i7eUzK9s+GJh5XVzDDQqgNdyNRrAkSjItpKGPcA= 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 A4DDD382B3C3 for ; Tue, 6 Dec 2022 23:31:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A4DDD382B3C3 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-532-ssGP3da4Om2QSI2WgYqi6A-1; Tue, 06 Dec 2022 18:31:51 -0500 X-MC-Unique: ssGP3da4Om2QSI2WgYqi6A-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E5E3229AA3B7 for ; Tue, 6 Dec 2022 23:31:50 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.17.99]) by smtp.corp.redhat.com (Postfix) with ESMTP id 96DD41121314; Tue, 6 Dec 2022 23:31:50 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed] analyzer: don't create bindings or binding keys for empty regions [PR107882] Date: Tue, 6 Dec 2022 18:31:46 -0500 Message-Id: <20221206233146.4111115-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.9 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+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" PR analyzer/107882 reports an ICE, due to trying to get a compound svalue for this binding: cluster for: a: key: {bytes 0-3} value: {UNKNOWN()} key: {empty} value: {UNKNOWN()} key: {bytes 4-7} value: {UNKNOWN()} where there's an binding to the unknown value of zero bits in size "somewhere" within "a" (perhaps between bits 3 and 4?) This makes no sense, so this patch adds an assertion that we never attempt to create a binding key for an empty region, and adds early rejection of attempts to get or set the values of such regions, fixing the ICE. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-4529-gdfe2ef7f2b6cac. gcc/analyzer/ChangeLog: PR analyzer/107882 * region-model.cc (region_model::get_store_value): Return an unknown value for empty regions. (region_model::set_value): Bail on empty regions. * region.cc (region::empty_p): New. * region.h (region::empty_p): New decl. * state-purge.cc (same_binding_p): Bail if either region is empty. * store.cc (binding_key::make): Assert that a concrete binding's bit_size must be > 0. (binding_cluster::mark_region_as_unknown): Bail on empty regions. (binding_cluster::get_binding): Likewise. (binding_cluster::remove_overlapping_bindings): Likewise. (binding_cluster::on_unknown_fncall): Don't conjure values for empty regions. (store::fill_region): Bail on empty regions. * store.h (class concrete_binding): Update comment to reflect that the range of bits must be non-empty. (concrete_binding::concrete_binding): Assert that bit range is non-empty. gcc/testsuite/ChangeLog: PR analyzer/107882 * gcc.dg/analyzer/memcpy-pr107882.c: New test. Signed-off-by: David Malcolm --- gcc/analyzer/region-model.cc | 8 +++++ gcc/analyzer/region.cc | 12 ++++++++ gcc/analyzer/region.h | 2 ++ gcc/analyzer/state-purge.cc | 4 +++ gcc/analyzer/store.cc | 30 +++++++++++++++---- gcc/analyzer/store.h | 8 +++-- .../gcc.dg/analyzer/memcpy-pr107882.c | 8 +++++ 7 files changed, 63 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/memcpy-pr107882.c diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 430c0e91175..18eaf22a5d1 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -2321,6 +2321,10 @@ const svalue * region_model::get_store_value (const region *reg, region_model_context *ctxt) const { + /* Getting the value of an empty region gives an unknown_svalue. */ + if (reg->empty_p ()) + return m_mgr->get_or_create_unknown_svalue (reg->get_type ()); + check_region_for_read (reg, ctxt); /* Special-case: handle var_decls in the constant pool. */ @@ -3159,6 +3163,10 @@ region_model::set_value (const region *lhs_reg, const svalue *rhs_sval, gcc_assert (lhs_reg); gcc_assert (rhs_sval); + /* Setting the value of an empty region is a no-op. */ + if (lhs_reg->empty_p ()) + return; + check_region_size (lhs_reg, rhs_sval, ctxt); check_region_for_write (lhs_reg, ctxt); diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc index 6d97590a83a..67ba9486980 100644 --- a/gcc/analyzer/region.cc +++ b/gcc/analyzer/region.cc @@ -671,6 +671,18 @@ region::symbolic_p () const return get_kind () == RK_SYMBOLIC; } +/* Return true if this region is known to be zero bits in size. */ + +bool +region::empty_p () const +{ + bit_size_t num_bits; + if (get_bit_size (&num_bits)) + if (num_bits == 0) + return true; + return false; +} + /* Return true if this is a region for a decl with name DECL_NAME. Intended for use when debugging (for assertions and conditional breakpoints). */ diff --git a/gcc/analyzer/region.h b/gcc/analyzer/region.h index ecae887edaf..6d8bcfb8868 100644 --- a/gcc/analyzer/region.h +++ b/gcc/analyzer/region.h @@ -233,6 +233,8 @@ public: bool is_named_decl_p (const char *decl_name) const; + bool empty_p () const; + protected: region (complexity c, unsigned id, const region *parent, tree type); diff --git a/gcc/analyzer/state-purge.cc b/gcc/analyzer/state-purge.cc index 6fac18a2bbc..e9bcb4b0345 100644 --- a/gcc/analyzer/state-purge.cc +++ b/gcc/analyzer/state-purge.cc @@ -813,7 +813,11 @@ same_binding_p (const region *reg_a, const region *reg_b, { if (reg_a->get_base_region () != reg_b->get_base_region ()) return false; + if (reg_a->empty_p ()) + return false; const binding_key *bind_key_a = binding_key::make (store_mgr, reg_a); + if (reg_b->empty_p ()) + return false; const binding_key *bind_key_b = binding_key::make (store_mgr, reg_b); return bind_key_a == bind_key_b; } diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc index 99939b7ea70..dd8ebaa7374 100644 --- a/gcc/analyzer/store.cc +++ b/gcc/analyzer/store.cc @@ -127,8 +127,12 @@ binding_key::make (store_manager *mgr, const region *r) { bit_size_t bit_size; if (r->get_bit_size (&bit_size)) - return mgr->get_concrete_binding (offset.get_bit_offset (), - bit_size); + { + /* Must be non-empty. */ + gcc_assert (bit_size > 0); + return mgr->get_concrete_binding (offset.get_bit_offset (), + bit_size); + } else return mgr->get_symbolic_binding (r); } @@ -1464,6 +1468,9 @@ binding_cluster::mark_region_as_unknown (store_manager *mgr, const region *reg_for_overlap, uncertainty_t *uncertainty) { + if (reg_to_bind->empty_p ()) + return; + remove_overlapping_bindings (mgr, reg_for_overlap, uncertainty); /* Add a default binding to "unknown". */ @@ -1516,6 +1523,8 @@ const svalue * binding_cluster::get_binding (store_manager *mgr, const region *reg) const { + if (reg->empty_p ()) + return NULL; const binding_key *reg_binding = binding_key::make (mgr, reg); const svalue *sval = m_map.get (reg_binding); if (sval) @@ -1800,6 +1809,8 @@ binding_cluster::remove_overlapping_bindings (store_manager *mgr, const region *reg, uncertainty_t *uncertainty) { + if (reg->empty_p ()) + return; const binding_key *reg_binding = binding_key::make (mgr, reg); const region *cluster_base_reg = get_base_region (); @@ -2007,11 +2018,14 @@ binding_cluster::on_unknown_fncall (const gcall *call, { m_map.empty (); - /* Bind it to a new "conjured" value using CALL. */ - const svalue *sval - = mgr->get_svalue_manager ()->get_or_create_conjured_svalue + if (!m_base_region->empty_p ()) + { + /* Bind it to a new "conjured" value using CALL. */ + const svalue *sval + = mgr->get_svalue_manager ()->get_or_create_conjured_svalue (m_base_region->get_type (), call, m_base_region, p); - bind (mgr, m_base_region, sval); + bind (mgr, m_base_region, sval); + } m_touched = true; } @@ -2742,6 +2756,10 @@ store::purge_region (store_manager *mgr, const region *reg) void store::fill_region (store_manager *mgr, const region *reg, const svalue *sval) { + /* Filling an empty region is a no-op. */ + if (reg->empty_p ()) + return; + const region *base_reg = reg->get_base_region (); if (base_reg->symbolic_for_unknown_ptr_p () || !base_reg->tracked_p ()) diff --git a/gcc/analyzer/store.h b/gcc/analyzer/store.h index 6243ec65ea1..30284eb7803 100644 --- a/gcc/analyzer/store.h +++ b/gcc/analyzer/store.h @@ -356,8 +356,8 @@ struct byte_range byte_size_t m_size_in_bytes; }; -/* Concrete subclass of binding_key, for describing a concrete range of - bits within the binding_map (e.g. "bits 8-15"). */ +/* Concrete subclass of binding_key, for describing a non-empty + concrete range of bits within the binding_map (e.g. "bits 8-15"). */ class concrete_binding : public binding_key { @@ -367,7 +367,9 @@ public: concrete_binding (bit_offset_t start_bit_offset, bit_size_t size_in_bits) : m_bit_range (start_bit_offset, size_in_bits) - {} + { + gcc_assert (!m_bit_range.empty_p ()); + } bool concrete_p () const final override { return true; } hashval_t hash () const diff --git a/gcc/testsuite/gcc.dg/analyzer/memcpy-pr107882.c b/gcc/testsuite/gcc.dg/analyzer/memcpy-pr107882.c new file mode 100644 index 00000000000..4ecb0fd973f --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/memcpy-pr107882.c @@ -0,0 +1,8 @@ +void +foo (int *x, int y) +{ + int *a = x, *b = (int *) &a; + + __builtin_memcpy (b + 1, x, y); + foo (a, 0); +}