From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x22f.google.com (mail-lj1-x22f.google.com [IPv6:2a00:1450:4864:20::22f]) by sourceware.org (Postfix) with ESMTPS id 31ABA3858D33 for ; Fri, 31 Mar 2023 09:24:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 31ABA3858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-x22f.google.com with SMTP id z42so22335565ljq.13 for ; Fri, 31 Mar 2023 02:24:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1680254691; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=fo/kquQdGs+SADT6w5jycufEgt3Lh4J6kvxjJ9osYhI=; b=bX1oiX6ET2/wKTIoJ4LqZr57JDjYUI9ooFAaUSuA0EJZ/m7HC6eCzn68ooWACdlEeQ rkiaAbxFh8z+6BhweYWq3amvPJbqQ2x9eWgcf7KpUTc763BARJPrnuyjgDTVmZNsZlZK dAvsk5EHLhwQTpY5mnrjvI1A4XkpPCh9xc+hYXsZMtHUITt/OBTWSoYo+TEUUptRMBvU k0YgvtpWJNnHJBDUFqYbBshwl1nJOdeCxyBG9x2VmpPMaLQoqvGA+mVV9F83CL6+NTmW kjiuYPzgIWMTCfbEkWr/jrecAKaJdfOKYXWCTpv+14XPi8FeLcl0qQxMScjil4pPiEN6 u16g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680254691; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=fo/kquQdGs+SADT6w5jycufEgt3Lh4J6kvxjJ9osYhI=; b=Rk0pyB1SNw/btSMTLDQnllfoxL9Dzk1iWMG+QjoBbw6Dm5P6sTXi7DduBQU0HC08/x PSndMNZryI2RsMpUBkTqVLLD9BBpTi4Td0M5xvPV8eco9W7m7+19CwBkvVeJ09juoNuj zX18LZZBiBcYn30rTzBawk7TTUljLy+76eRTqodhhtmNd/ojDEOVFReW2h6K1wk96cOx KXgehhl3RW7IYrdW78FUFdO8TNEYQgtdKQzUPn+Kqg1Ey0Mv8KIMfjyIb3cNfLTe2jx7 bfycqa6/Fs3MvQpVZzHkESZkBpYG62dIcekNKEurAmkZ0k0p6jBGicjIwUE/snT+29D8 meeQ== X-Gm-Message-State: AAQBX9e9JCygIgxBra6/D6LeoBgE9LYmW9wyC/yGYS690/lJ7jgmQVFW ASyoPdWNNUPs6+yPIl7jVC+g0vRgQXwPtLInG+I= X-Google-Smtp-Source: AKy350bF8+sCl4RQ4riesOEFHfTYd+JilxFkrhpnNVdOL/CTRCw797e2l6FezrcZAM1QaROyYEWQ7PrAoFlJ1I6zghQ= X-Received: by 2002:a2e:95d8:0:b0:29a:9053:ed1b with SMTP id y24-20020a2e95d8000000b0029a9053ed1bmr8126176ljh.3.1680254691545; Fri, 31 Mar 2023 02:24:51 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Fri, 31 Mar 2023 11:24:39 +0200 Message-ID: Subject: Re: [PATCH] ipa: Avoid constructing aggregate jump functions with huge offsets (PR 109303) To: Martin Jambor Cc: GCC Patches , Jan Hubicka Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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: On Fri, Mar 31, 2023 at 11:18=E2=80=AFAM Martin Jambor wr= ote: > > Hi, > > On Fri, Mar 31 2023, Richard Biener wrote: > > On Fri, Mar 31, 2023 at 10:46=E2=80=AFAM Martin Jambor wrote: > >> > >> Hi, > >> > >> we are in the process of changing data structures holding information > >> about constants passed by reference and in aggregates to use unsigned > >> int offsets rather than HOST_WIDE_INT (which was selected simply > >> because that is what fell out of get_ref_base_and_extent at that time) > >> in order to conserve memory, especially at WPA time. > >> > >> PR 109303 testcase discovers that we do not properly check that we > >> only create jump functions with offsets (plus sizes) that fit into the > >> smaller type. This patch adds the necessary check. > >> > >> Bootstrapped and tested on x86_64-linux. OK for master? > >> > >> Thanks, > >> > >> Martin > >> > >> > >> gcc/ChangeLog: > >> > >> 2023-03-30 Martin Jambor > >> > >> PR ipa/109303 > >> * ipa-prop.cc (determine_known_aggregate_parts): Check that th= e > >> offset + size will be representable in unsigned int. > >> > >> gcc/testsuite/ChangeLog: > >> > >> 2023-03-30 Jakub Jelinek > >> Martin Jambor > >> > >> PR ipa/109303 > >> * gcc.dg/pr109303.c: New test. > >> --- > >> gcc/ipa-prop.cc | 4 +++- > >> gcc/testsuite/gcc.dg/pr109303.c | 24 ++++++++++++++++++++++++ > >> 2 files changed, 27 insertions(+), 1 deletion(-) > >> create mode 100644 gcc/testsuite/gcc.dg/pr109303.c > >> > >> diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc > >> index de45dbccf16..9ffd49b590c 100644 > >> --- a/gcc/ipa-prop.cc > >> +++ b/gcc/ipa-prop.cc > >> @@ -2086,7 +2086,9 @@ determine_known_aggregate_parts (struct ipa_func= _body_info *fbi, > >> whether its value is clobbered any other dominating one. = */ > >> if ((content->value.pass_through.formal_id >=3D 0 > >> || content->value.pass_through.operand) > >> - && !clobber_by_agg_contents_list_p (all_list, content)) > >> + && !clobber_by_agg_contents_list_p (all_list, content) > >> + && (content->offset + content->size - arg_offset > >> + <=3D (HOST_WIDE_INT) UINT_MAX * BITS_PER_UNIT)) > >> { > > > > it does seem a bit misplaced since after the if we add the same > > 'content' to another > > list anyway. > > The other list is a clobber list, as we crawl backwards from the call > statement searching for stores, we also look whether we have already > encountered a store of something else to an overlapping area. In theory > we could have a store to a smaller data type, where the offset + size > would still fit unsigned int, be followed by a larger store, which would > not. We want the large store to end up in the clobber list so that the > smaller one does not. > > This is the place where we also calculate the size of the final > heap-allocated vector, so that is why eventually I put it there. > > > Wouldn't a more obvious place be where we end up truncating this sum? > > My reasoning was that since we know we would not be able to use it, it > makes sense to discard the data before we stream it from compilation to > WPA. > > Also, when we shorten the offset type also in ipa_agg_jf_item (which is > what I want to do next), this is where the check eventually needs to > be. OK, maybe add the above as comment then. OK with that change. Richard. > Thanks, > > Martin