From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp001-out.apm-internet.net (smtp001-out.apm-internet.net [85.119.248.222]) by sourceware.org (Postfix) with ESMTPS id 30F3F385C405 for ; Wed, 10 Nov 2021 20:24:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 30F3F385C405 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=sandoe.co.uk Received: (qmail 18131 invoked from network); 10 Nov 2021 20:24:04 -0000 X-APM-Out-ID: 16365758441812 X-APM-Authkey: 257869/1(257869/1) 9 Received: from unknown (HELO ?192.168.1.214?) (81.138.1.83) by smtp001.apm-internet.net with SMTP; 10 Nov 2021 20:24:04 -0000 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.21\)) Subject: Re: [PATCH] pch: Add support for PCH for relocatable executables From: Iain Sandoe In-Reply-To: Date: Wed, 10 Nov 2021 20:24:04 +0000 Cc: Richard Biener , GCC Patches , David Malcolm Content-Transfer-Encoding: quoted-printable Message-Id: References: <20211105095411.GG304296@tucnak> <20211105152515.GK304296@tucnak> <20211108114604.GI2710@tucnak> <20211108194807.GJ2710@tucnak> <6n83494-274o-4r5r-552n-8195p08748o7@fhfr.qr> <788587B8-9E00-4A7C-87B8-FBFBC6151070@sandoe.co.uk> <20211109121836.GQ2710@tucnak> To: Jakub Jelinek X-Mailer: Apple Mail (2.3445.104.21) X-Spam-Status: No, score=-9.0 required=5.0 tests=BAYES_00, KAM_COUK, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Wed, 10 Nov 2021 20:24:08 -0000 Hi Folks, > On 10 Nov 2021, at 08:14, Iain Sandoe wrote: >> On 9 Nov 2021, at 12:18, Jakub Jelinek via Gcc-patches = wrote: >>=20 >> On Tue, Nov 09, 2021 at 11:40:08AM +0000, Iain Sandoe wrote: >>> There were two issues, of which one remains and probably affects all = targets. >>> 2. This problem remains. This problem is also present on master without making any changes to the = PCH implementation - if one fixes up the read-in to simulate a corrupted = file, cc1 hangs (which means it=E2=80=99s no barrier to the revised PCH implementation) >>> - if we try to emit a diagnostic when the PCH read-in has failed, it = seems that >>> cc1 hangs somewhere in trying to lookup line table info. >>>=20 >>> - this was happening with the Darwin fixed PCH memory address = because it >>> was trying to report a fatal error in being unable to read the file = (or trying to >>> execute fancy_abort, in response to a segv). >>=20 >> I guess once we: >> /* Read in all the scalar variables. */ >> for (rt =3D gt_pch_scalar_rtab; *rt; rt++) >> for (rti =3D *rt; rti->base !=3D NULL; rti++) >> if (fread (rti->base, rti->stride, 1, f) !=3D 1) >> fatal_error (input_location, "cannot read PCH file: %m"); >>=20 >> /* Read in all the global pointers, in 6 easy loops. */ >> for (rt =3D gt_ggc_rtab; *rt; rt++) >> for (rti =3D *rt; rti->base !=3D NULL; rti++) >> for (i =3D 0; i < rti->nelt; i++) >> if (fread ((char *)rti->base + rti->stride * i, >> sizeof (void *), 1, f) !=3D 1) >> fatal_error (input_location, "cannot read PCH file: %m"); >> we overwrite the GTY(()) marked global vars including >> extern GTY(()) class line_maps *line_table; >> with pointers into the area we haven't mapped yet (or if the error = happens >> after that mmap but before everything is fixed up (e.g. the new = relocation >> processing), it is no wonder it doesn't work well. indeed. >>=20 >> Could we save line_table (and perhaps a few other vars) into non-GTY! = copies >> of them in ggc-common.c and instead of those fatal_error = (input_location, ...) >> calls in gt_pch_restore and ggc_pch_read call fatal_pch_error (...) = where >> void >> fatal_pch_error (const char *gmsg) >> { >> line_table =3D saved_line_table; >> // Restore anything else that is needed for fatal_error >> fatal_error (input_location, gmsg); >> } >=20 > That seems reasonable for the case that we call fatal_error from = ggc-common, but > I don=E2=80=99t think it will work if fancy_abort is called (for e.g. = a segv) - we might need to=20 > make a local fancy_abort() as well for that specific file, perhaps. >=20 > Or in some way defer overwriting the data until we=E2=80=99ve = succeeded in reading/relocating > the whole file (not sure what the largest PCH is we might encounter). (answering my own question) around 150Mb for largest libstdc++ and = similar for an=20 Objective-C include of Foundation + AppKit etc. The underlying reason here is that diagnostics have become much more = sophisticated, and they do all sorts of context checking and include the libcpp stuff = directly which is a lot of GTY(()) stuff. I cannot immediately see any small set of state that we can save / = restore around the PCH read in, Perhaps what would be more realistic would be a call into the = diagnostics stuff to say =E2=80=9Cdisable fancy stuff and just report the minimum=E2=80=9D, as noted, I don=E2=80=99t think this (second) issue is actually a = barrier to making the PCH change since it=E2=80=99s preexisting - I just ran into it while debugging = suitable VM addresses to use with ASLR on. thoughts? Iain