* [RFC] new substitute path when loading feature
@ 2008-05-13 18:56 Aleksandar Ristovski
2008-05-13 19:00 ` Daniel Jacobowitz
0 siblings, 1 reply; 29+ messages in thread
From: Aleksandar Ristovski @ 2008-05-13 18:56 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2864 bytes --]
Hello,
For us who deal with mixed-platform development environment it is common to debug binaries built on a different machine, often different OS. A real-life example:
development environment (sdk) is setup on a win32 machine, with headers and all. When compiled, binaries are being debugged on a linux-hosted gdb. Linux machine has the same development environment (sdk) only in different root directories. Where win32 may have C:/foo, linux will have /opt/foo, etc...
To properly translate the paths it is useful to be able to tell gdb to translate such paths into new environment. Existing "substitute-path" functionality seems like the most logical choice, and the diff exploits this to add new feature - rewriting source paths at readin time.
The difference can be observed if a binary built on a win32 system is loaded in gdb on linux. For example
(gdb) maint info symtabs
....
{ symtab C:/QNX632/target/qnx6/usr/include/sys/compiler_gnu.h
((struct symtab *) 0x8390808)
dirname c:/Temp/dirs/debug
fullname (null)
blockvector ((struct blockvector *) 0x8392f24)
linetable ((struct linetable *) (nil))
debugformat unknown
}
{ symtab ../main.c ((struct symtab *) 0x83907b0)
dirname c:/Temp/dirs/debug
fullname (null)
blockvector ((struct blockvector *) 0x8392f24) (primary)
linetable ((struct linetable *) 0x8392f9c)
debugformat DWARF 2
}
...
while with the patch:
(gdb) set substitute-path-at-source 1
(gdb) set substitute-path C:/QNX632 /opt/qnx632
(gdb) set substitute-path c:/Temp /tmp
(gdb) symbol-file /tmp/dirs/debug/main
Load new symbol table from "/tmp/dirs/debug/main"? (y or n) y
Reading symbols from /tmp/dirs/debug/main...done.
(gdb) maint info symtabs
....
{ symtab /opt/qnx632/target/qnx6/usr/include/sys/compiler_gnu.h
((struct symtab *) 0x8379c10)
dirname /tmp/dirs/debug
fullname (null)
blockvector ((struct blockvector *) 0x837c32c)
linetable ((struct linetable *) (nil))
debugformat unknown
}
{ symtab ../main.c ((struct symtab *) 0x8379bb8)
dirname /tmp/dirs/debug
fullname (null)
blockvector ((struct blockvector *) 0x837c32c) (primary)
linetable ((struct linetable *) 0x837c390)
debugformat DWARF 2
}
...
Thanks,
Aleksandar Ristovski
QNX Software Systems
ChangeLog:
2008-05-13 Aleksandar Ristovski <aristovski@qnx.com>
* Makefile.in (buildsym.o): Add new dependency.
* buildsym.c (source.h): New include.
(start_subfile): Rewrite path is substitute-path-at-source is set.
* source.c (substitute_pat_at_source): New variable.
(rewrite_source_path): Make external.
(_initialize_source): Add new set/show variable
'substitute-path-at-source'.
* source.h (rewrite_source_path substitute_pat_at_source): New
declarations.
* symfile.c (start_psymtab_common): Rewrite file name if
substitute-path-at-source is set.
[-- Attachment #2: substitute-path-at-source.diff --]
[-- Type: text/plain, Size: 6248 bytes --]
Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.1018
diff -u -p -r1.1018 Makefile.in
--- gdb/Makefile.in 9 May 2008 17:02:01 -0000 1.1018
+++ gdb/Makefile.in 13 May 2008 17:59:11 -0000
@@ -1998,7 +1998,7 @@ buildsym.o: buildsym.c $(defs_h) $(bfd_h
$(complaints_h) $(gdb_string_h) $(expression_h) $(bcache_h) \
$(filenames_h) $(macrotab_h) $(demangle_h) $(block_h) \
$(cp_support_h) $(dictionary_h) $(buildsym_h) $(stabsread_h) \
- $(addrmap_h)
+ $(addrmap_h) $(source_h)
c-exp.o: c-exp.c $(defs_h) $(gdb_string_h) $(expression_h) $(value_h) \
$(parser_defs_h) $(language_h) $(c_lang_h) $(bfd_h) $(symfile_h) \
$(objfiles_h) $(charset_h) $(block_h) $(cp_support_h) $(dfp_h)
Index: gdb/buildsym.c
===================================================================
RCS file: /cvs/src/src/gdb/buildsym.c,v
retrieving revision 1.60
diff -u -p -r1.60 buildsym.c
--- gdb/buildsym.c 17 Apr 2008 17:54:04 -0000 1.60
+++ gdb/buildsym.c 13 May 2008 17:59:11 -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
@@ -583,6 +584,24 @@ void
start_subfile (char *name, char *dirname)
{
struct subfile *subfile;
+ char *rwname = NULL;
+ char *rwdirname = NULL;
+
+ if (substitute_path_at_source)
+ {
+ rwname = rewrite_source_path (name);
+ rwdirname = rewrite_source_path (dirname);
+ }
+
+ if (rwname == NULL)
+ rwname = name;
+ else
+ make_cleanup (xfree, rwname);
+ if (rwdirname == NULL)
+ rwdirname = dirname;
+ else
+ make_cleanup (xfree, rwdirname);
+
/* See if this subfile is already known as a subfile of the current
main source file. */
@@ -593,7 +612,7 @@ start_subfile (char *name, char *dirname
/* If NAME is an absolute path, and this subfile is not, then
attempt to create an absolute path to compare. */
- if (IS_ABSOLUTE_PATH (name)
+ if (IS_ABSOLUTE_PATH (rwname)
&& !IS_ABSOLUTE_PATH (subfile->name)
&& subfile->dirname != NULL)
subfile_name = concat (subfile->dirname, SLASH_STRING,
@@ -601,7 +620,7 @@ start_subfile (char *name, char *dirname
else
subfile_name = subfile->name;
- if (FILENAME_CMP (subfile_name, name) == 0)
+ if (FILENAME_CMP (subfile_name, rwname) == 0)
{
current_subfile = subfile;
if (subfile_name != subfile->name)
@@ -623,9 +642,10 @@ start_subfile (char *name, char *dirname
current_subfile = subfile;
/* Save its name and compilation directory name */
- subfile->name = (name == NULL) ? NULL : savestring (name, strlen (name));
+ subfile->name =
+ (rwname == NULL) ? NULL : savestring (rwname, strlen (rwname));
subfile->dirname =
- (dirname == NULL) ? NULL : savestring (dirname, strlen (dirname));
+ (rwdirname == NULL) ? NULL : savestring (rwdirname, strlen (rwdirname));
/* Initialize line-number recording for this subfile. */
subfile->line_vector = NULL;
Index: gdb/source.c
===================================================================
RCS file: /cvs/src/src/gdb/source.c,v
retrieving revision 1.88
diff -u -p -r1.88 source.c
--- gdb/source.c 3 May 2008 06:13:21 -0000 1.88
+++ gdb/source.c 13 May 2008 17:59:12 -0000
@@ -91,6 +91,8 @@ static struct symtab *current_source_sym
static int current_source_line;
+int substitute_path_at_source = 0;
+
/* Default number of lines to print with commands like "list".
This is based on guessing how many long (i.e. more than chars_per_line
characters) lines there will be. To be completely correct, "list"
@@ -895,7 +897,7 @@ get_substitute_path_rule (const char *pa
Return NULL if no substitution rule was specified by the user,
or if no rule applied to the given PATH. */
-static char *
+char *
rewrite_source_path (const char *path)
{
const struct substitute_path_rule *rule = get_substitute_path_rule (path);
@@ -2027,4 +2029,12 @@ 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_zinteger_cmd ("substitute-path-at-source", class_files,
+ &substitute_path_at_source, _("\
+Activate path substitution rule at source."), _("\
+Show path substitution rule at source."), _("\
+When set, paths loaded from debug info will be rewritten before\n\
+being used in GDB."),
+ NULL, NULL, &setlist, &showlist);
}
Index: gdb/source.h
===================================================================
RCS file: /cvs/src/src/gdb/source.h,v
retrieving revision 1.9
diff -u -p -r1.9 source.h
--- gdb/source.h 1 Jan 2008 22:53:13 -0000 1.9
+++ gdb/source.h 13 May 2008 17:59:12 -0000
@@ -66,4 +66,9 @@ extern struct symtab_and_line set_curren
/* Reset any information stored about a default file and line to print. */
extern void clear_current_source_symtab_and_line (void);
+
+extern char *rewrite_source_path (const char *path);
+
+extern int substitute_path_at_source;
+
#endif
Index: gdb/symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.203
diff -u -p -r1.203 symfile.c
--- gdb/symfile.c 5 May 2008 16:13:49 -0000 1.203
+++ gdb/symfile.c 13 May 2008 17:59:13 -0000
@@ -3072,13 +3072,22 @@ start_psymtab_common (struct objfile *ob
struct partial_symbol **static_syms)
{
struct partial_symtab *psymtab;
+ char *rwfilename = NULL;
- psymtab = allocate_psymtab (filename, objfile);
+ if (substitute_path_at_source)
+ rwfilename = rewrite_source_path (filename);
+
+ if (rwfilename == NULL)
+ rwfilename = filename;
+
+ psymtab = allocate_psymtab (rwfilename, objfile);
psymtab->section_offsets = section_offsets;
psymtab->textlow = textlow;
psymtab->texthigh = psymtab->textlow; /* default */
psymtab->globals_offset = global_syms - objfile->global_psymbols.list;
psymtab->statics_offset = static_syms - objfile->static_psymbols.list;
+ if (rwfilename != filename)
+ xfree (rwfilename);
return (psymtab);
}
\f
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-13 18:56 [RFC] new substitute path when loading feature Aleksandar Ristovski
@ 2008-05-13 19:00 ` Daniel Jacobowitz
2008-05-13 20:24 ` Aleksandar Ristovski
0 siblings, 1 reply; 29+ messages in thread
From: Daniel Jacobowitz @ 2008-05-13 19:00 UTC (permalink / raw)
To: Aleksandar Ristovski; +Cc: gdb-patches
On Tue, May 13, 2008 at 02:06:57PM -0400, Aleksandar Ristovski wrote:
> To properly translate the paths it is useful to be able to tell gdb
> to translate such paths into new environment. Existing
> "substitute-path" functionality seems like the most logical choice,
> and the diff exploits this to add new feature - rewriting source
> paths at readin time.
Why is this feature useful? There shouldn't be anything that depends
on the paths being changed at readin time.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-13 19:00 ` Daniel Jacobowitz
@ 2008-05-13 20:24 ` Aleksandar Ristovski
2008-05-13 21:11 ` Daniel Jacobowitz
2008-05-13 21:12 ` Eli Zaretskii
0 siblings, 2 replies; 29+ messages in thread
From: Aleksandar Ristovski @ 2008-05-13 20:24 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
Daniel Jacobowitz wrote:
> On Tue, May 13, 2008 at 02:06:57PM -0400, Aleksandar Ristovski wrote:
>> To properly translate the paths it is useful to be able to tell gdb
>> to translate such paths into new environment. Existing
>> "substitute-path" functionality seems like the most logical choice,
>> and the diff exploits this to add new feature - rewriting source
>> paths at readin time.
>
> Why is this feature useful? There shouldn't be anything that depends
> on the paths being changed at readin time.
>
This is what prompted my change:
(gdb) set substitute-path c:/Temp /tmp
(gdb) symbol-file /tmp/dirs/debug/main2
Reading symbols from /tmp/dirs/debug/main2...done.
(gdb) b main
Breakpoint 1 at 0x8048418: file c:/Temp/dirs/main.c, line 3.
(gdb) l
1 c:/Temp/dirs/main.c: No such file or directory.
in c:/Temp/dirs/main.c
(gdb)
Now without my change, this will sometimes work, sometimes it won't. For example, if the program was compiled on windows like this:
C:\Temp\dirs\debug>gcc -g -O0 -c ../main.c -o main.o
things would work. However if built like this:
C:\Temp\dirs\debug>gcc -g -O0 -c c:\Temp\dirs\main.c -o main.o
things would not work (as shown above).
My other motivation was that throughout the code we use IS_ABSOLUTE_PATH which simply doesn't work if gdb was not configured for that filesystem flavour (i.e. when configured for POSIX path it does not understand Win32 path naming). With rewriting paths at source, things work as expected.
Further, in most cases when file lookup is done and binary contains alien path style (well, only when gdb is configured for POSIX but examinig win32 built binary) the path matches happen mostly by chance, i.e. by matching basename and not full path. This may (and probably will) cause problems every now and then when a binary contains two source files (whether CU or included file) with the same base name.
In any case, it seems right to be able to rewrite paths at readin and let gdb "see" the binaries as if they were built locally.
Thanks,
Aleksandar
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-13 20:24 ` Aleksandar Ristovski
@ 2008-05-13 21:11 ` Daniel Jacobowitz
2008-05-13 21:18 ` Aleksandar Ristovski
2008-05-13 21:12 ` Eli Zaretskii
1 sibling, 1 reply; 29+ messages in thread
From: Daniel Jacobowitz @ 2008-05-13 21:11 UTC (permalink / raw)
To: Aleksandar Ristovski; +Cc: gdb-patches
On Tue, May 13, 2008 at 02:55:41PM -0400, Aleksandar Ristovski wrote:
>
> Now without my change, this will sometimes work, sometimes it won't. For example, if the program was compiled on windows like this:
>
> C:\Temp\dirs\debug>gcc -g -O0 -c ../main.c -o main.o
>
> things would work. However if built like this:
>
> C:\Temp\dirs\debug>gcc -g -O0 -c c:\Temp\dirs\main.c -o main.o
>
> things would not work (as shown above).
Is this just because of the slashes? I have a patch which makes
Unix-configured GDB more tolerant of DOS paths and drive names. I
can post it if you like; I've been meaning to.
> In any case, it seems right to be able to rewrite paths at readin and let
> gdb "see" the binaries as if they were built locally.
This means you have to have the source path mapping worked out before
you know what files are in the program, so it's hard to fix up later
if you see a new unrelocated path.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-13 20:24 ` Aleksandar Ristovski
2008-05-13 21:11 ` Daniel Jacobowitz
@ 2008-05-13 21:12 ` Eli Zaretskii
1 sibling, 0 replies; 29+ messages in thread
From: Eli Zaretskii @ 2008-05-13 21:12 UTC (permalink / raw)
To: Aleksandar Ristovski; +Cc: drow, gdb-patches
> Date: Tue, 13 May 2008 14:55:41 -0400
> From: Aleksandar Ristovski <aristovski@qnx.com>
> CC: gdb-patches@sources.redhat.com
>
> Now without my change, this will sometimes work, sometimes it won't. For example, if the program was compiled on windows like this:
>
> C:\Temp\dirs\debug>gcc -g -O0 -c ../main.c -o main.o
>
> things would work. However if built like this:
>
> C:\Temp\dirs\debug>gcc -g -O0 -c c:\Temp\dirs\main.c -o main.o
>
> things would not work (as shown above).
Why wouldn't it be a good idea to fix this in GCC? It could normalize
all backslashes into forward slashes.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-13 21:11 ` Daniel Jacobowitz
@ 2008-05-13 21:18 ` Aleksandar Ristovski
2008-05-13 21:40 ` Daniel Jacobowitz
0 siblings, 1 reply; 29+ messages in thread
From: Aleksandar Ristovski @ 2008-05-13 21:18 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
Daniel Jacobowitz wrote:
> On Tue, May 13, 2008 at 02:55:41PM -0400, Aleksandar Ristovski wrote:
>> Now without my change, this will sometimes work, sometimes it won't. For example, if the program was compiled on windows like this:
>>
>> C:\Temp\dirs\debug>gcc -g -O0 -c ../main.c -o main.o
>>
>> things would work. However if built like this:
>>
>> C:\Temp\dirs\debug>gcc -g -O0 -c c:\Temp\dirs\main.c -o main.o
>>
>> things would not work (as shown above).
>
> Is this just because of the slashes? I have a patch which makes
> Unix-configured GDB more tolerant of DOS paths and drive names. I
> can post it if you like; I've been meaning to.
No, this particular issue is not because of the slashes, but rather due to IS_ABSOLUTE_PATH returning false on a path like "c:/Temp...".
>
>> In any case, it seems right to be able to rewrite paths at readin and let
>> gdb "see" the binaries as if they were built locally.
>
> This means you have to have the source path mapping worked out before
> you know what files are in the program, so it's hard to fix up later
> if you see a new unrelocated path.
>
True and I was thinking further along these lines to enable rewriting existing paths in psymtabs/symtabs on demand (i.e. a command that would tell gdb: look through all psymtabs/symtabs and rewrite paths)... but wanted to get a feedback first :-)
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-13 21:18 ` Aleksandar Ristovski
@ 2008-05-13 21:40 ` Daniel Jacobowitz
2008-05-15 17:37 ` Daniel Jacobowitz
0 siblings, 1 reply; 29+ messages in thread
From: Daniel Jacobowitz @ 2008-05-13 21:40 UTC (permalink / raw)
To: Aleksandar Ristovski; +Cc: gdb-patches
On Tue, May 13, 2008 at 03:11:22PM -0400, Aleksandar Ristovski wrote:
> No, this particular issue is not because of the slashes, but rather
> due to IS_ABSOLUTE_PATH returning false on a path like "c:/Temp...".
OK. I think I "fixed" FILENAME_CMP and not IS_ABSOLUTE_PATH, but it
would not be hard to do both.
I'll try to post something tonight.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-13 21:40 ` Daniel Jacobowitz
@ 2008-05-15 17:37 ` Daniel Jacobowitz
2008-05-15 19:29 ` Joel Brobecker
` (3 more replies)
0 siblings, 4 replies; 29+ messages in thread
From: Daniel Jacobowitz @ 2008-05-15 17:37 UTC (permalink / raw)
To: Aleksandar Ristovski, gdb-patches; +Cc: Eli Zaretskii
On Tue, May 13, 2008 at 03:20:41PM -0400, Daniel Jacobowitz wrote:
> On Tue, May 13, 2008 at 03:11:22PM -0400, Aleksandar Ristovski wrote:
> > No, this particular issue is not because of the slashes, but rather
> > due to IS_ABSOLUTE_PATH returning false on a path like "c:/Temp...".
>
> OK. I think I "fixed" FILENAME_CMP and not IS_ABSOLUTE_PATH, but it
> would not be hard to do both.
>
> I'll try to post something tonight.
Sorry, my existing patch was a mess so I had to rewrite it. I haven't
really tested this; it doesn't break a native Linux GDB in any case
that I consider significant. See the comments in defs.h and utils.c
for the details.
Does this fix the same problem you're working on? Does anyone see a
problem with the compromises I made here? Eli, I'd appreciate your
opinion.
--
Daniel Jacobowitz
CodeSourcery
2008-05-15 Daniel Jacobowitz <dan@codesourcery.com>
* defs.h (GDB_IS_DIR_SEPARATOR, GDB_IS_ABSOLUTE_PATH)
(GDB_FILENAME_CMP): New macros.
(gdb_filename_cmp): Declare.
* buildsym.c, dwarf2read.c, solib.c, source.c, symtab.c: Use the
new macros.
* utils.c (ldirname): Likewise.
(gdb_filename_cmp): New function.
Index: buildsym.c
===================================================================
RCS file: /cvs/src/src/gdb/buildsym.c,v
retrieving revision 1.60
diff -u -p -r1.60 buildsym.c
--- buildsym.c 17 Apr 2008 17:54:04 -0000 1.60
+++ buildsym.c 15 May 2008 15:41:01 -0000
@@ -593,15 +593,15 @@ start_subfile (char *name, char *dirname
/* If NAME is an absolute path, and this subfile is not, then
attempt to create an absolute path to compare. */
- if (IS_ABSOLUTE_PATH (name)
- && !IS_ABSOLUTE_PATH (subfile->name)
+ if (GDB_IS_ABSOLUTE_PATH (name)
+ && !GDB_IS_ABSOLUTE_PATH (subfile->name)
&& subfile->dirname != NULL)
subfile_name = concat (subfile->dirname, SLASH_STRING,
subfile->name, NULL);
else
subfile_name = subfile->name;
- if (FILENAME_CMP (subfile_name, name) == 0)
+ if (GDB_FILENAME_CMP (subfile_name, name) == 0)
{
current_subfile = subfile;
if (subfile_name != subfile->name)
Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.223
diff -u -p -r1.223 defs.h
--- defs.h 3 May 2008 22:20:13 -0000 1.223
+++ defs.h 15 May 2008 15:41:02 -0000
@@ -374,6 +374,24 @@ ULONGEST strtoulst (const char *num, con
char *ldirname (const char *filename);
+/* Versions of the filenames.h macros to use with filenames from debug
+ information, when the program may have been compiled on a DOS
+ filesystem even if GDB is running on a POSIX host. */
+
+/* Recognizing backslashes as directory separators has a low false
+ positive rate, because POSIX filenames rarely include them. They
+ are likely to appear in full paths in debug information. */
+#define GDB_IS_DIR_SEPARATOR(c) ((c) == '/' || (c) == '\\')
+
+/* Recognizing drive letters as absolute path prefixes has a low false
+ positive rate, because POSIX directories are rarely named "c:". We
+ must not prepend a directory prefix to drive letters. */
+#define GDB_IS_ABSOLUTE_PATH(f) \
+ (GDB_IS_DIR_SEPARATOR ((f)[0]) || (((f)[0]) && ((f)[1] == ':')))
+
+#define GDB_FILENAME_CMP(lhs, rhs) gdb_filename_cmp (lhs, rhs)
+int gdb_filename_cmp (const char *lhs, const char *rhs);
+
/* From demangle.c */
extern void set_demangling_style (char *);
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.263
diff -u -p -r1.263 dwarf2read.c
--- dwarf2read.c 5 May 2008 14:37:32 -0000 1.263
+++ dwarf2read.c 15 May 2008 15:41:02 -0000
@@ -2832,7 +2832,7 @@ read_file_scope (struct die_info *die, s
attr = dwarf2_attr (die, DW_AT_comp_dir, cu);
if (attr)
comp_dir = DW_STRING (attr);
- else if (name != NULL && IS_ABSOLUTE_PATH (name))
+ else if (name != NULL && GDB_IS_ABSOLUTE_PATH (name))
{
comp_dir = ldirname (name);
if (comp_dir != NULL)
@@ -7186,14 +7186,14 @@ dwarf_decode_lines (struct line_header *
if (fe.dir_index)
dir_name = lh->include_dirs[fe.dir_index - 1];
- if (!IS_ABSOLUTE_PATH (include_name) && dir_name != NULL)
+ if (!GDB_IS_ABSOLUTE_PATH (include_name) && dir_name != NULL)
{
include_name = concat (dir_name, SLASH_STRING,
include_name, (char *)NULL);
make_cleanup (xfree, include_name);
}
- if (!IS_ABSOLUTE_PATH (pst_filename) && pst->dirname != NULL)
+ if (!GDB_IS_ABSOLUTE_PATH (pst_filename) && pst->dirname != NULL)
{
pst_filename = concat (pst->dirname, SLASH_STRING,
pst_filename, (char *)NULL);
@@ -7274,7 +7274,7 @@ dwarf2_start_subfile (char *filename, ch
that represent full path names''. Thus ignoring dirname in the
`else' branch below isn't an issue. */
- if (!IS_ABSOLUTE_PATH (filename) && dirname != NULL)
+ if (!GDB_IS_ABSOLUTE_PATH (filename) && dirname != NULL)
fullname = concat (dirname, SLASH_STRING, filename, (char *)NULL);
else
fullname = filename;
@@ -9502,7 +9502,7 @@ file_full_name (int file, struct line_he
{
struct file_entry *fe = &lh->file_names[file - 1];
- if (IS_ABSOLUTE_PATH (fe->name))
+ if (GDB_IS_ABSOLUTE_PATH (fe->name))
return xstrdup (fe->name);
else
{
Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.101
diff -u -p -r1.101 solib.c
--- solib.c 7 Jan 2008 15:19:58 -0000 1.101
+++ solib.c 15 May 2008 15:41:02 -0000
@@ -150,7 +150,7 @@ solib_open (char *in_pathname, char **fo
gdb_sysroot_is_empty = (gdb_sysroot == NULL || *gdb_sysroot == 0);
- if (! IS_ABSOLUTE_PATH (in_pathname) || gdb_sysroot_is_empty)
+ if (! GDB_IS_ABSOLUTE_PATH (in_pathname) || gdb_sysroot_is_empty)
temp_pathname = in_pathname;
else
{
@@ -186,14 +186,14 @@ solib_open (char *in_pathname, char **fo
absolute at this point, make it relative. (openp will try and open the
file according to its absolute path otherwise, which is not what we want.)
Affects subsequent searches for this solib. */
- if (found_file < 0 && IS_ABSOLUTE_PATH (in_pathname))
+ if (found_file < 0 && GDB_IS_ABSOLUTE_PATH (in_pathname))
{
/* First, get rid of any drive letters etc. */
- while (!IS_DIR_SEPARATOR (*in_pathname))
+ while (!GDB_IS_DIR_SEPARATOR (*in_pathname))
in_pathname++;
/* Next, get rid of all leading dir separators. */
- while (IS_DIR_SEPARATOR (*in_pathname))
+ while (GDB_IS_DIR_SEPARATOR (*in_pathname))
in_pathname++;
}
Index: source.c
===================================================================
RCS file: /cvs/src/src/gdb/source.c,v
retrieving revision 1.88
diff -u -p -r1.88 source.c
--- source.c 3 May 2008 06:13:21 -0000 1.88
+++ source.c 15 May 2008 15:41:03 -0000
@@ -700,7 +700,7 @@ openp (const char *path, int opts, const
mode |= O_BINARY;
- if ((opts & OPF_TRY_CWD_FIRST) || IS_ABSOLUTE_PATH (string))
+ if ((opts & OPF_TRY_CWD_FIRST) || GDB_IS_ABSOLUTE_PATH (string))
{
int i;
@@ -720,16 +720,16 @@ openp (const char *path, int opts, const
if (!(opts & OPF_SEARCH_IN_PATH))
for (i = 0; string[i]; i++)
- if (IS_DIR_SEPARATOR (string[i]))
+ if (GDB_IS_DIR_SEPARATOR (string[i]))
goto done;
}
/* /foo => foo, to avoid multiple slashes that Emacs doesn't like. */
- while (IS_DIR_SEPARATOR(string[0]))
+ while (GDB_IS_DIR_SEPARATOR(string[0]))
string++;
/* ./foo => foo */
- while (string[0] == '.' && IS_DIR_SEPARATOR (string[1]))
+ while (string[0] == '.' && GDB_IS_DIR_SEPARATOR (string[1]))
string += 2;
alloclen = strlen (path) + strlen (string) + 2;
@@ -861,14 +861,14 @@ 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 (GDB_FILENAME_CMP (path_start, rule->from) != 0)
return 0;
/* Make sure that the region in the path that matches the substitution
rule is immediately followed by a directory separator (or the end of
string character). */
- if (path[from_len] != '\0' && !IS_DIR_SEPARATOR (path[from_len]))
+ if (path[from_len] != '\0' && !GDB_IS_DIR_SEPARATOR (path[from_len]))
return 0;
return 1;
@@ -1004,7 +1004,7 @@ find_and_open_source (struct objfile *ob
}
}
- if (IS_ABSOLUTE_PATH (filename))
+ if (GDB_IS_ABSOLUTE_PATH (filename))
{
/* If filename is absolute path, try the source path
substitution on it. */
@@ -1721,7 +1721,7 @@ strip_trailing_directory_separator (char
if (last < 0)
return; /* No stripping is needed if PATH is the empty string. */
- if (IS_DIR_SEPARATOR (path[last]))
+ if (GDB_IS_DIR_SEPARATOR (path[last]))
path[last] = '\0';
}
@@ -1735,7 +1735,7 @@ find_substitute_path_rule (const char *f
while (rule != NULL)
{
- if (FILENAME_CMP (rule->from, from) == 0)
+ if (GDB_FILENAME_CMP (rule->from, from) == 0)
return rule;
rule = rule->next;
}
@@ -1831,7 +1831,7 @@ show_substitute_path_command (char *args
while (rule != NULL)
{
- if (from == NULL || FILENAME_CMP (rule->from, from) == 0)
+ if (from == NULL || GDB_FILENAME_CMP (rule->from, from) == 0)
printf_filtered (" `%s' -> `%s'.\n", rule->from, rule->to);
rule = rule->next;
}
@@ -1871,7 +1871,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 || GDB_FILENAME_CMP (from, rule->from) == 0)
{
delete_substitute_path_rule (rule);
rule_found = 1;
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.181
diff -u -p -r1.181 symtab.c
--- symtab.c 5 May 2008 14:37:32 -0000 1.181
+++ symtab.c 15 May 2008 15:41:03 -0000
@@ -187,7 +187,7 @@ got_symtab:
ALL_SYMTABS (objfile, s)
{
- if (FILENAME_CMP (name, s->filename) == 0)
+ if (GDB_FILENAME_CMP (name, s->filename) == 0)
{
return s;
}
@@ -198,7 +198,7 @@ got_symtab:
if (full_path != NULL)
{
const char *fp = symtab_to_fullname (s);
- if (fp != NULL && FILENAME_CMP (full_path, fp) == 0)
+ if (fp != NULL && GDB_FILENAME_CMP (full_path, fp) == 0)
{
return s;
}
@@ -211,7 +211,7 @@ got_symtab:
{
char *rp = gdb_realpath (fullname);
make_cleanup (xfree, rp);
- if (FILENAME_CMP (real_path, rp) == 0)
+ if (GDB_FILENAME_CMP (real_path, rp) == 0)
{
return s;
}
@@ -224,7 +224,7 @@ got_symtab:
if (lbasename (name) == name)
ALL_SYMTABS (objfile, s)
{
- if (FILENAME_CMP (lbasename (s->filename), name) == 0)
+ if (GDB_FILENAME_CMP (lbasename (s->filename), name) == 0)
return s;
}
@@ -279,7 +279,7 @@ lookup_partial_symtab (const char *name)
ALL_PSYMTABS (objfile, pst)
{
- if (FILENAME_CMP (name, pst->filename) == 0)
+ if (GDB_FILENAME_CMP (name, pst->filename) == 0)
{
return (pst);
}
@@ -290,7 +290,7 @@ lookup_partial_symtab (const char *name)
{
psymtab_to_fullname (pst);
if (pst->fullname != NULL
- && FILENAME_CMP (full_path, pst->fullname) == 0)
+ && GDB_FILENAME_CMP (full_path, pst->fullname) == 0)
{
return pst;
}
@@ -305,7 +305,7 @@ lookup_partial_symtab (const char *name)
rp = gdb_realpath (pst->fullname);
make_cleanup (xfree, rp);
}
- if (rp != NULL && FILENAME_CMP (real_path, rp) == 0)
+ if (rp != NULL && GDB_FILENAME_CMP (real_path, rp) == 0)
{
return pst;
}
@@ -317,7 +317,7 @@ lookup_partial_symtab (const char *name)
if (lbasename (name) == name)
ALL_PSYMTABS (objfile, pst)
{
- if (FILENAME_CMP (lbasename (pst->filename), name) == 0)
+ if (GDB_FILENAME_CMP (lbasename (pst->filename), name) == 0)
return (pst);
}
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.187
diff -u -p -r1.187 utils.c
--- utils.c 24 Apr 2008 11:13:44 -0000 1.187
+++ utils.c 15 May 2008 15:41:03 -0000
@@ -3205,7 +3205,7 @@ ldirname (const char *filename)
const char *base = lbasename (filename);
char *dirname;
- while (base > filename && IS_DIR_SEPARATOR (base[-1]))
+ while (base > filename && GDB_IS_DIR_SEPARATOR (base[-1]))
--base;
if (base == filename)
@@ -3214,12 +3214,43 @@ ldirname (const char *filename)
dirname = xmalloc (base - filename + 2);
memcpy (dirname, filename, base - filename);
- /* On DOS based file systems, convert "d:foo" to "d:.", so that we
+ /* For DOS based file systems, convert "d:foo" to "d:.", so that we
create "d:./bar" later instead of the (different) "d:/bar". */
- if (base - filename == 2 && IS_ABSOLUTE_PATH (base)
- && !IS_DIR_SEPARATOR (filename[0]))
+ if (base - filename == 2 && GDB_IS_ABSOLUTE_PATH (base)
+ && !GDB_IS_DIR_SEPARATOR (filename[0]))
dirname[base++ - filename] = '.';
dirname[base - filename] = '\0';
return dirname;
}
+
+/* Compare two filenames. This version should be used instead of
+ FILENAME_CMP for filenames from debug information; it recognizes
+ equivalences from compiling on a DOS filesystem even if the
+ debugger is running on a POSIX host. */
+
+int
+gdb_filename_cmp (const char *lhs, const char *rhs)
+{
+ for (; *lhs || *rhs; lhs++, rhs++)
+ {
+#ifndef HAVE_DOS_BASED_FILE_SYSTEM
+ /* When debugging on a POSIX host, assume that each filename was
+ recorded with a single consistent capitalization during
+ compilation. Source trees are too likely to contain both
+ main.c and Main.c. */
+ if (*lhs == *rhs)
+ continue;
+#else
+ if (tolower (*lhs) == tolower (*rhs))
+ continue;
+#endif
+
+ if (*lhs == '/' && *rhs == '\\')
+ continue;
+ if (*lhs == '\\' && *rhs == '/')
+ continue;
+ return (int) *lhs - (int) *rhs;
+ }
+ return 0;
+}
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-15 17:37 ` Daniel Jacobowitz
@ 2008-05-15 19:29 ` Joel Brobecker
2008-05-15 19:30 ` Eli Zaretskii
` (2 subsequent siblings)
3 siblings, 0 replies; 29+ messages in thread
From: Joel Brobecker @ 2008-05-15 19:29 UTC (permalink / raw)
To: Aleksandar Ristovski, gdb-patches, Eli Zaretskii
> 2008-05-15 Daniel Jacobowitz <dan@codesourcery.com>
>
> * defs.h (GDB_IS_DIR_SEPARATOR, GDB_IS_ABSOLUTE_PATH)
> (GDB_FILENAME_CMP): New macros.
> (gdb_filename_cmp): Declare.
> * buildsym.c, dwarf2read.c, solib.c, source.c, symtab.c: Use the
> new macros.
> * utils.c (ldirname): Likewise.
> (gdb_filename_cmp): New function.
>
> Does this fix the same problem you're working on? Does anyone see a
> problem with the compromises I made here?
The compromises made here seem reasonable to me.
--
Joel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-15 17:37 ` Daniel Jacobowitz
2008-05-15 19:29 ` Joel Brobecker
@ 2008-05-15 19:30 ` Eli Zaretskii
2008-05-15 19:44 ` Daniel Jacobowitz
2008-05-15 20:05 ` Joel Brobecker
2008-05-15 19:35 ` Aleksandar Ristovski
2008-05-17 19:49 ` Vladimir Prus
3 siblings, 2 replies; 29+ messages in thread
From: Eli Zaretskii @ 2008-05-15 19:30 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: aristovski, gdb-patches
> Date: Thu, 15 May 2008 12:05:51 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Eli Zaretskii <eliz@gnu.org>
>
> On Tue, May 13, 2008 at 03:20:41PM -0400, Daniel Jacobowitz wrote:
> > On Tue, May 13, 2008 at 03:11:22PM -0400, Aleksandar Ristovski wrote:
> > > No, this particular issue is not because of the slashes, but rather
> > > due to IS_ABSOLUTE_PATH returning false on a path like "c:/Temp...".
> >
> > OK. I think I "fixed" FILENAME_CMP and not IS_ABSOLUTE_PATH, but it
> > would not be hard to do both.
> >
> > I'll try to post something tonight.
>
> Sorry, my existing patch was a mess so I had to rewrite it. I haven't
> really tested this; it doesn't break a native Linux GDB in any case
> that I consider significant. See the comments in defs.h and utils.c
> for the details.
>
> Does this fix the same problem you're working on? Does anyone see a
> problem with the compromises I made here? Eli, I'd appreciate your
> opinion.
Well, the patch will certainly work with DOS-ish file names on a Posix
host, but I'm worried about breaking file names that are perfectly
valid on Posix filesystems, but which just happen to use colons and
backslashes. I agree that a probability of meeting such file names on
Posix platforms is miniscule, but this patch leaves the user no fire
escape whatsoever when she does meet them.
Can we please augment this with some minimal band-aid for when
backslashes and colons are literally used in a file name? Something
like a user option to disable this feature and use normal Posix
file-name syntax? I figure this would be enough, since mixing object
files compiled on Posix and Windows platforms should be _really_ rare.
> +/* Compare two filenames. This version should be used instead of
> + FILENAME_CMP for filenames from debug information; it recognizes
> + equivalences from compiling on a DOS filesystem even if the
> + debugger is running on a POSIX host. */
> +
> +int
> +gdb_filename_cmp (const char *lhs, const char *rhs)
> +{
> + for (; *lhs || *rhs; lhs++, rhs++)
> + {
> +#ifndef HAVE_DOS_BASED_FILE_SYSTEM
> + /* When debugging on a POSIX host, assume that each filename was
> + recorded with a single consistent capitalization during
> + compilation. Source trees are too likely to contain both
> + main.c and Main.c. */
> + if (*lhs == *rhs)
> + continue;
> +#else
> + if (tolower (*lhs) == tolower (*rhs))
> + continue;
> +#endif
> +
> + if (*lhs == '/' && *rhs == '\\')
> + continue;
> + if (*lhs == '\\' && *rhs == '/')
> + continue;
> + return (int) *lhs - (int) *rhs;
> + }
> + return 0;
> +}
Will this work if the file name is encoded in UTF-8 or some other
multi-byte encoding, btw? We do want to support those, don't we?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-15 17:37 ` Daniel Jacobowitz
2008-05-15 19:29 ` Joel Brobecker
2008-05-15 19:30 ` Eli Zaretskii
@ 2008-05-15 19:35 ` Aleksandar Ristovski
2008-05-17 19:49 ` Vladimir Prus
3 siblings, 0 replies; 29+ messages in thread
From: Aleksandar Ristovski @ 2008-05-15 19:35 UTC (permalink / raw)
To: gdb-patches
Daniel Jacobowitz wrote:
> Does this fix the same problem you're working on? Does anyone see a
> problem with the compromises I made here? Eli, I'd appreciate your
> opinion.
>
I was considering something like this (except I changed the IS_ABSOLUTE_PATH macro to a be defined as a function call) but then didn't like the idea... there was also opposition to that approach when I brought it up on the mailing list.
But yes, this can be a solution too, although I expect that unix purists will complain about the possibility of having directory names with colon on POSIX file systems (GDB_IS_ABSOLUTE_PATH could return wrong result in such cases). The same stands for backslashes.
To sum it up: in our case it (probably - haven't tested) does fix our common-scenario, but not sure if it's a 'proper' solution - probably not; maybe if it was wrapped in an option as was suggested in my first attempt at 'universal' IS_ABSOLUTE_PATH?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-15 19:30 ` Eli Zaretskii
@ 2008-05-15 19:44 ` Daniel Jacobowitz
2008-05-15 22:40 ` Eli Zaretskii
2008-05-15 20:05 ` Joel Brobecker
1 sibling, 1 reply; 29+ messages in thread
From: Daniel Jacobowitz @ 2008-05-15 19:44 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: aristovski, gdb-patches
On Thu, May 15, 2008 at 09:55:05PM +0300, Eli Zaretskii wrote:
> Well, the patch will certainly work with DOS-ish file names on a Posix
> host, but I'm worried about breaking file names that are perfectly
> valid on Posix filesystems, but which just happen to use colons and
> backslashes. I agree that a probability of meeting such file names on
> Posix platforms is miniscule, but this patch leaves the user no fire
> escape whatsoever when she does meet them.
>
> Can we please augment this with some minimal band-aid for when
> backslashes and colons are literally used in a file name? Something
> like a user option to disable this feature and use normal Posix
> file-name syntax? I figure this would be enough, since mixing object
> files compiled on Posix and Windows platforms should be _really_ rare.
To have trouble I think you'd need to have two files in a project
whose name only differed by slashes: e.g. "a\b.c" and directory "a"
and "a/b.c". Mistakenly thinking a path is relative might cause some
problems but I can't think of how they would manifest.
I'm happy to add the band-aid; do you have an idea of what to call it?
set strict-filenames?
> Will this work if the file name is encoded in UTF-8 or some other
> multi-byte encoding, btw? We do want to support those, don't we?
The POSIX parts will work in UTF-8, since no multi-byte UTF-8
characters contain printable 7-bit ASCII. Other encodings, or with
the tolower check, I don't know.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-15 19:30 ` Eli Zaretskii
2008-05-15 19:44 ` Daniel Jacobowitz
@ 2008-05-15 20:05 ` Joel Brobecker
1 sibling, 0 replies; 29+ messages in thread
From: Joel Brobecker @ 2008-05-15 20:05 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Daniel Jacobowitz, aristovski, gdb-patches
> Can we please augment this with some minimal band-aid for when
> backslashes and colons are literally used in a file name? Something
> like a user option to disable this feature and use normal Posix
> file-name syntax? I figure this would be enough, since mixing object
> files compiled on Posix and Windows platforms should be _really_ rare.
That would make sense indeed.
--
Joel
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-15 19:44 ` Daniel Jacobowitz
@ 2008-05-15 22:40 ` Eli Zaretskii
2008-05-16 0:28 ` Daniel Jacobowitz
0 siblings, 1 reply; 29+ messages in thread
From: Eli Zaretskii @ 2008-05-15 22:40 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: aristovski, gdb-patches
> Date: Thu, 15 May 2008 15:16:42 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: aristovski@qnx.com, gdb-patches@sources.redhat.com
>
> I'm happy to add the band-aid; do you have an idea of what to call it?
> set strict-filenames?
How about "set support-dos-filenames" (on by default)?
> The POSIX parts will work in UTF-8, since no multi-byte UTF-8
> characters contain printable 7-bit ASCII. Other encodings, or with
> the tolower check, I don't know.
Supporting UTF-8 is good enough for me.
Thanks.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-15 22:40 ` Eli Zaretskii
@ 2008-05-16 0:28 ` Daniel Jacobowitz
2008-05-16 8:15 ` Aleksandar Ristovski
0 siblings, 1 reply; 29+ messages in thread
From: Daniel Jacobowitz @ 2008-05-16 0:28 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: aristovski, gdb-patches
On Thu, May 15, 2008 at 10:28:10PM +0300, Eli Zaretskii wrote:
> > Date: Thu, 15 May 2008 15:16:42 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > Cc: aristovski@qnx.com, gdb-patches@sources.redhat.com
> >
> > I'm happy to add the band-aid; do you have an idea of what to call it?
> > set strict-filenames?
>
> How about "set support-dos-filenames" (on by default)?
I like that better, thanks! I'll work on the option (not right now).
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 0:28 ` Daniel Jacobowitz
@ 2008-05-16 8:15 ` Aleksandar Ristovski
2008-05-16 8:19 ` Aleksandar Ristovski
` (2 more replies)
0 siblings, 3 replies; 29+ messages in thread
From: Aleksandar Ristovski @ 2008-05-16 8:15 UTC (permalink / raw)
To: gdb-patches; +Cc: Eli Zaretskii, Daniel Jacobowitz
Daniel Jacobowitz wrote:
> On Thu, May 15, 2008 at 10:28:10PM +0300, Eli Zaretskii wrote:
>>> Date: Thu, 15 May 2008 15:16:42 -0400
>>> From: Daniel Jacobowitz <drow@false.org>
>>> Cc: aristovski@qnx.com, gdb-patches@sources.redhat.com
>>>
>>> I'm happy to add the band-aid; do you have an idea of what to call it?
>>> set strict-filenames?
>> How about "set support-dos-filenames" (on by default)?
>
> I like that better, thanks! I'll work on the option (not right now).
>
You will probably want this wrapped in an if. In fact, I don't see how will it work properly with only one flag - we probably don't want to do case-insensitive compare by default, but we do need to compare case insensitive for dos-like file system.
#ifndef HAVE_DOS_BASED_FILE_SYSTEM
+ /* When debugging on a POSIX host, assume that each filename was
+ recorded with a single consistent capitalization during
+ compilation. Source trees are too likely to contain both
+ main.c and Main.c. */
+ if (*lhs == *rhs)
+ continue;
+#else
+ if (tolower (*lhs) == tolower (*rhs))
+ continue;
+#endif
+
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 8:15 ` Aleksandar Ristovski
@ 2008-05-16 8:19 ` Aleksandar Ristovski
2008-05-16 8:21 ` Daniel Jacobowitz
2008-05-16 15:26 ` Eli Zaretskii
2 siblings, 0 replies; 29+ messages in thread
From: Aleksandar Ristovski @ 2008-05-16 8:19 UTC (permalink / raw)
To: gdb-patches; +Cc: Eli Zaretskii, Daniel Jacobowitz
Daniel Jacobowitz wrote:
> On Thu, May 15, 2008 at 10:28:10PM +0300, Eli Zaretskii wrote:
>>> Date: Thu, 15 May 2008 15:16:42 -0400
>>> From: Daniel Jacobowitz <drow@false.org>
>>> Cc: aristovski@qnx.com, gdb-patches@sources.redhat.com
>>>
>>> I'm happy to add the band-aid; do you have an idea of what to call it?
>>> set strict-filenames?
>> How about "set support-dos-filenames" (on by default)?
>
> I like that better, thanks! I'll work on the option (not right now).
>
You will probably want this wrapped in an if. In fact, I don't see how will it work properly with only one flag - we probably don't want to do case-insensitive compare by default, but we do need to compare case insensitive for dos-like file system.
#ifndef HAVE_DOS_BASED_FILE_SYSTEM
+ /* When debugging on a POSIX host, assume that each filename was
+ recorded with a single consistent capitalization during
+ compilation. Source trees are too likely to contain both
+ main.c and Main.c. */
+ if (*lhs == *rhs)
+ continue;
+#else
+ if (tolower (*lhs) == tolower (*rhs))
+ continue;
+#endif
+
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 8:15 ` Aleksandar Ristovski
2008-05-16 8:19 ` Aleksandar Ristovski
@ 2008-05-16 8:21 ` Daniel Jacobowitz
2008-05-16 15:26 ` Mark Kettenis
2008-05-16 15:27 ` Aleksandar Ristovski
2008-05-16 15:26 ` Eli Zaretskii
2 siblings, 2 replies; 29+ messages in thread
From: Daniel Jacobowitz @ 2008-05-16 8:21 UTC (permalink / raw)
To: Aleksandar Ristovski; +Cc: gdb-patches, Eli Zaretskii
On Thu, May 15, 2008 at 03:40:26PM -0400, Aleksandar Ristovski wrote:
> You will probably want this wrapped in an if. In fact, I don't see
> how will it work properly with only one flag - we probably don't
> want to do case-insensitive compare by default, but we do need to
> compare case insensitive for dos-like file system.
Is this really necessary? As long as everything gives GDB the same
capitalization of the file (including debug info and any front end),
then we don't need to be case insensitive.
If you think this is an important feature I can make the variable
tri-state:
set support-dos-filenames (on|off|auto)
Auto would be case sensitive on POSIX hosts, on wouldn't, auto would
be the default.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 8:15 ` Aleksandar Ristovski
2008-05-16 8:19 ` Aleksandar Ristovski
2008-05-16 8:21 ` Daniel Jacobowitz
@ 2008-05-16 15:26 ` Eli Zaretskii
2 siblings, 0 replies; 29+ messages in thread
From: Eli Zaretskii @ 2008-05-16 15:26 UTC (permalink / raw)
To: Aleksandar Ristovski; +Cc: gdb-patches, drow
> Date: Thu, 15 May 2008 15:40:26 -0400
> From: Aleksandar Ristovski <aristovski@qnx.com>
> CC: Eli Zaretskii <eliz@gnu.org>, Daniel Jacobowitz <drow@false.org>
>
> You will probably want this wrapped in an if. In fact, I don't see
> how will it work properly with only one flag
Like this:
if (support_dos_filenames || HAVE_DOS_BASED_FILE_SYSTEM)
/* use DOS-ish comparisons */
else
/* use normal Unix comparisons, like what we do now */
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 8:21 ` Daniel Jacobowitz
@ 2008-05-16 15:26 ` Mark Kettenis
2008-05-16 15:27 ` Eli Zaretskii
2008-05-16 15:27 ` Aleksandar Ristovski
1 sibling, 1 reply; 29+ messages in thread
From: Mark Kettenis @ 2008-05-16 15:26 UTC (permalink / raw)
To: drow; +Cc: aristovski, gdb-patches, eliz
> Date: Thu, 15 May 2008 15:44:18 -0400
> From: Daniel Jacobowitz <drow@false.org>
>
> On Thu, May 15, 2008 at 03:40:26PM -0400, Aleksandar Ristovski wrote:
> > You will probably want this wrapped in an if. In fact, I don't see
> > how will it work properly with only one flag - we probably don't
> > want to do case-insensitive compare by default, but we do need to
> > compare case insensitive for dos-like file system.
>
> Is this really necessary? As long as everything gives GDB the same
> capitalization of the file (including debug info and any front end),
> then we don't need to be case insensitive.
>
> If you think this is an important feature I can make the variable
> tri-state:
>
> set support-dos-filenames (on|off|auto)
>
> Auto would be case sensitive on POSIX hosts, on wouldn't, auto would
> be the default.
Sorry, but I think this really is a bad idea. For a native debugger
on a POSIX system I really don't want the oddities of dos-like filenames.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 8:21 ` Daniel Jacobowitz
2008-05-16 15:26 ` Mark Kettenis
@ 2008-05-16 15:27 ` Aleksandar Ristovski
2008-05-16 15:27 ` Aleksandar Ristovski
2008-05-16 15:27 ` Eli Zaretskii
1 sibling, 2 replies; 29+ messages in thread
From: Aleksandar Ristovski @ 2008-05-16 15:27 UTC (permalink / raw)
Cc: Daniel Jacobowitz, gdb-patches, Eli Zaretskii
Daniel Jacobowitz wrote:
>
> Is this really necessary? As long as everything gives GDB the same
> capitalization of the file (including debug info and any front end),
> then we don't need to be case insensitive.
Yes, but it's all too easy to have mixed capitalizations (in general case) when building on win32... not that I think it is a common case, but still.
>
> If you think this is an important feature I can make the variable
> tri-state:
Well, it's a part of the same issue.
>
> set support-dos-filenames (on|off|auto)
>
> Auto would be case sensitive on POSIX hosts, on wouldn't, auto would
> be the default.
>
I think that would work.
@Eli: Your 'if' wouldn't work as expected for common posix-style paths (it would default to case insensitive comparison which is unacceptable for posix paths)
However, I'm not sure why you didn't like my original rewrite at readin... all the user would have to do for mixed path binaries is:
set substitute-path C: /
(which would make iberty's IS_ABSOLUTE_PATH work on posix-ly configured gdbs)
and this could be done by default in main :-)
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 15:27 ` Aleksandar Ristovski
2008-05-16 15:27 ` Aleksandar Ristovski
@ 2008-05-16 15:27 ` Eli Zaretskii
2008-05-16 19:21 ` Aleksandar Ristovski
2008-05-16 19:22 ` Aleksandar Ristovski
1 sibling, 2 replies; 29+ messages in thread
From: Eli Zaretskii @ 2008-05-16 15:27 UTC (permalink / raw)
To: Aleksandar Ristovski; +Cc: drow, gdb-patches
> Date: Thu, 15 May 2008 16:11:54 -0400
> From: Aleksandar Ristovski <aristovski@qnx.com>
> CC: Daniel Jacobowitz <drow@false.org>, gdb-patches@sources.redhat.com,
> Eli Zaretskii <eliz@gnu.org>
>
> @Eli: Your 'if' wouldn't work as expected for common posix-style paths (it would default to case insensitive comparison which is unacceptable for posix paths)
I think you misunderstood what I was suggesting, but we don't have to
argue about that, as long as you agree with Daniel's solution (because
I was in effect suggesting the same thing).
> However, I'm not sure why you didn't like my original rewrite at readin... all the user would have to do for mixed path binaries is:
>
> set substitute-path C: /
> (which would make iberty's IS_ABSOLUTE_PATH work on posix-ly configured gdbs)
>
> and this could be done by default in main :-)
I think this suggestion is so subtle that no one who has such a
problem would ever be able to guess that substitute-path is the
solution.
In other words, when a user has problem with DOS-style file names, she
will look for a solution whose name has something in common with "DOS"
and "file name", and substitute-path doesn't qualify. A good UI
should have high mnemonic significance to be helpful.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 15:27 ` Aleksandar Ristovski
@ 2008-05-16 15:27 ` Aleksandar Ristovski
2008-05-16 15:27 ` Eli Zaretskii
1 sibling, 0 replies; 29+ messages in thread
From: Aleksandar Ristovski @ 2008-05-16 15:27 UTC (permalink / raw)
To: gdb-patches; +Cc: Daniel Jacobowitz, gdb-patches, Eli Zaretskii
Daniel Jacobowitz wrote:
>
> Is this really necessary? As long as everything gives GDB the same
> capitalization of the file (including debug info and any front end),
> then we don't need to be case insensitive.
Yes, but it's all too easy to have mixed capitalizations (in general case) when building on win32... not that I think it is a common case, but still.
>
> If you think this is an important feature I can make the variable
> tri-state:
Well, it's a part of the same issue.
>
> set support-dos-filenames (on|off|auto)
>
> Auto would be case sensitive on POSIX hosts, on wouldn't, auto would
> be the default.
>
I think that would work.
@Eli: Your 'if' wouldn't work as expected for common posix-style paths (it would default to case insensitive comparison which is unacceptable for posix paths)
However, I'm not sure why you didn't like my original rewrite at readin... all the user would have to do for mixed path binaries is:
set substitute-path C: /
(which would make iberty's IS_ABSOLUTE_PATH work on posix-ly configured gdbs)
and this could be done by default in main :-)
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 15:26 ` Mark Kettenis
@ 2008-05-16 15:27 ` Eli Zaretskii
0 siblings, 0 replies; 29+ messages in thread
From: Eli Zaretskii @ 2008-05-16 15:27 UTC (permalink / raw)
To: Mark Kettenis; +Cc: drow, aristovski, gdb-patches
> Date: Thu, 15 May 2008 22:04:49 +0200 (CEST)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> CC: aristovski@qnx.com, gdb-patches@sources.redhat.com, eliz@gnu.org
>
> > set support-dos-filenames (on|off|auto)
> >
> > Auto would be case sensitive on POSIX hosts, on wouldn't, auto would
> > be the default.
>
> Sorry, but I think this really is a bad idea. For a native debugger
> on a POSIX system I really don't want the oddities of dos-like filenames.
Well, then you could set this option to never do that, couldn't you?
The problem is real, Mark. Why can't we give our users a way to cope
with it, as an option?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 15:27 ` Eli Zaretskii
@ 2008-05-16 19:21 ` Aleksandar Ristovski
2008-05-16 19:35 ` Eli Zaretskii
2008-05-16 19:22 ` Aleksandar Ristovski
1 sibling, 1 reply; 29+ messages in thread
From: Aleksandar Ristovski @ 2008-05-16 19:21 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: drow, gdb-patches
Eli Zaretskii wrote:
>> However, I'm not sure why you didn't like my original rewrite at readin... all the user would have to do for mixed path binaries is:
>>
>> set substitute-path C: /
>> (which would make iberty's IS_ABSOLUTE_PATH work on posix-ly configured gdbs)
>>
>> and this could be done by default in main :-)
>
> I think this suggestion is so subtle that no one who has such a
> problem would ever be able to guess that substitute-path is the
> solution.
Unless it was properly documented (for example, in a section named "Debugging cross compiled binaries" or something like that).
But ok, if I'm the only one who sees rewrite at readin as a useful thing, I'll leave it alone.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 15:27 ` Eli Zaretskii
2008-05-16 19:21 ` Aleksandar Ristovski
@ 2008-05-16 19:22 ` Aleksandar Ristovski
1 sibling, 0 replies; 29+ messages in thread
From: Aleksandar Ristovski @ 2008-05-16 19:22 UTC (permalink / raw)
To: gdb-patches; +Cc: drow
(sorry if this email is repeated, for some reason it never made it to the list)
Eli Zaretskii wrote:
>
>> However, I'm not sure why you didn't like my original rewrite at readin... all the user would have to do for mixed path binaries is:
>>
>> set substitute-path C: /
>> (which would make iberty's IS_ABSOLUTE_PATH work on posix-ly configured gdbs)
>>
>> and this could be done by default in main :-)
>
> I think this suggestion is so subtle that no one who has such a
> problem would ever be able to guess that substitute-path is the
> solution.
Unless it was properly documented (for example, in a section named "Debugging cross compiled binaries" or something like that).
But ok, if I'm the only one who sees rewrite at readin as a useful thing, I'll leave it alone.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-16 19:21 ` Aleksandar Ristovski
@ 2008-05-16 19:35 ` Eli Zaretskii
0 siblings, 0 replies; 29+ messages in thread
From: Eli Zaretskii @ 2008-05-16 19:35 UTC (permalink / raw)
To: Aleksandar Ristovski; +Cc: drow, gdb-patches
> Date: Fri, 16 May 2008 09:10:00 -0400
> From: Aleksandar Ristovski <aristovski@qnx.com>
> CC: drow@false.org, gdb-patches@sources.redhat.com
>
> > I think this suggestion is so subtle that no one who has such a
> > problem would ever be able to guess that substitute-path is the
> > solution.
>
> Unless it was properly documented (for example, in a section named "Debugging cross compiled binaries" or something like that).
You and I would then find it, perhaps, but unfortunately most users
don't really read the manual these days.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-15 17:37 ` Daniel Jacobowitz
` (2 preceding siblings ...)
2008-05-15 19:35 ` Aleksandar Ristovski
@ 2008-05-17 19:49 ` Vladimir Prus
2008-05-19 4:12 ` Vladimir Prus
3 siblings, 1 reply; 29+ messages in thread
From: Vladimir Prus @ 2008-05-17 19:49 UTC (permalink / raw)
To: gdb-patches
Daniel Jacobowitz wrote:
> On Tue, May 13, 2008 at 03:20:41PM -0400, Daniel Jacobowitz wrote:
>> On Tue, May 13, 2008 at 03:11:22PM -0400, Aleksandar Ristovski wrote:
>> > No, this particular issue is not because of the slashes, but rather
>> > due to IS_ABSOLUTE_PATH returning false on a path like "c:/Temp...".
>>
>> OK. I think I "fixed" FILENAME_CMP and not IS_ABSOLUTE_PATH, but it
>> would not be hard to do both.
>>
>> I'll try to post something tonight.
>
> Sorry, my existing patch was a mess so I had to rewrite it. I haven't
> really tested this; it doesn't break a native Linux GDB in any case
> that I consider significant. See the comments in defs.h and utils.c
> for the details.
I probably miss somethings, but it looks like the uses of IS_ABSOLUTE_PATH
in symtab.c:lookup_symtab and symtab.c:lookup_partial_symtab should be changed to
GDB_IS_ABSOLUTE_PATH, too?
Also, I see IS_ABSOLUTE_PATH been used in source.c:openp, inside the
"if (filename_opened)" block at the end, and your patch does not seem to change
that.
- Volodya
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RFC] new substitute path when loading feature
2008-05-17 19:49 ` Vladimir Prus
@ 2008-05-19 4:12 ` Vladimir Prus
0 siblings, 0 replies; 29+ messages in thread
From: Vladimir Prus @ 2008-05-19 4:12 UTC (permalink / raw)
To: gdb-patches
Vladimir Prus wrote:
> Daniel Jacobowitz wrote:
>
>> On Tue, May 13, 2008 at 03:20:41PM -0400, Daniel Jacobowitz wrote:
>>> On Tue, May 13, 2008 at 03:11:22PM -0400, Aleksandar Ristovski wrote:
>>> > No, this particular issue is not because of the slashes, but rather
>>> > due to IS_ABSOLUTE_PATH returning false on a path like "c:/Temp...".
>>>
>>> OK. I think I "fixed" FILENAME_CMP and not IS_ABSOLUTE_PATH, but it
>>> would not be hard to do both.
>>>
>>> I'll try to post something tonight.
>>
>> Sorry, my existing patch was a mess so I had to rewrite it. I haven't
>> really tested this; it doesn't break a native Linux GDB in any case
>> that I consider significant. See the comments in defs.h and utils.c
>> for the details.
>
> I probably miss somethings, but it looks like the uses of IS_ABSOLUTE_PATH
> in symtab.c:lookup_symtab and symtab.c:lookup_partial_symtab should be changed to
> GDB_IS_ABSOLUTE_PATH, too?
>
> Also, I see IS_ABSOLUTE_PATH been used in source.c:openp, inside the
> "if (filename_opened)" block at the end, and your patch does not seem to change
> that.
Ok, the last one probably need not be changed, since we appear to always
have host patch at this point. OTOH, I'm still unsure about symtab.c --
is calling lookup_symtab with the absolute path on the system where binary is
compiled (not where gdb is run) something we want to support? In general, it seems
that having IS_ABSOLUTE_PATH and GDB_IS_ABSOLUTE_PATH, and needing to use some reasoning
where to use which one is too risky -- should we use GDB_IS_ABSOLUTE_PATH everywhere?
- Volodya
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2008-05-17 16:22 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-05-13 18:56 [RFC] new substitute path when loading feature Aleksandar Ristovski
2008-05-13 19:00 ` Daniel Jacobowitz
2008-05-13 20:24 ` Aleksandar Ristovski
2008-05-13 21:11 ` Daniel Jacobowitz
2008-05-13 21:18 ` Aleksandar Ristovski
2008-05-13 21:40 ` Daniel Jacobowitz
2008-05-15 17:37 ` Daniel Jacobowitz
2008-05-15 19:29 ` Joel Brobecker
2008-05-15 19:30 ` Eli Zaretskii
2008-05-15 19:44 ` Daniel Jacobowitz
2008-05-15 22:40 ` Eli Zaretskii
2008-05-16 0:28 ` Daniel Jacobowitz
2008-05-16 8:15 ` Aleksandar Ristovski
2008-05-16 8:19 ` Aleksandar Ristovski
2008-05-16 8:21 ` Daniel Jacobowitz
2008-05-16 15:26 ` Mark Kettenis
2008-05-16 15:27 ` Eli Zaretskii
2008-05-16 15:27 ` Aleksandar Ristovski
2008-05-16 15:27 ` Aleksandar Ristovski
2008-05-16 15:27 ` Eli Zaretskii
2008-05-16 19:21 ` Aleksandar Ristovski
2008-05-16 19:35 ` Eli Zaretskii
2008-05-16 19:22 ` Aleksandar Ristovski
2008-05-16 15:26 ` Eli Zaretskii
2008-05-15 20:05 ` Joel Brobecker
2008-05-15 19:35 ` Aleksandar Ristovski
2008-05-17 19:49 ` Vladimir Prus
2008-05-19 4:12 ` Vladimir Prus
2008-05-13 21:12 ` Eli Zaretskii
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox