From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from omta001.cacentral1.a.cloudfilter.net (omta001.cacentral1.a.cloudfilter.net [3.97.99.32]) by sourceware.org (Postfix) with ESMTPS id D83D5383B786 for ; Sun, 22 May 2022 00:05:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D83D5383B786 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tuyoix.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tuyoix.net Received: from shw-obgw-4001a.ext.cloudfilter.net ([10.228.9.142]) by cmsmtp with ESMTP id sYnVnkb5wwtwGsZ5wnPYe5; Sun, 22 May 2022 00:05:16 +0000 Received: from fanir.tuyoix.net ([68.150.218.192]) by cmsmtp with ESMTP id sZ5znTET2E4bisZ5znb6rK; Sun, 22 May 2022 00:05:20 +0000 X-Authority-Analysis: v=2.4 cv=YdeuWidf c=1 sm=1 tr=0 ts=62897e40 a=LfNn7serMq+1bQZBlMsSfQ==:117 a=LfNn7serMq+1bQZBlMsSfQ==:17 a=oZkIemNP1mAA:10 a=M51BFTxLslgA:10 a=nlC_4_pT8q9DhB4Ho9EA:9 a=NEAV23lmAAAA:8 a=3I1X_3ewAAAA:8 a=mFnJSDkoaTzC2bisLKgA:9 a=QEXdDO2ut3YA:10 a=VG9N9RgkD3hcbI6YpJ1l:22 Received: from CLUIJ (cluij.tuyoix.net [192.168.144.15]) (authenticated bits=0) by fanir.tuyoix.net (8.17.1/8.17.1) with ESMTPSA id 24M05Hdp029009 (version=TLSv1 cipher=ECDHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 21 May 2022 18:05:18 -0600 Date: Sat, 21 May 2022 18:05:09 -0600 (Mountain Daylight Time) From: =?UTF-8?Q?Marc_Aur=C3=A8le_La_France?= To: Iain Buclaw cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] gdc 9, 10 and 11 bug fix In-Reply-To: <1653051283.jungooescn.astroid@pulse.none> Message-ID: References: <1652619184.mo716bh9xr.astroid@pulse.none> <1652785964.hma4ncd2fy.astroid@pulse.none> <1652803794.4apc2k19jf.astroid@pulse.none> <1653051283.jungooescn.astroid@pulse.none> User-Agent: Alpine 2.20 (WNT 67 2015-01-07) MIME-Version: 1.0 X-CMAE-Envelope: MS4xfEYRv4hoLKOq0bMi9A4MGXOgU4nDDHJwXsp17IH66ZwUm12w38CUtZOtY3z3NTiOtZYMzS+1yULk2QMgIgcoem8OyRexLy0euM//ZaZJ4hyXmAhzwFF9 DseZjParCq2TR5zyetgpfz68g6epeMxNL6XM97osYGr3T+aoaZDcO5T2ndRzktmfxYBoZkXd4xRFl8lPbs1/WfKRVJodr2kybnLZcIgwybIJgNlcNHnbv3KM X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, 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 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8BIT X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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: Sun, 22 May 2022 00:05:23 -0000 On Fri, 20 May 2022, Iain Buclaw wrote: > Excerpts from Marc Aurèle La France's message of Mai 20, 2022 6:56 am: >> On Tue, 17 May 2022, Marc Aurèle La France wrote: >>> On Tue, 17 May 2022, Iain Buclaw wrote: >>>> Excerpts from Marc Aurèle La France's message of Mai 17, 2022 5:31 pm: >>>>> On Tue, 17 May 2022, Iain Buclaw wrote: >>>>>> Excerpts from Marc Aurèle La France's message of Mai 16, 2022 11:34 pm: >>>>>>> On Sun, 15 May 2022, Iain Buclaw wrote: >>>>>>>> Excerpts from Marc Aurèle La France's message of Mai 12, 2022 10:29 pm: >>>>>>>>> No compiler has any business rejecting files for the sole crime of >>>>>>>>> being symlinked to. The following applies, modulo patch fuzz, to the >>>>>>>>> 9, 10 and 11 series of compilers. >>>>>>>>> Given my use of shadow trees, this bug attempted to prevent me from >>>>>>>>> building 12.1.0. The D-based gdc in 12.1.0 and up does not exhibit >>>>>>>>> this quirky behaviour. >>>>>>>> Thanks, I've checked upstream and see the following change: >>>>>>>> https://github.com/dlang/dmd/pull/11836/commits/ebda81e44fd0ca4b247a1860d9bef411c41c16cb >>>>>>>> It should be fine to just backport that. >>>>>>> Thanks for the pointer. >>>>>>> I ended up with the three slightly different diffs below, one each for >>>>>>> the 9, 10 and 11 branches. Each was rebuilt using 8.5.0, then used to >>>>>>> rebuild 12.1.0. All of this ran smoothly without complaint, although I >>>>>>> wouldn't want to do this on a 486... >>>>>>> Signed-off-by: Marc Aurèle La France >>>>>>> For GCC 9 ---------- 8< ---------- >>>>>>> >>>>>>> diff -aNpRruz -X /etc/diff.excludes gcc-9.4.0/gcc/d/dmd/root/filename.c >>>>>>> devel-9.4.0/gcc/d/dmd/root/filename.c >>>>>>> --- gcc-9.4.0/gcc/d/dmd/root/filename.c 2021-06-01 01:53:04.716474774 >>>>>>> -0600 >>>>>>> +++ devel-9.4.0/gcc/d/dmd/root/filename.c 2022-05-15 15:02:49.995441507 >>>>>>> -0600 >>>>>>> @@ -475,53 +475,7 @@ const char *FileName::safeSearchPath(Strings *path, >>>>>>> const char *name) >>>>>>> >>>>>>> return FileName::searchPath(path, name, false); >>>>>>> #elif POSIX >>>>>>> - /* Even with realpath(), we must check for // and disallow it >>>>>>> - */ >>>>>>> - for (const char *p = name; *p; p++) >>>>>>> - { >>>>>>> - char c = *p; >>>>>>> - if (c == '/' && p[1] == '/') >>>>>>> - { >>>>>>> - return NULL; >>>>>>> - } >>>>>>> - } >>>>>> I'd keep this check in, otherwise removing/replacing only the `if >>>>>> (path)` branch looks OK to me. >>>>> The corresponding D code doesn't care about double slashes and neither >>>>> should this. Also, the comment is misleading as realpath() would no >>>>> longer be used here. >>>> True, but the D front-end does check the path in other places now: >>>> https://github.com/dlang/dmd/blob/e9ba29d71b557fe079e95ee6554f116b24159bab/src/dmd/root/filename.d#L787-L803 >>>> https://github.com/dlang/dmd/blob/e9ba29d71b557fe079e95ee6554f116b24159bab/src/dmd/expressionsem.d#L5984-L6007 >>>> If we remove all checks, then there'd be nothing to prevent either >>>> import("/file") or import("../file") from being used. >>> There is still no check for double slashes. All I want here is to fix a C++ >>> -> D migration bug without leaving any detritus behind. >> Anything more on this? > My concern is that given: > pragma(msg, import("/etc/fstab")); > pragma(msg, import("../../../../../../etc/fstab")); > This will result in errors with both `gdc-11 -J.` and `gdc-12 -J.` > gdc-11: > --- > error: file "/etc/fstab" cannot be found or not in a path specified with -J > 1 | pragma(msg, import("/etc/fstab")); > | ^ > error: file "../../../../../../etc/fstab" cannot be found or not in a path specified with -J > 2 | pragma(msg, import("../../../../../../etc/fstab")); > | ^ > --- > gdc-12: > --- > error: absolute path is not allowed in import expression: ‘"/etc/fstab"’ > 1 | pragma(msg, import("/etc/fstab")); > | ^ > error: path refers to parent (‘..’) directory: ‘"../../../../../../etc/fstab"’ > 2 | pragma(msg, import("../../../../../../etc/fstab")); > | ^ > --- > With just this change alone, that means both forms will be permitted. False. What is checked is the name provided, and, if a symlink, not what it points to. You are making this far more complicated then it needs to be. Pursuant to the KISS principle, let's simply go back to the one-liner I posted earlier in this thread and call it a day. This way, you get to keep all other pathname checks you seem so intent on, and I get my bug fixed. Marc.