[v3] Do not abort compilation when dump file is /dev/*

  The `configure` scripts generated with autoconf often tests compiler
features by setting output to `/dev/null`, which then sets the dump
folder as being /dev/* and the compilation halts with an error because
GCC cannot create files in /dev/. This is a problem when configure is
testing for compiler features because it cannot tell if the failure was
due to unsupported features or any other problem, and disable it even
if it is working.

As an example, running configure overriding CFLAGS="-fdump-ipa-clones"
will result in several compiler-features as being disabled because of
gcc halting with an error creating files in /dev/*.

This commit fixes this issue by checking if the output file is
/dev/null or /dev/zero. In this case we use the current working
directory for dump output instead of the directory of the output
file because we cannot write to /dev/*.

gcc/ChangeLog
2021-11-16  Giuliano Belinassi  <gbelinassi@suse.de>

	* gcc.c (process_command): Skip dumpdir override if file is a
	not_actual_file_p.
	(not_actual_file_p): Return true if file is /dev/zero as well.
	* doc/invoke.texi: Update -dumpdir documentation.

gcc/testsuite/ChangeLog
2021-11-16  Giuliano Belinassi  <gbelinassi@suse.de>

	* gcc.dg/devnull-dump.c: New.

Signed-off-by: Giuliano Belinassi <gbelinassi@suse.de>
---
 gcc/doc/invoke.texi                 | 3 ++-
 gcc/gcc.c                           | 6 ++++--
 gcc/testsuite/gcc.dg/devnull-dump.c | 7 +++++++
 3 files changed, 13 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/devnull-dump.c
  

On Fri, 19 Nov 2021, Giuliano Belinassi wrote:

> The `configure` scripts generated with autoconf often tests compiler
> features by setting output to `/dev/null`, which then sets the dump
> folder as being /dev/* and the compilation halts with an error because
> GCC cannot create files in /dev/. This is a problem when configure is
> testing for compiler features because it cannot tell if the failure was
> due to unsupported features or any other problem, and disable it even
> if it is working.
> 
> As an example, running configure overriding CFLAGS="-fdump-ipa-clones"
> will result in several compiler-features as being disabled because of
> gcc halting with an error creating files in /dev/*.
> 
> This commit fixes this issue by checking if the output file is
> /dev/null or /dev/zero. In this case we use the current working
> directory for dump output instead of the directory of the output
> file because we cannot write to /dev/*.
> 
> gcc/ChangeLog
> 2021-11-16  Giuliano Belinassi  <gbelinassi@suse.de>
> 
> 	* gcc.c (process_command): Skip dumpdir override if file is a
> 	not_actual_file_p.
> 	(not_actual_file_p): Return true if file is /dev/zero as well.
> 	* doc/invoke.texi: Update -dumpdir documentation.
> 
> gcc/testsuite/ChangeLog
> 2021-11-16  Giuliano Belinassi  <gbelinassi@suse.de>
> 
> 	* gcc.dg/devnull-dump.c: New.
> 
> Signed-off-by: Giuliano Belinassi <gbelinassi@suse.de>
> ---
>  gcc/doc/invoke.texi                 | 3 ++-
>  gcc/gcc.c                           | 6 ++++--
>  gcc/testsuite/gcc.dg/devnull-dump.c | 7 +++++++
>  3 files changed, 13 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/devnull-dump.c
> 
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 6070288856c..4a17cd4d317 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -1877,7 +1877,8 @@ named @file{dir/bar.*}, combining the given @var{dumppfx} with the
>  default @var{dumpbase} derived from the primary output name.  Dump
>  outputs also take the input name suffix: @file{dir/bar.c.*}.
>  
> -It defaults to the location of the output file; options
> +It defaults to the location of the output file, unless the output
> +file is a special file like @code{/dev/null}. Options
>  @option{-save-temps=cwd} and @option{-save-temps=obj} override this
>  default, just like an explicit @option{-dumpdir} option.  In case
>  multiple such options are given, the last one prevails:
> diff --git a/gcc/gcc.c b/gcc/gcc.c
> index 506c2acc282..43d7cde1be9 100644
> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -5098,7 +5098,8 @@ process_command (unsigned int decoded_options_count,
>  
>    bool explicit_dumpdir = dumpdir;
>  
> -  if (!save_temps_overrides_dumpdir && explicit_dumpdir)
> +  if ((!save_temps_overrides_dumpdir && explicit_dumpdir)
> +      || (output_file && not_actual_file_p (output_file)))
>      {
>        /* Do nothing.  */
>      }
> @@ -10716,7 +10717,8 @@ static bool
>  not_actual_file_p (const char *name)
>  {
>    return (strcmp (name, "-") == 0
> -	  || strcmp (name, HOST_BIT_BUCKET) == 0);
> +	  || strcmp (name, HOST_BIT_BUCKET) == 0
> +	  || strcmp (name, "/dev/zero") == 0);
>  }

OK when you omit the change to include /dev/zero in not_actual_file_p.

Thanks,
Richard.

>  /* %:dumps spec function.  Take an optional argument that overrides
> diff --git a/gcc/testsuite/gcc.dg/devnull-dump.c b/gcc/testsuite/gcc.dg/devnull-dump.c
> new file mode 100644
> index 00000000000..378e0901c28
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/devnull-dump.c
> @@ -0,0 +1,7 @@
> +/* { dg-do assemble } */
> +/* { dg-options "-fdump-ipa-clones -o /dev/null" } */
> +
> +int main()
> +{
> +  return 0;
> +}
>
  

On Fri, 19 Nov 2021 15:12:35 +0100 (CET)
Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:

> On Fri, 19 Nov 2021, Giuliano Belinassi wrote:

> > -It defaults to the location of the output file; options
> > +It defaults to the location of the output file, unless the output
> > +file is a special file like @code{/dev/null}. Options

s/a special file like //

I'd say.
thanks,
> >  @option{-save-temps=cwd} and @option{-save-temps=obj} override this
> >  default, just like an explicit @option{-dumpdir} option.  In case
> >  multiple such options are given, the last one prevails:
> > diff --git a/gcc/gcc.c b/gcc/gcc.c
> > index 506c2acc282..43d7cde1be9 100644
> > --- a/gcc/gcc.c
> > +++ b/gcc/gcc.c
> > @@ -5098,7 +5098,8 @@ process_command (unsigned int decoded_options_count,
> >  
> >    bool explicit_dumpdir = dumpdir;
> >  
> > -  if (!save_temps_overrides_dumpdir && explicit_dumpdir)
> > +  if ((!save_temps_overrides_dumpdir && explicit_dumpdir)
> > +      || (output_file && not_actual_file_p (output_file)))
> >      {
> >        /* Do nothing.  */
> >      }
> > @@ -10716,7 +10717,8 @@ static bool
> >  not_actual_file_p (const char *name)
> >  {
> >    return (strcmp (name, "-") == 0
> > -	  || strcmp (name, HOST_BIT_BUCKET) == 0);
> > +	  || strcmp (name, HOST_BIT_BUCKET) == 0
> > +	  || strcmp (name, "/dev/zero") == 0);
> >  }  
> 
> OK when you omit the change to include /dev/zero in not_actual_file_p.
  

Hello, Giuliano, thanks for turning my suggestion into a patch!

On Nov 19, 2021, Richard Biener <rguenther@suse.de> wrote:

>> +/* { dg-options "-fdump-ipa-clones -o /dev/null" } */

May I suggest actually checking that the ipa-clones dump file is created
with the same name and the location it would without -o /dev/null?  I
think just using any of the normal dump file scanners would do.

There's a larger set of tests in gcc.misc/outputs.exp, where this test
would be a slightly better fit.  Copying some of the tests under
"Driver-chosen aux outputs", and some of the link tests that use
'$oaout', changing them to use -o /dev/null without any changes to the
expected output file names, would give us more coverage of expected
behavior than just checking that we just don't get an error.

Thanks again,
  

On Sun, 2021-11-21 at 00:47 -0300, Alexandre Oliva wrote:
> Hello, Giuliano, thanks for turning my suggestion into a patch!
> 
> On Nov 19, 2021, Richard Biener <rguenther@suse.de> wrote:
> 
> > > +/* { dg-options "-fdump-ipa-clones -o /dev/null" } */
> 
> May I suggest actually checking that the ipa-clones dump file is
> created
> with the same name and the location it would without -o /dev/null?  I
> think just using any of the normal dump file scanners would do.
> 
> There's a larger set of tests in gcc.misc/outputs.exp, where this
> test
> would be a slightly better fit.  Copying some of the tests under
> "Driver-chosen aux outputs", and some of the link tests that use
> '$oaout', changing them to use -o /dev/null without any changes to
> the
> expected output file names, would give us more coverage of expected
> behavior than just checking that we just don't get an error.
> 

Hi, Oliva.

The patch was already applied in trunk and backported to gcc-11. The
rationale behind it was that exiting with an error because it couldn't
write dumps into /dev/* confused configure. Simply not erroring
(through not dumping in /dev/ on -o /dev/null) was enough to fix
configure, and the testcase reflects that.

As for moving that test to gcc.misc, I am not sure if that deserves
another patch just for that, and then applying it on gcc-11. I could do
that if it is really important, OTOH.

Thank you,
Giuliano.

> Thanks again,
>