Re: [PATCH, v2] Add directory containing executable to relative search paths for .dwo files.

[Caroline Tice <cmtice@google.com>]

Hi Andrew,

As I said previously, your suggested patch works well and fixes the
problem.  I thought you were going to submit it, but I have not seen it  go
out for review or be committed.  Is there something else you need me to
do?  I would be happy to send your version for review if you would like?

-- Caroline
cmtice@google.com


On Wed, Mar 17, 2021 at 5:26 PM Andrew Burgess <andrew.burgess@embecosm.com>
wrote:

> * Caroline Tice <cmtice@google.com> [2021-03-04 07:48:37 -0800]:
>
> > 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),
>
> Most of these were written before the Dwarf assembler was available.
> If they were being written today then we'd be using the Dwarf
> assembler.
>
> >           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.
>
> I don't understand what problem you ran into here.  The path to the
> DWO file is encoded into the DWARF of the executable in a
> DW_AT_GNU_dwo_name attribute, which would be entirely generated within
> the .exp file, and so could contain whatever you like.  Likewise for
> the DW_AT_comp_dir.
>
> I made an attempt at writing a test using the dwarf assembler.  You
> will need to start by applying this patch to a clean GDB tree:
>
>    https://sourceware.org/pipermail/gdb-patches/2021-March/177049.html
>
> Then grab the patch below and give it a try.  It includes your fix,
> and a brand new test case.  Let me know if you don't think this is
> covering your change correctly, I'm sure we can get it working as
> needed.
>
> Thanks,
> Andrew
>
> ---
>
> commit 47bf19a7ed4c96a86d67329efb7e5ce983debddd
> Author: Andrew Burgess <andrew.burgess@embecosm.com>
> Date:   Wed Mar 17 22:54:22 2021 +0000
>
>     gdb: handle relative paths to DWO files
>
>     **** NOTE ****
>
>     The fix in dwarf2/read.c includes a hack to allow the fix to be
>     toggled on/off using environment variable GDB_NO_FIX.  This must be
>     removed before pushing upstream.
>
>     **** END NOTE ****
>
>     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.
>
>     gdb/ChangeLog:
>
>             * 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:
>
>             * gdb.dwarf2/fission-relative-dwo.c: New file.
>             * gdb.dwarf2/fission-relative-dwo.exp: New file.
>             * lib/dwarf.exp (extract_dwo_information): New proc.
>             (strip_dwo_information): New proc.
>             (build_executable_from_fission_assembler): Use
>             extract_dwo_information and strip_dwo_information.
>             (Dwarf::_debug_addr_index): New variable.
>             (Dwarf::_cu_is_fission): New variable.
>             (Dwarf::_handle_DW_FORM): Handle DW_OP_GNU_addr_index.
>             (Dwarf::_default_form): Supply a default for
> DW_AT_GNU_addr_base.
>             (Dwarf::_handle_macro_at_range): Use form
> DW_FORM_GNU_addr_index
>             if this is a fission CU.
>             (Dwarf::_location): Handle DW_OP_GNU_addr_index.
>             (Dwarf::debug_addr_index): New proc.
>             (Dwarf::cu): Initialise _cu_is_fission.
>             (Dwarf::tu): Likewise.
>             (Dwarf::assemble): Initialise _debug_addr_index.
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index d6881300fa6..522ccc2506e 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -12581,6 +12581,15 @@ try_open_dwop_file (dwarf2_per_objfile
> *per_objfile,
>    else
>      search_path = debug_file_directory;
>
> +  if (getenv ("GDB_NO_FIX") == nullptr)
> +    {
> +      /* 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, nullptr));
> +      search_path = search_path_holder.get ();
> +    }
> +
>    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..27f7f0dfb4b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/fission-relative-dwo.c
> @@ -0,0 +1,28 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2021 Free Software Foundation, Inc.
> +
> +   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/>.  */
> +
> +/* Our fake object.  */
> +int global_var[100];
> +
> +int
> +main (int argc, char **argv)
> +{
> +  asm ("main_label: .globl main_label");
> +
> +  return 0;
> +}
> 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..f9ab3b8c6a4
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/fission-relative-dwo.exp
> @@ -0,0 +1,145 @@
> +# Copyright 2021 Free Software Foundation, Inc.
> +
> +# 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
> +
> +# This test can only be run on targets which support DWARF-2 and use gas.
> +if {![dwarf2_support]} {
> +    return 0
> +}
> +
> +standard_testfile .c -dw.S
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
> +    return -1
> +}
> +
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    global srcfile gdb_test_file_name
> +
> +    set debug_addr_base -1
> +
> +    # The information that will be split out into the .dwo file.
> +    cu {fission 1} {
> +
> +       # Capture the current index into .debug_addr so we can fill in
> +       # DW_AT_GNU_addr_base later.
> +       set debug_addr_base [debug_addr_index]
> +
> +       compile_unit {
> +            {language @DW_LANG_C}
> +            {name ${srcfile}}
> +           {DW_AT_comp_dir .}
> +           {DW_AT_GNU_dwo_id 0x1234 DW_FORM_data8}
> +        } {
> +           declare_labels int4_type struct_type
> +
> +           int4_type: DW_TAG_base_type {
> +               {DW_AT_byte_size 4 DW_FORM_sdata}
> +               {DW_AT_encoding  @DW_ATE_signed}
> +               {DW_AT_name      integer}
> +           }
> +
> +           struct_type: DW_TAG_structure_type {
> +               {DW_AT_name "foo_t"}
> +               {DW_AT_byte_size 12 DW_FORM_sdata}
> +           } {
> +               member {
> +                   {name "aa"}
> +                   {type :$int4_type}
> +                   {data_member_location 0 data1}
> +               }
> +               member {
> +                   {name "bb"}
> +                   {type :$int4_type}
> +                   {data_member_location 4 data1}
> +               }
> +               member {
> +                   {name "cc"}
> +                   {type :$int4_type}
> +                   {data_member_location 8 data1}
> +               }
> +           }
> +
> +           DW_TAG_variable {
> +               {DW_AT_name global_var}
> +               {DW_AT_type :$struct_type}
> +               {DW_AT_location {
> +                   DW_OP_GNU_addr_index [gdb_target_symbol global_var]
> +               } SPECIAL_expr}
> +               {external 1 flag}
> +           }
> +
> +           subprogram {
> +               {external 1 flag}
> +               {DW_AT_name main DW_FORM_string}
> +               {MACRO_AT_func {main}}
> +           }
> +       }
> +    }
> +
> +    # The information that will remain in the .o file.
> +    cu {} {
> +       compile_unit {
> +            {DW_AT_GNU_dwo_name ${gdb_test_file_name}.dwo DW_FORM_strp}
> +           {DW_AT_comp_dir .}
> +           {DW_AT_GNU_dwo_id 0x1234 DW_FORM_data8}
> +           {DW_AT_GNU_addr_base $debug_addr_base}
> +        } {
> +           # Nothing.
> +       }
> +    }
> +}
> +
> +# Assemble the file containing the .dwo information, we can then split
> +# this out into a separate .dwo file.  Once we've extracted the DWO
> +# information from this object file we're done with it, we recompile
> +# this again in the BUILD_EXECUTABLE call below.
> +set object_file [standard_output_file ${binfile}-dwo.o]
> +if { [gdb_compile $asm_file $object_file object {nodebug}] != "" } {
> +    return -1
> +}
> +
> +# Split out the dwo sections into a dwo file.
> +if { [extract_dwo_information $object_file ${binfile}.dwo] == -1 } {
> +    perror "failed to extract dwo information"
> +    return -1
> +}
> +
> +# Build the executable, but don't use prepare_for_testing as we don't
> +# want to start GDB just yet.
> +if { [build_executable "failed to prepare" "${testfile}" \
> +         [list $srcfile $asm_file] {nodebug}] } {
> +    return -1
> +}
> +
> +# Just so we can't be found cheating, strip all the dwo information
> +# from the executable.
> +if { [strip_dwo_information ${binfile}] == -1 } {
> +    perror "failed to strip dwo information"
> +    return -1
> +}
> +
> +# Now we can start GDB.
> +clean_restart ${testfile}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +# Print the type of global_var.  This type information is entirely
> +# fictional, it only exists in the DWARF.  If we don't have the DWARF
> +# information then there's no way we can print this.
> +gdb_test "p global_var" " = \\{aa = 0, bb = 0, cc = 0\\}"
> diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
> index 4cd5e16c604..a41366f1747 100644
> --- a/gdb/testsuite/lib/dwarf.exp
> +++ b/gdb/testsuite/lib/dwarf.exp
> @@ -28,6 +28,38 @@ proc dwarf2_support {} {
>      return 0
>  }
>
> +# Use 'objcopy --extract-dwo to extract DWO information from
> +# OBJECT_FILE and place it into DWO_FILE.
> +#
> +# Return 0 on success, otherwise, return -1.
> +proc extract_dwo_information { object_file dwo_file } {
> +    set objcopy [gdb_find_objcopy]
> +    set command "$objcopy --extract-dwo $object_file $dwo_file"
> +    verbose -log "Executing $command"
> +    set result [catch "exec $command" output]
> +    verbose -log "objcopy --extract-dwo output: $output"
> +    if { $result == 1 } {
> +       return -1
> +    }
> +    return 0
> +}
> +
> +# Use 'objcopy --strip-dwo to remove DWO information from
> +# FILENAME.
> +#
> +# Return 0 on success, otherwise, return -1.
> +proc strip_dwo_information { filename } {
> +   set objcopy [gdb_find_objcopy]
> +    set command "$objcopy --strip-dwo $filename"
> +    verbose -log "Executing $command"
> +    set result [catch "exec $command" output]
> +    verbose -log "objcopy --strip-dwo output: $output"
> +    if { $result == 1 } {
> +       return -1
> +    }
> +    return 0
> +}
> +
>  # Build an executable from a fission-based .S file.
>  # This handles the extra work of splitting the .o into non-dwo and dwo
>  # pieces, making sure the .dwo is available if we're using
> cc-with-tweaks.sh
> @@ -54,26 +86,17 @@ proc build_executable_from_fission_assembler {
> testname executable sources optio
>      set object_file ${output_base}.o
>      set dwo_file ${output_base}.dwo
>      set object_options "object $options"
> -    set objcopy [gdb_find_objcopy]
>
>      set result [gdb_compile $source_file $object_file object $options]
>      if { "$result" != "" } {
>         return -1
>      }
>
> -    set command "$objcopy --extract-dwo $object_file $dwo_file"
> -    verbose -log "Executing $command"
> -    set result [catch "exec $command" output]
> -    verbose -log "objcopy --extract-dwo output: $output"
> -    if { $result == 1 } {
> +    if { [extract_dwo_information $object_file $dwo_file] == -1 } {
>         return -1
>      }
>
> -    set command "$objcopy --strip-dwo $object_file"
> -    verbose -log "Executing $command"
> -    set result [catch "exec $command" output]
> -    verbose -log "objcopy --strip-dwo output: $output"
> -    if { $result == 1 } {
> +    if { [strip_dwo_information $object_file] == -1 } {
>         return -1
>      }
>
> @@ -329,6 +352,14 @@ namespace eval Dwarf {
>      # The address size for debug ranges section.
>      variable _debug_ranges_64_bit
>
> +    # The index into the .debug_addr section (used for fission
> +    # generation).
> +    variable _debug_addr_index
> +
> +    # Flag, true if the current CU is contains fission information,
> +    # otherwise false.
> +    variable _cu_is_fission
> +
>      proc _process_one_constant {name value} {
>         variable _constants
>         variable _AT
> @@ -486,6 +517,18 @@ namespace eval Dwarf {
>                 _op .${_cu_addr_size}byte $value
>             }
>
> +           DW_FORM_GNU_addr_index {
> +               variable _debug_addr_index
> +               variable _cu_addr_size
> +
> +               _op .uleb128 ${_debug_addr_index}
> +               incr _debug_addr_index
> +
> +               _defer_output .debug_addr {
> +                   _op .${_cu_addr_size}byte $value
> +               }
> +           }
> +
>             DW_FORM_data2 -
>             DW_FORM_ref2 {
>                 _op .2byte $value
> @@ -553,7 +596,6 @@ namespace eval Dwarf {
>             DW_FORM_strx3 -
>             DW_FORM_strx4 -
>
> -           DW_FORM_GNU_addr_index -
>             DW_FORM_GNU_str_index -
>
>             default {
> @@ -609,6 +651,9 @@ namespace eval Dwarf {
>             DW_AT_name {
>                 return DW_FORM_string
>             }
> +           DW_AT_GNU_addr_base {
> +               return DW_FORM_sec_offset
> +           }
>         }
>         return ""
>      }
> @@ -649,6 +694,8 @@ namespace eval Dwarf {
>      # Handle macro attribute MACRO_AT_range.
>
>      proc _handle_macro_at_range { attr_value } {
> +       variable _cu_is_fission
> +
>         if {[llength $attr_value] != 1} {
>             error "usage: MACRO_AT_range { func }"
>         }
> @@ -658,10 +705,14 @@ namespace eval Dwarf {
>         set src ${srcdir}/${subdir}/${srcfile}
>         set result [function_range $func $src]
>
> -       _handle_attribute DW_AT_low_pc [lindex $result 0] \
> -           DW_FORM_addr
> +       set form DW_FORM_addr
> +       if { $_cu_is_fission } {
> +           set form DW_FORM_GNU_addr_index
> +       }
> +
> +       _handle_attribute DW_AT_low_pc [lindex $result 0] $form
>         _handle_attribute DW_AT_high_pc \
> -           "[lindex $result 0] + [lindex $result 1]" DW_FORM_addr
> +           "[lindex $result 0] + [lindex $result 1]" $form
>      }
>
>      # Handle macro attribute MACRO_AT_func.
> @@ -929,6 +980,18 @@ namespace eval Dwarf {
>                     _op .${addr_size}byte [lindex $line 1]
>                 }
>
> +               DW_OP_GNU_addr_index {
> +                   variable _debug_addr_index
> +                   variable _cu_addr_size
> +
> +                   _op .uleb128 ${_debug_addr_index}
> +                   incr _debug_addr_index
> +
> +                   _defer_output .debug_addr {
> +                       _op .${_cu_addr_size}byte [lindex $line 1]
> +                   }
> +               }
> +
>                 DW_OP_regx {
>                     _op .uleb128 [lindex $line 1]
>                 }
> @@ -1050,6 +1113,14 @@ namespace eval Dwarf {
>         }
>      }
>
> +    # Return the current value of _DEBUG_ADDR_INDEX.  The return value
> +    # of this function can be used for the value of
> +    # DW_AT_GNU_addr_base.
> +    proc debug_addr_index {} {
> +       variable _debug_addr_index
> +       return ${_debug_addr_index}
> +    }
> +
>      # Emit a DWARF CU.
>      # OPTIONS is a list with an even number of elements containing
>      # option-name and option-value pairs.
> @@ -1073,12 +1144,13 @@ namespace eval Dwarf {
>         variable _cu_version
>         variable _cu_addr_size
>         variable _cu_offset_size
> +       variable _cu_is_fission
>
>         # Establish the defaults.
>         set is_64 0
>         set _cu_version 4
>         set _cu_addr_size default
> -       set fission 0
> +       set _cu_is_fission 0
>         set section ".debug_info"
>         set _abbrev_section ".debug_abbrev"
>
> @@ -1088,7 +1160,7 @@ namespace eval Dwarf {
>                 is_64 { set is_64 $value }
>                 version { set _cu_version $value }
>                 addr_size { set _cu_addr_size $value }
> -               fission { set fission $value }
> +               fission { set _cu_is_fission $value }
>                 default { error "unknown option $name" }
>             }
>         }
> @@ -1100,7 +1172,7 @@ namespace eval Dwarf {
>             }
>         }
>         set _cu_offset_size [expr { $is_64 ? 8 : 4 }]
> -       if { $fission } {
> +       if { $_cu_is_fission } {
>             set section ".debug_info.dwo"
>             set _abbrev_section ".debug_abbrev.dwo"
>         }
> @@ -1180,12 +1252,13 @@ namespace eval Dwarf {
>         variable _cu_version
>         variable _cu_addr_size
>         variable _cu_offset_size
> +       variable _cu_is_fission
>
>         # Establish the defaults.
>         set is_64 0
>         set _cu_version 4
>         set _cu_addr_size default
> -       set fission 0
> +       set _cu_is_fission 0
>         set section ".debug_types"
>         set _abbrev_section ".debug_abbrev"
>
> @@ -1194,7 +1267,7 @@ namespace eval Dwarf {
>                 is_64 { set is_64 $value }
>                 version { set _cu_version $value }
>                 addr_size { set _cu_addr_size $value }
> -               fission { set fission $value }
> +               fission { set _cu_is_fission $value }
>                 default { error "unknown option $name" }
>             }
>         }
> @@ -1206,7 +1279,7 @@ namespace eval Dwarf {
>             }
>         }
>         set _cu_offset_size [expr { $is_64 ? 8 : 4 }]
> -       if { $fission } {
> +       if { $_cu_is_fission } {
>             set section ".debug_types.dwo"
>             set _abbrev_section ".debug_abbrev.dwo"
>         }
> @@ -2056,6 +2129,7 @@ namespace eval Dwarf {
>         variable _line_saw_program
>         variable _line_header_end_label
>         variable _debug_ranges_64_bit
> +       variable _debug_addr_index
>
>         if {!$_initialized} {
>             _read_constants
> @@ -2074,6 +2148,8 @@ namespace eval Dwarf {
>         set _line_saw_program 0
>         set _debug_ranges_64_bit [is_64_target]
>
> +       set _debug_addr_index 0
> +
>         # Not "uplevel" here, because we want to evaluate in this
>         # namespace.  This is somewhat bad because it means we can't
>         # readily refer to outer variables.
>