[PATCH] gdc 9, 10 and 11 bug fix

[PATCH] gdc 9, 10 and 11 bug fix

[Marc Aurèle La France]

Greetings.

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.

Please Reply-To-All.

Thanks and have a great day.

Marc.

Signed-off-by: Marc Aurèle La France <tsi@tuyoix.net>

diff -aNpRruz -X /etc/diff.excludes gcc-11.3.0/gcc/d/dmd/root/filename.c devel-11.3.0/gcc/d/dmd/root/filename.c
--- gcc-11.3.0/gcc/d/dmd/root/filename.c	2022-04-21 01:58:53.151586473 -0600
+++ devel-11.3.0/gcc/d/dmd/root/filename.c	2022-05-11 16:30:54.398488793 -0600
@@ -522,7 +522,7 @@ const char *FileName::safeSearchPath(Strings *path, const char *name)
             //      "strncmp(cpath, cname, %i)=%i\n", exists(cname),
             //      strlen(cpath), strncmp(cpath, cname, strlen(cpath)));
             // exists and name is *really* a "child" of path
-            if (exists(cname) && strncmp(cpath, cname, strlen(cpath)) == 0)
+            if (exists(cname))
             {
                 ::free(const_cast<char *>(cpath));
                 const char *p = mem.xstrdup(cname);

Re: [PATCH] gdc 9, 10 and 11 bug fix

[Iain Buclaw]

Excerpts from Marc Aurèle La France's message of Mai 12, 2022 10:29 pm:
> Greetings.
> 
> 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.
> 

Hi Marc,

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.

Iain.

Re: [PATCH] gdc 9, 10 and 11 bug fix

[Marc Aurèle La France]

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...

Thanks again and have a great day.

Marc.

Signed-off-by: Marc Aurèle La France <tsi@tuyoix.net>

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;
-        }
-    }
-
-    if (path)
-    {
-        /* Each path is converted to a cannonical name and then a check is done to see
-         * that the searched name is really a child one of the the paths searched.
-         */
-        for (size_t i = 0; i < path->dim; i++)
-        {
-            const char *cname = NULL;
-            const char *cpath = canonicalName((*path)[i]);
-            //printf("FileName::safeSearchPath(): name=%s; path=%s; cpath=%s\n",
-            //      name, (char *)path->data[i], cpath);
-            if (cpath == NULL)
-                goto cont;
-            cname = canonicalName(combine(cpath, name));
-            //printf("FileName::safeSearchPath(): cname=%s\n", cname);
-            if (cname == NULL)
-                goto cont;
-            //printf("FileName::safeSearchPath(): exists=%i "
-            //      "strncmp(cpath, cname, %i)=%i\n", exists(cname),
-            //      strlen(cpath), strncmp(cpath, cname, strlen(cpath)));
-            // exists and name is *really* a "child" of path
-            if (exists(cname) && strncmp(cpath, cname, strlen(cpath)) == 0)
-            {
-                ::free(const_cast<char *>(cpath));
-                const char *p = mem.xstrdup(cname);
-                ::free(const_cast<char *>(cname));
-                return p;
-            }
-cont:
-            if (cpath)
-                ::free(const_cast<char *>(cpath));
-            if (cname)
-                ::free(const_cast<char *>(cname));
-        }
-    }
-    return NULL;
+    return searchPath(path, name, false);
 #else
     assert(0);
 #endif

For GCC 10  ---------- 8< ----------

diff -aNpRruz -X /etc/diff.excludes gcc-10.3.0/gcc/d/dmd/root/filename.c devel-10.3.0/gcc/d/dmd/root/filename.c
--- gcc-10.3.0/gcc/d/dmd/root/filename.c	2021-04-08 05:56:28.277743190 -0600
+++ devel-10.3.0/gcc/d/dmd/root/filename.c	2022-05-15 14:13:15.028032823 -0600
@@ -476,53 +476,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;
-        }
-    }
-
-    if (path)
-    {
-        /* Each path is converted to a cannonical name and then a check is done to see
-         * that the searched name is really a child one of the the paths searched.
-         */
-        for (size_t i = 0; i < path->dim; i++)
-        {
-            const char *cname = NULL;
-            const char *cpath = canonicalName((*path)[i]);
-            //printf("FileName::safeSearchPath(): name=%s; path=%s; cpath=%s\n",
-            //      name, (char *)path->data[i], cpath);
-            if (cpath == NULL)
-                goto cont;
-            cname = canonicalName(combine(cpath, name));
-            //printf("FileName::safeSearchPath(): cname=%s\n", cname);
-            if (cname == NULL)
-                goto cont;
-            //printf("FileName::safeSearchPath(): exists=%i "
-            //      "strncmp(cpath, cname, %i)=%i\n", exists(cname),
-            //      strlen(cpath), strncmp(cpath, cname, strlen(cpath)));
-            // exists and name is *really* a "child" of path
-            if (exists(cname) && strncmp(cpath, cname, strlen(cpath)) == 0)
-            {
-                ::free(const_cast<char *>(cpath));
-                const char *p = mem.xstrdup(cname);
-                ::free(const_cast<char *>(cname));
-                return p;
-            }
-cont:
-            if (cpath)
-                ::free(const_cast<char *>(cpath));
-            if (cname)
-                ::free(const_cast<char *>(cname));
-        }
-    }
-    return NULL;
+    return searchPath(path, name, false);
 #else
     assert(0);
 #endif

For GCC 11  ---------- 8< ----------

diff -aNpRruz -X /etc/diff.excludes gcc-11.3.0/gcc/d/dmd/root/filename.c devel-11.3.0/gcc/d/dmd/root/filename.c
--- gcc-11.3.0/gcc/d/dmd/root/filename.c	2022-04-21 01:58:53.000000000 -0600
+++ devel-11.3.0/gcc/d/dmd/root/filename.c	2022-05-15 14:10:16.512781192 -0600
@@ -490,53 +490,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;
-        }
-    }
-
-    if (path)
-    {
-        /* Each path is converted to a cannonical name and then a check is done to see
-         * that the searched name is really a child one of the the paths searched.
-         */
-        for (size_t i = 0; i < path->length; i++)
-        {
-            const char *cname = NULL;
-            const char *cpath = canonicalName((*path)[i]);
-            //printf("FileName::safeSearchPath(): name=%s; path=%s; cpath=%s\n",
-            //      name, (char *)path->data[i], cpath);
-            if (cpath == NULL)
-                goto cont;
-            cname = canonicalName(combine(cpath, name));
-            //printf("FileName::safeSearchPath(): cname=%s\n", cname);
-            if (cname == NULL)
-                goto cont;
-            //printf("FileName::safeSearchPath(): exists=%i "
-            //      "strncmp(cpath, cname, %i)=%i\n", exists(cname),
-            //      strlen(cpath), strncmp(cpath, cname, strlen(cpath)));
-            // exists and name is *really* a "child" of path
-            if (exists(cname) && strncmp(cpath, cname, strlen(cpath)) == 0)
-            {
-                ::free(const_cast<char *>(cpath));
-                const char *p = mem.xstrdup(cname);
-                ::free(const_cast<char *>(cname));
-                return p;
-            }
-cont:
-            if (cpath)
-                ::free(const_cast<char *>(cpath));
-            if (cname)
-                ::free(const_cast<char *>(cname));
-        }
-    }
-    return NULL;
+    return searchPath(path, name, false);
 #else
     assert(0);
 #endif

Re: [PATCH] gdc 9, 10 and 11 bug fix

[Iain Buclaw]

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...
> 
> Thanks again and have a great day.
> 
> Marc.
> 
> Signed-off-by: Marc Aurèle La France <tsi@tuyoix.net>
> 
> 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.

Iain.

Re: [PATCH] gdc 9, 10 and 11 bug fix

[Marc Aurèle La France]

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 <tsi@tuyoix.net>

>> 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.

Marc.

Re: [PATCH] gdc 9, 10 and 11 bug fix

[Iain Buclaw]

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 <tsi@tuyoix.net>
> 
>>> 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.

Iain.

Re: [PATCH] gdc 9, 10 and 11 bug fix

[Marc Aurèle La France]

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 <tsi@tuyoix.net>

>>>> 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.

Marc.

Re: [PATCH] gdc 9, 10 and 11 bug fix

[Marc Aurèle La France]

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 <tsi@tuyoix.net>

>>>>> 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?

Marc.

Re: [PATCH] gdc 9, 10 and 11 bug fix

[Iain Buclaw]

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 <tsi@tuyoix.net>
> 
>>>>>> 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"));
      |             ^
absimport.d:2:1: note: while evaluat
---

With just this change alone, that means both forms will be permitted.
If we're going to fix how searchPath checks imports, might as well move
in the direction of the v2.100 behaviour in gdc-12, and include a couple
checks for absolute paths and `..` in the string.

Regards,
Iain.

Re: [PATCH] gdc 9, 10 and 11 bug fix

[Marc Aurèle La France]

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 <tsi@tuyoix.net>

>>>>>>> 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.