From patchwork Sun May 17 09:12:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 1292085 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: 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=bgeskB1w; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49PxHf6ZH4z9sT6 for ; Sun, 17 May 2020 19:12:28 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1286E3851C00; Sun, 17 May 2020 09:12:23 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1286E3851C00 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1589706743; bh=zEC6wNmDzYhR+m/yEcXXoqtRZbX5BdCpePAeqJsFIAs=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=bgeskB1wtVqDYF8/NGWZg/ImhNmBY2rw1yiWi3XK9x2cLiO38MpqxiG2nnSgbOcI2 7AcZBjK+2NkN5tVybFM75F3j4moK8rvpcW/0YUo8LzPIICyZHA8Zj+mqbuHFYJtCaO PqJ3cvNOmVT89mkh9gmURt8IcJRD+rOJTNalnIUQ= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 41DA5385BF81 for ; Sun, 17 May 2020 09:12:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 41DA5385BF81 Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-15-0cnHR-eqOPqsj4ScBFpyRQ-1; Sun, 17 May 2020 05:12:16 -0400 X-MC-Unique: 0cnHR-eqOPqsj4ScBFpyRQ-1 Received: by mail-wr1-f71.google.com with SMTP id p13so2619925wrt.1 for ; Sun, 17 May 2020 02:12:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:cc:from:subject:message-id:date:user-agent :mime-version:content-language; bh=zEC6wNmDzYhR+m/yEcXXoqtRZbX5BdCpePAeqJsFIAs=; b=EHSZ64NRspKp1gzQ4tCM6AKRy0irmtCPbe7bamgHa/wBohAsNcAICotaXORRZmr7jQ rbl6uttY8OuNJz8iwg5m8tODXT4ryBQdiRwdeA02a73cwjyaNn2h566pAe8e3bi+XN7K laSPXVc5VSQ34sVd0geGWYhSWjk9zPjcgmCKD6g2tfzp5eIyjxVRiELyCHlnc7MFrCQP HjTqGERbK+0CxLmK5FjFmP8bnUjzdITcTv/NvWvnFzyln4iPl+y2OEyA9TPPYipAHPTz LDojgDlK0hOFfG+YPK8uvFURjsSpKgZqKCkt8xb7ZejvpgRbBc72u/Wtm1G3rYcAlEjq eukQ== X-Gm-Message-State: AOAM530OSSgsfHxyy8tAdXHw8EAZA2QRw9Ybys1Bj/goJI2juQVL5gQk GZIa/oqB27ruX5FwzOWcTml20U8pURjiiEZ+tigOoJuxijGV2ZpJNeRzSNk1QwF4jSLt/tFib2k M3HDzE8JQRZD6F2IsVw== X-Received: by 2002:a05:6000:81:: with SMTP id m1mr13584748wrx.59.1589706735700; Sun, 17 May 2020 02:12:15 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwr4hwTLgFZaX8Akvk2nYGFm7q6w+BvNLLamn1VSWs6NmBSs53s6dgBWh7V00KFM/WZmmUjWw== X-Received: by 2002:a05:6000:81:: with SMTP id m1mr13584722wrx.59.1589706735456; Sun, 17 May 2020 02:12:15 -0700 (PDT) Received: from abulafia.quesejoda.com ([93.176.151.136]) by smtp.gmail.com with ESMTPSA id 37sm11475103wrk.61.2020.05.17.02.12.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 17 May 2020 02:12:14 -0700 (PDT) To: gcc-patches Subject: [committed] Move array bounds checking out of vrp_prop and into its own class. Message-ID: <0adcf9b6-a37b-cef1-2850-e3bd8700c3ab@redhat.com> Date: Sun, 17 May 2020 11:12:13 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.0 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_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Aldy Hernandez via Gcc-patches From: Aldy Hernandez Reply-To: Aldy Hernandez Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Howdy. There's really no reason why the array bounds code should live in tree-vrp, when all it really needs is a get_value_range() call back. Particularly painful is that it partially lives inside the vrp_prop class. This patch moves the array bounds code into its own class, while taking in a vr_values in the constructor which is ONLY used to get at get_value_range(). It is my ultimate desire to move this class into its own file (as a follow-up), minimizing distraction in tree-vrp.c. It can take a vr_values (or similar object), and can be made to work with any ranger (evrp, vrp, or ranger). Approved by Jeff, and committed to mainline. Aldy commit 65d44272bd988f695a9b5fa7e1b88c266c3089cb Author: Aldy Hernandez Date: Tue May 5 18:40:44 2020 +0200 Move array bounds checking out of vrp_prop and into its own class. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 4f48d443235..64681dd97fe 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,15 @@ +2020-05-17 Aldy Hernandez + + * tree-vrp.c (class vrp_prop): Move check_all_array_refs, + check_array_ref, check_mem_ref, and search_for_addr_array + into new class... + (class array_bounds_checker): ...here. + (class check_array_bounds_dom_walker): Adjust to use + array_bounds_checker. + (check_all_array_refs): Move into array_bounds_checker and rename + to check. + (class vrp_folder): Make fold_predicate_in private. + 2020-05-15 Jeff Law * config/h8300/h8300.md (SFI iterator): New iterator for diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 1001c7f41d8..6e3510856a5 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -3521,7 +3521,7 @@ vrp_insert::insert_range_assertions (void) class vrp_prop : public ssa_propagation_engine { - public: +public: enum ssa_prop_result visit_stmt (gimple *, edge *, tree *) FINAL OVERRIDE; enum ssa_prop_result visit_phi (gphi *) FINAL OVERRIDE; @@ -3529,12 +3529,10 @@ class vrp_prop : public ssa_propagation_engine void vrp_initialize (struct function *); void vrp_finalize (bool); - void check_all_array_refs (void); - bool check_array_ref (location_t, tree, bool); - bool check_mem_ref (location_t, tree, bool); - void search_for_addr_array (tree, location_t); class vr_values vr_values; + +private: /* Temporary delegator to minimize code churn. */ const value_range_equiv *get_value_range (const_tree op) { return vr_values.get_value_range (op); } @@ -3552,6 +3550,29 @@ class vrp_prop : public ssa_propagation_engine void extract_range_from_phi_node (gphi *phi, value_range_equiv *vr) { vr_values.extract_range_from_phi_node (phi, vr); } }; + +/* Array bounds checking pass. */ + +class array_bounds_checker +{ + friend class check_array_bounds_dom_walker; + +public: + array_bounds_checker (struct function *fun, class vr_values *v) + : fun (fun), ranges (v) { } + void check (); + +private: + static tree check_array_bounds (tree *tp, int *walk_subtree, void *data); + bool check_array_ref (location_t, tree, bool ignore_off_by_one); + bool check_mem_ref (location_t, tree, bool ignore_off_by_one); + void check_addr_expr (location_t, tree); + const value_range_equiv *get_value_range (const_tree op) + { return ranges->get_value_range (op); } + struct function *fun; + class vr_values *ranges; +}; + /* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible arrays and "struct" hacks. If VRP can determine that the array subscript is a constant, check if it is outside valid @@ -3561,8 +3582,8 @@ class vrp_prop : public ssa_propagation_engine Returns true if a warning has been issued. */ bool -vrp_prop::check_array_ref (location_t location, tree ref, - bool ignore_off_by_one) +array_bounds_checker::check_array_ref (location_t location, tree ref, + bool ignore_off_by_one) { if (TREE_NO_WARNING (ref)) return false; @@ -3760,8 +3781,8 @@ vrp_prop::check_array_ref (location_t location, tree ref, Returns true if a warning has been issued. */ bool -vrp_prop::check_mem_ref (location_t location, tree ref, - bool ignore_off_by_one) +array_bounds_checker::check_mem_ref (location_t location, tree ref, + bool ignore_off_by_one) { if (TREE_NO_WARNING (ref)) return false; @@ -4038,7 +4059,7 @@ vrp_prop::check_mem_ref (location_t location, tree ref, address of an ARRAY_REF, and call check_array_ref on it. */ void -vrp_prop::search_for_addr_array (tree t, location_t location) +array_bounds_checker::check_addr_expr (location_t location, tree t) { /* Check each ARRAY_REF and MEM_REF in the reference chain. */ do @@ -4122,14 +4143,12 @@ vrp_prop::search_for_addr_array (tree t, location_t location) } } -/* walk_tree() callback that checks if *TP is - an ARRAY_REF inside an ADDR_EXPR (in which an array - subscript one outside the valid range is allowed). Call - check_array_ref for each ARRAY_REF found. The location is - passed in DATA. */ +/* Callback for walk_tree to check a tree for out of bounds array + accesses. The array_bounds_checker class is passed in DATA. */ -static tree -check_array_bounds (tree *tp, int *walk_subtree, void *data) +tree +array_bounds_checker::check_array_bounds (tree *tp, int *walk_subtree, + void *data) { tree t = *tp; struct walk_stmt_info *wi = (struct walk_stmt_info *) data; @@ -4143,14 +4162,16 @@ check_array_bounds (tree *tp, int *walk_subtree, void *data) *walk_subtree = TRUE; bool warned = false; - vrp_prop *vrp_prop = (class vrp_prop *)wi->info; + array_bounds_checker *checker = (array_bounds_checker *) wi->info; if (TREE_CODE (t) == ARRAY_REF) - warned = vrp_prop->check_array_ref (location, t, false/*ignore_off_by_one*/); + warned = checker->check_array_ref (location, t, + false/*ignore_off_by_one*/); else if (TREE_CODE (t) == MEM_REF) - warned = vrp_prop->check_mem_ref (location, t, false /*ignore_off_by_one*/); + warned = checker->check_mem_ref (location, t, + false /*ignore_off_by_one*/); else if (TREE_CODE (t) == ADDR_EXPR) { - vrp_prop->search_for_addr_array (t, location); + checker->check_addr_expr (location, t); *walk_subtree = FALSE; } /* Propagate the no-warning bit to the outer expression. */ @@ -4160,26 +4181,26 @@ check_array_bounds (tree *tp, int *walk_subtree, void *data) return NULL_TREE; } -/* A dom_walker subclass for use by vrp_prop::check_all_array_refs, - to walk over all statements of all reachable BBs and call - check_array_bounds on them. */ +/* A dom_walker subclass for use by check_all_array_refs, to walk over + all statements of all reachable BBs and call check_array_bounds on + them. */ class check_array_bounds_dom_walker : public dom_walker { - public: - check_array_bounds_dom_walker (vrp_prop *prop) +public: + check_array_bounds_dom_walker (array_bounds_checker *checker) : dom_walker (CDI_DOMINATORS, /* Discover non-executable edges, preserving EDGE_EXECUTABLE flags, so that we can merge in information on non-executable edges from vrp_folder . */ REACHABLE_BLOCKS_PRESERVING_FLAGS), - m_prop (prop) {} + checker (checker) { } ~check_array_bounds_dom_walker () {} edge before_dom_children (basic_block) FINAL OVERRIDE; - private: - vrp_prop *m_prop; +private: + array_bounds_checker *checker; }; /* Implementation of dom_walker::before_dom_children. @@ -4201,9 +4222,9 @@ check_array_bounds_dom_walker::before_dom_children (basic_block bb) memset (&wi, 0, sizeof (wi)); - wi.info = m_prop; + wi.info = checker; - walk_gimple_op (stmt, check_array_bounds, &wi); + walk_gimple_op (stmt, array_bounds_checker::check_array_bounds, &wi); } /* Determine if there's a unique successor edge, and if so, return @@ -4213,11 +4234,10 @@ check_array_bounds_dom_walker::before_dom_children (basic_block bb) return find_taken_edge (bb, NULL_TREE); } -/* Walk over all statements of all reachable BBs and call check_array_bounds - on them. */ +/* Entry point into array bounds checking pass. */ void -vrp_prop::check_all_array_refs () +array_bounds_checker::check () { check_array_bounds_dom_walker w (this); w.walk (ENTRY_BLOCK_PTR_FOR_FN (fun)); @@ -4851,14 +4871,15 @@ vrp_prop::visit_phi (gphi *phi) class vrp_folder : public substitute_and_fold_engine { - public: +public: vrp_folder () : substitute_and_fold_engine (/* Fold all stmts. */ true) { } tree get_value (tree) FINAL OVERRIDE; bool fold_stmt (gimple_stmt_iterator *) FINAL OVERRIDE; - bool fold_predicate_in (gimple_stmt_iterator *); class vr_values *vr_values; +private: + bool fold_predicate_in (gimple_stmt_iterator *); /* Delegators. */ tree vrp_evaluate_conditional (tree_code code, tree op0, tree op1, gimple *stmt) @@ -5293,7 +5314,10 @@ vrp_prop::vrp_finalize (bool warn_array_bounds_p) vrp_folder.substitute_and_fold (); if (warn_array_bounds && warn_array_bounds_p) - check_all_array_refs (); + { + array_bounds_checker array_checker (fun, &vr_values); + array_checker.check (); + } } /* Main entry point to VRP (Value Range Propagation). This pass is