Message ID | cd5986cd3374bda0deb6bd495d0dcec61dcaefa0.camel@mengyan1223.wang |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E5F343857C48 for <patchwork@sourceware.org>; Thu, 18 Nov 2021 11:01:52 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E5F343857C48 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1637233312; bh=aCIgGNtU/cxphQZHFNNQnRwn4jpL8tG4geuZFqV4RDk=; h=Subject:To:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=T5xm1cci4EuqZuoR4oByAUfqHHHUlgzFaE0l2QR2vVCAIg+E/JLhQSY0sw0Xbk3KF Vr7++iB0j9cF2qbz9GxRAbD35RNzBv2RSquHaOx+GuTh/ApIGS5Ncv0L4U9fjXIQAG lUkS4djOZT8FIBGg62p+0HNc9vB2n5/sCjfT95bM= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mengyan1223.wang (mengyan1223.wang [89.208.246.23]) by sourceware.org (Postfix) with ESMTPS id 137F53858406 for <gcc-patches@gcc.gnu.org>; Thu, 18 Nov 2021 11:01:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 137F53858406 Received: from [IPv6:240e:35a:10fb:fb00:dc73:854d:832e:2] (unknown [IPv6:240e:35a:10fb:fb00:dc73:854d:832e:2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature ECDSA (P-384) server-digest SHA384) (Client did not present a certificate) (Authenticated sender: xry111@mengyan1223.wang) by mengyan1223.wang (Postfix) with ESMTPSA id 0213D660E5; Thu, 18 Nov 2021 06:01:17 -0500 (EST) Message-ID: <cd5986cd3374bda0deb6bd495d0dcec61dcaefa0.camel@mengyan1223.wang> Subject: [PATCH] fixincludes: don't abort() on access failure [PR103306] To: gcc-patches@gcc.gnu.org Date: Thu, 18 Nov 2021 19:01:11 +0800 Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.42.1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3039.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, JMQ_SPF_NEUTRAL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham 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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Xi Ruoyao via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Xi Ruoyao <xry111@mengyan1223.wang> Cc: Jakub Jelinek <jakub@redhat.com>, Zdenek Sojka <zsojka@seznam.cz>, Bruce Korb <bkorb@gnu.org> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
fixincludes: don't abort() on access failure [PR103306]
|
|
Commit Message
Xi Ruoyao
Nov. 18, 2021, 11:01 a.m. UTC
Some distro may ship dangling symlinks in include directories, triggers the access failure. Skip it and continue to next header instead of being to panic. Restore to old behavior before r12-5234 but without resurrecting the problematic getcwd() call, by using the environment variable "INPUT" exported by fixinc.sh. Tested on x86_64-linux-gnu, with a dangling symlink in /usr/include. fixincludes/ PR bootstrap/103306 * fixincl.c (process): Don't call abort(). --- fixincludes/fixincl.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
Comments
On 11/18/2021 4:01 AM, Xi Ruoyao via Gcc-patches wrote: > Some distro may ship dangling symlinks in include directories, triggers > the access failure. Skip it and continue to next header instead of > being to panic. > > Restore to old behavior before r12-5234 but without resurrecting the > problematic getcwd() call, by using the environment variable "INPUT" > exported by fixinc.sh. > > Tested on x86_64-linux-gnu, with a dangling symlink in /usr/include. > > fixincludes/ > > PR bootstrap/103306 > * fixincl.c (process): Don't call abort(). > --- > fixincludes/fixincl.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/fixincludes/fixincl.c b/fixincludes/fixincl.c > index a17b65866c3..81939ee5ffa 100644 > --- a/fixincludes/fixincl.c > +++ b/fixincludes/fixincl.c > @@ -1352,10 +1352,19 @@ process (void) > > if (access (pz_curr_file, R_OK) != 0) > { > - /* Some really strange error happened. */ > - fprintf (stderr, "Cannot access %s: %s\n", pz_curr_file, > + /* It may happens if for e. g. the distro ships some broken symlinks > + * in /usr/include. */ > + > + /* "INPUT" is exported in fixinc.sh, which is the pwd where fixincl > + * runs. It's used instead of getcwd to avoid allocating a buffer > + * with unknown length. */ Formatting nits. We don't use '*' at the start of comment lines. Drop the '*' like this /* blah blah blah more text. */ > + const char *cwd = getenv ("INPUT"); > + if (!cwd) > + cwd = "the working directory"; > + > + fprintf (stderr, "Cannot access %s from %s: %s\n", pz_curr_file, cwd, > xstrerror (errno)); > - abort (); > + return; > } If INPUT is always exported, why not just print it? ie, would CWD after actually be NULL? jeff
On Mon, 2021-11-22 at 17:37 -0700, Jeff Law wrote: > > > On 11/18/2021 4:01 AM, Xi Ruoyao via Gcc-patches wrote: > > Some distro may ship dangling symlinks in include directories, > > triggers > > the access failure. Skip it and continue to next header instead of > > being to panic. > > > > Restore to old behavior before r12-5234 but without resurrecting the > > problematic getcwd() call, by using the environment variable "INPUT" > > exported by fixinc.sh. > > > > Tested on x86_64-linux-gnu, with a dangling symlink in /usr/include. > > > > fixincludes/ > > > > PR bootstrap/103306 > > * fixincl.c (process): Don't call abort(). > > --- > > fixincludes/fixincl.c | 15 ++++++++++++--- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/fixincludes/fixincl.c b/fixincludes/fixincl.c > > index a17b65866c3..81939ee5ffa 100644 > > --- a/fixincludes/fixincl.c > > +++ b/fixincludes/fixincl.c > > @@ -1352,10 +1352,19 @@ process (void) > > > > if (access (pz_curr_file, R_OK) != 0) > > { > > - /* Some really strange error happened. */ > > - fprintf (stderr, "Cannot access %s: %s\n", pz_curr_file, > > + /* It may happens if for e. g. the distro ships some broken symlinks > > + * in /usr/include. */ > > + > > + /* "INPUT" is exported in fixinc.sh, which is the pwd where fixincl > > + * runs. It's used instead of getcwd to avoid allocating a buffer > > + * with unknown length. */ > Formatting nits. We don't use '*' at the start of comment lines. Drop > the '*' like this > > /* blah blah blah > more text. */ Strangely contrib/check_GNU_style.sh does not warn about this. > > > + const char *cwd = getenv ("INPUT"); > > + if (!cwd) > > + cwd = "the working directory"; > > + > > + fprintf (stderr, "Cannot access %s from %s: %s\n", pz_curr_file, cwd, > > xstrerror (errno)); > > - abort (); > > + return; > > } > If INPUT is always exported, why not just print it? ie, would CWD after > actually be NULL? INPUT is set by fixinc.sh. During GCC building process fixincl is always invoked by fixinc.sh. However someone may run fixincl executable directly for debugging.
On 11/23/2021 2:31 AM, Xi Ruoyao wrote: > On Mon, 2021-11-22 at 17:37 -0700, Jeff Law wrote: >> >> On 11/18/2021 4:01 AM, Xi Ruoyao via Gcc-patches wrote: >>> Some distro may ship dangling symlinks in include directories, >>> triggers >>> the access failure. Skip it and continue to next header instead of >>> being to panic. >>> >>> Restore to old behavior before r12-5234 but without resurrecting the >>> problematic getcwd() call, by using the environment variable "INPUT" >>> exported by fixinc.sh. >>> >>> Tested on x86_64-linux-gnu, with a dangling symlink in /usr/include. >>> >>> fixincludes/ >>> >>> PR bootstrap/103306 >>> * fixincl.c (process): Don't call abort(). >>> --- >>> fixincludes/fixincl.c | 15 ++++++++++++--- >>> 1 file changed, 12 insertions(+), 3 deletions(-) >>> >>> diff --git a/fixincludes/fixincl.c b/fixincludes/fixincl.c >>> index a17b65866c3..81939ee5ffa 100644 >>> --- a/fixincludes/fixincl.c >>> +++ b/fixincludes/fixincl.c >>> @@ -1352,10 +1352,19 @@ process (void) >>> >>> if (access (pz_curr_file, R_OK) != 0) >>> { >>> - /* Some really strange error happened. */ >>> - fprintf (stderr, "Cannot access %s: %s\n", pz_curr_file, >>> + /* It may happens if for e. g. the distro ships some broken symlinks >>> + * in /usr/include. */ >>> + >>> + /* "INPUT" is exported in fixinc.sh, which is the pwd where fixincl >>> + * runs. It's used instead of getcwd to avoid allocating a buffer >>> + * with unknown length. */ >> Formatting nits. We don't use '*' at the start of comment lines. Drop >> the '*' like this >> >> /* blah blah blah >> more text. */ > Strangely contrib/check_GNU_style.sh does not warn about this. It should. Though in fairness, that checker is new relative to the overall live of the GCC project and obviously not 100% complete. Patches are always appreciated :-) > >>> + const char *cwd = getenv ("INPUT"); >>> + if (!cwd) >>> + cwd = "the working directory"; >>> + >>> + fprintf (stderr, "Cannot access %s from %s: %s\n", pz_curr_file, cwd, >>> xstrerror (errno)); >>> - abort (); >>> + return; >>> } >> If INPUT is always exported, why not just print it? ie, would CWD after >> actually be NULL? > INPUT is set by fixinc.sh. During GCC building process fixincl is > always invoked by fixinc.sh. However someone may run fixincl executable > directly for debugging. Good point. With the formatting nit fixed, this is fine for the trunk. Thanks, jeff
diff --git a/fixincludes/fixincl.c b/fixincludes/fixincl.c index a17b65866c3..81939ee5ffa 100644 --- a/fixincludes/fixincl.c +++ b/fixincludes/fixincl.c @@ -1352,10 +1352,19 @@ process (void) if (access (pz_curr_file, R_OK) != 0) { - /* Some really strange error happened. */ - fprintf (stderr, "Cannot access %s: %s\n", pz_curr_file, + /* It may happens if for e. g. the distro ships some broken symlinks + * in /usr/include. */ + + /* "INPUT" is exported in fixinc.sh, which is the pwd where fixincl + * runs. It's used instead of getcwd to avoid allocating a buffer + * with unknown length. */ + const char *cwd = getenv ("INPUT"); + if (!cwd) + cwd = "the working directory"; + + fprintf (stderr, "Cannot access %s from %s: %s\n", pz_curr_file, cwd, xstrerror (errno)); - abort (); + return; } pz_curr_data = load_file (pz_curr_file);