[PATCH] fixincludes: don't abort() on access failure [PR103306]

[Jeff Law]

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