From patchwork Tue Mar 3 20:29:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 1248653 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=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-520563-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha1 header.s=default header.b=o5rExYgN; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=MYXI5QXK; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48X7sd5007z9sNg for ; Wed, 4 Mar 2020 07:29:39 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type:content-transfer-encoding; q=dns; s=default; b=upg cX7cQveM61o/hsj8S0sfMetiA/2ZL9ASUF2pfc+qHWZKzK5BljXY5MXLW8iNgfl1 RO0YRMj7zKhCXlc21RKZ20NSt/V7pfAsCzDv881kMkNPYXtNgPLCuE4np1g2kXXK H9BdzWzq9q0gBNiLxfoNPqfW7hd6Qt/pHw+kcjV4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type:content-transfer-encoding; s=default; bh=t/tq9HhcI G/fUZZUw5qbCfCL9Zk=; b=o5rExYgNZRhMtQP1bimthnv3HBeGbEi02TMghpXs3 f2sO85nq82aS7DMRWtMq+udOrblFOcVg3QaiLelkO6aLHLiqzk0PCSVUpl6pHGuA dP1Dda1m+KH7GHxNvY5sE6m6+2GtU+SJtONKzMYCIpYuK/7JZ5vH0cbA4Nhiy/KB rI= Received: (qmail 67548 invoked by alias); 3 Mar 2020 20:29:32 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 67540 invoked by uid 89); 3 Mar 2020 20:29:31 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-8.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=anticipation X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-1.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (205.139.110.61) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 03 Mar 2020 20:29:30 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1583267368; h=from:from:reply-to: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=KdHoYrpWW18K6XkyOKCevXsAQaHttUc1FbW+Qj2HHY4=; b=MYXI5QXKft1EgomuYYZshr37LJONVfAAVfoJdzm7Yh8KwSvmJP5N2olAYtwVCuSnQkl3NW law9lSRsg/4wj5p5x+1gsVbr5mw00IDI4eJ308o8yR8tFUf7pOAOT2QQBIkQqSTSwSn9Rr aLEfUDvpGh0KIpVYgezPtTjvndBOa5I= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-294-Wl7TnHoEOqa5YP4SgmSRhw-1; Tue, 03 Mar 2020 15:29:25 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 48A6F8017CC; Tue, 3 Mar 2020 20:29:24 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-37.ams2.redhat.com [10.36.116.37]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D12575DA2C; Tue, 3 Mar 2020 20:29:23 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id 023KTLHE028484; Tue, 3 Mar 2020 21:29:22 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id 023KTKXb028483; Tue, 3 Mar 2020 21:29:20 +0100 Date: Tue, 3 Mar 2020 21:29:20 +0100 From: Jakub Jelinek To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] sccvn: Avoid overflows in push_partial_def Message-ID: <20200303202920.GD2156@tucnak> Reply-To: Jakub Jelinek MIME-Version: 1.0 User-Agent: Mutt/1.11.3 (2019-02-01) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-IsSubscribed: yes Hi! The following patch attempts to avoid dangerous overflows in the various push_partial_def HOST_WIDE_INT computations. This is achieved by performing the subtraction offset2i - offseti in the push_partial_def function and before doing that doing some tweaks. If a constant store (non-CONSTRUCTOR) is too large (perhaps just hypothetical case), native_encode_expr would fail for it, but we don't necessarily need to fail right away, instead we can treat it like non-constant store and if it is already shadowed, we can ignore it. Otherwise, if it at most 64-byte and the caller ensured that there is a range overlap and push_partial_def ensures the load is at most 64-byte, I think we should be fine, offset (relative to the load) can be from -64*8+1 to 64*8-1 only and size at most 64*8, so no risks of overflowing HOST_WIDE_INT computations. For CONSTRUCTOR (or non-constant) stores, those can be indeed arbitrarily large, the caller just checks that both the absolute offset and size fit into signed HWI. But, we store the same bytes in that case over and over (both in the {} case where it is all 0, and in the hypothetical future case where we handle in push_partial_def also memset (, 123, )), so we can tweak the write range for our purposes. For {} store we could just cap it at the start offset and/or offset+size because all the bits are 0, but I wrote it in anticipation of the memset case and so the relative offset can now be down to -7 and similarly size can grow up to 64 bytes + 14 bits, all this trying to preserve the offset difference % BITS_PER_UNIT or end as well. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? I've tried to construct a testcase and came with /* PR tree-optimization/93582 */ /* { dg-do compile { target lp64 } } */ union U { struct A { unsigned long long a : 1, b : 62, c : 1; } a; unsigned long long i; }; unsigned long long foo (char *p) { __builtin_memset (p - 0xfffffffffffffffULL, 0, 0xffffffffffffffeULL); __builtin_memset (p + 1, 0, 0xffffffffffffffeULL); union U *q = (union U *) (void *) (p - 4); q->a.b = -1; return q->i; } With this testcase, one can see signed integer overflows in the compiler without the patch. But unfortunately even with the patch it isn't optimized as it should. I believe the problem is in: gimple *def = SSA_NAME_DEF_STMT (ref2); if (is_gimple_assign (def) && gimple_assign_rhs_code (def) == POINTER_PLUS_EXPR && gimple_assign_rhs1 (def) == TREE_OPERAND (base, 0) && poly_int_tree_p (gimple_assign_rhs2 (def)) && (wi::to_poly_offset (gimple_assign_rhs2 (def)) << LOG2_BITS_PER_UNIT).to_shwi (&offset2)) { where POINTER_PLUS_EXPR last operand has sizetype type, thus unsigned, and in the testcase gimple_assign_rhs2 (def) is thus 0xf000000000000001ULL which multiplied by 8 doesn't fit into signed HWI. If it would be treated as signed offset instead, it would fit (-0xfffffffffffffffLL, multiplied by 8 is -0x7ffffffffffffff8LL). Unfortunately with the poly_int obfuscation I'm not sure how to convert it from unsigned to signed poly_int. 2020-03-03 Jakub Jelinek * tree-ssa-sccvn.c (vn_walk_cb_data::push_partial_def): Add offseti argument. Change pd argument so that it can be modified. Turn constant non-CONSTRUCTOR store into non-constant if it is too large. Adjust offset and size of CONSTRUCTOR or non-constant store to avoid overflows. (vn_walk_cb_data::vn_walk_cb_data, vn_reference_lookup_3): Adjust callers. Jakub --- gcc/tree-ssa-sccvn.c.jj 2020-03-03 11:20:52.761545034 +0100 +++ gcc/tree-ssa-sccvn.c 2020-03-03 16:22:49.387657379 +0100 @@ -1716,7 +1716,7 @@ struct vn_walk_cb_data else pd.offset = pos; pd.size = tz; - void *r = push_partial_def (pd, 0, 0, prec); + void *r = push_partial_def (pd, 0, 0, 0, prec); gcc_assert (r == NULL_TREE); } pos += tz; @@ -1733,8 +1733,9 @@ struct vn_walk_cb_data } ~vn_walk_cb_data (); void *finish (alias_set_type, alias_set_type, tree); - void *push_partial_def (const pd_data& pd, - alias_set_type, alias_set_type, HOST_WIDE_INT); + void *push_partial_def (pd_data pd, + alias_set_type, alias_set_type, HOST_WIDE_INT, + HOST_WIDE_INT); vn_reference_t vr; ao_ref orig_ref; @@ -1817,8 +1818,9 @@ pd_tree_dealloc (void *, void *) on failure. */ void * -vn_walk_cb_data::push_partial_def (const pd_data &pd, +vn_walk_cb_data::push_partial_def (pd_data pd, alias_set_type set, alias_set_type base_set, + HOST_WIDE_INT offseti, HOST_WIDE_INT maxsizei) { const HOST_WIDE_INT bufsize = 64; @@ -1831,6 +1833,27 @@ vn_walk_cb_data::push_partial_def (const || BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN) return (void *)-1; + /* Turn too large constant stores into non-constant stores. */ + if (CONSTANT_CLASS_P (pd.rhs) && pd.size > bufsize * BITS_PER_UNIT) + pd.rhs = error_mark_node; + + /* And for non-constant or CONSTRUCTOR stores shrink them to only keep at + most a partial byte before and/or after the region. */ + if (!CONSTANT_CLASS_P (pd.rhs)) + { + if (pd.offset < offseti) + { + HOST_WIDE_INT o = ROUND_DOWN (offseti - pd.offset, BITS_PER_UNIT); + gcc_assert (pd.size > o); + pd.size -= o; + pd.offset += o; + } + if (pd.size > maxsizei) + pd.size = maxsizei + ((pd.size - maxsizei) % BITS_PER_UNIT); + } + + pd.offset -= offseti; + bool pd_constant_p = (TREE_CODE (pd.rhs) == CONSTRUCTOR || CONSTANT_CLASS_P (pd.rhs)); if (partial_defs.is_empty ()) @@ -2736,9 +2759,9 @@ vn_reference_lookup_3 (ao_ref *ref, tree { pd_data pd; pd.rhs = build_constructor (NULL_TREE, NULL); - pd.offset = offset2i - offseti; + pd.offset = offset2i; pd.size = leni << LOG2_BITS_PER_UNIT; - return data->push_partial_def (pd, 0, 0, maxsizei); + return data->push_partial_def (pd, 0, 0, offseti, maxsizei); } } @@ -2785,11 +2808,11 @@ vn_reference_lookup_3 (ao_ref *ref, tree by a later def. */ pd_data pd; pd.rhs = gimple_assign_rhs1 (def_stmt); - pd.offset = offset2i - offseti; + pd.offset = offset2i; pd.size = size2i; return data->push_partial_def (pd, ao_ref_alias_set (&lhs_ref), ao_ref_base_alias_set (&lhs_ref), - maxsizei); + offseti, maxsizei); } } } @@ -2936,11 +2959,11 @@ vn_reference_lookup_3 (ao_ref *ref, tree if (TREE_CODE (rhs) == SSA_NAME) rhs = SSA_VAL (rhs); pd.rhs = rhs; - pd.offset = offset2i - offseti; + pd.offset = offset2i; pd.size = size2i; return data->push_partial_def (pd, ao_ref_alias_set (&lhs_ref), ao_ref_base_alias_set (&lhs_ref), - maxsizei); + offseti, maxsizei); } } } @@ -3014,11 +3037,11 @@ vn_reference_lookup_3 (ao_ref *ref, tree { pd_data pd; pd.rhs = SSA_VAL (def_rhs); - pd.offset = offset2i - offseti; + pd.offset = offset2i; pd.size = size2i; return data->push_partial_def (pd, ao_ref_alias_set (&lhs_ref), ao_ref_base_alias_set (&lhs_ref), - maxsizei); + offseti, maxsizei); } } } @@ -3154,7 +3177,7 @@ vn_reference_lookup_3 (ao_ref *ref, tree pd.size = maxsizei; return data->push_partial_def (pd, ao_ref_alias_set (&lhs_ref), ao_ref_base_alias_set (&lhs_ref), - maxsizei); + 0, maxsizei); } }