Re: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

Re: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[asmwarrior]

On 2011-8-12 23:50, Pierre Muller wrote:
>    Using current CVS source,
> I am unable to handle files having Dos style
> directory specifications inside the stabs debugging information
> (but I don't think that this is stabs specific).
> Release 7.3 has the same problem...
>
>    The program test.exe below has been compiled with Free Pascal
> for win32 target (more or less mingw).
>    When I try to insert a break point at a line of current file,
> the addr_string computed is "e:/pas/trunk/fpcsrc/ide/test.pas:166".
>    But locate_first_half function stops at the first colon
> and GDB complains because file "e" is not found.
>
>    I first tried to add double-quotes around the file name,
> but this was not enough... I suspect that the other changes
> below that I had to add are just errors in the current implementation...
>    See below for submitted patch.
>
>    Nevertheless, this implementation will probably fail miserably for
> file names containing double-quotes, not sure if this is allowed
> on some OS's or FileSystems...
>
>    Comments most welcome
> Pierre Muller
> GDB pascal language maintainer
>
>
>>>>> GDB session illustrating the problem
>
> E:\pas\trunk\fpcsrc\ide>gdbcvsmult test
> GNU gdb (GDB) 7.3.50.20110805-cvs
> Copyright (C) 2011 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later
> <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "i686-pc-mingw32".
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>...
> Reading symbols from E:\pas\trunk\fpcsrc\ide/test.exe...done.
> (gdb) li main
> 157     end;
> 158
> 159     var i,j : longint;
> 160         Length : longint;
> 161
> 162     BEGIN
> 163     {$ifdef m68k}
> 164       asm
> 165         beq @L13
> 166         bhi @L13
> (gdb) b 166
> Breakpoint 1 at 0x40170d: file e:/pas/trunk/fpcsrc/ide/test.pas, line 166.
> (gdb) r
> Starting program: E:\pas\trunk\fpcsrc\ide/test.exe
> [New Thread 1116.0xe38]
> Error in re-setting breakpoint 1: No source file named e.
> Obj1.Z=1
> Hello world!
> ParamCount = 0
> Paramstr(0) = E:\pas\trunk\fpcsrc\ide\test.exe
> FALSE
> dummy for browser
> dummy for browser
> 0
> [Inferior 1 (process 1116) exited with code 04]
> (gdb)
>
>
>
>
>
>
> 2011-08-12  Pierre Muller<muller@ics.u-strasbg.fr>
>
> 	* linespec.c (build_canonical_line_spec): Handle Dos style filenames
> 	by adding double-quotes around the file name.
> 	(locate_first_half): Exit at the end of double-quoted part.
> 	Never stop inside double-quoted part.
> 	(symtab_from_filename): Move past the ending double-quote if
> 	IS_QUOTE_ENCLOSED is set.
>
>
> Index: src/gdb/linespec.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/linespec.c,v
> retrieving revision 1.128
> diff -u -p -r1.128 linespec.c
> --- src/gdb/linespec.c	5 Jul 2011 20:30:19 -0000	1.128
> +++ src/gdb/linespec.c	12 Aug 2011 15:30:43 -0000
> @@ -455,7 +455,11 @@ build_canonical_line_spec (struct symtab
>     else
>       {
>         canonical_name = xmalloc (strlen (filename) + 30);
> -      sprintf (canonical_name, "%s:%d", filename, sal->line);
> +      /* Quote filenames containing ':' characters to avoid problems.  */
> +      if (strchr (filename, ':') != NULL&&  filename[0] != '"')
> +	sprintf (canonical_name, "\"%s\":%d", filename, sal->line);
> +      else
> +	sprintf (canonical_name, "%s:%d", filename, sal->line);
>       }
>     canonical_arr[0] = canonical_name;
>   }
> @@ -1214,12 +1218,16 @@ locate_first_half (char **argptr, int *i
>   	{
>   	  break;
>   	}
> +      /* Consider a double quote to be the end of the first half.  */
> +      if (*is_quote_enclosed&&  p[0] == '"')
> +	return p;
> +
>         /* Check for the end of the first half of the linespec.  End of
>            line, a tab, a colon or a space.  But if enclosed in double
> -	 quotes we do not break on enclosed spaces.  */
> +	 quotes we do not break on enclosed spaces, nor on colons.  */
>         if (!*p
>   	  || p[0] == '\t'
> -	  || (p[0] == ':')
> +	  || ((p[0] == ':')&&  !*is_quote_enclosed)
>   	  || ((p[0] == ' ')&&  !*is_quote_enclosed))
>   	break;
>         if (p[0] == '.'&&  strchr (p, ':') == NULL)
> @@ -1796,12 +1804,10 @@ symtab_from_filename (char **argptr, cha
>     char *p1;
>     char *copy;
>     struct symtab *file_symtab;
> -
> +
>     p1 = p;
>     while (p != *argptr&&  p[-1] == ' ')
>       --p;
> -  if ((*p == '"')&&  is_quote_enclosed)
> -    --p;
>     copy = (char *) alloca (p - *argptr + 1);
>     memcpy (copy, *argptr, p - *argptr);
>     /* It may have the ending quote right after the file name.  */
> @@ -1822,8 +1828,11 @@ symtab_from_filename (char **argptr, cha
>         throw_error (NOT_FOUND_ERROR, _("No source file named %s."), copy);
>       }
>
> +  if (p1[0] == '"'&&  is_quote_enclosed)
> +    p1++;
>     /* Discard the file name from the arg.  */
>     p = p1 + 1;
> +
>     while (*p == ' ' || *p == '\t')
>       p++;
>     *argptr = p;
>
>
It is exact the problem I reported here:
http://sourceware.org/bugzilla/show_bug.cgi?id=12843

As Keith said, I can solve this problem only by revert this change:
------------------------------
if (!*p
|| p[0] == '\t'
- || ((p[0] == ':')
- && ((p[1] == ':') || (strchr (p + 1, ':') == NULL)))
+ || (p[0] == ':')
|| ((p[0] == ' ') && !*is_quote_enclosed))
------------------------------

Then, everything works fine.

@Pierre Muller:
I think adding quotes internally is not quite good. That's from my 
personal point.

there is another issue when locating the absolute the source path if the 
app is build with relative paths. I have a patch to solve them in that 
following discussion.

http://sourceware.org/ml/gdb/2011-06/msg00102.html
a simplified patch used on mingw official 7.3 release can be found here:
http://article.gmane.org/gmane.comp.gnu.mingw.user/37199


asmwarrior
ollydbg from codeblocks' forum

Re: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[Eli Zaretskii]

> From: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
> Date: Fri, 12 Aug 2011 17:50:11 +0200
> 
>   Using current CVS source, 
> I am unable to handle files having Dos style
> directory specifications inside the stabs debugging information
> (but I don't think that this is stabs specific).
> Release 7.3 has the same problem...
> 
>   The program test.exe below has been compiled with Free Pascal
> for win32 target (more or less mingw).
>   When I try to insert a break point at a line of current file,
> the addr_string computed is "e:/pas/trunk/fpcsrc/ide/test.pas:166".
>   But locate_first_half function stops at the first colon
> and GDB complains because file "e" is not found.
> 
>   I first tried to add double-quotes around the file name,
> but this was not enough... I suspect that the other changes
> below that I had to add are just errors in the current implementation...
>   See below for submitted patch.
> 
>   Nevertheless, this implementation will probably fail miserably for
> file names containing double-quotes, not sure if this is allowed
> on some OS's or FileSystems...
> 
>   Comments most welcome

Thanks for working on this, but I don't like the idea of quoting file
names internally just to work around this problem.  I think we need to
teach linespec about DOS-style file names instead.

Re: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[Tom Tromey]

Eli> Thanks for working on this, but I don't like the idea of quoting file
Eli> names internally just to work around this problem.

Why?

Tom

Re: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: Pierre Muller <pierre.muller@ics-cnrs.unistra.fr>,
>         gdb-patches@sourceware.org
> Date: Fri, 12 Aug 2011 11:24:56 -0600
> 
> Eli> Thanks for working on this, but I don't like the idea of quoting file
> Eli> names internally just to work around this problem.
> 
> Why?

It's ugly and fragile, because the original name could be quoted.

RE: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Eli Zaretskii
> Envoyé : vendredi 12 août 2011 19:47
> À : Tom Tromey
> Cc : pierre.muller@ics-cnrs.unistra.fr; gdb-patches@sourceware.org
> Objet : Re: [RFC] Fix problems related to Mingw/DJGPP file names
containing
> colons
> 
> > From: Tom Tromey <tromey@redhat.com>
> > Cc: Pierre Muller <pierre.muller@ics-cnrs.unistra.fr>,
> >         gdb-patches@sourceware.org
> > Date: Fri, 12 Aug 2011 11:24:56 -0600
> >
> > Eli> Thanks for working on this, but I don't like the idea of quoting
file
> > Eli> names internally just to work around this problem.
> >
> > Why?
> 
> It's ugly and fragile, because the original name could be quoted.

+      /* Quote filenames containing ':' characters to avoid problems.  */
+      if (strchr (filename, ':') != NULL && filename[0] != '"')
+	sprintf (canonical_name, "\"%s\":%d", filename, sal->line);

  The second condition already takes care of not
quoting already quoted file names.

Regarding the reverting of 
As Keith said, I can solve this problem only by revert this change:
------------------------------
if (!*p
|| p[0] == '\t'
- || ((p[0] == ':')
- && ((p[1] == ':') || (strchr (p + 1, ':') == NULL)))
+ || (p[0] == ':')


This probably creates problems with some C++ 
cases that use '::' patterns, no?

  Pierre

Re: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[Eli Zaretskii]

> From: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
> Cc: <gdb-patches@sourceware.org>
> Date: Sat, 13 Aug 2011 12:10:53 +0200
> 
> > It's ugly and fragile, because the original name could be quoted.
> 
> +      /* Quote filenames containing ':' characters to avoid problems.  */
> +      if (strchr (filename, ':') != NULL && filename[0] != '"')
> +	sprintf (canonical_name, "\"%s\":%d", filename, sal->line);
> 
>   The second condition already takes care of not
> quoting already quoted file names.

What if the original name already includes quote characters?  That can
happen on Posix platforms.

RE: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Eli Zaretskii
> Envoyé : samedi 13 août 2011 12:51
> À : Pierre Muller
> Cc : tromey@redhat.com; gdb-patches@sourceware.org
> Objet : Re: [RFC] Fix problems related to Mingw/DJGPP file names
containing
> colons
> 
> > From: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
> > Cc: <gdb-patches@sourceware.org>
> > Date: Sat, 13 Aug 2011 12:10:53 +0200
> >
> > > It's ugly and fragile, because the original name could be quoted.
> >
> > +      /* Quote filenames containing ':' characters to avoid problems.
*/
> > +      if (strchr (filename, ':') != NULL && filename[0] != '"')
> > +	sprintf (canonical_name, "\"%s\":%d", filename, sal->line);
> >
> >   The second condition already takes care of not
> > quoting already quoted file names.
> 
> What if the original name already includes quote characters?  That can
> happen on Posix platforms.
  You are right, but we are saved by the fact
that colons are not allow on those platforms, isn't it?
(otherwise all lists of directories like in the PATH
environment variable would have a problem...)

  But there might be some 'exotic' file systems that allow
both double-quotes and colons as valid characters in their
filenames. In those cases, we would be in trouble...

  The main reason for now is that the code is acting directly on 
addr_string pchar and should not modify it, otherwise,
we could simply call a more complete quoting function that
would escape internal colons (but of course we should then also
look for and handle those escaped colons...)

  I am wondering how spaces within filenames are handled 
inside GDB on mingw32...
  I just tested... No very well :(

  If you have "test space with name.exe" and "test.exe"
executables within the same directory,
GDB seems to load the correct file but runs the false one because
no quoting is added to CreateProcess call....


Pierre

Re: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[DJ Delorie]

>   But there might be some 'exotic' file systems that allow
> both double-quotes and colons as valid characters in their
> filenames.

Any unix-like OS supports that:

$ touch 'foo"bar:grill'
-rw-r--r-- 1 dj games       0 Aug 13 16:46 foo"bar:grill

The only two characters you *can't* put in a file name or subdirectory
name are the NUL character and possibly '/' (I've seen ways to make
filenames with / in them).  Every other character is 100% legal.

>   I am wondering how spaces within filenames are handled 
> inside GDB on mingw32...
>   I just tested... No very well :(

Any unix-like OS also supports spaces in filenames!

$ touch 'foo bar'
-rw-r--r-- 1 dj games       0 Aug 13 16:47 foo bar

These are not new problems, nor are they dos/windows specific.

Re: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[Mark Kettenis]

> From: "Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>
> Date: Sat, 13 Aug 2011 22:42:16 +0200
> 
> > What if the original name already includes quote characters?  That can
> > happen on Posix platforms.
>   You are right, but we are saved by the fact
> that colons are not allow on those platforms, isn't it?
> (otherwise all lists of directories like in the PATH
> environment variable would have a problem...)

borodin$ touch :
borodin$ ls -l :
-rw-r--r--  1 kettenis  wheel  0 Aug 13 23:16 :

Of course sane people wouldn't use it for exacty the reson you cite
above.

>   But there might be some 'exotic' file systems that allow
> both double-quotes and colons as valid characters in their
> filenames. In those cases, we would be in trouble...

borodin$ touch \"
borodin$ ls -l \"
-rw-r--r--  1 kettenis  wheel  0 Aug 13 23:20 "

So it's not that exotic.  Note however that I need to escape the
double-quote to prevent the shell from interpreting it as a quote.

Perhaps that's the solution here?  If you use "quotation" as well as
"escaping" you should be able to express anything you want on a
command line.

Re: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[Tom Tromey]

>>>>> "Mark" == Mark Kettenis <mark.kettenis@xs4all.nl> writes:

Mark> Perhaps that's the solution here?  If you use "quotation" as well as
Mark> "escaping" you should be able to express anything you want on a
Mark> command line.

One funny thing about linespecs is that the quoting syntax isn't
actually documented -- at least, I couldn't find it in the manual.

I tend to think this gives us some freedom to change it a little, on the
theory that probably not many people are already using both quotes and
backslashes.

That theory may run aground on the unfortunate fact that MI's
-break-insert exposes linespecs to the MI clients.  I wonder what they
do for Windows-style paths right now.


I have been contemplating changing linespec to pre-tokenize.  Right now
if you read linespec.c, code to pull out the next relevant token is
spread out all over the file.  I think it would simplify and strengthen
the implementation if this were done in a single pass up-front.

Tom

Re: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>   The main reason for now is that the code is acting directly on 
Pierre> addr_string pchar and should not modify it, otherwise,
Pierre> we could simply call a more complete quoting function that
Pierre> would escape internal colons (but of course we should then also
Pierre> look for and handle those escaped colons...)

If you, or anybody, is planning a major overhaul of linespec, I'd
appreciate it if you could coordinate with me first; my "ambiguous
linespec" patch touches linespec heavily and I would prefer not to have
to redo it all.

The simplest way to coordinate would be to pull from
archer-tromey-ambiguous-linespec.

Tom

[RFC] Fix problems related to Mingw/DJGPP file names containing colons

[Pierre Muller]

  Using current CVS source, 
I am unable to handle files having Dos style
directory specifications inside the stabs debugging information
(but I don't think that this is stabs specific).
Release 7.3 has the same problem...

  The program test.exe below has been compiled with Free Pascal
for win32 target (more or less mingw).
  When I try to insert a break point at a line of current file,
the addr_string computed is "e:/pas/trunk/fpcsrc/ide/test.pas:166".
  But locate_first_half function stops at the first colon
and GDB complains because file "e" is not found.

  I first tried to add double-quotes around the file name,
but this was not enough... I suspect that the other changes
below that I had to add are just errors in the current implementation...
  See below for submitted patch.

  Nevertheless, this implementation will probably fail miserably for
file names containing double-quotes, not sure if this is allowed
on some OS's or FileSystems...

  Comments most welcome
Pierre Muller
GDB pascal language maintainer


>>>> GDB session illustrating the problem

E:\pas\trunk\fpcsrc\ide>gdbcvsmult test
GNU gdb (GDB) 7.3.50.20110805-cvs
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-pc-mingw32".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from E:\pas\trunk\fpcsrc\ide/test.exe...done.
(gdb) li main
157     end;
158
159     var i,j : longint;
160         Length : longint;
161
162     BEGIN
163     {$ifdef m68k}
164       asm
165         beq @L13
166         bhi @L13
(gdb) b 166
Breakpoint 1 at 0x40170d: file e:/pas/trunk/fpcsrc/ide/test.pas, line 166.
(gdb) r
Starting program: E:\pas\trunk\fpcsrc\ide/test.exe
[New Thread 1116.0xe38]
Error in re-setting breakpoint 1: No source file named e.
Obj1.Z=1
Hello world!
ParamCount = 0
Paramstr(0) = E:\pas\trunk\fpcsrc\ide\test.exe
FALSE
dummy for browser
dummy for browser
0
[Inferior 1 (process 1116) exited with code 04]
(gdb)






2011-08-12  Pierre Muller  <muller@ics.u-strasbg.fr>

	* linespec.c (build_canonical_line_spec): Handle Dos style filenames
	by adding double-quotes around the file name.
	(locate_first_half): Exit at the end of double-quoted part.
	Never stop inside double-quoted part.
	(symtab_from_filename): Move past the ending double-quote if
	IS_QUOTE_ENCLOSED is set.


Index: src/gdb/linespec.c
===================================================================
RCS file: /cvs/src/src/gdb/linespec.c,v
retrieving revision 1.128
diff -u -p -r1.128 linespec.c
--- src/gdb/linespec.c	5 Jul 2011 20:30:19 -0000	1.128
+++ src/gdb/linespec.c	12 Aug 2011 15:30:43 -0000
@@ -455,7 +455,11 @@ build_canonical_line_spec (struct symtab
   else
     {
       canonical_name = xmalloc (strlen (filename) + 30);
-      sprintf (canonical_name, "%s:%d", filename, sal->line);
+      /* Quote filenames containing ':' characters to avoid problems.  */
+      if (strchr (filename, ':') != NULL && filename[0] != '"')
+	sprintf (canonical_name, "\"%s\":%d", filename, sal->line);
+      else
+	sprintf (canonical_name, "%s:%d", filename, sal->line);
     }
   canonical_arr[0] = canonical_name;
 }
@@ -1214,12 +1218,16 @@ locate_first_half (char **argptr, int *i
 	{
 	  break;
 	}
+      /* Consider a double quote to be the end of the first half.  */
+      if (*is_quote_enclosed && p[0] == '"')
+	return p;
+
       /* Check for the end of the first half of the linespec.  End of
          line, a tab, a colon or a space.  But if enclosed in double
-	 quotes we do not break on enclosed spaces.  */
+	 quotes we do not break on enclosed spaces, nor on colons.  */
       if (!*p
 	  || p[0] == '\t'
-	  || (p[0] == ':')
+	  || ((p[0] == ':') && !*is_quote_enclosed)
 	  || ((p[0] == ' ') && !*is_quote_enclosed))
 	break;
       if (p[0] == '.' && strchr (p, ':') == NULL)
@@ -1796,12 +1804,10 @@ symtab_from_filename (char **argptr, cha
   char *p1;
   char *copy;
   struct symtab *file_symtab;
-  
+
   p1 = p;
   while (p != *argptr && p[-1] == ' ')
     --p;
-  if ((*p == '"') && is_quote_enclosed)
-    --p;
   copy = (char *) alloca (p - *argptr + 1);
   memcpy (copy, *argptr, p - *argptr);
   /* It may have the ending quote right after the file name.  */
@@ -1822,8 +1828,11 @@ symtab_from_filename (char **argptr, cha
       throw_error (NOT_FOUND_ERROR, _("No source file named %s."), copy);
     }
 
+  if (p1[0] == '"' && is_quote_enclosed)
+    p1++;
   /* Discard the file name from the arg.  */
   p = p1 + 1;
+  
   while (*p == ' ' || *p == '\t')
     p++;
   *argptr = p;

Re: [RFC] Fix problems related to Mingw/DJGPP file names containing colons

[Keith Seitz]

On 08/12/2011 08:50 AM, Pierre Muller wrote:
>    I first tried to add double-quotes around the file name,
> but this was not enough... I suspect that the other changes
> below that I had to add are just errors in the current implementation...
>    See below for submitted patch.

I remember this bug. I believe I introduced this with one of my many 
hackings on linespec.c. This is gdb/12843, about which I'd nearly forgotten.

I was going to work on this patch after 12266, but that bug has lasted 
much longer than I expected.

Unfortunately, your patch causes several test suite regressions for me 
(as it is right now):

+FAIL: gdb.base/break.exp: breakpoint quoted function (got interactive 
prompt)
+FAIL: gdb.base/break.exp: breakpoint info
+FAIL: gdb.base/break.exp: info break 2 4 6
+FAIL: gdb.base/break.exp: info break 3-5
+FAIL: gdb.base/break.exp: check disable with history values
+FAIL: gdb.base/break.exp: check disable with convenience values
+FAIL: gdb.base/break.exp: run until quoted breakpoint
+FAIL: gdb.base/break.exp: run until file:linenum breakpoint (the 
program exited)
+FAIL: gdb.base/break.exp: step onto breakpoint (the program is no 
longer running)
+FAIL: gdb.base/break.exp: continue to breakpoint at } (the program is 
no longer running)
+FAIL: gdb.base/sepdebug.exp: breakpoint quoted function (got 
interactive prompt)
+FAIL: gdb.base/sepdebug.exp: breakpoint info
+FAIL: gdb.base/sepdebug.exp: run until quoted breakpoint
+FAIL: gdb.base/sepdebug.exp: run until file:linenum breakpoint (the 
program exited)
+FAIL: gdb.base/sepdebug.exp: step onto breakpoint (the program is no 
+FAIL: gdb.cp/cmpd-minsyms.exp: setting breakpoint at "int 
GDB<char>::even_harder<int>(char)"
+FAIL: gdb.cp/overload.exp: list overloaded function with function ptr 
args - quotes around argument

If it's okay with you, I'd like to take this bug myself. I will drop 
everything and work on it immediately.

Keith