PATCH: PowerPC simulator doesn't handle spaces in filenames

PATCH: PowerPC simulator doesn't handle spaces in filenames

[Mark Mitchell]

The PowerPC simulator simulates bits of openprom functionality.  In
particular, it feeds the program file name into the openprom device
tree.  However, filenames containing a space or backslash (both of
which are very common on Windows, but can also occur on UNIX), were
not quoted.  (Here, quoting means enclosing the entire filename in
double-quotes, and protecting backslashes by double-quoting them.)

Tested by verifying that I could run a PowerPC program with a space
and/or backslash in its name, on both x86_64-unknown-linux-gnu and
i686-mingw32. 

OK?

--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

2006-01-23  Mark Mitchell  <mark@codesourcery.com>

	* emul_netbsd.c (emul_netbsd_create): Quote file-name property.
	* emul_unix.c (emul_unix_create): Likewise.
	* tree.c (libiberty.h): Include it.
	(tree_quote_property): New function.
	* tree.h (tree_quote_property): Declare.

Index: emul_netbsd.c
===================================================================
RCS file: /cvs/src/src/sim/ppc/emul_netbsd.c,v
retrieving revision 1.6
diff -c -5 -p -r1.6 emul_netbsd.c
*** emul_netbsd.c	28 Nov 2005 23:19:39 -0000	1.6
--- emul_netbsd.c	24 Jan 2006 05:50:57 -0000
*************** emul_netbsd_create(device *root,
*** 1381,1390 ****
--- 1381,1391 ----
    unsigned_word top_of_stack;
    unsigned stack_size;
    int elf_binary;
    os_emul_data *bsd_data;
    device *vm;
+   char *filename;
  
    /* check that this emulation is really for us */
    if (name != NULL && strcmp(name, "netbsd") != 0)
      return NULL;
    if (image == NULL)
*************** emul_netbsd_create(device *root,
*** 1415,1426 ****
    vm = tree_parse(root, "/openprom/vm");
    tree_parse(vm, "./stack-base 0x%lx",
  	     (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./nr-bytes 0x%x", stack_size);
  
    tree_parse(root, "/openprom/vm/map-binary/file-name %s",
! 	     bfd_get_filename(image));
  
    /* finish the init */
    tree_parse(root, "/openprom/init/register/pc 0x%lx",
  	     (unsigned long)bfd_get_start_address(image));
    tree_parse(root, "/openprom/init/register/sp 0x%lx",
--- 1416,1429 ----
    vm = tree_parse(root, "/openprom/vm");
    tree_parse(vm, "./stack-base 0x%lx",
  	     (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./nr-bytes 0x%x", stack_size);
  
+   filename = tree_quote_property (bfd_get_filename(image));
    tree_parse(root, "/openprom/vm/map-binary/file-name %s",
! 	     filename);
!   free (filename);
  
    /* finish the init */
    tree_parse(root, "/openprom/init/register/pc 0x%lx",
  	     (unsigned long)bfd_get_start_address(image));
    tree_parse(root, "/openprom/init/register/sp 0x%lx",
Index: emul_unix.c
===================================================================
RCS file: /cvs/src/src/sim/ppc/emul_unix.c,v
retrieving revision 1.2
diff -c -5 -p -r1.2 emul_unix.c
*** emul_unix.c	28 Nov 2005 23:19:39 -0000	1.2
--- emul_unix.c	24 Jan 2006 05:50:57 -0000
*************** emul_unix_create(device *root,
*** 949,958 ****
--- 949,959 ----
    unsigned_word top_of_stack;
    unsigned stack_size;
    int elf_binary;
    os_emul_data *data;
    device *vm;
+   char *filename;
  
    /* merge any emulation specific entries into the device tree */
  
    /* establish a few defaults */
    if (image->xvec->flavour == bfd_target_elf_flavour) {
*************** emul_unix_create(device *root,
*** 977,988 ****
  		  (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./stack-base 0x%lx",
  	     (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./nr-bytes 0x%x", stack_size);
  
    tree_parse(root, "/openprom/vm/map-binary/file-name %s",
! 	     bfd_get_filename(image));
  
    /* finish the init */
    tree_parse(root, "/openprom/init/register/pc 0x%lx",
  	     (unsigned long)bfd_get_start_address(image));
    tree_parse(root, "/openprom/init/register/sp 0x%lx",
--- 978,991 ----
  		  (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./stack-base 0x%lx",
  	     (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./nr-bytes 0x%x", stack_size);
  
+   filename = tree_quote_property (bfd_get_filename(image));
    tree_parse(root, "/openprom/vm/map-binary/file-name %s",
! 	     filename);
!   free (filename);
  
    /* finish the init */
    tree_parse(root, "/openprom/init/register/pc 0x%lx",
  	     (unsigned long)bfd_get_start_address(image));
    tree_parse(root, "/openprom/init/register/sp 0x%lx",
Index: tree.c
===================================================================
RCS file: /cvs/src/src/sim/ppc/tree.c,v
retrieving revision 1.4
diff -c -5 -p -r1.4 tree.c
*** tree.c	25 Mar 2005 20:40:02 -0000	1.4
--- tree.c	24 Jan 2006 05:50:59 -0000
***************
*** 43,52 ****
--- 43,53 ----
  #endif
  #endif
  
  #include <ctype.h>
  
+ #include "libiberty.h"
  
  /* manipulate/lookup device names */
  
  typedef struct _name_specifier {
    /* components in the full length name */
*************** parse_integer_property(device *current,
*** 623,632 ****
--- 624,677 ----
      device_add_array_property(current, property_name, words,
                                sizeof(words[0]) * nr_entries);
    }
  }
  
+ /* PROPERTY_VALUE is a raw property value.  Quote it as required by
+    parse_string_property.  It is the caller's responsibility to free
+    the memory returned.  */
+ 
+ EXTERN_TREE\
+ (char *)
+ tree_quote_property(const char *property_value)
+ {
+   char *p;
+   char *ret;
+   const char *chp;
+   int backslashes;
+ 
+   /* Count backslashes in PROPERTY_VALUE.  */
+   backslashes = 0;
+   for (chp = property_value; *chp; ++chp)
+     if (*chp == '\\')
+       ++backslashes;
+   
+   ret = (char *) xmalloc (strlen (property_value) 
+ 			  + 2 /* quotes */
+ 			  + backslashes
+ 			  + 1 /* terminator */);
+ 
+   p = ret;
+   /* Add the opening quote.  */
+   *p++ = '"';
+   /* Copy the value.  */
+   for (chp = property_value; *chp; ++chp)
+     if (*chp == '\\')
+       {
+ 	/* Double backslashes.  */
+ 	*p++ = '\\';
+ 	*p++ = '\\';
+       }
+     else
+       *p++ = *chp;
+   /* Add the closing quote.  */
+   *p++ = '"';
+   /* Terminate the string.  */
+   *p++ = '\0';
+ 
+   return ret;
+ }
  
  /* <string> ... */
  
  STATIC_INLINE_TREE\
  (void)
Index: tree.h
===================================================================
RCS file: /cvs/src/src/sim/ppc/tree.h,v
retrieving revision 1.1.1.1
diff -c -5 -p -r1.1.1.1 tree.h
*** tree.h	16 Apr 1999 01:35:12 -0000	1.1.1.1
--- tree.h	24 Jan 2006 05:50:59 -0000
***************
*** 40,49 ****
--- 40,53 ----
     initialized they will be restored to their initial value.
  
     */
  
  EXTERN_TREE\
+ (char*) tree_quote_property
+ (const char *property_value);
+ 
+ EXTERN_TREE\
  (device *) tree_parse
  (device *root,
   const char *fmt,
   ...) __attribute__ ((format (printf, 2, 3)));
  

Re: PATCH: PowerPC simulator doesn't handle spaces in filenames

[Daniel Jacobowitz]

On Mon, Jan 23, 2006 at 09:59:43PM -0800, Mark Mitchell wrote:
> 
> The PowerPC simulator simulates bits of openprom functionality.  In
> particular, it feeds the program file name into the openprom device
> tree.  However, filenames containing a space or backslash (both of
> which are very common on Windows, but can also occur on UNIX), were
> not quoted.  (Here, quoting means enclosing the entire filename in
> double-quotes, and protecting backslashes by double-quoting them.)
> 
> Tested by verifying that I could run a PowerPC program with a space
> and/or backslash in its name, on both x86_64-unknown-linux-gnu and
> i686-mingw32. 
> 
> OK?

You've escaped backslashes, but what about double quotes?  I assume
this eventually goes to parse_string_property.  It's not immediately
obvious, but the unescaping loop there will unescape both backslashes
and double quotes (see print_string).

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PowerPC simulator doesn't handle spaces in filenames

[Mark Mitchell]

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

Daniel Jacobowitz wrote:

> You've escaped backslashes, but what about double quotes?  I assume
> this eventually goes to parse_string_property.  It's not immediately
> obvious, but the unescaping loop there will unescape both backslashes
> and double quotes (see print_string).

Thanks for the quick review; good catch.

Updated thusly, which, I confirmed, works for a filename containing both
a space and a double-quote on x86_64-unknown-linux-gnu.

OK to apply?

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

[-- Attachment #2: gdb.sim.patch --]
[-- Type: text/plain, Size: 6286 bytes --]

2006-01-24  Mark Mitchell  <mark@codesourcery.com>

	* emul_netbsd.c (emul_netbsd_create): Quote file-name property.
	* emul_unix.c (emul_unix_create): Likewise.
	* tree.c (libiberty.h): Include it.
	(tree_quote_property): New function.
	* tree.h (tree_quote_property): Declare.

Index: emul_netbsd.c
===================================================================
RCS file: /cvs/src/src/sim/ppc/emul_netbsd.c,v
retrieving revision 1.6
diff -c -5 -p -r1.6 emul_netbsd.c
*** emul_netbsd.c	28 Nov 2005 23:19:39 -0000	1.6
--- emul_netbsd.c	25 Jan 2006 06:19:47 -0000
*************** emul_netbsd_create(device *root,
*** 1381,1390 ****
--- 1381,1391 ----
    unsigned_word top_of_stack;
    unsigned stack_size;
    int elf_binary;
    os_emul_data *bsd_data;
    device *vm;
+   char *filename;
  
    /* check that this emulation is really for us */
    if (name != NULL && strcmp(name, "netbsd") != 0)
      return NULL;
    if (image == NULL)
*************** emul_netbsd_create(device *root,
*** 1415,1426 ****
    vm = tree_parse(root, "/openprom/vm");
    tree_parse(vm, "./stack-base 0x%lx",
  	     (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./nr-bytes 0x%x", stack_size);
  
    tree_parse(root, "/openprom/vm/map-binary/file-name %s",
! 	     bfd_get_filename(image));
  
    /* finish the init */
    tree_parse(root, "/openprom/init/register/pc 0x%lx",
  	     (unsigned long)bfd_get_start_address(image));
    tree_parse(root, "/openprom/init/register/sp 0x%lx",
--- 1416,1429 ----
    vm = tree_parse(root, "/openprom/vm");
    tree_parse(vm, "./stack-base 0x%lx",
  	     (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./nr-bytes 0x%x", stack_size);
  
+   filename = tree_quote_property (bfd_get_filename(image));
    tree_parse(root, "/openprom/vm/map-binary/file-name %s",
! 	     filename);
!   free (filename);
  
    /* finish the init */
    tree_parse(root, "/openprom/init/register/pc 0x%lx",
  	     (unsigned long)bfd_get_start_address(image));
    tree_parse(root, "/openprom/init/register/sp 0x%lx",
Index: emul_unix.c
===================================================================
RCS file: /cvs/src/src/sim/ppc/emul_unix.c,v
retrieving revision 1.2
diff -c -5 -p -r1.2 emul_unix.c
*** emul_unix.c	28 Nov 2005 23:19:39 -0000	1.2
--- emul_unix.c	25 Jan 2006 06:19:47 -0000
*************** emul_unix_create(device *root,
*** 949,958 ****
--- 949,959 ----
    unsigned_word top_of_stack;
    unsigned stack_size;
    int elf_binary;
    os_emul_data *data;
    device *vm;
+   char *filename;
  
    /* merge any emulation specific entries into the device tree */
  
    /* establish a few defaults */
    if (image->xvec->flavour == bfd_target_elf_flavour) {
*************** emul_unix_create(device *root,
*** 977,988 ****
  		  (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./stack-base 0x%lx",
  	     (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./nr-bytes 0x%x", stack_size);
  
    tree_parse(root, "/openprom/vm/map-binary/file-name %s",
! 	     bfd_get_filename(image));
  
    /* finish the init */
    tree_parse(root, "/openprom/init/register/pc 0x%lx",
  	     (unsigned long)bfd_get_start_address(image));
    tree_parse(root, "/openprom/init/register/sp 0x%lx",
--- 978,991 ----
  		  (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./stack-base 0x%lx",
  	     (unsigned long)(top_of_stack - stack_size));
    tree_parse(vm, "./nr-bytes 0x%x", stack_size);
  
+   filename = tree_quote_property (bfd_get_filename(image));
    tree_parse(root, "/openprom/vm/map-binary/file-name %s",
! 	     filename);
!   free (filename);
  
    /* finish the init */
    tree_parse(root, "/openprom/init/register/pc 0x%lx",
  	     (unsigned long)bfd_get_start_address(image));
    tree_parse(root, "/openprom/init/register/sp 0x%lx",
Index: tree.c
===================================================================
RCS file: /cvs/src/src/sim/ppc/tree.c,v
retrieving revision 1.4
diff -c -5 -p -r1.4 tree.c
*** tree.c	25 Mar 2005 20:40:02 -0000	1.4
--- tree.c	25 Jan 2006 06:19:49 -0000
***************
*** 43,52 ****
--- 43,53 ----
  #endif
  #endif
  
  #include <ctype.h>
  
+ #include "libiberty.h"
  
  /* manipulate/lookup device names */
  
  typedef struct _name_specifier {
    /* components in the full length name */
*************** parse_integer_property(device *current,
*** 623,632 ****
--- 624,677 ----
      device_add_array_property(current, property_name, words,
                                sizeof(words[0]) * nr_entries);
    }
  }
  
+ /* PROPERTY_VALUE is a raw property value.  Quote it as required by
+    parse_string_property.  It is the caller's responsibility to free
+    the memory returned.  */
+ 
+ EXTERN_TREE\
+ (char *)
+ tree_quote_property(const char *property_value)
+ {
+   char *p;
+   char *ret;
+   const char *chp;
+   int quotees;
+ 
+   /* Count characters need quotes in PROPERTY_VALUE.  */
+   quotees = 0;
+   for (chp = property_value; *chp; ++chp)
+     if (*chp == '\\' || *chp == '"')
+       ++quotees;
+   
+   ret = (char *) xmalloc (strlen (property_value) 
+ 			  + 2 /* quotes */
+ 			  + quotees
+ 			  + 1 /* terminator */);
+ 
+   p = ret;
+   /* Add the opening quote.  */
+   *p++ = '"';
+   /* Copy the value.  */
+   for (chp = property_value; *chp; ++chp)
+     if (*chp == '\\' || *chp == '"')
+       {
+ 	/* Quote this character.  */ 
+ 	*p++ = '\\';
+ 	*p++ = *chp;
+       }
+     else
+       *p++ = *chp;
+   /* Add the closing quote.  */
+   *p++ = '"';
+   /* Terminate the string.  */
+   *p++ = '\0';
+ 
+   return ret;
+ }
  
  /* <string> ... */
  
  STATIC_INLINE_TREE\
  (void)
Index: tree.h
===================================================================
RCS file: /cvs/src/src/sim/ppc/tree.h,v
retrieving revision 1.1.1.1
diff -c -5 -p -r1.1.1.1 tree.h
*** tree.h	16 Apr 1999 01:35:12 -0000	1.1.1.1
--- tree.h	25 Jan 2006 06:19:49 -0000
***************
*** 40,49 ****
--- 40,53 ----
     initialized they will be restored to their initial value.
  
     */
  
  EXTERN_TREE\
+ (char*) tree_quote_property
+ (const char *property_value);
+ 
+ EXTERN_TREE\
  (device *) tree_parse
  (device *root,
   const char *fmt,
   ...) __attribute__ ((format (printf, 2, 3)));
  

Re: PATCH: PowerPC simulator doesn't handle spaces in filenames

[Daniel Jacobowitz]

On Tue, Jan 24, 2006 at 10:23:16PM -0800, Mark Mitchell wrote:
> 2006-01-24  Mark Mitchell  <mark@codesourcery.com>
> 
> 	* emul_netbsd.c (emul_netbsd_create): Quote file-name property.
> 	* emul_unix.c (emul_unix_create): Likewise.
> 	* tree.c (libiberty.h): Include it.
> 	(tree_quote_property): New function.
> 	* tree.h (tree_quote_property): Declare.

Yes, this is fine.  Thanks.

-- 
Daniel Jacobowitz
CodeSourcery

Re: PATCH: PowerPC simulator doesn't handle spaces in filenames

[Mark Mitchell]

Daniel Jacobowitz wrote:
> On Tue, Jan 24, 2006 at 10:23:16PM -0800, Mark Mitchell wrote:
> 
>>2006-01-24  Mark Mitchell  <mark@codesourcery.com>
>>
>>	* emul_netbsd.c (emul_netbsd_create): Quote file-name property.
>>	* emul_unix.c (emul_unix_create): Likewise.
>>	* tree.c (libiberty.h): Include it.
>>	(tree_quote_property): New function.
>>	* tree.h (tree_quote_property): Declare.
> 
> 
> Yes, this is fine.  Thanks.

Committed, thanks!

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713