[PATCH] Fix PR50293 - LTO plugin with space in path

[PATCH] Fix PR50293 - LTO plugin with space in path

[Joey Ye]

Mainline and 4.7 failed to use LTO when toolchain is installed to a path
with space in it. Resulting in error message like:

xxxx/ld.exe: c:/program: error loading plugin
collect2.exe: error: ld returned 1 exit status

Root cause is when GCC driver process specs, it doesn't handle plugin file
name properly. Here is a patch fixing it.

Bootstraped and make check on x86 and aarch32, no regression.

OK to trunk and 4.7?

ChangeLog:

	PR lto/50293
	* gcc.c (convert_white_space): New function.
	(main): Handles white space in function name.

Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c   (revision 195189)
+++ gcc/gcc.c   (working copy)
@@ -265,6 +265,7 @@
 static const char *compare_debug_auxbase_opt_spec_function (int, const char
**);
 static const char *pass_through_libs_spec_func (int, const char **);
 static const char *replace_extension_spec_func (int, const char **);
+static char * convert_white_space(char * orig);


 /* The Specs Language

@@ -6595,6 +6596,7 @@
                                    X_OK, false);
   if (lto_wrapper_file)
     {
+      lto_wrapper_file = convert_white_space(lto_wrapper_file);
       lto_wrapper_spec = lto_wrapper_file;
       obstack_init (&collect_obstack);
       obstack_grow (&collect_obstack, "COLLECT_LTO_WRAPPER=",
@@ -7005,12 +7007,13 @@
                              + strlen (fuse_linker_plugin), 0))
 #endif
            {
-             linker_plugin_file_spec = find_a_file (&exec_prefixes,
+             char * temp_spec = find_a_file (&exec_prefixes,
                                                     LTOPLUGINSONAME, R_OK,
                                                     false);
-             if (!linker_plugin_file_spec)
+             if (!temp_spec)
                fatal_error ("-fuse-linker-plugin, but %s not found",
                             LTOPLUGINSONAME);
+             linker_plugin_file_spec = convert_white_space(temp_spec);
            }
 #endif
          lto_gcc_spec = argv[0];
@@ -8506,3 +8509,30 @@
   free (name);
   return result;
 }
+
+/* Insert back slash before spaces in a string, to avoid path
+   that has space in it broken into multiple arguments.  */
+
+static char * convert_white_space(char * orig)
+{
+  int len, number_of_space = 0;
+  char *p = orig;
+  if (orig == NULL) return NULL;
+
+  for (len=0; p[len]; len++)
+    if (p[len] == ' ' || p[len] == '\t') number_of_space ++;
+
+  if (number_of_space)
+    {
+      char * new_spec = (char *)xmalloc(len + number_of_space + 1);
+      int j,k;
+      for (j=0, k=0; j<=len; j++, k++)
+       {
+         if (p[j] == ' ' || p[j] == '\t') new_spec[k++]='\\';
+         new_spec[k] = p[j];
+       }
+      free(CONST_CAST(char *, orig));
+      return new_spec;
+  }
+  else return orig;
+}

Re: [PATCH] Fix PR50293 - LTO plugin with space in path

[Joseph S. Myers]

On Sun, 17 Feb 2013, Joey Ye wrote:

> +static char * convert_white_space(char * orig);

Please fix formatting in many places in this patch to follow the GNU 
Coding Standards.  No space after '*', but space before '('; there seem to 
be various other formatting problems as well.

> +/* Insert back slash before spaces in a string, to avoid path
> +   that has space in it broken into multiple arguments.  */

That doesn't seem to be a proper specification of the interface to this 
function.  What are the semantics of ORIG?  A string that is a filename, 
or something else?  What are the exact semantics of the return value for 
quoting - is it correct for the function to convert a (backslash, space) 
pair to (backslash, backslash, space) or not?  Is anything special in the 
return value other than backslash and space, and how are any special 
characters in the return value to be interpreted?

As it seems like this function frees the argument (why?) this also needs 
to be specified in the comment as part of the semantics of the function.

It would be a good idea for you to give a more detailed explanation in the 
next version of the patch submission of how the path, before the patch, 
got processed so that the spaces were wrongly interpreted.  That might 
help make clearer whether the interface to this new function is actually 
correct, since the subsequent operations on the return value should act as 
an inverse to the operation carried out by this function.

-- 
Joseph S. Myers
joseph@codesourcery.com

RE: [PATCH] Fix PR50293 - LTO plugin with space in path

[Joey Ye]

Joseph, Thanks for your valuable comments. See my reply and new patch below.

> -----Original Message-----
> From: Joseph Myers [mailto:joseph@codesourcery.com]
> Sent: Monday, February 18, 2013 06:16
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Fix PR50293 - LTO plugin with space in path
> 
> On Sun, 17 Feb 2013, Joey Ye wrote:
> 
> > +static char * convert_white_space(char * orig);
> 
> Please fix formatting in many places in this patch to follow the GNU
> Coding Standards.  No space after '*', but space before '('; there seem
> to
> be various other formatting problems as well.
My bad. All fixed.

> 
> > +/* Insert back slash before spaces in a string, to avoid path
> > +   that has space in it broken into multiple arguments.  */
> 
> That doesn't seem to be a proper specification of the interface to this
> function.  What are the semantics of ORIG?  A string that is a filename,
> or something else?  What are the exact semantics of the return value for
> quoting - is it correct for the function to convert a (backslash, space)
> pair to (backslash, backslash, space) or not?  Is anything special in
> the
> return value other than backslash and space, and how are any special
> characters in the return value to be interpreted?
> 
> As it seems like this function frees the argument (why?) this also needs
> to be specified in the comment as part of the semantics of the function.
This function might need a string longer than original one to accommodate
additional back slashes. So it has to xmalloc a new string. The original
string should be freed in such a case. However, it is tedious to caller
to figure out that conversion does happens and free the orig. The solution
is for this function to free it when conversion happens.

By doing so it is required that orig must be allocated and can be freed,
as the newly added comments described explicitly.
> 
> It would be a good idea for you to give a more detailed explanation in
> the
> next version of the patch submission of how the path, before the patch,
> got processed so that the spaces were wrongly interpreted.  That might
> help make clearer whether the interface to this new function is actually
> correct, since the subsequent operations on the return value should act
> as
> an inverse to the operation carried out by this function.
> 
> --
> Joseph S. Myers
> joseph@codesourcery.com

Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 195189)
+++ gcc/gcc.c	(working copy)
@@ -265,6 +265,7 @@
 static const char *compare_debug_auxbase_opt_spec_function (int, const char
**);
 static const char *pass_through_libs_spec_func (int, const char **);
 static const char *replace_extension_spec_func (int, const char **);
+static char * convert_white_space (char *);
 

 /* The Specs Language
 
@@ -6595,6 +6596,7 @@
 				    X_OK, false);
   if (lto_wrapper_file)
     {
+      lto_wrapper_file = convert_white_space (lto_wrapper_file);
       lto_wrapper_spec = lto_wrapper_file;
       obstack_init (&collect_obstack);
       obstack_grow (&collect_obstack, "COLLECT_LTO_WRAPPER=",
@@ -7005,12 +7007,13 @@
 			      + strlen (fuse_linker_plugin), 0))
 #endif
 	    {
-	      linker_plugin_file_spec = find_a_file (&exec_prefixes,
+	      char * temp_spec = find_a_file (&exec_prefixes,
 						     LTOPLUGINSONAME, R_OK,
 						     false);
-	      if (!linker_plugin_file_spec)
+	      if (!temp_spec)
 		fatal_error ("-fuse-linker-plugin, but %s not found",
 			     LTOPLUGINSONAME);
+	      linker_plugin_file_spec = convert_white_space (temp_spec);
 	    }
 #endif
 	  lto_gcc_spec = argv[0];
@@ -8506,3 +8509,52 @@
   free (name);
   return result;
 }
+
+/* Insert back slash before spaces in orig (usually a file path), to 
+   avoid being broken by spec parser.
+
+   This function is needed as do_spec_1 treats white space (' ' and '\t')
+   as the end of an argument. But in case of -plugin /usr/gcc
install/xxx.so,
+   the filename should be treated as a single argument rather than being
+   broken into multiple. Solution is to insert '\\' before the space in a 
+   filename.
+   
+   This function converts and only converts all occurrance of ' ' 
+   to '\\' + ' ' and '\t' to '\\' + '\t'.  For example:
+   "a b"  -> "a\\ b"
+   "a  b" -> "a\\ \\ b"
+   "a\tb" -> "a\\\tb"
+   "a\\ b" -> "a\\\\ b"
+
+   orig: input null-terminating string that was allocated by xalloc. The
+   memory it points to might be freed in this function. Behavior undefined
+   if orig isn't xalloced or is freed already at entry.
+
+   Return: orig if no conversion needed. orig if conversion needed but no
+   sufficient memory for a new string. Otherwise a newly allocated string
+   that was converted from orig.  */
+
+static char * convert_white_space (char *orig)
+{
+  int len, number_of_space = 0;
+  if (orig == NULL) return orig;
+
+  for (len=0; orig[len]; len++)
+    if (orig[len] == ' ' || orig[len] == '\t') number_of_space ++;
+
+  if (number_of_space)
+    {
+      char * new_spec = (char *)xmalloc (len + number_of_space + 1);
+      int j,k;
+      if (new_spec == NULL) return orig;
+
+      for (j=0, k=0; j<=len; j++, k++)
+	{
+	  if (orig[j] == ' ' || orig[j] == '\t') new_spec[k++] = '\\';
+	  new_spec[k] = orig[j];
+	}
+      free (orig);
+      return new_spec;
+  }
+  else return orig;
+}

RE: [PATCH] Fix PR50293 - LTO plugin with space in path

[Joey Ye]

[-- Attachment #1: Type: text/plain, Size: 6103 bytes --]

Ping

> -----Original Message-----
> From: Joey Ye [mailto:joey.ye@arm.com]
> Sent: Monday, February 18, 2013 11:32
> To: 'Joseph Myers'
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] Fix PR50293 - LTO plugin with space in path
> 
> Joseph, Thanks for your valuable comments. See my reply and new patch
> below.
> 
> > -----Original Message-----
> > From: Joseph Myers [mailto:joseph@codesourcery.com]
> > Sent: Monday, February 18, 2013 06:16
> > To: Joey Ye
> > Cc: gcc-patches@gcc.gnu.org
> > Subject: Re: [PATCH] Fix PR50293 - LTO plugin with space in path
> >
> > On Sun, 17 Feb 2013, Joey Ye wrote:
> >
> > > +static char * convert_white_space(char * orig);
> >
> > Please fix formatting in many places in this patch to follow the GNU
> > Coding Standards.  No space after '*', but space before '('; there
> seem
> > to
> > be various other formatting problems as well.
> My bad. All fixed.
> 
> >
> > > +/* Insert back slash before spaces in a string, to avoid path
> > > +   that has space in it broken into multiple arguments.  */
> >
> > That doesn't seem to be a proper specification of the interface to
> this
> > function.  What are the semantics of ORIG?  A string that is a
> filename,
> > or something else?  What are the exact semantics of the return value
> for
> > quoting - is it correct for the function to convert a (backslash,
> space)
> > pair to (backslash, backslash, space) or not?  Is anything special in
> > the
> > return value other than backslash and space, and how are any special
> > characters in the return value to be interpreted?
> >
> > As it seems like this function frees the argument (why?) this also
> needs
> > to be specified in the comment as part of the semantics of the
> function.
> This function might need a string longer than original one to
> accommodate
> additional back slashes. So it has to xmalloc a new string. The original
> string should be freed in such a case. However, it is tedious to caller
> to figure out that conversion does happens and free the orig. The
> solution
> is for this function to free it when conversion happens.
> 
> By doing so it is required that orig must be allocated and can be freed,
> as the newly added comments described explicitly.
> >
> > It would be a good idea for you to give a more detailed explanation in
> > the
> > next version of the patch submission of how the path, before the patch,
> > got processed so that the spaces were wrongly interpreted.  That might
> > help make clearer whether the interface to this new function is
> actually
> > correct, since the subsequent operations on the return value should
> act
> > as
> > an inverse to the operation carried out by this function.
> >
> > --
> > Joseph S. Myers
> > joseph@codesourcery.com
> 
> Index: gcc/gcc.c
> ===================================================================
> --- gcc/gcc.c	(revision 195189)
> +++ gcc/gcc.c	(working copy)
> @@ -265,6 +265,7 @@
>  static const char *compare_debug_auxbase_opt_spec_function (int, const
> char **);
>  static const char *pass_through_libs_spec_func (int, const char **);
>  static const char *replace_extension_spec_func (int, const char **);
> +static char * convert_white_space (char *);
> 
> 
>  /* The Specs Language
> 
> @@ -6595,6 +6596,7 @@
>  				    X_OK, false);
>    if (lto_wrapper_file)
>      {
> +      lto_wrapper_file = convert_white_space (lto_wrapper_file);
>        lto_wrapper_spec = lto_wrapper_file;
>        obstack_init (&collect_obstack);
>        obstack_grow (&collect_obstack, "COLLECT_LTO_WRAPPER=",
> @@ -7005,12 +7007,13 @@
>  			      + strlen (fuse_linker_plugin), 0))
>  #endif
>  	    {
> -	      linker_plugin_file_spec = find_a_file (&exec_prefixes,
> +	      char * temp_spec = find_a_file (&exec_prefixes,
>  						     LTOPLUGINSONAME, R_OK,
>  						     false);
> -	      if (!linker_plugin_file_spec)
> +	      if (!temp_spec)
>  		fatal_error ("-fuse-linker-plugin, but %s not found",
>  			     LTOPLUGINSONAME);
> +	      linker_plugin_file_spec = convert_white_space (temp_spec);
>  	    }
>  #endif
>  	  lto_gcc_spec = argv[0];
> @@ -8506,3 +8509,52 @@
>    free (name);
>    return result;
>  }
> +
> +/* Insert back slash before spaces in orig (usually a file path), to
> +   avoid being broken by spec parser.
> +
> +   This function is needed as do_spec_1 treats white space (' ' and
> '\t')
> +   as the end of an argument. But in case of -plugin /usr/gcc
> install/xxx.so,
> +   the filename should be treated as a single argument rather than
> being
> +   broken into multiple. Solution is to insert '\\' before the space in
> a
> +   filename.
> +
> +   This function converts and only converts all occurrance of ' '
> +   to '\\' + ' ' and '\t' to '\\' + '\t'.  For example:
> +   "a b"  -> "a\\ b"
> +   "a  b" -> "a\\ \\ b"
> +   "a\tb" -> "a\\\tb"
> +   "a\\ b" -> "a\\\\ b"
> +
> +   orig: input null-terminating string that was allocated by xalloc.
> The
> +   memory it points to might be freed in this function. Behavior
> undefined
> +   if orig isn't xalloced or is freed already at entry.
> +
> +   Return: orig if no conversion needed. orig if conversion needed but
> no
> +   sufficient memory for a new string. Otherwise a newly allocated
> string
> +   that was converted from orig.  */
> +
> +static char * convert_white_space (char *orig)
> +{
> +  int len, number_of_space = 0;
> +  if (orig == NULL) return orig;
> +
> +  for (len=0; orig[len]; len++)
> +    if (orig[len] == ' ' || orig[len] == '\t') number_of_space ++;
> +
> +  if (number_of_space)
> +    {
> +      char * new_spec = (char *)xmalloc (len + number_of_space + 1);
> +      int j,k;
> +      if (new_spec == NULL) return orig;
> +
> +      for (j=0, k=0; j<=len; j++, k++)
> +	{
> +	  if (orig[j] == ' ' || orig[j] == '\t') new_spec[k++] = '\\';
> +	  new_spec[k] = orig[j];
> +	}
> +      free (orig);
> +      return new_spec;
> +  }
> +  else return orig;
> +}

[-- Attachment #2: space_in_plugin_spec-20130218.patch --]
[-- Type: application/octet-stream, Size: 2982 bytes --]

Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 195189)
+++ gcc/gcc.c	(working copy)
@@ -265,6 +265,7 @@
 static const char *compare_debug_auxbase_opt_spec_function (int, const char **);
 static const char *pass_through_libs_spec_func (int, const char **);
 static const char *replace_extension_spec_func (int, const char **);
+static char * convert_white_space (char *);
 \f
 /* The Specs Language
 
@@ -6595,6 +6596,7 @@
 				    X_OK, false);
   if (lto_wrapper_file)
     {
+      lto_wrapper_file = convert_white_space (lto_wrapper_file);
       lto_wrapper_spec = lto_wrapper_file;
       obstack_init (&collect_obstack);
       obstack_grow (&collect_obstack, "COLLECT_LTO_WRAPPER=",
@@ -7005,12 +7007,13 @@
 			      + strlen (fuse_linker_plugin), 0))
 #endif
 	    {
-	      linker_plugin_file_spec = find_a_file (&exec_prefixes,
+	      char * temp_spec = find_a_file (&exec_prefixes,
 						     LTOPLUGINSONAME, R_OK,
 						     false);
-	      if (!linker_plugin_file_spec)
+	      if (!temp_spec)
 		fatal_error ("-fuse-linker-plugin, but %s not found",
 			     LTOPLUGINSONAME);
+	      linker_plugin_file_spec = convert_white_space (temp_spec);
 	    }
 #endif
 	  lto_gcc_spec = argv[0];
@@ -8506,3 +8509,52 @@
   free (name);
   return result;
 }
+
+/* Insert back slash before spaces in orig (usually a file path), to 
+   avoid being broken by spec parser.
+
+   This function is needed as do_spec_1 treats white space (' ' and '\t')
+   as the end of an argument. But in case of -plugin /usr/gcc install/xxx.so,
+   the filename should be treated as a single argument rather than being
+   broken into multiple. Solution is to insert '\\' before the space in a 
+   filename.
+   
+   This function converts and only converts all occurrance of ' ' 
+   to '\\' + ' ' and '\t' to '\\' + '\t'.  For example:
+   "a b"  -> "a\\ b"
+   "a  b" -> "a\\ \\ b"
+   "a\tb" -> "a\\\tb"
+   "a\\ b" -> "a\\\\ b"
+
+   orig: input null-terminating string that was allocated by xalloc. The
+   memory it points to might be freed in this function. Behavior undefined
+   if orig isn't xalloced or is freed already at entry.
+
+   Return: orig if no conversion needed. orig if conversion needed but no
+   sufficient memory for a new string. Otherwise a newly allocated string
+   that was converted from orig.  */
+
+static char * convert_white_space (char *orig)
+{
+  int len, number_of_space = 0;
+  if (orig == NULL) return orig;
+
+  for (len=0; orig[len]; len++)
+    if (orig[len] == ' ' || orig[len] == '\t') number_of_space ++;
+
+  if (number_of_space)
+    {
+      char * new_spec = (char *)xmalloc (len + number_of_space + 1);
+      int j,k;
+      if (new_spec == NULL) return orig;
+
+      for (j=0, k=0; j<=len; j++, k++)
+	{
+	  if (orig[j] == ' ' || orig[j] == '\t') new_spec[k++] = '\\';
+	  new_spec[k] = orig[j];
+	}
+      free (orig);
+      return new_spec;
+  }
+  else return orig;
+}

RE: [PATCH] Fix PR50293 - LTO plugin with space in path

[Joseph S. Myers]

On Mon, 18 Feb 2013, Joey Ye wrote:

> +static char * convert_white_space (char *);

No space after "*".

> -	      linker_plugin_file_spec = find_a_file (&exec_prefixes,
> +	      char * temp_spec = find_a_file (&exec_prefixes,
>  						     LTOPLUGINSONAME, R_OK,
>  						     false);

The indentation of the following lines looks odd after this patch; unless 
that's just an effect of TABs plus quoting, make sure they are reindented 
to line up with the new position of the opening '('.

> +/* Insert back slash before spaces in orig (usually a file path), to 

Capitalize variable names when referring to the value of the variable, so 
ORIG; likewise elsewhere in this comment.  Single work "backslash".

> +   the filename should be treated as a single argument rather than being

"file name" should be two words, according to the GNU Coding Standards.

> +   This function converts and only converts all occurrance of ' ' 

"occurrence"

> +   Return: orig if no conversion needed. orig if conversion needed but no
> +   sufficient memory for a new string. Otherwise a newly allocated string

Returning wrong results on insufficient memory doesn't make sense.  
Anyway, xmalloc always exits the program if there is insufficient memory, 
so you don't need any code to allow for that case.

> +static char * convert_white_space (char *orig)

Newline, not space, between return type and function name, so that the 
function name comes at the start of the line.

> +  if (orig == NULL) return orig;

The comment didn't mention NULL as a valid argument, and it doesn't appear 
NULL can actually be passed to this function.  So don't include code to 
handle that case.

> +  for (len=0; orig[len]; len++)

Spaces around "=".

> +    if (orig[len] == ' ' || orig[len] == '\t') number_of_space ++;

No space before "++", but put the body of the "if" on a separate line.

> +      char * new_spec = (char *)xmalloc (len + number_of_space + 1);

No space after "*".  Space in the cast after "(char *)".

> +      int j,k;

Space after ",".

> +      if (new_spec == NULL) return orig;

As discussed above, not needed.

> +      for (j=0, k=0; j<=len; j++, k++)

Spaces around "=" and "<=".

> +	  if (orig[j] == ' ' || orig[j] == '\t') new_spec[k++] = '\\';

Put the "if" both on a separate line.

> +  else return orig;

Put the "else" body on a separate line.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Fix PR50293 - LTO plugin with space in path

[Georg-Johann Lay]

Joey Ye schrieb:
> Ping
> 
>> Subject: RE: [PATCH] Fix PR50293 - LTO plugin with space in path

Does this patch also work with MS-Windows as host, i.e. with \ as path 
separator?

>>>> +static char * convert_white_space(char * orig);
>>> Please fix formatting in many places in this patch to follow the GNU
>>> Coding Standards.  No space after '*', but space before '('; there
>> seem
>>> to
>>> be various other formatting problems as well.
>> My bad. All fixed.
>>
>>>> +/* Insert back slash before spaces in a string, to avoid path
>>>> +   that has space in it broken into multiple arguments.  */
>>> That doesn't seem to be a proper specification of the interface to
>> this
>>> function.  What are the semantics of ORIG?  A string that is a
>> filename,
>>> or something else?  What are the exact semantics of the return value
>> for
>>> quoting - is it correct for the function to convert a (backslash,
>> space)
>>> pair to (backslash, backslash, space) or not?  Is anything special in
>>> the
>>> return value other than backslash and space, and how are any special
>>> characters in the return value to be interpreted?
>>>
>>> As it seems like this function frees the argument (why?) this also
>> needs
>>> to be specified in the comment as part of the semantics of the
>> function.
>> This function might need a string longer than original one to
>> accommodate
>> additional back slashes. So it has to xmalloc a new string. The original
>> string should be freed in such a case. However, it is tedious to caller
>> to figure out that conversion does happens and free the orig. The
>> solution
>> is for this function to free it when conversion happens.
>>
>> By doing so it is required that orig must be allocated and can be freed,
>> as the newly added comments described explicitly.
>>> It would be a good idea for you to give a more detailed explanation in
>>> the
>>> next version of the patch submission of how the path, before the patch,
>>> got processed so that the spaces were wrongly interpreted.  That might
>>> help make clearer whether the interface to this new function is
>> actually
>>> correct, since the subsequent operations on the return value should
>> act
>>> as
>>> an inverse to the operation carried out by this function.
>>>
>>> --
>>> Joseph S. Myers
>>> joseph@codesourcery.com
>> Index: gcc/gcc.c
>> ===================================================================
>> --- gcc/gcc.c	(revision 195189)
>> +++ gcc/gcc.c	(working copy)
>> @@ -265,6 +265,7 @@
>>  static const char *compare_debug_auxbase_opt_spec_function (int, const
>> char **);
>>  static const char *pass_through_libs_spec_func (int, const char **);
>>  static const char *replace_extension_spec_func (int, const char **);
>> +static char * convert_white_space (char *);
>>
>>
>>  /* The Specs Language
>>
>> @@ -6595,6 +6596,7 @@
>>  				    X_OK, false);
>>    if (lto_wrapper_file)
>>      {
>> +      lto_wrapper_file = convert_white_space (lto_wrapper_file);
>>        lto_wrapper_spec = lto_wrapper_file;
>>        obstack_init (&collect_obstack);
>>        obstack_grow (&collect_obstack, "COLLECT_LTO_WRAPPER=",
>> @@ -7005,12 +7007,13 @@
>>  			      + strlen (fuse_linker_plugin), 0))
>>  #endif
>>  	    {
>> -	      linker_plugin_file_spec = find_a_file (&exec_prefixes,
>> +	      char * temp_spec = find_a_file (&exec_prefixes,
>>  						     LTOPLUGINSONAME, R_OK,
>>  						     false);
>> -	      if (!linker_plugin_file_spec)
>> +	      if (!temp_spec)
>>  		fatal_error ("-fuse-linker-plugin, but %s not found",
>>  			     LTOPLUGINSONAME);
>> +	      linker_plugin_file_spec = convert_white_space (temp_spec);
>>  	    }
>>  #endif
>>  	  lto_gcc_spec = argv[0];
>> @@ -8506,3 +8509,52 @@
>>    free (name);
>>    return result;
>>  }
>> +
>> +/* Insert back slash before spaces in orig (usually a file path), to
>> +   avoid being broken by spec parser.
>> +
>> +   This function is needed as do_spec_1 treats white space (' ' and
>> '\t')
>> +   as the end of an argument. But in case of -plugin /usr/gcc
>> install/xxx.so,
>> +   the filename should be treated as a single argument rather than
>> being
>> +   broken into multiple. Solution is to insert '\\' before the space in
>> a
>> +   filename.
>> +
>> +   This function converts and only converts all occurrance of ' '
>> +   to '\\' + ' ' and '\t' to '\\' + '\t'.  For example:
>> +   "a b"  -> "a\\ b"
>> +   "a  b" -> "a\\ \\ b"
>> +   "a\tb" -> "a\\\tb"
>> +   "a\\ b" -> "a\\\\ b"
>> +
>> +   orig: input null-terminating string that was allocated by xalloc.
>> The
>> +   memory it points to might be freed in this function. Behavior
>> undefined
>> +   if orig isn't xalloced or is freed already at entry.
>> +
>> +   Return: orig if no conversion needed. orig if conversion needed but
>> no
>> +   sufficient memory for a new string. Otherwise a newly allocated
>> string
>> +   that was converted from orig.  */
>> +
>> +static char * convert_white_space (char *orig)
>> +{
>> +  int len, number_of_space = 0;
>> +  if (orig == NULL) return orig;
>> +
>> +  for (len=0; orig[len]; len++)
>> +    if (orig[len] == ' ' || orig[len] == '\t') number_of_space ++;
>> +
>> +  if (number_of_space)
>> +    {
>> +      char * new_spec = (char *)xmalloc (len + number_of_space + 1);
>> +      int j,k;
>> +      if (new_spec == NULL) return orig;
>> +
>> +      for (j=0, k=0; j<=len; j++, k++)
>> +	{
>> +	  if (orig[j] == ' ' || orig[j] == '\t') new_spec[k++] = '\\';
>> +	  new_spec[k] = orig[j];
>> +	}
>> +      free (orig);
>> +      return new_spec;
>> +  }
>> +  else return orig;
>> +}

RE: [PATCH] Fix PR50293 - LTO plugin with space in path

[Joey Ye]

[-- Attachment #1: Type: text/plain, Size: 6142 bytes --]

> -----Original Message-----
> From: Joseph Myers [mailto:joseph@codesourcery.com]
> Sent: Monday, March 04, 2013 00:49
> To: Joey Ye
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] Fix PR50293 - LTO plugin with space in path
> 
> On Mon, 18 Feb 2013, Joey Ye wrote:
> 
> > +static char * convert_white_space (char *);
> 
> No space after "*".
Fixed
> 
> > -	      linker_plugin_file_spec = find_a_file (&exec_prefixes,
> > +	      char * temp_spec = find_a_file (&exec_prefixes,
> >  						     LTOPLUGINSONAME, R_OK,
> >  						     false);
> 
> The indentation of the following lines looks odd after this patch;
> unless
> that's just an effect of TABs plus quoting, make sure they are
> reindented
> to line up with the new position of the opening '('.
They was unchanged. But since the line with '(' changed, it need changing
too.

Fixed
> 
> > +/* Insert back slash before spaces in orig (usually a file path), to
> 
> Capitalize variable names when referring to the value of the variable,
> so
> ORIG; likewise elsewhere in this comment.  Single work "backslash".
> 
> > +   the filename should be treated as a single argument rather than
> being
> 
> "file name" should be two words, according to the GNU Coding Standards.
> 
> > +   This function converts and only converts all occurrance of ' '
> 
> "occurrence"
> 
> > +   Return: orig if no conversion needed. orig if conversion needed
> but no
> > +   sufficient memory for a new string. Otherwise a newly allocated
> string
> 
> Returning wrong results on insufficient memory doesn't make sense.
> Anyway, xmalloc always exits the program if there is insufficient memory,
> so you don't need any code to allow for that case.
Fixed. Though it is conflicting with secure coding practice.
> 
> > +static char * convert_white_space (char *orig)
> 
> Newline, not space, between return type and function name, so that the
> function name comes at the start of the line.
Fixed.

> 
> > +  if (orig == NULL) return orig;
> 
> The comment didn't mention NULL as a valid argument, and it doesn't
> appear
> NULL can actually be passed to this function.  So don't include code to
> handle that case.
Fixed.

> 
> > +  for (len=0; orig[len]; len++)
> 
> Spaces around "=".
Fixed.

> 
> > +    if (orig[len] == ' ' || orig[len] == '\t') number_of_space ++;
> 
> No space before "++", but put the body of the "if" on a separate line.
Fixed.
> 
> > +      char * new_spec = (char *)xmalloc (len + number_of_space + 1);
> 
> No space after "*".  Space in the cast after "(char *)".
Fixed.
> 
> > +      int j,k;
> 
> Space after ",".
Fixed.
> 
> > +      if (new_spec == NULL) return orig;
> 
> As discussed above, not needed.
Removed.
> 
> > +      for (j=0, k=0; j<=len; j++, k++)
> 
> Spaces around "=" and "<=".
Fixed.
> 
> > +	  if (orig[j] == ' ' || orig[j] == '\t') new_spec[k++] = '\\';
> 
> Put the "if" both on a separate line.
Fixed.
> 
> > +  else return orig;
> 
> Put the "else" body on a separate line.
Fixed.
> 
> --
> Joseph S. Myers
> joseph@codesourcery.com

Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 195189)
+++ gcc/gcc.c	(working copy)
@@ -265,6 +265,7 @@
 static const char *compare_debug_auxbase_opt_spec_function (int, const char
**);
 static const char *pass_through_libs_spec_func (int, const char **);
 static const char *replace_extension_spec_func (int, const char **);
+static char *convert_white_space (char *);
 

 /* The Specs Language
 
@@ -6595,6 +6596,7 @@
 				    X_OK, false);
   if (lto_wrapper_file)
     {
+      lto_wrapper_file = convert_white_space (lto_wrapper_file);
       lto_wrapper_spec = lto_wrapper_file;
       obstack_init (&collect_obstack);
       obstack_grow (&collect_obstack, "COLLECT_LTO_WRAPPER=",
@@ -7005,12 +7007,13 @@
 			      + strlen (fuse_linker_plugin), 0))
 #endif
 	    {
-	      linker_plugin_file_spec = find_a_file (&exec_prefixes,
-						     LTOPLUGINSONAME, R_OK,
-						     false);
-	      if (!linker_plugin_file_spec)
+	      char *temp_spec = find_a_file (&exec_prefixes,
+					     LTOPLUGINSONAME, R_OK,
+					     false);
+	      if (!temp_spec)
 		fatal_error ("-fuse-linker-plugin, but %s not found",
 			     LTOPLUGINSONAME);
+	      linker_plugin_file_spec = convert_white_space (temp_spec);
 	    }
 #endif
 	  lto_gcc_spec = argv[0];
@@ -8506,3 +8509,51 @@
   free (name);
   return result;
 }
+
+/* Insert backslash before spaces in ORIG (usually a file path), to 
+   avoid being broken by spec parser.
+
+   This function is needed as do_spec_1 treats white space (' ' and '\t')
+   as the end of an argument. But in case of -plugin /usr/gcc
install/xxx.so,
+   the file name should be treated as a single argument rather than being
+   broken into multiple. Solution is to insert '\\' before the space in a 
+   file name.
+   
+   This function converts and only converts all occurrence of ' ' 
+   to '\\' + ' ' and '\t' to '\\' + '\t'.  For example:
+   "a b"  -> "a\\ b"
+   "a  b" -> "a\\ \\ b"
+   "a\tb" -> "a\\\tb"
+   "a\\ b" -> "a\\\\ b"
+
+   orig: input null-terminating string that was allocated by xalloc. The
+   memory it points to might be freed in this function. Behavior undefined
+   if ORIG wasn't xalloced or was freed already at entry.
+
+   Return: ORIG if no conversion needed. Otherwise a newly allocated string
+   that was converted from ORIG.  */
+
+static char *
+convert_white_space (char *orig)
+{
+  int len, number_of_space = 0;
+
+  for (len = 0; orig[len]; len++)
+    if (orig[len] == ' ' || orig[len] == '\t') number_of_space++;
+
+  if (number_of_space)
+    {
+      char *new_spec = (char *)xmalloc (len + number_of_space + 1);
+      int j, k;
+      for (j = 0, k = 0; j <= len; j++, k++)
+	{
+	  if (orig[j] == ' ' || orig[j] == '\t')
+	    new_spec[k++] = '\\';
+	  new_spec[k] = orig[j];
+	}
+      free (orig);
+      return new_spec;
+  }
+  else
+    return orig;
+}

[-- Attachment #2: space_in_plugin_spec-20130304.patch.txt --]
[-- Type: text/plain, Size: 2994 bytes --]

Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 195189)
+++ gcc/gcc.c	(working copy)
@@ -265,6 +265,7 @@
 static const char *compare_debug_auxbase_opt_spec_function (int, const char **);
 static const char *pass_through_libs_spec_func (int, const char **);
 static const char *replace_extension_spec_func (int, const char **);
+static char *convert_white_space (char *);
 \f
 /* The Specs Language
 
@@ -6595,6 +6596,7 @@
 				    X_OK, false);
   if (lto_wrapper_file)
     {
+      lto_wrapper_file = convert_white_space (lto_wrapper_file);
       lto_wrapper_spec = lto_wrapper_file;
       obstack_init (&collect_obstack);
       obstack_grow (&collect_obstack, "COLLECT_LTO_WRAPPER=",
@@ -7005,12 +7007,13 @@
 			      + strlen (fuse_linker_plugin), 0))
 #endif
 	    {
-	      linker_plugin_file_spec = find_a_file (&exec_prefixes,
-						     LTOPLUGINSONAME, R_OK,
-						     false);
-	      if (!linker_plugin_file_spec)
+	      char *temp_spec = find_a_file (&exec_prefixes,
+					     LTOPLUGINSONAME, R_OK,
+					     false);
+	      if (!temp_spec)
 		fatal_error ("-fuse-linker-plugin, but %s not found",
 			     LTOPLUGINSONAME);
+	      linker_plugin_file_spec = convert_white_space (temp_spec);
 	    }
 #endif
 	  lto_gcc_spec = argv[0];
@@ -8506,3 +8509,51 @@
   free (name);
   return result;
 }
+
+/* Insert backslash before spaces in ORIG (usually a file path), to 
+   avoid being broken by spec parser.
+
+   This function is needed as do_spec_1 treats white space (' ' and '\t')
+   as the end of an argument. But in case of -plugin /usr/gcc install/xxx.so,
+   the file name should be treated as a single argument rather than being
+   broken into multiple. Solution is to insert '\\' before the space in a 
+   file name.
+   
+   This function converts and only converts all occurrence of ' ' 
+   to '\\' + ' ' and '\t' to '\\' + '\t'.  For example:
+   "a b"  -> "a\\ b"
+   "a  b" -> "a\\ \\ b"
+   "a\tb" -> "a\\\tb"
+   "a\\ b" -> "a\\\\ b"
+
+   orig: input null-terminating string that was allocated by xalloc. The
+   memory it points to might be freed in this function. Behavior undefined
+   if ORIG wasn't xalloced or was freed already at entry.
+
+   Return: ORIG if no conversion needed. Otherwise a newly allocated string
+   that was converted from ORIG.  */
+
+static char *
+convert_white_space (char *orig)
+{
+  int len, number_of_space = 0;
+
+  for (len = 0; orig[len]; len++)
+    if (orig[len] == ' ' || orig[len] == '\t') number_of_space++;
+
+  if (number_of_space)
+    {
+      char *new_spec = (char *)xmalloc (len + number_of_space + 1);
+      int j, k;
+      for (j = 0, k = 0; j <= len; j++, k++)
+	{
+	  if (orig[j] == ' ' || orig[j] == '\t')
+	    new_spec[k++] = '\\';
+	  new_spec[k] = orig[j];
+	}
+      free (orig);
+      return new_spec;
+  }
+  else
+    return orig;
+}

RE: [PATCH] Fix PR50293 - LTO plugin with space in path

[Joey Ye]

> -----Original Message-----
> From: Georg-Johann Lay [mailto:gjl@gcc.gnu.org]
> Sent: Monday, March 04, 2013 02:42
> To: Joey Ye
> Cc: 'Joseph Myers'; gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Fix PR50293 - LTO plugin with space in path
> 
> Joey Ye schrieb:
> > Ping
> >
> >> Subject: RE: [PATCH] Fix PR50293 - LTO plugin with space in path
> 
> Does this patch also work with MS-Windows as host, i.e. with \ as path
> separator?
This patch itself doesn't handle '\'. As it should in comments:
"a\\ b" -> "a\\\\ b"

But I doubt \ is a problem, as Mingw or Cygwin changes it to / instead.

- Joey

RE: [PATCH] Fix PR50293 - LTO plugin with space in path

[Joseph S. Myers]

On Mon, 4 Mar 2013, Joey Ye wrote:

> +      char *new_spec = (char *)xmalloc (len + number_of_space + 1);

Space in cast between "(char *)" and "xmalloc".  OK with that change.

-- 
Joseph S. Myers
joseph@codesourcery.com