Re: [patch 0/3] Support .dwp with the name of symlinked binary file

[Doug Evans <dje at google dot com>]

Jan Kratochvil writes:
 > Hi Doug,
 > 
 > this patch is made upon Doug's request where they use this case:
 > 	$ ../gdb gdb.base/start-symlink 
 > 	gdb.base/start*
 > 	gdb.base/start-symlink -> start*
 > 	gdb.base/start-symlink.dwp

More specifically,
bin/start -> /foo/abc/def
bin/start.dwp -> /foo/ghi/jkl

 > I was proposing to symlink also all .dwp files in such case, for example
 > Fedora symlinks all .debug files - but Doug did not accept that:
 > -rwxr-xr-x 1 root root  92560 Feb 19  2013 /lib64/libz.so.1.2.7*
 > lrwxrwxrwx 1 root root     13 Aug 19 07:32 /lib64/libz.so.1 -> libz.so.1.2.7*
 > -r--r--r-- 1 root root 167247 Feb 19  2013 /usr/lib/debug/lib64/libz.so.1.2.7.debug
 > lrwxrwxrwx 1 root root     19 Aug 19 08:03 /usr/lib/debug/lib64/libz.so.1.debug -> libz.so.1.2.7.debug

I couldn't use it because I can't change what's provided to gdb.
There's an underlying abstraction layer implemented with symlinks,
and there's not necessarily a useful relationship between what's above
the boundary and what's below it.

 > This patchset depends on:
 > 	Re: [patch] [7.6.1] Fix argv[0] symlink regression (PR 15415)
 > 	https://sourceware.org/ml/gdb-patches/2013-08/msg00789.html
 > 	Message-ID: <20130827140915.GA17861@host2.jankratochvil.net>
 > 
 > 
 > GDB 7.6 got .dwp support
 > (I have tested c2f14511388ab029f3bda0f5227eab67e04daac5)
 > and it worked for:
 > 	$ ../gdb gdb.base/start-symlink 
 > 	gdb.base/start*
 > 	gdb.base/start-symlink -> start*
 > 	gdb.base/start-symlink.dwp
 > although it did not work for:
 > 	$ ../gdb gdb.base/start-symlink 
 > 	gdb.base/start*
 > 	gdb.base/start-symlink -> start*
 > 	gdb.base/start.dwp
 > 
 > Then there was a patch:
 > commit bfacf227ec8ee6b1c73311e323bd93c1eddd9ca6
 > Author: Jan Kratochvil <jan.kratochvil@redhat.com>
 > Date:   Sun Feb 3 15:54:14 2013 +0000
 >         Replace xfullpath calls by gdb_realpath calls.
 > 
 > and the functionality has reversed, it no longer worked for:
 > 	$ ../gdb gdb.base/start-symlink 
 > 	gdb.base/start*
 > 	gdb.base/start-symlink -> start*
 > 	gdb.base/start-symlink.dwp
 > while it started to work for:
 > 	$ ../gdb gdb.base/start-symlink 
 > 	gdb.base/start*
 > 	gdb.base/start-symlink -> start*
 > 	gdb.base/start.dwp
 > 
 > This issue affects only DWP due to
 > dwarf2read.c:open_and_init_dwp_file():
 >   dwp_name = xstrprintf ("%s.dwp", dwarf2_per_objfile->objfile->name);
 > 
 > and it does not affect .debug files as their filename is stored in
 > .gnu_debuglink.
 > 
 > This patchset can be considered as an extension of the existing released FSF
 > GDB 7.6 DWP functionality, it is not fixing any FSF release regression as 
 > GDB 7.5 did not have DWP support.
 > 
 > With xfullpath before the BFD cache was less efficient (cache misses for
 > symlinked files with different basename).  Nowadays with gdb_realpath one
 > cannot undo it, therefore I have moved the gdb_realpath() call from openp() to
 > gdb_bfd_open() so that GDB code still knows the original filename before
 > gdb_realpath().
 > 
 > This patch has negative performance impact: (1) openp() is called less times
 > than gdb_bfd_open() so gdb_realpath() will be now called in more cases.
 > (2) As gdb_realpath() call is left in most of the openp() calls (due to
 > OPF_RETURN_REALPATH set there) gdb_realpath() will be now called twice.
 > These cases could be optimized, see /home/user/file below.

For reference sake,
several instances that could measurably impact performance (because
there can be a lot of them in one session, e.g., solibs, fission dwo files)
call openp and pass the descriptor to gdb_bfd_open.
Calling openp for dwo files doesn't need realpath.
Can we remove OPF_RETURN_REALPATH when doing openp (solib) ? [ref: solib_find]
We kind of have to to fix the dwp problem for solibs.

 > IMO the real problem is that gdb_realpath() is currently written very
 > ineffectively.  It could cache most of the stat() results even for different
 > filenames as most of their parent directories are the same.  Such optimization
 > is not in this patchset.

Right.
I know of at least one implementation of this optimization.
I'm not sure what the status of it is, but it's easy enough to do over.

 > In fact all the gdb_realpath() calls are IMO a bug, it is more convenient to
 > see I am debugging /home/user/file than to see it is /.sdb5/home/user/file
 > (due to common 'ln -s .sdb5/home /home' joining of partitions etc.).
 > As this goes against some GDB goal I do not understand I have not changed it
 > everywhere yet.  Still the [patch 3/3] already makes such change for objfiles
 > and exec file, such as in "info files" (but not for source files).

Agreed.