Re: [PATCH] Prune invalid filename due to makefile syntax.
On 1/23/20 4:54 AM, Martin Liška wrote: On 1/22/20 9:12 AM, Richard Biener wrote: Ah, maybe it's worth to export this functionality to libiberty? That would make sense to me. Can you please Nathan factor out the function to libiberty? you're not the boss of me :) But seriously, I'm not really going to have time to invest in this in the near future. nathan -- Nathan Sidwell
Re: [PATCH] Prune invalid filename due to makefile syntax.
On 1/22/20 9:12 AM, Richard Biener wrote: Ah, maybe it's worth to export this functionality to libiberty? That would make sense to me. Can you please Nathan factor out the function to libiberty? Thanks, Martin
Re: [PATCH] Prune invalid filename due to makefile syntax.
On Tue, Jan 21, 2020 at 5:40 PM Nathan Sidwell wrote: > > On 1/21/20 11:26 AM, Richard Biener wrote: > > On January 21, 2020 4:37:00 PM GMT+01:00, Jan Hubicka > > wrote: > > >> I would say we want to fix that... Even GCC gets idea to put # info > >> filenames and I am sure except for HHVM there are others. > > > > Can't w just quote them somehow? > > IIUC there are two issues > > 1) Make's syntax. Its quoting is baroque, (and incomplete?). See the > comment in libcpp/mkdeps.c, which I think is curtesy of ZackW. I see > that the just-released Make 4.3 has changed something to do with # and > its need to be quoted in some contexts. Ah, maybe it's worth to export this functionality to libiberty? > 2) Bad chars for the underlying filesystem. But, if the temp files are > on the same FS as the thing they're temping for, then there shouldn't be > a conflict. If they are different, then one might hope the temp-fs > allows a superset. Yeah, so I'd suggest to simply avoid the bad names being created from the WPA stage and instead sanitize it there already. That would make it also consistent when debugging. Not to say that using a tempfile by default when not doing -save-temps would be appropriate anyway. Richard. > nathan > > -- > Nathan Sidwell
Re: [PATCH] Prune invalid filename due to makefile syntax.
On 1/21/20 11:26 AM, Richard Biener wrote: On January 21, 2020 4:37:00 PM GMT+01:00, Jan Hubicka wrote: I would say we want to fix that... Even GCC gets idea to put # info filenames and I am sure except for HHVM there are others. Can't w just quote them somehow? IIUC there are two issues 1) Make's syntax. Its quoting is baroque, (and incomplete?). See the comment in libcpp/mkdeps.c, which I think is curtesy of ZackW. I see that the just-released Make 4.3 has changed something to do with # and its need to be quoted in some contexts. 2) Bad chars for the underlying filesystem. But, if the temp files are on the same FS as the thing they're temping for, then there shouldn't be a conflict. If they are different, then one might hope the temp-fs allows a superset. nathan -- Nathan Sidwell
Re: [PATCH] Prune invalid filename due to makefile syntax.
On January 21, 2020 4:37:00 PM GMT+01:00, Jan Hubicka wrote: >> On 1/21/20 4:08 PM, Jan Hubicka wrote: >> > I think this is not enough - you need to take into consideration >all >> > special characters used by make and bash, such as $ and others... >> >> Hm, you are right. Do you have a reasonable list which we should >support? >No - I guess one needs to dig into manual, try them out or just assume >that all letters in filenams except for a-z, 0-9, _, -, ., ',' and >perhaps more common caracters needs to be taken out? It is used only >to >make more meaningful temp filenames, right? >> Or should we leave this known limitation? >I would say we want to fix that... Even GCC gets idea to put # info >filenames and I am sure except for HHVM there are others. Can't w just quote them somehow? Richard. > >Honza >> >> Martin
Re: [PATCH] Prune invalid filename due to makefile syntax.
> On 1/21/20 4:08 PM, Jan Hubicka wrote: > > I think this is not enough - you need to take into consideration all > > special characters used by make and bash, such as $ and others... > > Hm, you are right. Do you have a reasonable list which we should support? No - I guess one needs to dig into manual, try them out or just assume that all letters in filenams except for a-z, 0-9, _, -, ., ',' and perhaps more common caracters needs to be taken out? It is used only to make more meaningful temp filenames, right? > Or should we leave this known limitation? I would say we want to fix that... Even GCC gets idea to put # info filenames and I am sure except for HHVM there are others. Honza > > Martin
Re: [PATCH] Prune invalid filename due to makefile syntax.
On 1/21/20 4:08 PM, Jan Hubicka wrote: I think this is not enough - you need to take into consideration all special characters used by make and bash, such as $ and others... Hm, you are right. Do you have a reasonable list which we should support? Or should we leave this known limitation? Martin
Re: [PATCH] Prune invalid filename due to makefile syntax.
On 1/21/20 4:15 PM, Andreas Schwab wrote:
On Jan 21 2020, Martin Liška wrote:
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index fe8f292f877..f2504cc5b4f 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1241,6 +1241,16 @@ jobserver_active_p (void)
&& is_valid_fd (wfd));
}
+/* Prune invalid characters that can't be in name due to Makefile syntax. */
+
+static void
+prune_filename_for_make (char *filename)
+{
+ for (unsigned i = 0; i < strlen (filename); i++)
That's quadratic runtime.
Andreas.
Thanks for heads up.
Martin
>From 88b3817dc377ecf030300078514eb5749c361406 Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Tue, 21 Jan 2020 16:03:36 +0100
Subject: [PATCH] Prune invalid filename due to makefile syntax.
gcc/ChangeLog:
2020-01-21 Martin Liska
PR driver/93057
* lto-wrapper.c (prune_filename_for_make): Prune
characters like '#'.
---
gcc/lto-wrapper.c | 12
1 file changed, 12 insertions(+)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index fe8f292f877..81d0fdb2410 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1241,6 +1241,17 @@ jobserver_active_p (void)
&& is_valid_fd (wfd));
}
+/* Prune invalid characters that can't be in name due to Makefile syntax. */
+
+static void
+prune_filename_for_make (char *filename)
+{
+ unsigned length = strlen (filename);
+ for (unsigned i = 0; i < length; i++)
+if (filename[i] == '#')
+ filename[i] = '_';
+}
+
/* Execute gcc. ARGC is the number of arguments. ARGV contains the arguments. */
static void
@@ -1780,6 +1791,7 @@ cont:
obstack_grow (&env_obstack, input_name, strlen (input_name) - 2);
obstack_grow (&env_obstack, ".ltrans.o", sizeof (".ltrans.o"));
output_name = XOBFINISH (&env_obstack, char *);
+ prune_filename_for_make (output_name);
/* Adjust the dumpbase if the linker output file was seen. */
if (linker_output)
--
2.24.1
Re: [PATCH] Prune invalid filename due to makefile syntax.
On Jan 21 2020, Martin Liška wrote:
> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
> index fe8f292f877..f2504cc5b4f 100644
> --- a/gcc/lto-wrapper.c
> +++ b/gcc/lto-wrapper.c
> @@ -1241,6 +1241,16 @@ jobserver_active_p (void)
> && is_valid_fd (wfd));
> }
>
> +/* Prune invalid characters that can't be in name due to Makefile syntax. */
> +
> +static void
> +prune_filename_for_make (char *filename)
> +{
> + for (unsigned i = 0; i < strlen (filename); i++)
That's quadratic runtime.
Andreas.
--
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Re: [PATCH] Prune invalid filename due to makefile syntax.
> Hi.
>
> The patch strips '#' in filenames used for Makefile.
>
> Ready to be installed?
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2020-01-21 Martin Liska
>
> PR driver/93057
> * lto-wrapper.c (prune_filename_for_make): Prune
> characters like '#'.
I think this is not enough - you need to take into consideration all
special characters used by make and bash, such as $ and others...
Honza
> ---
> gcc/lto-wrapper.c | 11 +++
> 1 file changed, 11 insertions(+)
>
>
> diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
> index fe8f292f877..f2504cc5b4f 100644
> --- a/gcc/lto-wrapper.c
> +++ b/gcc/lto-wrapper.c
> @@ -1241,6 +1241,16 @@ jobserver_active_p (void)
> && is_valid_fd (wfd));
> }
>
> +/* Prune invalid characters that can't be in name due to Makefile syntax. */
> +
> +static void
> +prune_filename_for_make (char *filename)
> +{
> + for (unsigned i = 0; i < strlen (filename); i++)
> +if (filename[i] == '#')
> + filename[i] = '_';
> +}
> +
> /* Execute gcc. ARGC is the number of arguments. ARGV contains the
> arguments. */
>
> static void
> @@ -1780,6 +1790,7 @@ cont:
> obstack_grow (&env_obstack, input_name, strlen (input_name) - 2);
> obstack_grow (&env_obstack, ".ltrans.o", sizeof (".ltrans.o"));
> output_name = XOBFINISH (&env_obstack, char *);
> + prune_filename_for_make (output_name);
>
> /* Adjust the dumpbase if the linker output file was seen. */
> if (linker_output)
>
[PATCH] Prune invalid filename due to makefile syntax.
Hi.
The patch strips '#' in filenames used for Makefile.
Ready to be installed?
Thanks,
Martin
gcc/ChangeLog:
2020-01-21 Martin Liska
PR driver/93057
* lto-wrapper.c (prune_filename_for_make): Prune
characters like '#'.
---
gcc/lto-wrapper.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index fe8f292f877..f2504cc5b4f 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -1241,6 +1241,16 @@ jobserver_active_p (void)
&& is_valid_fd (wfd));
}
+/* Prune invalid characters that can't be in name due to Makefile syntax. */
+
+static void
+prune_filename_for_make (char *filename)
+{
+ for (unsigned i = 0; i < strlen (filename); i++)
+if (filename[i] == '#')
+ filename[i] = '_';
+}
+
/* Execute gcc. ARGC is the number of arguments. ARGV contains the arguments. */
static void
@@ -1780,6 +1790,7 @@ cont:
obstack_grow (&env_obstack, input_name, strlen (input_name) - 2);
obstack_grow (&env_obstack, ".ltrans.o", sizeof (".ltrans.o"));
output_name = XOBFINISH (&env_obstack, char *);
+ prune_filename_for_make (output_name);
/* Adjust the dumpbase if the linker output file was seen. */
if (linker_output)
