From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x834.google.com (mail-qt1-x834.google.com [IPv6:2607:f8b0:4864:20::834]) by sourceware.org (Postfix) with ESMTPS id C5AE33858D32 for ; Thu, 7 Jul 2022 08:02:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C5AE33858D32 Received: by mail-qt1-x834.google.com with SMTP id ck6so21633968qtb.7 for ; Thu, 07 Jul 2022 01:02:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=KXmXN00jWurKOnW6hw6eMjCN3EYdAJSWh/q4QwjiyRE=; b=N+QSvo+q4m7TsMIXQedFkB9m0hV4xSkReCtMIoqweKcAh9rXkEshjRvcCGP7BjpVaa vzy4RcUH9MfFS4FUWZ1gPyCTa4q5M9jRU6mfUqsm3yWYfuZeo+4k7GnHsTqLERJDnEcf wFFaswDlU67vHgSUSY9SJhDvF7Sc/4UPqCKm41tTgBdQnm0jKaMraZuAnBqCg5PbWWWg CmghpWr8IiYaCxLFPbldnMIHaCPZn59JeLWVDYvWIs/vvrLAWAbEInGlKb+q6Fa6rRvU bX/Tpru7riL5omn1RKLWAYI+tgYrSJ//0U6z9Ya2OHdbeQOuCfj4hlalvC8WV3FwYjQx hCEw== X-Gm-Message-State: AJIora+d3jRqJ2la6eQadDY56MAbvSPh78974GeGTD1OZtOvNqRiVi+l iai/DZoT7TobmA3KU3COphdS9dZMNnUPBdX7xFA= X-Google-Smtp-Source: AGRyM1uJF/om5mWAuiNfA6hLoyOe/P+OIE1+omTBveHCGXy24LjTIo1wf0Xs/xhdOa5/dxZFNIGjbiNxtJuoYKuYRN4= X-Received: by 2002:a05:6214:27c6:b0:472:ff4f:f103 with SMTP id ge6-20020a05621427c600b00472ff4ff103mr13303951qvb.55.1657180940086; Thu, 07 Jul 2022 01:02:20 -0700 (PDT) MIME-Version: 1.0 References: <6CB6B076-0635-4DE8-861E-F8EBC0B696B4@gmail.com> <7306B3D0-01A8-47F8-8BE2-F5E312FC94D1@oracle.com> <8ef7b17b-5a12-b718-7b04-f8ae7e0611bb@gmail.com> In-Reply-To: From: Richard Biener Date: Thu, 7 Jul 2022 10:02:08 +0200 Message-ID: Subject: Re: [GCC 13][PATCH] PR101836: Add a new option -fstrict-flex-array[=n] and use it in __builtin_object_size To: Qing Zhao Cc: Martin Sebor , Jakub Jelinek , gcc-patches Paul A Clarke via , kees Cook Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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-List-Received-Date: Thu, 07 Jul 2022 08:02:22 -0000 On Wed, Jul 6, 2022 at 4:20 PM Qing Zhao wrote: > > (Sorry for the late reply, just came back from a short vacation.) > > > On Jul 4, 2022, at 2:49 AM, Richard Biener = wrote: > > > > On Fri, Jul 1, 2022 at 5:32 PM Martin Sebor wrote: > >> > >> On 7/1/22 08:01, Qing Zhao wrote: > >>> > >>> > >>>> On Jul 1, 2022, at 8:59 AM, Jakub Jelinek wrote: > >>>> > >>>> On Fri, Jul 01, 2022 at 12:55:08PM +0000, Qing Zhao wrote: > >>>>> If so, comparing to the current implemenation to have all the check= ing in middle-end, what=E2=80=99s the > >>>>> major benefit of moving part of the checking into FE, and leaving t= he other part in middle-end? > >>>> > >>>> The point is recording early what FIELD_DECLs could be vs. can't pos= sibly be > >>>> treated like flexible array members and just use that flag in the de= cisions > >>>> in the current routines in addition to what it is doing. > >>> > >>> Okay. > >>> > >>> Based on the discussion so far, I will do the following: > >>> > >>> 1. Add a new flag =E2=80=9CDECL_NOT_FLEXARRAY=E2=80=9D to FIELD_DECL; > >>> 2. In C/C++ FE, set the new flag =E2=80=9CDECL_NOT_FLEXARRAY=E2=80=9D= for a FIELD_DECL based on [0], [1], > >>> [] and the option -fstrict-flex-array, and whether it=E2=80=99s t= he last field of the DECL_CONTEXT. > >>> 3. In Middle end, Add a new utility routine is_flexible_array_member= _p, which bases on > >>> DECL_NOT_FLEXARRAY + array_at_struct_end_p to decide whether the = array > >>> reference is a real flexible array member reference. > > > > I would just update all existing users, not introduce another wrapper > > that takes DECL_NOT_FLEXARRAY > > into account additionally. > > Okay. > > > >>> > >>> > >>> Middle end currently is quite mess, array_at_struct_end_p, component_= ref_size, and all the phases that > >>> use these routines need to be updated, + new testing cases for each o= f the phases. > >>> > >>> > >>> So, I still plan to separate the patch set into 2 parts: > >>> > >>> Part A: the above 1 + 2 + 3, and use these new utilities in tre= e-object-size.cc to resolve PR101836 first. > >>> Then kernel can use __FORTIFY_SOURCE correctly; > >>> > >>> Part B: update all other phases with the new utilities + new tes= ting cases + resolving regressions. > >>> > >>> Let me know if you have any comment and suggestion. > >> > >> It might be worth considering whether it should be possible to control > >> the "flexible array" property separately for each trailing array membe= r > >> via either a #pragma or an attribute in headers that can't change > >> the struct layout but that need to be usable in programs compiled with > >> stricter -fstrict-flex-array=3DN settings. > > > > Or an decl attribute. > > Yes, it might be necessary to add a corresponding decl attribute > > strict_flex_array (N) > > Which is attached to a trailing structure array member to provide the use= r a finer control when -fstrict-flex-array=3DN is specified. > > So, I will do the following: > > > *****User interface: > > 1. command line option: > -fstrict-flex-array=3DN (N=3D0, 1, 2, 3) > 2. decl attribute: > strict_flex_array (N) (N=3D0, 1, 2, 3) > > > *****Implementation: > > 1. Add a new flag =E2=80=9CDECL_NOT_FLEXARRAY=E2=80=9D to FIELD_DECL; > 2. In C/C++ FE, set the new flag =E2=80=9CDECL_NOT_FLEXARRAY=E2=80=9D for= a FIELD_DECL based on [0], [1], > [], the option -fstrict-flex-array, the attribute strict_flex_array,= and whether it=E2=80=99s the last field > of the DECL_CONTEXT. > 3. In Middle end, update all users of =E2=80=9Carray_at_struct_end_p" o= r =E2=80=9Ccomponent_ref_size=E2=80=9D, or any place that treats > Trailing array as flexible array member with the new flag DECL_NOT_F= LEXARRAY. > (Still think we need a new consistent utility routine here). > > > I still plan to separate the patch set into 2 parts: > > Part A: the above 1 + 2 + 3, and use these new utilities in tree-obje= ct-size.cc to resolve PR101836 first. > Then kernel can use __FORTIFY_SOURCE correctly. > Part B: update all other phases with the new utilities + new testing c= ases + resolving regressions. > > > Let me know any more comment or suggestion. Sounds good. Part 3. is "optimization" and reasonable to do separately, I'm not sure you need 'B' (since we're not supposed to have new utilities), but instead I'd do '3.' as part of 'B', just changing the pieces th resolve PR101836 for part 'A'. Richard. > Thanks a lot. > > Qing > >