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

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

[Giuliano Belinassi]

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 dump folder is /dev/.
If yes, then it just informs the user and disables dumping, but does
not halt the compilation and the compiler retuns 0 to the shell.

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

	* dumpfile.c (dump_open): Do not halt compilation when file
	matches /dev/*.

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/dumpfile.c                      | 17 ++++++++++++++++-
 gcc/testsuite/gcc.dg/devnull-dump.c |  7 +++++++
 2 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/devnull-dump.c

diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
index 8169daf7f59..b1dbfb371af 100644
--- a/gcc/dumpfile.c
+++ b/gcc/dumpfile.c
@@ -378,7 +378,22 @@ dump_open (const char *filename, bool trunc)
   FILE *stream = fopen (filename, trunc ? "w" : "a");
 
   if (!stream)
-    error ("could not open dump file %qs: %m", filename);
+    {
+      /* Autoconf tests compiler functionalities by setting output to /dev/null.
+	 In this case, if dumps are enabled, it will try to set the output
+	 folder to /dev/*, which is of course invalid and the compiler will exit
+	 with an error, resulting in configure script reporting the tested
+	 feature as being unavailable. Here we test this case by checking if the
+	 output file prefix has /dev/ and only inform the user in this case
+	 rather than refusing to compile.  */
+
+      const char *const slash_dev = "/dev/";
+      if (strncmp(slash_dev, filename, strlen(slash_dev)) == 0)
+	inform (UNKNOWN_LOCATION,
+		"could not open dump file %qs: %m. Dumps are disabled.", filename);
+      else
+	error ("could not open dump file %qs: %m", filename);
+    }
   return stream;
 }
 
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;
+}
-- 
2.33.1

Re: [PATCH] Do not abort compilation when dump file is /dev/*

[Richard Biener]

On Tue, 16 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 dump folder is /dev/.
> If yes, then it just informs the user and disables dumping, but does
> not halt the compilation and the compiler retuns 0 to the shell.

I think matching '/dev/*' is broken.  Either failure to open a dump
file should be an error or not.  Btw, some configure tests check
for compiler output as well which still breaks in this case.

IMHO a more reasonable thing to do would be to not treat
-o /dev/null as a source for -dumpdir and friends.  Alex?
You did the last re-org, where'd we put such special casing?

Of course configure checks using -fdump-... with -o /dev/null are
a bit fragile.  They could use -fdump-...=FILENAME

Thnaks,
Richard.

> gcc/ChangeLog
> 2021-11-16  Giuliano Belinassi  <gbelinassi@suse.de>
> 
> 	* dumpfile.c (dump_open): Do not halt compilation when file
> 	matches /dev/*.
> 
> 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/dumpfile.c                      | 17 ++++++++++++++++-
>  gcc/testsuite/gcc.dg/devnull-dump.c |  7 +++++++
>  2 files changed, 23 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/devnull-dump.c
> 
> diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> index 8169daf7f59..b1dbfb371af 100644
> --- a/gcc/dumpfile.c
> +++ b/gcc/dumpfile.c
> @@ -378,7 +378,22 @@ dump_open (const char *filename, bool trunc)
>    FILE *stream = fopen (filename, trunc ? "w" : "a");
>  
>    if (!stream)
> -    error ("could not open dump file %qs: %m", filename);
> +    {
> +      /* Autoconf tests compiler functionalities by setting output to /dev/null.
> +	 In this case, if dumps are enabled, it will try to set the output
> +	 folder to /dev/*, which is of course invalid and the compiler will exit
> +	 with an error, resulting in configure script reporting the tested
> +	 feature as being unavailable. Here we test this case by checking if the
> +	 output file prefix has /dev/ and only inform the user in this case
> +	 rather than refusing to compile.  */
> +
> +      const char *const slash_dev = "/dev/";
> +      if (strncmp(slash_dev, filename, strlen(slash_dev)) == 0)
> +	inform (UNKNOWN_LOCATION,
> +		"could not open dump file %qs: %m. Dumps are disabled.", filename);
> +      else
> +	error ("could not open dump file %qs: %m", filename);
> +    }
>    return stream;
>  }
>  
> 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;
> +}
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

Re: [PATCH] Do not abort compilation when dump file is /dev/*

[Martin Liška]

On 11/16/21 17:52, Giuliano Belinassi via Gcc-patches wrote:
> |+ if (strncmp(slash_dev, filename, strlen(slash_dev)) == 0)|

Btw. you can use startswith function.

Cheers,
Martin

Re: [PATCH] Do not abort compilation when dump file is /dev/*

[Giuliano Belinassi]

On Thu, 2021-11-18 at 10:43 +0100, Richard Biener wrote:
> On Tue, 16 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 dump folder is
> > /dev/.
> > If yes, then it just informs the user and disables dumping, but
> > does
> > not halt the compilation and the compiler retuns 0 to the shell.
> 
> I think matching '/dev/*' is broken.  Either failure to open a dump
> file should be an error or not.  Btw, some configure tests check
> for compiler output as well which still breaks in this case.

Hi,

IMHO gcc shouldn't fail when it cannot write the dump files because
compilation could still continue compiling even if the dumps cannot be
created. However, I chose to do not because the code may be issuing an
error for other reasons, and matching for /dev/ fixes the test case. 

> 
> IMHO a more reasonable thing to do would be to not treat
> -o /dev/null as a source for -dumpdir and friends.  Alex?
> You did the last re-org, where'd we put such special casing?
> 
> Of course configure checks using -fdump-... with -o /dev/null are
> a bit fragile.  They could use -fdump-...=FILENAME
> 

But that would also set all clones output as being equal to FILENAME
and don't create one dump for every compiled file, no?

Thanks,
Giuliano.

> Thnaks,
> Richard.
> 
> > gcc/ChangeLog
> > 2021-11-16  Giuliano Belinassi  <gbelinassi@suse.de>
> > 
> >         * dumpfile.c (dump_open): Do not halt compilation when file
> >         matches /dev/*.
> > 
> > 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/dumpfile.c                      | 17 ++++++++++++++++-
> >  gcc/testsuite/gcc.dg/devnull-dump.c |  7 +++++++
> >  2 files changed, 23 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/devnull-dump.c
> > 
> > diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> > index 8169daf7f59..b1dbfb371af 100644
> > --- a/gcc/dumpfile.c
> > +++ b/gcc/dumpfile.c
> > @@ -378,7 +378,22 @@ dump_open (const char *filename, bool trunc)
> >    FILE *stream = fopen (filename, trunc ? "w" : "a");
> >  
> >    if (!stream)
> > -    error ("could not open dump file %qs: %m", filename);
> > +    {
> > +      /* Autoconf tests compiler functionalities by setting output
> > to /dev/null.
> > +        In this case, if dumps are enabled, it will try to set the
> > output
> > +        folder to /dev/*, which is of course invalid and the
> > compiler will exit
> > +        with an error, resulting in configure script reporting the
> > tested
> > +        feature as being unavailable. Here we test this case by
> > checking if the
> > +        output file prefix has /dev/ and only inform the user in
> > this case
> > +        rather than refusing to compile.  */
> > +
> > +      const char *const slash_dev = "/dev/";
> > +      if (strncmp(slash_dev, filename, strlen(slash_dev)) == 0)
> > +       inform (UNKNOWN_LOCATION,
> > +               "could not open dump file %qs: %m. Dumps are
> > disabled.", filename);
> > +      else
> > +       error ("could not open dump file %qs: %m", filename);
> > +    }
> >    return stream;
> >  }
> >  
> > 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;
> > +}
> > 
> 

Re: [PATCH] Do not abort compilation when dump file is /dev/*

[Richard Biener]

On Thu, 18 Nov 2021, Giuliano Belinassi wrote:

> On Thu, 2021-11-18 at 10:43 +0100, Richard Biener wrote:
> > On Tue, 16 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 dump folder is
> > > /dev/.
> > > If yes, then it just informs the user and disables dumping, but
> > > does
> > > not halt the compilation and the compiler retuns 0 to the shell.
> > 
> > I think matching '/dev/*' is broken.  Either failure to open a dump
> > file should be an error or not.  Btw, some configure tests check
> > for compiler output as well which still breaks in this case.
> 
> Hi,
> 
> IMHO gcc shouldn't fail when it cannot write the dump files because
> compilation could still continue compiling even if the dumps cannot be
> created. However, I chose to do not because the code may be issuing an
> error for other reasons, and matching for /dev/ fixes the test case. 
> 
> > 
> > IMHO a more reasonable thing to do would be to not treat
> > -o /dev/null as a source for -dumpdir and friends.  Alex?
> > You did the last re-org, where'd we put such special casing?
> > 
> > Of course configure checks using -fdump-... with -o /dev/null are
> > a bit fragile.  They could use -fdump-...=FILENAME
> > 
> 
> But that would also set all clones output as being equal to FILENAME
> and don't create one dump for every compiled file, no?

Yes.  But then whoever blindly adds -fdump- to CFLAGS over the
run of configure is really to blame.  Yes, I agree - "silent"
failure is bad, but that's also the fault of configure since it
cannot distinguish between real and accidential fails when you
supply wrong CFLAGS.

So I'm not sure if -fdump- should be special cased.  But for
sure -o /dev/null might not be a good source for our heuristic
way to determine the aux file prefix and directory.

Richard.

> Thanks,
> Giuliano.
> 
> > Thnaks,
> > Richard.
> > 
> > > gcc/ChangeLog
> > > 2021-11-16  Giuliano Belinassi  <gbelinassi@suse.de>
> > > 
> > >         * dumpfile.c (dump_open): Do not halt compilation when file
> > >         matches /dev/*.
> > > 
> > > 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/dumpfile.c                      | 17 ++++++++++++++++-
> > >  gcc/testsuite/gcc.dg/devnull-dump.c |  7 +++++++
> > >  2 files changed, 23 insertions(+), 1 deletion(-)
> > >  create mode 100644 gcc/testsuite/gcc.dg/devnull-dump.c
> > > 
> > > diff --git a/gcc/dumpfile.c b/gcc/dumpfile.c
> > > index 8169daf7f59..b1dbfb371af 100644
> > > --- a/gcc/dumpfile.c
> > > +++ b/gcc/dumpfile.c
> > > @@ -378,7 +378,22 @@ dump_open (const char *filename, bool trunc)
> > >    FILE *stream = fopen (filename, trunc ? "w" : "a");
> > >  
> > >    if (!stream)
> > > -    error ("could not open dump file %qs: %m", filename);
> > > +    {
> > > +      /* Autoconf tests compiler functionalities by setting output
> > > to /dev/null.
> > > +        In this case, if dumps are enabled, it will try to set the
> > > output
> > > +        folder to /dev/*, which is of course invalid and the
> > > compiler will exit
> > > +        with an error, resulting in configure script reporting the
> > > tested
> > > +        feature as being unavailable. Here we test this case by
> > > checking if the
> > > +        output file prefix has /dev/ and only inform the user in
> > > this case
> > > +        rather than refusing to compile.  */
> > > +
> > > +      const char *const slash_dev = "/dev/";
> > > +      if (strncmp(slash_dev, filename, strlen(slash_dev)) == 0)
> > > +       inform (UNKNOWN_LOCATION,
> > > +               "could not open dump file %qs: %m. Dumps are
> > > disabled.", filename);
> > > +      else
> > > +       error ("could not open dump file %qs: %m", filename);
> > > +    }
> > >    return stream;
> > >  }
> > >  
> > > 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;
> > > +}
> > > 
> > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

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

[Giuliano Belinassi]

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 on -o /dev/null
        or -o /dev/zero.

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                 |  4 +++-
 gcc/gcc.c                           | 19 +++++++++++++++----
 gcc/testsuite/gcc.dg/devnull-dump.c |  7 +++++++
 3 files changed, 25 insertions(+), 5 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..4c056606e0c 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -1877,7 +1877,9 @@ 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 @option{-o /dev/null}
+or @option{-o /dev/zero} is passed to the compiler: on those cases the @option{cwd}
+will be used instead. 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..0173018ac04 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -5109,10 +5109,21 @@ process_command (unsigned int decoded_options_count,
     {
       free (dumpdir);
       dumpdir = NULL;
-      temp = lbasename (output_file);
-      if (temp != output_file)
-	dumpdir = xstrndup (output_file,
-			    strlen (output_file) - strlen (temp));
+      if (strcmp(output_file, "/dev/null") == 0
+	  || strcmp(output_file, "/dev/zero") == 0)
+	{
+	  /* If output is /dev/null or /dev/zero, we cannot set the dumpdir to
+	     /dev/ because no files can be created on that directory.  In this
+	     case, we do nothing.  */
+	}
+      else
+	{
+	  /* Set dumpdir as being the same directory where the output file is.  */
+	  temp = lbasename (output_file);
+	  if (temp != output_file)
+	    dumpdir = xstrndup (output_file,
+				strlen (output_file) - strlen (temp));
+	}
     }
   else if (dumpdir)
     {
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;
+}
-- 
2.33.1

Re: [PATCH v2] Do not abort compilation when dump file is /dev/*

[Richard Biener]

On Thu, 18 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/*.

Comments below.

> gcc/ChangeLog
> 2021-11-16  Giuliano Belinassi  <gbelinassi@suse.de>
> 
> 	* gcc.c (process_command): Skip dumpdir override on -o /dev/null
>         or -o /dev/zero.
> 
> 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                 |  4 +++-
>  gcc/gcc.c                           | 19 +++++++++++++++----
>  gcc/testsuite/gcc.dg/devnull-dump.c |  7 +++++++
>  3 files changed, 25 insertions(+), 5 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..4c056606e0c 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -1877,7 +1877,9 @@ 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 @option{-o /dev/null}
> +or @option{-o /dev/zero} is passed to the compiler: on those cases the @option{cwd}
> +will be used instead. Options

"It defaults to the location of the output file, unless the output
file is a special file like @code{/dev/null}."

>  @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..0173018ac04 100644
> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -5109,10 +5109,21 @@ process_command (unsigned int decoded_options_count,
>      {
>        free (dumpdir);
>        dumpdir = NULL;
> -      temp = lbasename (output_file);
> -      if (temp != output_file)
> -	dumpdir = xstrndup (output_file,
> -			    strlen (output_file) - strlen (temp));
> +      if (strcmp(output_file, "/dev/null") == 0
> +	  || strcmp(output_file, "/dev/zero") == 0)

Please merge this into the condition of the block and update the comment

  /* If -save-temps=obj and -o name, create the prefix to use for %b.
     Otherwise just make -save-temps=obj the same as -save-temps=cwd.  */
  else if (save_temps_flag != SAVE_TEMPS_CWD && output_file != NULL)

please split out

          strcmp(output_file, "/dev/null") == 0
          || strcmp(output_file, "/dev/zero") == 0

into a bool special_output_location (const char *) function.  For
example on Windows the special output location would be NUL
(appearantly any path ending in NUL works like that - huh).

Note it might be possible to stat() the file and simply behave
this way whenever it exists and is not a regular file.  For start
just matching literal /dev/null is good enough IMHO (did you
run into cases with /dev/zero?).

Thanks,
Richard.

> +	{
> +	  /* If output is /dev/null or /dev/zero, we cannot set the dumpdir to
> +	     /dev/ because no files can be created on that directory.  In this
> +	     case, we do nothing.  */
> +	}
> +      else
> +	{
> +	  /* Set dumpdir as being the same directory where the output file is.  */
> +	  temp = lbasename (output_file);
> +	  if (temp != output_file)
> +	    dumpdir = xstrndup (output_file,
> +				strlen (output_file) - strlen (temp));
> +	}
>      }
>    else if (dumpdir)
>      {
> 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;
> +}
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

Re: [PATCH] Do not abort compilation when dump file is /dev/*

[Alexandre Oliva]

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

> IMHO a more reasonable thing to do would be to not treat
> -o /dev/null as a source for -dumpdir and friends.  Alex?

+1

I think we already have some special-casing for /dev/null somewhere.

> You did the last re-org, where'd we put such special casing?

I think we're missing something like this, to avoid messing with dumpdir
with -o /dev/null.  We already use the same function when computing
outbase just below this.

diff --git a/gcc/gcc.c b/gcc/gcc.c
index 506c2acc282d6..a986728fb91d6 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 != NULL && not_actual_file_p (output_file)))
     {
       /* Do nothing.  */
     }


-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

Re: [PATCH] Do not abort compilation when dump file is /dev/*

[Richard Biener]

On Fri, 19 Nov 2021, Alexandre Oliva wrote:

> On Nov 18, 2021, Richard Biener <rguenther@suse.de> wrote:
> 
> > IMHO a more reasonable thing to do would be to not treat
> > -o /dev/null as a source for -dumpdir and friends.  Alex?
> 
> +1
> 
> I think we already have some special-casing for /dev/null somewhere.

Grepping finds me the following in system.h which is already checked
for in gcc.c in a few places indeed.

/* Provide a default for the HOST_BIT_BUCKET.
   This suffices for POSIX-like hosts.  */

#ifndef HOST_BIT_BUCKET
#define HOST_BIT_BUCKET "/dev/null"
#endif


> > You did the last re-org, where'd we put such special casing?
> 
> I think we're missing something like this, to avoid messing with dumpdir
> with -o /dev/null.  We already use the same function when computing
> outbase just below this.

Ah yeah, not_actual_file_p should do the trick indeed.  Giuliano, can
you update the patch like below?  I think we should still adjust
documentation as you did.

> diff --git a/gcc/gcc.c b/gcc/gcc.c
> index 506c2acc282d6..a986728fb91d6 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 != NULL && not_actual_file_p (output_file)))
>      {
>        /* Do nothing.  */
>      }
> 

Re: [PATCH] Do not abort compilation when dump file is /dev/*

[Bernhard Reutner-Fischer]

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

> On Fri, 19 Nov 2021, Alexandre Oliva wrote:
> 
> > On Nov 18, 2021, Richard Biener <rguenther@suse.de> wrote:
> >   
> > > IMHO a more reasonable thing to do would be to not treat
> > > -o /dev/null as a source for -dumpdir and friends.  Alex?  
> > 
> > +1
> > 
> > I think we already have some special-casing for /dev/null somewhere.  
> 
> Grepping finds me the following in system.h which is already checked
> for in gcc.c in a few places indeed.
> 
> /* Provide a default for the HOST_BIT_BUCKET.
>    This suffices for POSIX-like hosts.  */
> 
> #ifndef HOST_BIT_BUCKET
> #define HOST_BIT_BUCKET "/dev/null"
> #endif
> 
> 
> > > You did the last re-org, where'd we put such special casing?  
> > 
> > I think we're missing something like this, to avoid messing with dumpdir
> > with -o /dev/null.  We already use the same function when computing
> > outbase just below this.  
> 
> Ah yeah, not_actual_file_p should do the trick indeed.  Giuliano, can
> you update the patch like below?  I think we should still adjust
> documentation as you did.

But that wouldn't cater for the general problem that the dumpdir is not
writable, no? Why not just simply check access W_OK of the dumpdir?

Otherwise a dumpdir /dev/full or anyother such path will cause the same
thing i guess.

thanks,
> 
> > diff --git a/gcc/gcc.c b/gcc/gcc.c
> > index 506c2acc282d6..a986728fb91d6 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 != NULL && not_actual_file_p (output_file)))
> >      {
> >        /* Do nothing.  */
> >      }
> >   

Re: [PATCH] Do not abort compilation when dump file is /dev/*

[Richard Biener]

On Fri, 19 Nov 2021, Bernhard Reutner-Fischer wrote:

> On Fri, 19 Nov 2021 10:35:26 +0100 (CET)
> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> > On Fri, 19 Nov 2021, Alexandre Oliva wrote:
> > 
> > > On Nov 18, 2021, Richard Biener <rguenther@suse.de> wrote:
> > >   
> > > > IMHO a more reasonable thing to do would be to not treat
> > > > -o /dev/null as a source for -dumpdir and friends.  Alex?  
> > > 
> > > +1
> > > 
> > > I think we already have some special-casing for /dev/null somewhere.  
> > 
> > Grepping finds me the following in system.h which is already checked
> > for in gcc.c in a few places indeed.
> > 
> > /* Provide a default for the HOST_BIT_BUCKET.
> >    This suffices for POSIX-like hosts.  */
> > 
> > #ifndef HOST_BIT_BUCKET
> > #define HOST_BIT_BUCKET "/dev/null"
> > #endif
> > 
> > 
> > > > You did the last re-org, where'd we put such special casing?  
> > > 
> > > I think we're missing something like this, to avoid messing with dumpdir
> > > with -o /dev/null.  We already use the same function when computing
> > > outbase just below this.  
> > 
> > Ah yeah, not_actual_file_p should do the trick indeed.  Giuliano, can
> > you update the patch like below?  I think we should still adjust
> > documentation as you did.
> 
> But that wouldn't cater for the general problem that the dumpdir is not
> writable, no? Why not just simply check access W_OK of the dumpdir?
> 
> Otherwise a dumpdir /dev/full or anyother such path will cause the same
> thing i guess.

I think those cases are OK to diagnose.  Just choosing a
not_actual_file_p output as base to derive the dump directory is
bad.

Richard.

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

[Giuliano Belinassi]

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);
 }
 
 /* %: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;
+}
-- 
2.33.1

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

[Richard Biener]

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;
> +}
> 

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

[Bernhard Reutner-Fischer]

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.

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

[Alexandre Oliva]

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,

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>

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

[Giuliano Belinassi]

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