This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 0/3] Support .dwp with the name of symlinked binary file
- From: Doug Evans <dje at google dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Tue, 3 Sep 2013 15:01:09 -0700
- Subject: Re: [patch 0/3] Support .dwp with the name of symlinked binary file
- Authentication-results: sourceware.org; auth=none
- References: <20130828160529 dot GA23977 at host2 dot jankratochvil dot net>
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.