From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa4.mentor.iphmx.com (esa4.mentor.iphmx.com [68.232.137.252]) by sourceware.org (Postfix) with ESMTPS id 8A9143858CD1 for ; Wed, 5 Jul 2023 07:51:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8A9143858CD1 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="6.01,182,1684828800"; d="scan'208,223";a="10985954" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa4.mentor.iphmx.com with ESMTP; 04 Jul 2023 23:51:11 -0800 IronPort-SDR: /WUYEjF+fvX6zOsgMLovkWrz9xHKBpdZuCjN7H+ql4HdfldEtAbvP5hrcrNxCXtP7DV7j9HrEi fKrUZKuGfhTLFXn8IfeLHBaNUK3UpR6yA3rrUTRMEzPKJSY7R1eF1boO1PHsUQtDl8RziIJaj/ /62naLW+qiIFvTqKdcUkd1BhbC1bfmToDKCxMCLsFPkAXPsTACQno4voDj8qIHGJuInFhtuXps a730sgHARqEG46CnBHDOsDszWCyjDdH+qBcidMO2LxW0gSllBUDSmuH6Vto7EEGwWdjamUwLOf 5fw= From: Thomas Schwinge To: Lewis Hyatt , Subject: GTY: Explicitly reject 'string_length' option for (fields in) global variables (was: [PATCH] pch: Fix streaming of strings with embedded null bytes) In-Reply-To: References: User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/28.2 (x86_64-pc-linux-gnu) Date: Wed, 5 Jul 2023 09:50:58 +0200 Message-ID: <87bkgqvlst.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-12.mgc.mentorg.com (139.181.222.12) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,SPF_HELO_PASS,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 List-Id: --=-=-= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi! On 2022-10-18T18:14:54-0400, Lewis Hyatt via Gcc-patches wrote: > [...] add a new > GTY option "string_length" so that gt_pch_note_object can be informed the > actual length it ought to use, [...] > --- a/gcc/doc/gty.texi > +++ b/gcc/doc/gty.texi > @@ -196,7 +196,26 @@ static GTY((length("reg_known_value_size"))) rtx *re= g_known_value; > Note that the @code{length} option is only meant for use with arrays of > non-atomic objects, that is, objects that contain pointers pointing to > other GTY-managed objects. For other GC-allocated arrays and strings > -you should use @code{atomic}. > +you should use @code{atomic} or @code{string_length}. > + > +@findex string_length > +@item string_length ("@var{expression}") > + > +In order to simplify production of PCH, a structure member that is a pla= in > +array of bytes (an optionally @code{const} and/or @code{unsigned} @code{= char > +*}) is treated specially by the infrastructure. Even if such an array ha= s not > +been allocated in GC-controlled memory, it will still be written properl= y into > +a PCH. The machinery responsible for this needs to know the length of t= he > +data; by default, the length is determined by calling @code{strlen} on t= he > +pointer. The @code{string_length} option specifies an alternate way to > +determine the length, such as by inspecting another struct member: > + > +@smallexample > +struct GTY(()) non_terminated_string @{ > + size_t sz; > + const char * GTY((string_length ("%h.sz"))) data; > +@}; > +@end smallexample In preparation for another thing I'm working on, OK to push the attached "GTY: Explicitly reject 'string_length' option for (fields in) global varia= bles" (with pointing to this message)? Gr=C3=BC=C3=9Fe Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename="0001-GTY-Explicitly-reject-string_length-option-for-field.patch" >From 9130fe7873c2e1b44ab2449bfe022837e26f710c Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Tue, 4 Jul 2023 11:46:50 +0200 Subject: [PATCH] GTY: Explicitly reject 'string_length' option for (fields in) global variables This is preparational for another thing that I'm working on. No change in behavior -- other than a more explicit error message. The 'string_length' option currently is not supported for (fields in) global variables. For example, if we apply the following (made-up) changes: --- gcc/c-family/c-cppbuiltin.cc +++ gcc/c-family/c-cppbuiltin.cc @@ -1777 +1777 @@ struct GTY(()) lazy_hex_fp_value_struct - const char *hex_str; + const char * GTY((string_length("strlen(%h.hex_str) + 1"))) hex_str; --- gcc/varasm.cc +++ gcc/varasm.cc @@ -66 +66 @@ along with GCC; see the file COPYING3. If not see -extern GTY(()) const char *first_global_object_name; +extern GTY((string_length("strlen(%h.first_global_object_name) + 1"))) const char *first_global_object_name; ..., we get: [...] build/gengtype \ -S [...]/source-gcc/gcc -I gtyp-input.list -w tmp-gtype.state /bin/sh [...]/source-gcc/gcc/../move-if-change tmp-gtype.state gtype.state build/gengtype \ -r gtype.state [...]/source-gcc/gcc/varasm.cc:66: global `first_global_object_name' has unknown option `string_length' [...]/source-gcc/gcc/c-family/c-cppbuiltin.cc:1789: field `hex_str' of global `lazy_hex_fp_values[0]' has unknown option `string_length' make[2]: *** [Makefile:2890: s-gtype] Error 1 [...] These errors occur when writing "GC roots", where -- per my understanding -- 'string_length' isn't relevant for actual GC purposes. However, like elsewhere, it is for PCH purposes, and simply accepting 'string_length' here isn't sufficient: we'll still get '(gt_pointer_walker) >_pch_n_S' used in the 'struct ggc_root_tab' instances, and there's no easy way to change that to instead use 'gt_pch_n_S2' with explicit 'size_t string_len' argument. (At least not sufficiently easy to justify spending any further time on, given that I don't have an actual use for that feature.) So, until an actual need arises, and/or to avoid the next person looking into this having to figure out the same thing again, let's just document this limitation: [...]/source-gcc/gcc/varasm.cc:66: option `string_length' not supported for global `first_global_object_name' [...]/source-gcc/gcc/c-family/c-cppbuiltin.cc:1789: option `string_length' not supported for field `hex_str' of global `lazy_hex_fp_values[0]' This amends commit f3b957ea8b9dadfb1ed30f24f463529684b7a36a "pch: Fix streaming of strings with embedded null bytes". gcc/ * gengtype.cc (write_root, write_roots): Explicitly reject 'string_length' option. * doc/gty.texi (GTY Options) : Document. --- gcc/doc/gty.texi | 4 ++++ gcc/gengtype.cc | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/gcc/doc/gty.texi b/gcc/doc/gty.texi index 9fc509128fe..24799aa6781 100644 --- a/gcc/doc/gty.texi +++ b/gcc/doc/gty.texi @@ -217,6 +217,10 @@ struct GTY(()) non_terminated_string @{ @}; @end smallexample +The @code{string_length} option currently is not supported for (fields +in) global variables. +@c + @findex skip @item skip diff --git a/gcc/gengtype.cc b/gcc/gengtype.cc index 7763f40e9ab..04dbb0de8bd 100644 --- a/gcc/gengtype.cc +++ b/gcc/gengtype.cc @@ -4337,6 +4337,11 @@ write_root (outf_p f, pair_p v, type_p type, const char *name, int has_length, else if (strcmp (o->name, "desc") == 0 && o->kind == OPTION_STRING) desc = o->info.string; + else if (strcmp (o->name, "string_length") == 0) + /* See 'doc/gty.texi'. */ + error_at_line (line, + "option `%s' not supported for field `%s' of global `%s'", + o->name, fld->name, name); else error_at_line (line, "field `%s' of global `%s' has unknown option `%s'", @@ -4535,6 +4540,11 @@ write_roots (pair_p variables, bool emit_pch) deletable_p = 1; else if (strcmp (o->name, "cache") == 0) ; + else if (strcmp (o->name, "string_length") == 0) + /* See 'doc/gty.texi'. */ + error_at_line (&v->line, + "option `%s' not supported for global `%s'", + o->name, v->name); else error_at_line (&v->line, "global `%s' has unknown option `%s'", -- 2.34.1 --=-=-=--