| Message ID | 144dce4b-4163-c4b0-2817-aebe74cbca0f@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Prune invalid filename due to makefile syntax. | expand |
> Hi. > > The patch strips '#' in filenames used for Makefile. > > Ready to be installed? > Thanks, > Martin > > gcc/ChangeLog: > > 2020-01-21 Martin Liska <mliska@suse.cz> > > 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) >
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.
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
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
> 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
On January 21, 2020 4:37:00 PM GMT+01:00, Jan Hubicka <hubicka@ucw.cz> 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
On 1/21/20 11:26 AM, Richard Biener wrote: > On January 21, 2020 4:37:00 PM GMT+01:00, Jan Hubicka <hubicka@ucw.cz> 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
On Tue, Jan 21, 2020 at 5:40 PM Nathan Sidwell <nathan@acm.org> wrote: > > On 1/21/20 11:26 AM, Richard Biener wrote: > > On January 21, 2020 4:37:00 PM GMT+01:00, Jan Hubicka <hubicka@ucw.cz> 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
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
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
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)