[RFC] gdb_realpath causes problems with GVD

[RFC] gdb_realpath causes problems with GVD

[Joel Brobecker]

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

Machine: x86-inux

The problem appears when some of the files used to build the
application are symbolic links and when GDB annotations are activated.
For us, this happens when GVD, our graphical front-end, drives GDB.

First, the context: In Ada, the filename associated to a unit need to
follow a strict convention, and this convention is dictated by the
compiler. For instance, the GNAT convention says that the spec of
package Hello must be located in file hello.ads. But the Apex naming
schemes says that it must be named hello.1.ada. The trouble starts when
somebody wants to develop/navigate using Apex, while building and
debugging using GNAT. In order to solve this naming issue, it has been
decided to keep the Apex filenames, and to create one symbolic link per
file which name follows GNAT naming scheme. So far, so good.

Now, here is an example of this situation, where I replace Ada by C,
which shows the problem we encountered. Supose we have directory
called symlinks, in which we have one C file, called toto.C, and one
symbolic link to this file called toto.c:
<<
void
break_me (void)
{
}

int
main (void)
{
  break_me ();
}
>>

We build the executable using the symlink:

        % gcc -g -o toto toto.c

Let's debug toto to see what happens:
<<
GNU gdb 2002-03-19-cvs
[...]
This GDB was configured as "i686-pc-linux-gnu"...
(gdb) set annotate 1
(gdb) b break_me
Breakpoint 1 at 0x804846b: file toto.c, line 4.
(gdb) run
Starting program: /bonn.a/brobecke/symlinks/toto 

Breakpoint 1, break_me () at toto.c:4
yy/bonn.a/brobecke/symlinks/toto.C:4:23:beg:0x804846b
                            ^^^^^^
>>

As you see, GDB has translated toto.c into toto.C. This translation
causes GDB to think that the inferior stopped in a file named toto.C
(which is not known to GDB, since the compiler used only toto.c). As a
consequence, when the user tries to put breakpoints using the GVD, GVD
issues the following break command

   (gdb) break toto.C:4

to which GDB answers
   
   No source file named toto.C

The translation is performed by gdb_realpath. I searched the gdb-patches
archives, and found the reason for this translation in a message from
Tom Tromey. I think I found a way to keep the fix to his problem and
then at the same time fix our issue: instead of canonicalizing the
entire filename, I suggest that we only expand the directory prefix
(ie the part returned by the "dirname" unix command).

Here is a patch which implements this idea. I'd like to get your opinion
on it. Ideally, I'll like to commit this change if it is ok. Here is the
change log:

2002-03-19  Joel Brobecker  <brobecker@gnat.com>

	* utils.c (gdb_canonicalize_path): New function.
	(gdb_realpath_fallback): New function.
	(gdb_realpath): Restrict the canonicalization to the directory
	prefix rather than canonicalizing the entire filename, to avoid
	returning a filename unknown to GDB when the file is actually a
	symbolic link.

This change was tested

BTW: I just a message requesting a change in the exact same function.
I haven't integrated it yet, because I don't know if it will be accepted
or not. If it is accepted, I can merge it and resubmit.

Thanks,
-- 
Joel

[-- Attachment #2: utils.c.diff --]
[-- Type: text/plain, Size: 4450 bytes --]

Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.70
diff -c -3 -p -r1.70 utils.c
*** utils.c	2002/03/19 02:51:07	1.70
--- utils.c	2002/03/19 16:08:08
*************** string_to_core_addr (const char *my_stri
*** 2531,2558 ****
    return addr;
  }
  
- char *
- gdb_realpath (const char *filename)
- {
  #if defined(HAVE_REALPATH)
  # if defined (PATH_MAX)
!   char buf[PATH_MAX];
  #  define USE_REALPATH
  # elif defined (MAXPATHLEN)
!   char buf[MAXPATHLEN];
  #  define USE_REALPATH
  # elif defined (HAVE_UNISTD_H) && defined(HAVE_ALLOCA)
!   char *buf = alloca ((size_t)pathconf ("/", _PC_PATH_MAX));
  #  define USE_REALPATH
  # endif
  #endif /* HAVE_REALPATH */
  
! #if defined(USE_REALPATH)
!   char *rp = realpath (filename, buf);
!   return xstrdup (rp ? rp : filename);
! #elif defined(HAVE_CANONICALIZE_FILE_NAME)
!   return canonicalize_file_name (filename);
  #else
    return xstrdup (filename);
  #endif
  }
--- 2531,2640 ----
    return addr;
  }
  
  #if defined(HAVE_REALPATH)
  # if defined (PATH_MAX)
! static const int max_rp_buffer_size = PATH_MAX;
  #  define USE_REALPATH
  # elif defined (MAXPATHLEN)
! static const int max_rp_buffer_size = MAXPATHLEN;
  #  define USE_REALPATH
  # elif defined (HAVE_UNISTD_H) && defined(HAVE_ALLOCA)
! static const int max_rp_buffer_size = pathconf ("/", _PC_PATH_MAX);
  #  define USE_REALPATH
  # endif
  #endif /* HAVE_REALPATH */
+ 
+ /*
+  * gdb_canonicalize_path
+  *
+  * Sets RESOLVED_PATH to the canonicalized form of FILENAME, as realpath ()
+  * does. If there is no error, it returns a pointer to the resolved path.
+  * Otherwise, returns null.
+  *
+  * Returns null on platforms where no canonicalization routine (such as
+  * realpath for instance) are available.
+  */
+ static char *
+ gdb_canonicalize_path (const char *path, char *resolved_path)
+ {
+ #if defined (USE_REALPATH)
+   return realpath (path, resolved_path);
+   
+ #elif defined (HAVE_CANONICALIZE_FILE_NAME)
+   const char *canonicalized = canonicalize_file_name (path);
+ 
+   if (!canonicalized)
+     return null;
  
!   strcpy (resolved_path, canonicalized);
!   return resolved_path;
! 
  #else
+   return null;
+ 
+ #endif
+ }
+ 
+ /*
+  * gdb_realpath_fallback
+  *
+  * This is the fallback version of gdb_realpath when there is no
+  * canonicalization routine (such as realpath for instance) available.
+  * In this case, we simply return a copy of FILENAME.
+  */
+ static char *
+ gdb_realpath_fallback (const char *filename)
+ {
    return xstrdup (filename);
+ }
+ 
+ /*
+  * gdb_realpath
+  *
+  * Return a copy of FILENAME, with its directory prefix canonicalized,
+  * as for realpath() (see "man realpath" for more details on what 
+  * this function does), or simply a copy of FILENAME on platforms 
+  * where no canonicalization routine is available.
+  *
+  * We don't want to canonicalize the entire FILENAME, because
+  * the canonicalization routines used to perform the operation also
+  * expand symbolic links. If FILENAME is itself a link to another file,
+  * it can lead GDB to translate FILENAME into another filename that GDB
+  * does not know about. This can confuse graphical front-ends for GDB
+  * which, like GVD, rely on the filename displayed by GDB to get the
+  * current file:line location, for example.
+  */
+ char *
+ gdb_realpath (const char *filename)
+ {
+   const char *base_name = lbasename (filename);
+   char *dir_name;
+   char *buf;
+   char *rp = NULL;
+ 
+ #if !defined (USE_REALPATH) || !defined (HAVE_CANONICALIZE_FILE_NAME)
+   return gdb_realpath_fallback (filename);
  #endif
+ 
+   /* If basename and filename are equal, then there is no path to
+      canonicalize. Just return a copy of filename */
+   if (base_name == filename)
+     return xstrdup (filename);
+     
+   dir_name = alloca ((size_t) (base_name - filename + 1));
+   strncpy (dir_name, filename, base_name - filename);
+ 
+   /* Allocate enough space to contain the largest path possible returned
+      by realpath, plus the SLASH_STRING and the base_name */
+   buf = alloca ((size_t) max_rp_buffer_size +
+                 strlen (SLASH_STRING) + strlen (base_name));
+   rp = gdb_canonicalize_path (dir_name, buf);
+  
+   if (rp == NULL)
+     return xstrdup (filename);
+  
+   strcat (buf, SLASH_STRING);
+   strcat (buf, base_name);
+   return xstrdup (buf);
  }
+ 

Re: [RFC] gdb_realpath causes problems with GVD

[Eli Zaretskii]

On Tue, 19 Mar 2002, Joel Brobecker wrote:

> As you see, GDB has translated toto.c into toto.C. This translation
> causes GDB to think that the inferior stopped in a file named toto.C
> (which is not known to GDB, since the compiler used only toto.c).

Are you saying that gdb_realpath shouldn't follow symlinks?

> The translation is performed by gdb_realpath. I searched the gdb-patches
> archives, and found the reason for this translation in a message from
> Tom Tromey. I think I found a way to keep the fix to his problem and
> then at the same time fix our issue: instead of canonicalizing the
> entire filename, I suggest that we only expand the directory prefix
> (ie the part returned by the "dirname" unix command).

Is this a complete solution?  That is, will it work in a situation 
slightly different from your, e.g., when one of the directories in the 
full file name is also a symlink?

Re: [RFC] gdb_realpath causes problems with GVD

[Daniel Jacobowitz]

On Tue, Mar 19, 2002 at 07:16:33PM +0200, Eli Zaretskii wrote:
> 
> On Tue, 19 Mar 2002, Joel Brobecker wrote:
> 
> > As you see, GDB has translated toto.c into toto.C. This translation
> > causes GDB to think that the inferior stopped in a file named toto.C
> > (which is not known to GDB, since the compiler used only toto.c).
> 
> Are you saying that gdb_realpath shouldn't follow symlinks?

Well, that it shouldn't follow file symlinks.

> > The translation is performed by gdb_realpath. I searched the gdb-patches
> > archives, and found the reason for this translation in a message from
> > Tom Tromey. I think I found a way to keep the fix to his problem and
> > then at the same time fix our issue: instead of canonicalizing the
> > entire filename, I suggest that we only expand the directory prefix
> > (ie the part returned by the "dirname" unix command).
> 
> Is this a complete solution?  That is, will it work in a situation 
> slightly different from your, e.g., when one of the directories in the 
> full file name is also a symlink?

I believe it will.  We'll have a canonical name for each directory a
source file was built out of; if the source file was a link, well, it's
still the name we were given for the source file.  Does that seem right
to you?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFC] gdb_realpath causes problems with GVD

[Joel Brobecker]

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

> I believe it will.  We'll have a canonical name for each directory a
> source file was built out of; if the source file was a link, well, it's
> still the name we were given for the source file.  Does that seem right
> to you?

To give an example, I used the following scenario:

  <prefix>/build
          /toplevel
             /symlink
               toto.C
               toto.c -> toto.C
          /toplevel_link -> toplevel

I then went inside <prefix>/build and compiled toto.c using the
following command:

  % gcc -g -o toto ../toplevel_link/symlinks/toto.c

I then retried the same test with GDB (break into break_me and
run until the bp is hit, to see the annotation):
<<
(gdb) set annotate 1
(gdb) b break_me
Breakpoint 1 at 0x804846b: file ../toplevel_link/symlinks/toto.c, line
4.
(gdb) run
Starting program: /bonn.a/brobecke/build/hello 

Breakpoint 1, break_me () at ../toplevel_link/symlinks/toto.c:4
yy/bonn.a/brobecke/toplevel/symlinks/toto.c:4:23:beg:0x804846b
>>

As you see, the directory name has been expanded, but not the filename.

And I also have to appologize. I sent a wrong patch, which is missing one
tiny line after the strncpy where I add a missing '\000'. I am attaching
the correct one this time.

-- 
Joel

[-- Attachment #2: utils.c.diff --]
[-- Type: text/plain, Size: 4495 bytes --]

Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.70
diff -c -3 -p -r1.70 utils.c
*** utils.c	2002/03/19 02:51:07	1.70
--- utils.c	2002/03/19 17:53:01
*************** string_to_core_addr (const char *my_stri
*** 2531,2558 ****
    return addr;
  }
  
- char *
- gdb_realpath (const char *filename)
- {
  #if defined(HAVE_REALPATH)
  # if defined (PATH_MAX)
!   char buf[PATH_MAX];
  #  define USE_REALPATH
  # elif defined (MAXPATHLEN)
!   char buf[MAXPATHLEN];
  #  define USE_REALPATH
  # elif defined (HAVE_UNISTD_H) && defined(HAVE_ALLOCA)
!   char *buf = alloca ((size_t)pathconf ("/", _PC_PATH_MAX));
  #  define USE_REALPATH
  # endif
  #endif /* HAVE_REALPATH */
  
! #if defined(USE_REALPATH)
!   char *rp = realpath (filename, buf);
!   return xstrdup (rp ? rp : filename);
! #elif defined(HAVE_CANONICALIZE_FILE_NAME)
!   return canonicalize_file_name (filename);
  #else
    return xstrdup (filename);
  #endif
  }
--- 2531,2641 ----
    return addr;
  }
  
  #if defined(HAVE_REALPATH)
  # if defined (PATH_MAX)
! static const int max_rp_buffer_size = PATH_MAX;
  #  define USE_REALPATH
  # elif defined (MAXPATHLEN)
! static const int max_rp_buffer_size = MAXPATHLEN;
  #  define USE_REALPATH
  # elif defined (HAVE_UNISTD_H) && defined(HAVE_ALLOCA)
! static const int max_rp_buffer_size = pathconf ("/", _PC_PATH_MAX);
  #  define USE_REALPATH
  # endif
  #endif /* HAVE_REALPATH */
+ 
+ /*
+  * gdb_canonicalize_path
+  *
+  * Sets RESOLVED_PATH to the canonicalized form of FILENAME, as realpath ()
+  * does. If there is no error, it returns a pointer to the resolved path.
+  * Otherwise, returns null.
+  *
+  * Returns null on platforms where no canonicalization routine (such as
+  * realpath for instance) are available.
+  */
+ static char *
+ gdb_canonicalize_path (const char *path, char *resolved_path)
+ {
+ #if defined (USE_REALPATH)
+   return realpath (path, resolved_path);
+   
+ #elif defined (HAVE_CANONICALIZE_FILE_NAME)
+   const char *canonicalized = canonicalize_file_name (path);
+ 
+   if (!canonicalized)
+     return null;
  
!   strcpy (resolved_path, canonicalized);
!   return resolved_path;
! 
  #else
+   return null;
+ 
+ #endif
+ }
+ 
+ /*
+  * gdb_realpath_fallback
+  *
+  * This is the fallback version of gdb_realpath when there is no
+  * canonicalization routine (such as realpath for instance) available.
+  * In this case, we simply return a copy of FILENAME.
+  */
+ static char *
+ gdb_realpath_fallback (const char *filename)
+ {
    return xstrdup (filename);
+ }
+ 
+ /*
+  * gdb_realpath
+  *
+  * Return a copy of FILENAME, with its directory prefix canonicalized,
+  * as for realpath() (see "man realpath" for more details on what 
+  * this function does), or simply a copy of FILENAME on platforms 
+  * where no canonicalization routine is available.
+  *
+  * We don't want to canonicalize the entire FILENAME, because
+  * the canonicalization routines used to perform the operation also
+  * expand symbolic links. If FILENAME is itself a link to another file,
+  * it can lead GDB to translate FILENAME into another filename that GDB
+  * does not know about. This can confuse graphical front-ends for GDB
+  * which, like GVD, rely on the filename displayed by GDB to get the
+  * current file:line location, for example.
+  */
+ char *
+ gdb_realpath (const char *filename)
+ {
+   const char *base_name = lbasename (filename);
+   char *dir_name;
+   char *buf;
+   char *rp = NULL;
+ 
+ #if !defined (USE_REALPATH) || !defined (HAVE_CANONICALIZE_FILE_NAME)
+   return gdb_realpath_fallback (filename);
  #endif
+ 
+   /* If basename and filename are equal, then there is no path to
+      canonicalize. Just return a copy of filename */
+   if (base_name == filename)
+     return xstrdup (filename);
+     
+   dir_name = alloca ((size_t) (base_name - filename + 1));
+   strncpy (dir_name, filename, base_name - filename);
+   dir_name[base_name - filename] = '\000';
+ 
+   /* Allocate enough space to contain the largest path possible returned
+      by realpath, plus the SLASH_STRING and the base_name */
+   buf = alloca ((size_t) max_rp_buffer_size +
+                 strlen (SLASH_STRING) + strlen (base_name));
+   rp = gdb_canonicalize_path (dir_name, buf);
+  
+   if (rp == NULL)
+     return xstrdup (filename);
+  
+   strcat (buf, SLASH_STRING);
+   strcat (buf, base_name);
+   return xstrdup (buf);
  }
+ 

Re: [RFC] gdb_realpath causes problems with GVD

[Eli Zaretskii]

> Date: Tue, 19 Mar 2002 12:33:57 -0500
> From: Daniel Jacobowitz <drow@mvista.com>
> 
> > Is this a complete solution?  That is, will it work in a situation 
> > slightly different from your, e.g., when one of the directories in the 
> > full file name is also a symlink?
> 
> I believe it will.  We'll have a canonical name for each directory a
> source file was built out of; if the source file was a link, well, it's
> still the name we were given for the source file.  Does that seem right
> to you?

I'm not sure yet.  My doubt stems from the fact that directories are
also recorded in the debug info, at least with some formats (stabs,
DWARF2).  One place in GDB where we use this is in file-name
completion, for example when you type "break /foo TAB" and want GDB
to complete this to "break /foobar/foo.c" (assuming that there's only
one file foo.c in that directory that was compiled into the program).

Will this break if symlinks are followed in the directory part, but
not in the file-name part?

Re: [RFC] gdb_realpath causes problems with GVD

[Joel Brobecker]

> I'm not sure yet.  My doubt stems from the fact that directories are
> also recorded in the debug info, at least with some formats (stabs,
> DWARF2).  One place in GDB where we use this is in file-name
> completion, for example when you type "break /foo TAB" and want GDB
> to complete this to "break /foobar/foo.c" (assuming that there's only
> one file foo.c in that directory that was compiled into the program).
> 
> Will this break if symlinks are followed in the directory part, but
> not in the file-name part?

It seems it does. With the same example where I built toto from
toplevel_link/symlink (reminder toplevel_link is a symbolic link to
toplevel), here is what is recoded in stabs,

<<
.stabs "/bonn.a/brobecke/toplevel_link/symlinks/",100,0,0,.Ltext0
                         ^^^^^^^^^^^^^
.stabs "toto.c",100,0,0,.Ltext0
>>

And here is a GDB session showing that with both paths:
<<
(gdb) b break_me
Breakpoint 1 at 0x8048433: file toto.c, line 4.
(gdb) set annotate 1
(gdb) run
Starting program: /bonn.a/brobecke/toplevel/symlinks/toto 

Breakpoint 1, break_me () at toto.c:4
yy/bonn.a/brobecke/toplevel/symlinks/toto.c:4:23:beg:0x8048433
(gdb) b /bonn.a/brobecke/toplevel/symlinks/toto.c:4
Note: breakpoint 1 also set at pc 0x8048433.
Breakpoint 2 at 0x8048433: file toto.c, line 4.
(gdb) b /bonn.a/brobecke/toplevel_link/symlinks/toto.c:5
Breakpoint 3 at 0x8048438: file toto.c, line 5.
>>

-- 
Joel

Re: [RFC] gdb_realpath causes problems with GVD

[Eli Zaretskii]

On Tue, 19 Mar 2002, Joel Brobecker wrote:

> > I'm not sure yet.  My doubt stems from the fact that directories are
> > also recorded in the debug info, at least with some formats (stabs,
> > DWARF2).  One place in GDB where we use this is in file-name
> > completion, for example when you type "break /foo TAB" and want GDB
> > to complete this to "break /foobar/foo.c" (assuming that there's only
> > one file foo.c in that directory that was compiled into the program).
> > 
> > Will this break if symlinks are followed in the directory part, but
> > not in the file-name part?
> 
> It seems it does.

I'm not sure I understand what are you saying: does completion work or 
does it break in that case?  I cannot figure out from your script
whether you tried completion, and if so, in what part of the script.

Re: [RFC] gdb_realpath causes problems with GVD

[Joel Brobecker]

> I'm not sure I understand what are you saying: does completion work or 
> does it break in that case?  I cannot figure out from your script
> whether you tried completion, and if so, in what part of the script.

Ah, sorry, I understand what you meant now. I have a sad news: filename
completion does not work in the case you mentionned, but this issue is
not related to my change. I tried with an unmodified GDB, and got the
same behavior. I also tried without a simple example without any
symbolic link, and it did not work either. It only completes if we enter
the filename without the directory prefix, with and without my change.
If this used to work, it looks like you spotted a regression!

Shall I go ahead and implement Andrew's suggestions?
-- 
Joel

Re: [RFC] gdb_realpath causes problems with GVD

[Eli Zaretskii]

On Wed, 20 Mar 2002, Joel Brobecker wrote:

> I have a sad news: filename
> completion does not work in the case you mentionned, but this issue is
> not related to my change. I tried with an unmodified GDB, and got the
> same behavior.

What version of GDB was that, and what debug info format does your 
compiler use?

> I also tried without a simple example without any
> symbolic link, and it did not work either. It only completes if we enter
> the filename without the directory prefix, with and without my change.

This certainly works for me, at least in GDB 5.1.1 with stabs and DWARF2 
debug info.  It doesn't work with COFF debug info, but that's because 
COFF doesn't record the leading directories, only the basename.

Running "objdump --debugging" on the executable should show whether full 
file names with leading directories are recorded or not.

> Shall I go ahead and implement Andrew's suggestions?

I don't have anything against Andrew's suggestions, but they only change 
minor (although important) clerical details.

I don't call the shots in this matter, but I'd certainly hate to lose the 
file-name completion due to this patch.  My suggestion would be to find 
a version of GDB where completion does work, and then see whether your 
changes have any adverse effect on it, before making the decision 
whether to accept the patch.

Re: [RFC] gdb_realpath causes problems with GVD

[Joel Brobecker]

> What version of GDB was that, and what debug info format does your 
> compiler use?

It was with GNU gdb 2002-03-19-cvs, and I used stabs. I verified in
the assembly file that the path is stored in the debug information, and
that objdump --debugging gives me the path as well.

> This certainly works for me, at least in GDB 5.1.1 with stabs and DWARF2 
> debug info.  It doesn't work with COFF debug info, but that's because 
> COFF doesn't record the leading directories, only the basename.

I tried it with several combinations of GCC, GDB, and debug format, and
did not get it to work until I changed the compilation command to
provide the fullpath of the file I was compiling:

    gcc -g -o toto <fullpath>/toto.c
    
In this case, completion works, and I verified that my change does not
break completion.

As an aside after all my experiments, it seems that the break command is
only using the information corresponding to the DW_AT_name entry (in
stabs, this is the second N_SO entry)...

Is there anything else that I missed regarding this file-completion
issue?
-- 
Joel

Re: [RFC] gdb_realpath causes problems with GVD

[Eli Zaretskii]

> Date: Wed, 20 Mar 2002 13:03:30 +0100
> From: Joel Brobecker <brobecker@ACT-Europe.FR>
> 
> I tried it with several combinations of GCC, GDB, and debug format, and
> did not get it to work until I changed the compilation command to
> provide the fullpath of the file I was compiling:
> 
>     gcc -g -o toto <fullpath>/toto.c
>     
> In this case, completion works, and I verified that my change does not
> break completion.

In that case, I have no objections to the patch.  Thanks for testing
this.

Re: [RFC] gdb_realpath causes problems with GVD

[Andrew Cagney]

> On Wed, 20 Mar 2002, Joel Brobecker wrote:
> 
> 
>> I have a sad news: filename
>> completion does not work in the case you mentionned, but this issue is
>> not related to my change. I tried with an unmodified GDB, and got the
>> same behavior.
> 
> 
> What version of GDB was that, and what debug info format does your 
> compiler use?
> 
> 
>> I also tried without a simple example without any
>> symbolic link, and it did not work either. It only completes if we enter
>> the filename without the directory prefix, with and without my change.
> 
> 
> This certainly works for me, at least in GDB 5.1.1 with stabs and DWARF2 
> debug info.  It doesn't work with COFF debug info, but that's because 
> COFF doesn't record the leading directories, only the basename.

If the auto-configury fails, gdb_realpath() returns xstrdup of the 
original string.  Since 5.1.1 people have been wacking that bit of 
configury and the corresponding function so that it ``works'' (as in 
implements realpath() semantics) on more platforms.  I think it is that 
``wacking'' that has revealed the regression - in 5.1.1 gdb_realpath() 
on your platform probably wasn't doing anything.

Joel's patch makes things slightly better - it stops the basename part 
from being messed up (what happens if there is no path?) - while neither 
helping nor hindering the regression.  It does need a testcase.  Since 
the completer has regressions, xfullpath() can't be indirectly tested 
using that -> need to directly test xfullpath() -> need the white-box 
directory -> stay tuned.

Joel, can you please create a testcase along the lines of 
gdb.base/selftest.exp that tests xfullpath() using gdb.  I'll find a 
directory for you to put it in!

--

This just leaves us with the regression.  The question here is should 
the path expansion be disabled or the problems fixed.

enjoy,
Andrew

Re: [RFC] gdb_realpath causes problems with GVD

[Joel Brobecker]

> Joel, can you please create a testcase along the lines of 
> gdb.base/selftest.exp that tests xfullpath() using gdb.  I'll find a 
> directory for you to put it in!

Wildo, it may take a bit of time though, because I've never had time to
actually look in details at how the testsuite works (shame on me).

-- 
Joel

Re: [RFC] gdb_realpath causes problems with GVD

[Andrew Cagney]

Joel,

I suspect you're right.

Two comments:

I think gdb_realpath() needs a re-name - it no longer implements 
realpath() semantics.  xfullpath() comes to mind (`x' for `xmalloc', 
`fullpath' cos it is close to realpath :-) (Better ideas? Does libiberty 
already provide an equivalent?)

I don't think trying to rewriting the gdb_realpath() autoconfigury magic 
is a good idea (the rewrite as two comile errors).  Instead just assume 
that gdb_realpath() works and use that, concat() and xfree().  I think 
the first rule of autoconf is, if it ain't `broke' don't fix it :-)(1)

enjoy,
Andrew

(1) Yes, I know, I've got that NULL canonicalize patch to commit :-)

Re: [RFC] gdb_realpath causes problems with GVD

[Joel Brobecker]

> I think gdb_realpath() needs a re-name - it no longer implements 
> realpath() semantics.  xfullpath() comes to mind (`x' for `xmalloc', 
> `fullpath' cos it is close to realpath :-) (Better ideas? Does libiberty 
> already provide an equivalent?)

I agree. I will wait to make sure everybody agrees to the change, and
to see if any other suggestion arises. I'll then perform the name
change.

> I don't think trying to rewriting the gdb_realpath() autoconfigury magic 
> is a good idea (the rewrite as two comile errors).  Instead just assume 
> that gdb_realpath() works and use that, concat() and xfree().

Here is how I understand your suggestion: Rewrite gdb_realpath which
will always "work", ie return a path.

And then implement xfullpath to use it in all cases:

char *
xfullpath (const char *filename)
{
  const char *base_name = [...];
  char *dir_name;
  char *rp;
  char *temp_result;
  char *result;

  dir_name = [...];
  rp = gdb_realpath (dir_name); /* rp will always be non-null */
  temp_result = concat (rp, SLASH_CHAR, base_name);
  result = xstrdup (temp_result);

  xfree (rp);
  xfree (temp_result);

  return result;
}
  

-- 
Joel

Re: [RFC] gdb_realpath causes problems with GVD

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@ACT-Europe.FR> writes:

Joel> The problem appears when some of the files used to build the
Joel> application are symbolic links and when GDB annotations are
Joel> activated.  For us, this happens when GVD, our graphical
Joel> front-end, drives GDB.

Joel> Breakpoint 1, break_me () at toto.c:4
Joel> yy/bonn.a/brobecke/symlinks/toto.C:4:23:beg:0x804846b

Joel> As you see, GDB has translated toto.c into toto.C. This
Joel> translation causes GDB to think that the inferior stopped in a
Joel> file named toto.C (which is not known to GDB, since the compiler
Joel> used only toto.c). As a consequence, when the user tries to put
Joel> breakpoints using the GVD, GVD issues the following break
Joel> command
Joel>    (gdb) break toto.C:4

gdb tells GVD that the file is "/bonn.a/brobecke/symlinks/toto.C".
Why does GVD then use just the base name?
Won't this be incorrect if there is more than one file with the same
name?  (The very problem my original patch was intended to fix.)
Wouldn't changing GVD to pass the exact same file name back to gdb
give correct results in every case?

Joel> I think I found a way to keep the fix to his problem and then at
Joel> the same time fix our issue: instead of canonicalizing the
Joel> entire filename, I suggest that we only expand the directory
Joel> prefix (ie the part returned by the "dirname" unix command).

I suspect this won't be correct in all cases, but I don't have a ready
counterexample.

Tom

Re: [RFC] gdb_realpath causes problems with GVD

[Joel Brobecker]

> gdb tells GVD that the file is "/bonn.a/brobecke/symlinks/toto.C".
> Why does GVD then use just the base name?

That's a very good point, and I'll followup with the GVD developers.

There is something in the break command that I haven't understood,
because:

      (gdb) b toto.C:5
      No source file named toto.C.
      (gdb) b /bonn.a/brobecke/toplevel/symlinks/toto.C:4
      Note: breakpoint 1 also set at pc 0x8048583.
      Breakpoint 2 at 0x8048583: file /bonn.a/brobecke/toplevel_link/symlinks/toto.c, line 4.

This seems odd to me that GDB refuses a breakpoint on toto.C, but accepts
a breakpoint on /bonn.a/.../toto.C? I also noticed an inconsistency in the
filename used in the "Breakpoint 2 at ..." line, should this be also
normalized?

> I suspect this won't be correct in all cases, but I don't have a ready
> counterexample.

It would be useful if you could find such a counter example, because it
would help me for future work in GDB to undertand if there was something
I missed.

Supposing that this problem can be corrected entirely in GVD, should I
withdraw my change request? I would still prefer GDB to display toto.c
rather than toto.C as the basename part, but I don't have a strong
opinion so the advice of all GDB developers would be welcome.

-- 
Joel

Re: [RFC] gdb_realpath causes problems with GVD

[Joel Brobecker]

> It would be useful if you could find such a counter example, because it
> would help me for future work in GDB to undertand if there was something
> I missed.

I think a counter example would definitely be helpful.

> Supposing that this problem can be corrected entirely in GVD, should I
> withdraw my change request? I would still prefer GDB to display toto.c
> rather than toto.C as the basename part, but I don't have a strong
> opinion so the advice of all GDB developers would be welcome.

Based on a discussion I had with the GVD developers, I think that
there is still some work needed to be done in GDB. When you start GVD on
a program, GVD does an "info sources" to get the list of files, and then
presents this list in a tree on which the user can click to view the
file, put breakpoints, etc.

The list looks like this:
<<
toto.c, ../wcsmbs/wchar.h, ../iconv/gconv.h, ../include/gconv.h, 
../sysdeps/unix/sysv/linux/i386/bits/wchar.h, 
>>

As you see, there is no path information regarding toto.c. In my
relatively modest experience, I find that the most common case of
compiler invocation is 

    "compiler <switches> <relative_path_or_no_path>/<filename>"

This is means that most of the time, info sources will print a list of
filenames with either relative path or no path at all. In order to
locate a file, GVD needs to ask GDB. This is what it does using
the "info line toto.c:1" command.

To which GDB answers "<some_path>/toto.C". But then, GDB refuses to put
a breakpoint on toto.C unless one uses the full path. GDB is
inconsistent here, and I really believe this inconsistency needs to be
fixed.

Either we modify GDB to be able to accept breakpoints on toto.C, or
we make sure toto.c is not translated into toto.C. I think the shortest
route is to avoid the toto.c -> toto.C translation.

Comments?
-- 
Joel

Re: [RFC] gdb_realpath causes problems with GVD

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@ACT-Europe.FR> writes:

Joel> Based on a discussion I had with the GVD developers, I think
Joel> that there is still some work needed to be done in GDB. When you
Joel> start GVD on a program, GVD does an "info sources" to get the
Joel> list of files, and then presents this list in a tree on which
Joel> the user can click to view the file, put breakpoints, etc.

Ok.

Joel> This is means that most of the time, info sources will print a
Joel> list of filenames with either relative path or no path at
Joel> all. In order to locate a file, GVD needs to ask GDB. This is
Joel> what it does using the "info line toto.c:1" command.

Yes.  The problem here is that gdb is giving an absolute path to GVD.
GVD then sends just the basename back to gdb.  This doesn't really
make sense.

I think GVD has two choices.

First, it could use the path information as given by info sources.
This is just the paths given in the symtab.

Second, it could use the path information as printed by `b'.

Instead, GVD is taking the basename of the path in question.  That is
a questionable operation.  What if there are two files with the same
base name?  This is the scenario that lead to the patch in the first
place.

Unfortunately I think Insight also falls down here.  I haven't looked
into it recently though.

Perhaps GVD is doing what it does in order to be compatible with older
versions of gdb.  I can sympathize with that.  I suspect that using
the full name as printed by `info sources' will work with any version
of gdb, though I have not tested that theory.  Another choice would be
for GVD to use the gdb version number to decide whether it should send
base- or full-names.


BTW, I finally remembered the logic behind the apparent
lookup_symtab() weirdness.

Users want "b foo.c:57" to "just work".  If there's only one foo.c in
your program, this is unambiguous.  In this situation we have to get
the debugger to find foo.c for us.  We can't use realpath information,
because we don't have a base directory to start from.  So we search
through the info in the symtab to find the file.

On the other hand, debugger UIs already know an absolute path to a
file.  If the program has multiple files with the same base name, you
want your UI to always set a breakpoint (e.g., in response to a mouse
click in the source window) in the correct file.  This is where the
realpath information comes in -- both gdb and the UI can find an
absolute path to the file, but they might be different.  If you just
use the base name here then you wind up in a confusing, ambiguous
situation; see my original posts for real examples.

Joel> To which GDB answers "<some_path>/toto.C". But then, GDB refuses
Joel> to put a breakpoint on toto.C unless one uses the full path. GDB
Joel> is inconsistent here, and I really believe this inconsistency
Joel> needs to be fixed.

You could modify lookup_symtab() to also check against the base name
of the realpath.  That would work, although it would add the
possibility of more ambiguous choices.  Anyway, as I've said earlier I
think the primary problem here is in GVD.

Joel> Either we modify GDB to be able to accept breakpoints on toto.C,
Joel> or we make sure toto.c is not translated into toto.C. I think
Joel> the shortest route is to avoid the toto.c -> toto.C translation.

Suppose I'm editing this file in Emacs.  Emacs, at least as I have it
configured, likes to follow symlinks.  So Emacs thinks the file is
"/a/b/toto.C".

In the symtab we have "toto.c".  We compiled it in the directory "/c/d"
using `gcc -c -g toto.c'.

Now in Emacs I use C-x SPC to set a breakpoint in toto.C.  Emacs
(well, a hacked version that works with this feature) sends:

    b /a/b/toto.C:57

Currently gdb will search through the symtabs.  It will see something
`toto.c' and use realpath to turn it into /a/b/toto.C.  And since
this is equal to the argument to `b', the breakpoint will be set.

With your proposed change, when we're searching through the symtabs
we'll find `toto.c' and turn it into `/a/b/toto.c'.  This isn't equal
to the argument to `b', so gdb will give an error.

At least, that's what I think.  I haven't tried your patch.  What do
you think of this?

Tom

Re: [RFC] gdb_realpath causes problems with GVD

[Joel Brobecker]

> Yes.  The problem here is that gdb is giving an absolute path to GVD.
> GVD then sends just the basename back to gdb.  This doesn't really
> make sense.

Right, and the GVD team is now aware of this issue. Let's now leave GVD
out of the picture, as it was the tool that uncovered the issue, but the
issue is actually independent from GVD.

> Suppose I'm editing this file in Emacs.  Emacs, at least as I have it
> configured, likes to follow symlinks.  So Emacs thinks the file is
> "/a/b/toto.C".
> 
> In the symtab we have "toto.c".  We compiled it in the directory "/c/d"
> using `gcc -c -g toto.c'.
> 
> Now in Emacs I use C-x SPC to set a breakpoint in toto.C.  Emacs
> (well, a hacked version that works with this feature) sends:
> 
>     b /a/b/toto.C:57
> 
> Currently gdb will search through the symtabs.  It will see something
> `toto.c' and use realpath to turn it into /a/b/toto.C.  And since
> this is equal to the argument to `b', the breakpoint will be set.
> 
> With your proposed change, when we're searching through the symtabs
> we'll find `toto.c' and turn it into `/a/b/toto.c'.  This isn't equal
> to the argument to `b', so gdb will give an error.

I think it is wrong to force the user to follow links (I'm trying to
find a not so strong way to voice my opinion, but not being a native
speaker I can't find any). There are some development systems where the
links are automagically, and the user sometimes does not know about
them.

Let me suggest the following: 
  - Use xfullpath when printing the filename in "info line"
  - Try both xfullpath and then  gdb_realpath when setting breakpoints.

That way, we remain consistent between the filenames known to the user,
the filenames displayed by GDB, but at the same time being lenient in
what we accept.

What do you think? Would that work for you?
-- 
Joel

Re: [RFC] gdb_realpath causes problems with GVD

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@ACT-Europe.FR> writes:

Joel> I think it is wrong to force the user to follow links (I'm
Joel> trying to find a not so strong way to voice my opinion, but not
Joel> being a native speaker I can't find any).

I don't understand.

The way things stand right now, if the user uses an absolute path,
then realpath is used to determine whether the file matches.  Note
that any absolute path will work here.

If the user provides a relative path -- which is indeed what most
humans, as opposed to GUI wrappers, actually do -- then the realpath
stuff isn't used.

So users can always use the file name that they know (the one that is
in their Makefile), and gdb will understand.  I don't see when a user
would be forced to follow a link.

The case that is causing you problems is when gdb prints the real file
name, and then the user tries to set a breakpoint using just the
basename of that file name.  This seems like an unlikely scenario to
me.

Joel> Let me suggest the following: 
Joel>   - Use xfullpath when printing the filename in "info line"
Joel>   - Try both xfullpath and then  gdb_realpath when setting breakpoints.

Joel> That way, we remain consistent between the filenames known to the user,
Joel> the filenames displayed by GDB, but at the same time being lenient in
Joel> what we accept.

Joel> What do you think? Would that work for you?

If it doesn't affect the feature I care about, then it doesn't matter
to me.  The feature in question is having a way to tell gdb
unambiguously which file a breakpoint should appear in.  The current
mechanism for this is to use an absolute path.

Another choice would be to compare what the user types in against the
basename of the realpath.  This isn't any more ambiguous than what
already goes on in the relative-path case.  This would work around the
GVD bug, and it wouldn't affect the full-path case.

Tom

Re: [RFC] gdb_realpath causes problems with GVD

[Joel Brobecker]

> I don't understand.

We both agree that the most common case is currently correctly handled
by GDB. The cases I am concerned about are corner-cases, but they did
happen, and I would like GDB to remain consistent: if it tells me about
file bla.C, I expect it to know about bla.C later, even if I don't
provide the path to this file.

> If it doesn't affect the feature I care about, then it doesn't matter
> to me. 

Good, because this is exactly what I was suggesting: make GDB consistent
using the new xfullpath function, but at the same time not breaking your
important feature by makin GDB lenient enough to accept the following
syntaxes:
  1/ break toto.c:1
  2/ break /<fullpath>/toto.c:1
  3/ break /<fullpath>/toto.C:1
where fullpath can be resolved but does not need to.

  - break toto.C:1 is still not accepted.

> The feature in question is having a way to tell gdb unambiguously
> which file a breakpoint should appear in.  The current mechanism for
> this is to use an absolute path.

Well actually, I actually broke this feature if you follow links, that
it is was still working using syntax 2 of the above, so to my defense I
only broke half of this feature :-).

Fortunately, with my latest change, GDB is now printing the "correct"
file name, and all 3 syntaxes are accepted by GDB. You seem OK with
that, but I'd like to have other people's opinion.

BTW: I have the patch ready for submission. I need to work on a new
testcase before I submit it, but the current testsuite already shows one
extra PASS. I need to find a tool to analyze 2 logs that tell me what
has changed...

-- 
Joel

Re: [RFC] gdb_realpath causes problems with GVD

[Andrew Cagney]

> BTW: I have the patch ready for submission. I need to work on a new
> testcase before I submit it, but the current testsuite already shows one
> extra PASS. I need to find a tool to analyze 2 logs that tell me what
> has changed...

Er, if the patch as is fixes a an existing test failure, I don't see 
anyone worrying about formalities such as a new test case :-)

Andrew

Re: [RFC] gdb_realpath causes problems with GVD

[Daniel Jacobowitz]

On Wed, Mar 27, 2002 at 10:34:43PM -0500, Andrew Cagney wrote:
> >BTW: I have the patch ready for submission. I need to work on a new
> >testcase before I submit it, but the current testsuite already shows one
> >extra PASS. I need to find a tool to analyze 2 logs that tell me what
> >has changed...

Diff, by the way - on the gdb.sum files, it's fairly high signal-noise.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFC] gdb_realpath causes problems with GVD

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@ACT-Europe.FR> writes:

Joel> There is something in the break command that I haven't
Joel> understood, because:

Joel>       (gdb) b toto.C:5
Joel>       No source file named toto.C.
Joel>       (gdb) b /bonn.a/brobecke/toplevel/symlinks/toto.C:4
Joel>       Note: breakpoint 1 also set at pc 0x8048583.
Joel>       Breakpoint 2 at 0x8048583: file /bonn.a/brobecke/toplevel_link/symlinks/toto.c, line 4.

Joel> This seems odd to me that GDB refuses a breakpoint on toto.C,
Joel> but accepts a breakpoint on /bonn.a/.../toto.C?

If you give an absolute path to the break command, then gdb will try
to find and use the real path.

If you give a relative path to the break command, gdb won't do this.
It will just compare what you gave it to what is in the symtab.  It
might compare the base name if you didn't give an exact match.

See symtab.c:lookup_symtab() for the code.

Joel> I also noticed an inconsistency in the filename used in the
Joel> "Breakpoint 2 at ..." line, should this be also normalized?

I don't know.

Joel> Supposing that this problem can be corrected entirely in GVD,
Joel> should I withdraw my change request?

My opinion is that the primary bug here is in GVD.  However there are
subsidiary bugs as well.

Joel> I would still prefer GDB to display toto.c rather than toto.C as
Joel> the basename part, but I don't have a strong opinion so the
Joel> advice of all GDB developers would be welcome.

I agree it would be best if gdb could somehow tell GVD about the file
name as it appears in the symtab.  Then GVD could display this file
name if it so chose.

My reasoning here is that the file name in the symtab is most likely
the one the user will recognize, since it is the one that he (or the
Makefile or whatever) passed to the compiler.

Tom