This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: proposal: substitute-path handles foreign dir separators
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org, Eli Zaretskii <eliz at gnu dot org>
- Cc: Raphael Zulliger <zulliger at indel dot ch>
- Date: Fri, 17 Dec 2010 17:50:11 +0000
- Subject: Re: proposal: substitute-path handles foreign dir separators
- References: <4D0B7125.2010203@indel.ch> <834oach14b.fsf@gnu.org>
On Friday 17 December 2010 15:26:12, Eli Zaretskii wrote:
> > Date: Fri, 17 Dec 2010 15:18:13 +0100
> > From: Raphael Zulliger <zulliger@indel.ch>
> >
> > 1. Most importantly: Do we want such a 'feature'? Personally, I think
> > it makes sense to have such a feature.
>
> There were discussions on this a few months ago, you may wish to look
> them up.
>
> FWIW, I'm not opposed to having this feature, but I think it should
> not be on by default on Posix platforms, because they support file
> names with literal backslashes.
>
> > 2. I created this patch because I actually debug binaries on a Linux
> > box which have been created on a Windows machine (GNU toolchain/Cygwin).
> > Empirically, I've realized that 'substitute-path' doesn't work for this
> > purpose. After having single-stepped GDB and modified its code slightly,
> > I ended up with this patch. The point is that I figured it out
> > empirically. It could the case that I justed missed an already existing
> > solution. Please let me know if my patch is crap because there exists
> > another way of handling my use case.
>
> I don't think your approach is ``crap'', but the patch needs work,
> IMO. See below.
>
> > ! if (path[from_len] != '\0' && (!IS_UNIX_DIR_SEPARATOR (path[from_len]) && !IS_DOS_DIR_SEPARATOR (path[from_len])))
>
> This isn't the right way, for 2 reasons:
>
> . It makes the code uglier than it must be.
>
> . The support for DOS-style file names is based on compile-time
> macros, so it cannot be turned on and off. Thus, you leave no fire
> escape for Unix users who just happen to have file names with
> literal backslashes, improbable as that may be.
>
> The first part could be taken care of by crafting a single macro that
> replaces the two macros. The second part calls for an option which
> could be tested in the same macro.
>
> > ! if (IS_UNIX_ABSOLUTE_PATH (filename) || IS_DOS_ABSOLUTE_PATH(filename))
>
> Similarly here.
>
> > if (rewritten_filename != NULL)
> > {
> > + /* Especially for embedded systems, it may be the case that a
> > + binary has been built on Windows but the embedded system is now
> > + being debugged on a Unix machine (and vice versa). In order to
> > + make path substitution work on such 'mixed' path styles, we need
> > + to convert foreign dir separators to native ones. */
> > + #ifdef HAVE_DOS_BASED_FILE_SYSTEM
> > + const char from = '/';
> > + const char to = '\\';
> > + #else
> > + const char from = '\\';
> > + const char to = '/';
> > + #endif
> > + unsigned int i=0;
> > + for( i=0; i<strlen(rewritten_filename); ++i ) {
> > + if( rewritten_filename[i] == from ) {
> > + rewritten_filename[i] = to;
> > + }
> > + }
>
> In this part, I simply don't understand why you needed the
> HAVE_DOS_BASED_FILE_SYSTEM branch. DOS/Windows file-system calls
> support forward slashes just fine, so there's no need to rewrite the
> slashes.
>
> Finally, please wait for other opinions, as I'm not sure mine is in
> consensus.
>
Here's a patch I wrote a few months back when I was working on
the "set/show target-file-system-kind" support, but put it on
the back burner and never looked at it again. It adds a new
"set source-filenames-matching-scheme dos/unix" command to select
the behavior at run-time, and adds a new source_filename_cmp function
to be used whenever we are comparing source file names instead of
using FILENAME_CMP. I never did an exaustive check on all FILENAME_CMP
calls that should be replaced, and neither did much testing on it
when I wrote it. I've new comfirmed it still builds, but didn't retest
more than that.
--
Pedro Alves
9999-99-99 Pedro Alves <pedro@codesourcery.com>
gdb/
* buildsym.c: Include "source.h".
(start_subfile): Use source_filename_cmp instead of FILENAME_CMP.
* symtab.c: Remove comment on reason of filenames.h inclusion.
(lookup_symtab): Use source_filename_cmp instead of FILENAME_CMP.
(append_exact_match_to_sals): Ditto.
* psymtab.c (lookup_partial_symtab): Ditto.
* source.c (substitute_path_rule_matches): Ditto.
(find_substitute_path_rule): Ditto.
(show_substitute_path_command): Ditto.
(unset_substitute_path_command): Ditto.
(source_file_names_dos_based, source_file_names_unix)
(source_file_names_modes, source_file_names_mode): New.
(show_source_filenames_matching_scheme_command): New.
(source_filename_cmp): New.
(_initialize_source): Install new set/show
source-filenames-matching-scheme commands.
* source.h (source_filename_cmp): Declare.
* dwarf2read.c: Include source.h.
(dw2_lookup_symtab): Use source_filename_cmp instead of
FILENAME_CMP.
include/
* filenames.h (filename_cmp): Delete declaration.
(dos_filename_cmp, unix_filename_cmp): New declarations.
(FILENAME_CMP): Reimplement.
libiberty/
* filename_cmp.c (filename_cmp): Delete, split into...
(dos_filename_cmp, unix_filename_cmp): ... these new functions.
---
gdb/buildsym.c | 3 +-
gdb/dwarf2read.c | 3 +-
gdb/psymtab.c | 8 ++---
gdb/source.c | 66 ++++++++++++++++++++++++++++++++++++++++++++---
gdb/source.h | 13 +++++++++
gdb/symtab.c | 18 ++++++------
include/filenames.h | 10 +++++--
libiberty/filename_cmp.c | 13 ++++-----
8 files changed, 107 insertions(+), 27 deletions(-)
Index: src/gdb/buildsym.c
===================================================================
--- src.orig/gdb/buildsym.c 2010-10-06 23:13:47.000000000 +0100
+++ src/gdb/buildsym.c 2010-12-17 17:11:42.000000000 +0000
@@ -44,6 +44,7 @@
#include "cp-support.h"
#include "dictionary.h"
#include "addrmap.h"
+#include "source.h"
/* Ask buildsym.h to define the vars it normally declares `extern'. */
#define EXTERN
@@ -544,7 +545,7 @@ start_subfile (const char *name, const c
else
subfile_name = subfile->name;
- if (FILENAME_CMP (subfile_name, name) == 0)
+ if (source_filename_cmp (subfile_name, name) == 0)
{
current_subfile = subfile;
if (subfile_name != subfile->name)
Index: src/gdb/symtab.c
===================================================================
--- src.orig/gdb/symtab.c 2010-11-09 16:13:38.000000000 +0000
+++ src/gdb/symtab.c 2010-12-17 17:13:03.000000000 +0000
@@ -37,7 +37,7 @@
#include "inferior.h"
#include "linespec.h"
#include "source.h"
-#include "filenames.h" /* for FILENAME_CMP */
+#include "filenames.h"
#include "objc-lang.h"
#include "d-lang.h"
#include "ada-lang.h"
@@ -180,7 +180,7 @@ got_symtab:
ALL_SYMTABS (objfile, s)
{
- if (FILENAME_CMP (name, s->filename) == 0)
+ if (source_filename_cmp (name, s->filename) == 0)
{
return s;
}
@@ -192,7 +192,7 @@ got_symtab:
{
const char *fp = symtab_to_fullname (s);
- if (fp != NULL && FILENAME_CMP (full_path, fp) == 0)
+ if (fp != NULL && source_filename_cmp (full_path, fp) == 0)
{
return s;
}
@@ -207,7 +207,7 @@ got_symtab:
char *rp = gdb_realpath (fullname);
make_cleanup (xfree, rp);
- if (FILENAME_CMP (real_path, rp) == 0)
+ if (source_filename_cmp (real_path, rp) == 0)
{
return s;
}
@@ -220,7 +220,7 @@ got_symtab:
if (lbasename (name) == name)
ALL_SYMTABS (objfile, s)
{
- if (FILENAME_CMP (lbasename (s->filename), name) == 0)
+ if (source_filename_cmp (lbasename (s->filename), name) == 0)
return s;
}
@@ -2201,11 +2201,11 @@ find_line_symtab (struct symtab *symtab,
struct linetable *l;
int ind;
- if (FILENAME_CMP (symtab->filename, s->filename) != 0)
+ if (source_filename_cmp (symtab->filename, s->filename) != 0)
continue;
if (symtab->fullname != NULL
&& symtab_to_fullname (s) != NULL
- && FILENAME_CMP (symtab->fullname, s->fullname) != 0)
+ && source_filename_cmp (symtab->fullname, s->fullname) != 0)
continue;
l = LINETABLE (s);
ind = find_line_common (l, line, &exact);
@@ -4513,14 +4513,14 @@ append_exact_match_to_sals (char *filena
ALL_PSPACES (pspace)
ALL_PSPACE_SYMTABS (pspace, objfile, symtab)
{
- if (FILENAME_CMP (filename, symtab->filename) == 0)
+ if (source_filename_cmp (filename, symtab->filename) == 0)
{
struct linetable *l;
int len;
if (fullname != NULL
&& symtab_to_fullname (symtab) != NULL
- && FILENAME_CMP (fullname, symtab->fullname) != 0)
+ && source_filename_cmp (fullname, symtab->fullname) != 0)
continue;
l = LINETABLE (symtab);
if (!l)
Index: src/gdb/psymtab.c
===================================================================
--- src.orig/gdb/psymtab.c 2010-11-25 13:03:20.000000000 +0000
+++ src/gdb/psymtab.c 2010-12-17 17:11:42.000000000 +0000
@@ -81,7 +81,7 @@ lookup_partial_symtab (struct objfile *o
ALL_OBJFILE_PSYMTABS (objfile, pst)
{
- if (FILENAME_CMP (name, pst->filename) == 0)
+ if (source_filename_cmp (name, pst->filename) == 0)
{
return (pst);
}
@@ -92,7 +92,7 @@ lookup_partial_symtab (struct objfile *o
{
psymtab_to_fullname (pst);
if (pst->fullname != NULL
- && FILENAME_CMP (full_path, pst->fullname) == 0)
+ && source_filename_cmp (full_path, pst->fullname) == 0)
{
return pst;
}
@@ -107,7 +107,7 @@ lookup_partial_symtab (struct objfile *o
rp = gdb_realpath (pst->fullname);
make_cleanup (xfree, rp);
}
- if (rp != NULL && FILENAME_CMP (real_path, rp) == 0)
+ if (rp != NULL && source_filename_cmp (real_path, rp) == 0)
{
return pst;
}
@@ -119,7 +119,7 @@ lookup_partial_symtab (struct objfile *o
if (lbasename (name) == name)
ALL_OBJFILE_PSYMTABS (objfile, pst)
{
- if (FILENAME_CMP (lbasename (pst->filename), name) == 0)
+ if (source_filename_cmp (lbasename (pst->filename), name) == 0)
return (pst);
}
Index: src/gdb/source.c
===================================================================
--- src.orig/gdb/source.c 2010-11-09 16:20:32.000000000 +0000
+++ src/gdb/source.c 2010-12-17 17:33:20.000000000 +0000
@@ -909,7 +909,7 @@ substitute_path_rule_matches (const stru
strncpy (path_start, path, from_len);
path_start[from_len] = '\0';
- if (FILENAME_CMP (path_start, rule->from) != 0)
+ if (source_filename_cmp (path_start, rule->from) != 0)
return 0;
/* Make sure that the region in the path that matches the substitution
@@ -1751,7 +1751,7 @@ find_substitute_path_rule (const char *f
while (rule != NULL)
{
- if (FILENAME_CMP (rule->from, from) == 0)
+ if (source_filename_cmp (rule->from, from) == 0)
return rule;
rule = rule->next;
}
@@ -1847,7 +1847,7 @@ show_substitute_path_command (char *args
while (rule != NULL)
{
- if (from == NULL || FILENAME_CMP (rule->from, from) == 0)
+ if (from == NULL || source_filename_cmp (rule->from, from) == 0)
printf_filtered (" `%s' -> `%s'.\n", rule->from, rule->to);
rule = rule->next;
}
@@ -1887,7 +1887,7 @@ unset_substitute_path_command (char *arg
{
struct substitute_path_rule *next = rule->next;
- if (from == NULL || FILENAME_CMP (from, rule->from) == 0)
+ if (from == NULL || source_filename_cmp (from, rule->from) == 0)
{
delete_substitute_path_rule (rule);
rule_found = 1;
@@ -1943,6 +1943,46 @@ set_substitute_path_command (char *args,
forget_cached_source_info ();
}
+/* Allow the user to configure the debugger behavior with respect to
+ source filename matching. */
+
+static const char source_file_names_dos_based[] = "dos-based";
+static const char source_file_names_unix[] = "unix";
+static const char *source_file_names_modes[] =
+{
+ source_file_names_dos_based,
+ source_file_names_unix
+};
+
+/* Handle binaries compiled on DOS-based filesystems (e.g, Windows),
+ by default, even if GDB itself is not running on such a system.
+ Such binaries may contain debug info with source paths the native
+ path handling functions wouldn't understand (e.g., backslash as
+ directory separator, drive names, and case insensitivity). The
+ risk of this going wrong is very minor in practice, so it's more
+ useful to leave this as default. */
+static const char *source_file_names_mode = source_file_names_dos_based;
+
+static void
+show_source_filenames_matching_scheme_command (struct ui_file *file,
+ int from_tty,
+ struct cmd_list_element *c,
+ const char *value)
+{
+ fprintf_filtered (file, _("\
+File name matching scheme for source paths is \"%s\".\n"),
+ value);
+}
+
+int
+source_filename_cmp (const char *s1, const char *s2)
+{
+ if (source_file_names_mode == source_file_names_dos_based)
+ return dos_filename_cmp (s1, s2);
+ else
+ return unix_filename_cmp (s1, s2);
+}
+
void
_initialize_source (void)
@@ -2058,4 +2098,22 @@ Usage: show substitute-path [FROM]\n\
Print the rule for substituting FROM in source file names. If FROM\n\
is not specified, print all substitution rules."),
&showlist);
+
+ add_setshow_enum_cmd ("source-filenames-matching-scheme",
+ class_support,
+ source_file_names_modes,
+ &source_file_names_mode, _("\
+Set file name matching scheme for source paths"), _("\
+Set file name matching scheme for source paths"),
+ _("\
+If `unix', source paths (e.g., program source paths included in debug \n\
+info) starting the forward slash (`/') character are considered \n\
+absolute, and the directory separator character is the forward slash \n\
+(`/'). If `dos-based' (which is the default), source paths starting with \n\
+a drive name (e.g., `c:'), are also considered absolute, the backslash \n\
+(`\\') is also considered a directory separator, and source filename \n\
+comparisons are not case sensitive."),
+ NULL, /* setfunc */
+ show_source_filenames_matching_scheme_command,
+ &setlist, &showlist);
}
Index: src/gdb/source.h
===================================================================
--- src.orig/gdb/source.h 2010-03-12 19:12:13.000000000 +0000
+++ src/gdb/source.h 2010-12-17 17:11:42.000000000 +0000
@@ -68,4 +68,17 @@ extern void clear_current_source_symtab_
/* Add a source path substitution rule. */
extern void add_substitute_path_rule (char *, char *);
+
+/* Return zero if the two file names S1 and S2 are equivalent. If not
+ equivalent, the returned value is similar to what strcmp would
+ return. In other words, it returns a negative value if S1 is less
+ than S2, or a positive value if S2 is greater than S2.
+
+ This function does not normalize file names. As a result, this
+ function will treat filenames that are spelled differently as
+ different even in the case when the two filenames point to the same
+ underlying file. However, it does handle the fact that if `set
+ source-filenames-matching-scheme' mode is set to `dos-based',
+ forward and backward slashes are equal. */
+extern int source_filename_cmp (const char *s1, const char *s2);
#endif
Index: src/gdb/dwarf2read.c
===================================================================
--- src.orig/gdb/dwarf2read.c 2010-12-13 16:31:03.000000000 +0000
+++ src/gdb/dwarf2read.c 2010-12-17 17:30:37.000000000 +0000
@@ -57,6 +57,7 @@
#include "vec.h"
#include "c-lang.h"
#include "valprint.h"
+#include "source.h"
#include <fcntl.h>
#include "gdb_string.h"
@@ -2285,7 +2286,7 @@ dw2_lookup_symtab (struct objfile *objfi
{
const char *this_name = file_data->file_names[j];
- if (FILENAME_CMP (name, this_name) == 0)
+ if (source_filename_cmp (name, this_name) == 0)
{
*result = dw2_instantiate_symtab (objfile, per_cu);
return 1;
Index: src/include/filenames.h
===================================================================
--- src.orig/include/filenames.h 2010-06-16 10:55:28.000000000 +0100
+++ src/include/filenames.h 2010-12-17 17:11:42.000000000 +0000
@@ -70,8 +70,14 @@ extern "C" {
(IS_DIR_SEPARATOR_1 (dos_based, (f)[0]) \
|| HAS_DRIVE_SPEC_1 (dos_based, f))
-extern int filename_cmp (const char *s1, const char *s2);
-#define FILENAME_CMP(s1, s2) filename_cmp(s1, s2)
+extern int dos_filename_cmp (const char *s1, const char *s2);
+extern int unix_filename_cmp (const char *s1, const char *s2);
+
+#if (HAVE_DOS_BASED_FILE_SYSTEM)
+# define FILENAME_CMP(s1, s2) dos_filename_cmp (s1, s2)
+#else
+# define FILENAME_CMP(s1, s2) unix_filename_cmp (s1, s2)
+#endif
#ifdef __cplusplus
}
Index: src/libiberty/filename_cmp.c
===================================================================
--- src.orig/libiberty/filename_cmp.c 2007-05-04 00:40:11.000000000 +0100
+++ src/libiberty/filename_cmp.c 2010-12-17 17:11:42.000000000 +0000
@@ -48,11 +48,14 @@ and backward slashes are equal.
*/
int
-filename_cmp (const char *s1, const char *s2)
+unix_filename_cmp (const char *s1, const char *s2)
+{
+ return strcmp (s1, s2);
+}
+
+int
+dos_filename_cmp (const char *s1, const char *s2)
{
-#ifndef HAVE_DOS_BASED_FILE_SYSTEM
- return strcmp(s1, s2);
-#else
for (;;)
{
int c1 = TOLOWER (*s1);
@@ -73,6 +76,4 @@ filename_cmp (const char *s1, const char
s1++;
s2++;
}
-#endif
}
-