[PATCH, v2] Add directory containing executable to relative search paths for .dwo files.
Caroline Tice
cmtice@google.com
Thu Mar 4 15:48:37 GMT 2021
Hi Andrew,
I updated my change to gdb/dwarf2/read.c as you suggested (spaces
before '(' and use nullptr).
I spent a long time looking into using Dwarf::assemble in my testcase,
to make it hardware agnostic (although I'd like to point out
that most of the rest of the fission tests specifically only work for
x86_64), but in the end found that it would not work for me. The
problems that I was unable to make this work for was that I needed to
generate not only the binary but the .dwo file; I needed reasonably
valid DWARF in both of them; and the path name for the .dwo file had
to be relative not absolute -- the way gdb compiles the test suites it
always uses complete absolute paths for all of its files.
As an alternative I created 4 different versions of the test: x86_64,
i386, arm32 and aarch64. I have attached a new version of the patch
with these changes.
Is this ok to commit now?
-- Caroline
cmtice@google.com
gdb/ChangeLog
2021-03-04 Caroline Tice <cmtice@google.com>
* dwarf2/read.c (try_open_dwop_file): Add path for the binary
to the search paths used resolve relative location of .dwo file.
gdb/testsuite/ChangeLog
2021-03-04 Caroline Tice <cmtice@google.com>
* gdb.dwarf2/fission-relative-dwo.c: New file.
* gdb.dwarf2/fission-relative-dwo-x86_64.S: New file.
* gdb.dwarf2/fission-relative-dwo-x86_64.exp: New file.
* gdb.dwarf2/fission-relative-dwo-i386.S: New file.
* gdb.dwarf2/fission-relative-dwo-i386.exp: New file.
* gdb.dwarf2/fission-relative-dwo-aarch64.S: New file.
* gdb.dwarf2/fission-relative-dwo-aarch64.exp: New file.
* gdb.dwarf2/fission-relative-dwo-arm32.S: New file.
* gdb.dwarf2/fission-relative-dwo-arm32.exp: New file.
On Wed, Mar 3, 2021 at 1:53 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>
> * Caroline Tice via Gdb-patches <gdb-patches@sourceware.org> [2021-03-02 16:42:08 -0800]:
>
> > DWARF allows .dwo file paths to be relative rather than absolute. When
> > they are relative, DWARF uses DW_AT_comp_dir to find the .dwo file.
> > DW_AT_comp_dir can also be relative, making the entire search patch
> > for the .dwo file relative. In this case, GDB currently searches
> > relative to its current working directory, i.e. the directory from
> > which the debugger was launched, but not relative to the directory
> > containing the built binary. This cannot be right, as the compiler,
> > when generating the relative paths, knows where it's building the
> > binary but can have no idea where the debugger will be launched. The
> > correct thing is to add the directory containing the binary to the
> > search paths used for resolving relative locations of dwo files. That
> > is what this patch does.
> >
> > I have run this through the regression testsuite with no regressions,
> > and I have verified that my new testcase passes with
> > this patch and fails without it.
> >
> > Is this patch OK to commit?
> >
> > -- Caroline Tice
> > cmtice@google.com
> >
> > gdb/ChangeLog
> >
> > 2021-03-02 Caroline Tice <cmtice@google.com>
> >
> > * dwarf2/read.c (try_open_dwop_file): Add path for the binary
> > to the search paths used resolve relative location of .dwo file.
> >
> > gdb/testsuite/ChangeLog
> >
> > 2021-03-02 Caroline Tice <cmtice@google.com>
> >
> > * gdb.dwarf2/fission-relative-dwo.c: New file.
> > * gdb.dwarf2/fission-relative-dwo.S: New file.
> > * gdb.dwarf2/fission-relative-dwo.exp: New file.
>
> > From 83056b931839107f131c0059377a011876df051b Mon Sep 17 00:00:00 2001
> > From: Caroline Tice <cmtice@google.com>
> > Date: Tue, 2 Mar 2021 16:15:54 -0800
> > Subject: [PATCH] Add directory containing executable to relative search paths
> > for .dwo files.
> >
> > DWARF allows .dwo file paths to be relative rather than absolute. When
> > they are relative, DWARF uses DW_AT_comp_dir to find the .dwo
> > file. DW_AT_comp_dir can also be relative, making the entire search
> > patch for the .dwo file relative. In this case, GDB currently searches
> > relative to its current working directory, i.e. the directory from
> > which the debugger was launched, but not relative to the directory
> > containing the built binary. This cannot be right, as the compiler,
> > which generating the relative paths, knows where it's building the
> > binary but can have no idea where the debugger will be launched. The
> > correct thing is to add the directory containing the binary to the
> > search paths used for resolving relative locations of dwo files. That
> > is what this patch does.
> > ---
> > gdb/dwarf2/read.c | 6 +
> > .../gdb.dwarf2/fission-relative-dwo.S | 407 ++++++++++++++++++
> > .../gdb.dwarf2/fission-relative-dwo.c | 10 +
> > .../gdb.dwarf2/fission-relative-dwo.exp | 52 +++
> > 4 files changed, 475 insertions(+)
> > create mode 100644 gdb/testsuite/gdb.dwarf2/fission-relative-dwo.S
> > create mode 100644 gdb/testsuite/gdb.dwarf2/fission-relative-dwo.c
> > create mode 100644 gdb/testsuite/gdb.dwarf2/fission-relative-dwo.exp
> >
> > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> > index d4f13229ced..7cac5027d41 100644
> > --- a/gdb/dwarf2/read.c
> > +++ b/gdb/dwarf2/read.c
> > @@ -12809,6 +12809,12 @@ try_open_dwop_file (dwarf2_per_objfile *per_objfile,
> > else
> > search_path = debug_file_directory;
> >
> > + /* Add the path for the executable binary to the list of search paths. */
> > + search_path_holder.reset(
> > + concat(ldirname(per_objfile->objfile->original_name).c_str(),
> > + dirname_separator_string, search_path, (char *) NULL));
> > + search_path = search_path_holder.get();
>
> Missing whitespace before many of the '(' in this code. Also I think
> you should replace '(char *) NULL' with just 'nullptr'.
>
> > +
> > openp_flags flags = OPF_RETURN_REALPATH;
> > if (is_dwp)
> > flags |= OPF_SEARCH_IN_PATH;
> > diff --git a/gdb/testsuite/gdb.dwarf2/fission-relative-dwo.c b/gdb/testsuite/gdb.dwarf2/fission-relative-dwo.c
> > new file mode 100644
> > index 00000000000..4c5f0b88b0b
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.dwarf2/fission-relative-dwo.c
> > @@ -0,0 +1,10 @@
> > +
> > +int main (int argc, char **argv)
> > +{
> > + int x = 10;
> > + char y[3] = { 'a', 'b', 'c' };
> > +
> > + x += (int) y[0] + (int) y[1];
> > +
> > + return 0;
> > +}
>
> Test source files should include a copyright header, and should follow
> GNU/GDB coding standard, unless the deviation is critical for the test
> (which is not the case here).
>
>
> > diff --git a/gdb/testsuite/gdb.dwarf2/fission-relative-dwo.exp b/gdb/testsuite/gdb.dwarf2/fission-relative-dwo.exp
> > new file mode 100644
> > index 00000000000..3d0b9b9fb5f
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.dwarf2/fission-relative-dwo.exp
> > @@ -0,0 +1,52 @@
> > +# Copyright 2013-2021 Free Software Foundation, Inc.
>
> The from date should represent the first time you posted this patch to
> the mailing list. I didn't check to see if this patch was posted in
> 2013.
>
> > +
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 3 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> > +
> > +load_lib dwarf.exp
> > +
> > +# We run objcopy locally to split out the .dwo file.
> > +if [is_remote host] {
> > + return 0
> > +}
> > +
> > +# This test can only be run on targets which support DWARF-2 and use gas.
> > +if ![dwarf2_support] {
> > + return 0
> > +}
> > +
> > +# This test can only be run on x86-64 targets.
> > +if {![istarget x86_64-*] || ![is_lp64_target]} {
> > + return 0
> > +}
> > +
> > +standard_testfile .S
> > +
> > +set dwo [standard_output_file "fission-relative-dwo.dwo"]
> > +
> > +if [build_executable_from_fission_assembler \
> > + "$testfile.exp" "$binfile" "$srcfile" \
> > + [list nodebug additional_flags=-DDWO=$dwo]] {
> > + return -1
> > +}
>
> Rather than using a fixed assembler file I would like to suggest you
> write this test using GDB's DWARF assembler functionality. This will
> allow the test to run for any target, not just x86-64.
>
> It's certainly easy to set the DW_AT_comp_dir to any value you need
> using that tool. From your patch description I don't know what else
> you would need to modify in the DWARF, maybe nothing?
>
> If you look for .exp files containing 'Dwarf::assemble' you'll find
> loads of examples that you can draw inspiration from, but feel free to
> ask if you have specific questions.
>
> > +
> > +clean_restart $binfile
> > +
> > +if ![runto_main] {
> > + return -1
> > +}
> > +
> > +# Verify gdb can find argc.
> > +
> > +gdb_test "p argc" " = 1"
>
> I guess you're looking for argc just to prove that GDB has found the
> DWO file.
>
> If you switch to GDB's DWARF assembler then generating debug
> information to find a local variable is harder. I would suggest that
> instead, you make the test file something like this:
>
> int global_var;
>
> int
> main (void)
> {
> asm ("main_label: .globl main_label");
> return 0;
> }
>
> Then in the generated DWARF give global_var a typedef type, your test
> then becomes something like:
>
> gdb_test "ptype global_var" "type = ....."
>
> Thanks,
> Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v2-0001-Add-directory-containing-executable-to-relative-s.patch
Type: application/octet-stream
Size: 41066 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/gdb-patches/attachments/20210304/ce60ec6a/attachment-0001.obj>
More information about the Gdb-patches
mailing list