* [PATCH][gdb] Write index for dwz -m file
@ 2019-05-07 14:42 Tom de Vries
2019-05-07 16:13 ` [RFC, gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs Tom de Vries
2019-05-11 2:36 ` [PATCH][gdb] Write index for dwz -m file Simon Marchi
0 siblings, 2 replies; 11+ messages in thread
From: Tom de Vries @ 2019-05-07 14:42 UTC (permalink / raw)
To: gdb-patches, Simon Marchi
Hi,
With target board cc-with-dwz-m, we run into the following failures:
...
FAIL: gdb.base/index-cache.exp: test_cache_enabled_hit: check index-cache stats
FAIL: gdb.dwarf2/gdb-index.exp: index used
FAIL: gdb.dwarf2/gdb-index.exp: index used after symbol reloading
...
The problem in both test-cases is that given an executable foo.exe with
.gnu_debugaltlink file foo.dwz, gdb writes the index for both files into a
single file.
Fix this by writing the index for foo.dwz to a seperate file. [ Note that
this does not fix contrib/gdb-add-index.sh. ]
Tested on x86_64-linux with native and cc-with-dwz-m.
The only remaining regressions of cc-with-dwz-m compared to native are:
- PR24508 (gdb.base/maint.exp)
- PR24503 (gdb.base/max-depth.exp and gdb.cp/var-tag.exp)
OK for trunk?
Thanks,
- Tom
[gdb] Write index for dwz -m file
gdb/ChangeLog:
2019-05-07 Simon Marchi <simon.marchi@efficios.com>
PR gdb/24445
* dwarf-index-cache.c (index_cache::store): Save index of
.gnu_debugaltlink file to seperate cache file.
* dwarf-index-write.c (write_gdbindex): Add and handle new parameter
dwz_out_file.
(write_psymtabs_to_index): Add and handle new parameter dwz_basename.
(save_gdb_index_command): Update write_psymtabs_to_index call.
* dwarf-index-write.h (write_psymtabs_to_index): Update declaration.
* dwarf2read.c (struct dwz_file): Move ...
* dwarf2read.h (struct dwz_file): ... here.
gdb/testsuite/ChangeLog:
2019-05-07 Tom de Vries <tdevries@suse.de>
PR gdb/24445
* gdb.dwarf2/gdb-index.exp: Update dwz file with generated index.
---
gdb/dwarf-index-cache.c | 29 +++++++++++--
gdb/dwarf-index-write.c | 90 ++++++++++++++++++++++++++++++++++++++---
gdb/dwarf-index-write.h | 2 +-
gdb/dwarf2read.c | 28 +------------
gdb/dwarf2read.h | 34 ++++++++++++++++
5 files changed, 145 insertions(+), 38 deletions(-)
---
gdb/dwarf-index-cache.c | 27 +++++++++--
gdb/dwarf-index-write.c | 87 +++++++++++++++++++++++++++++++---
gdb/dwarf-index-write.h | 2 +-
gdb/dwarf2read.c | 28 +----------
gdb/dwarf2read.h | 34 +++++++++++++
gdb/testsuite/gdb.dwarf2/gdb-index.exp | 15 ++++++
6 files changed, 156 insertions(+), 37 deletions(-)
diff --git a/gdb/dwarf-index-cache.c b/gdb/dwarf-index-cache.c
index 9513891a45..8d583bd81b 100644
--- a/gdb/dwarf-index-cache.c
+++ b/gdb/dwarf-index-cache.c
@@ -102,14 +102,34 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
return;
}
+ std::string build_id_str = build_id_to_string (build_id);
+
+ const bfd_build_id *dwz_build_id = NULL;
+ gdb::optional<std::string> dwz_build_id_str;
+ const dwz_file *dwz = dwarf2_get_dwz_file (dwarf2_per_objfile);
+ const char *dwz_build_id_ptr = NULL;
+ if (dwz != nullptr)
+ {
+ dwz_build_id = build_id_bfd_get (dwz->dwz_bfd.get ());
+
+ if (dwz_build_id == nullptr)
+ {
+ if (debug_index_cache)
+ printf_unfiltered ("index cache: dwz objfile %s has no build id\n",
+ dwz->filename ());
+ return;
+ }
+
+ dwz_build_id_str = build_id_to_string (dwz_build_id);
+ dwz_build_id_ptr = dwz_build_id_str->c_str ();
+ }
+
if (m_dir.empty ())
{
warning (_("The index cache directory name is empty, skipping store."));
return;
}
- std::string build_id_str = build_id_to_string (build_id);
-
try
{
/* Try to create the containing directory. */
@@ -127,7 +147,8 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
/* Write the index itself to the directory, using the build id as the
filename. */
write_psymtabs_to_index (dwarf2_per_objfile, m_dir.c_str (),
- build_id_str.c_str (), dw_index_kind::GDB_INDEX);
+ build_id_str.c_str (), dwz_build_id_ptr,
+ dw_index_kind::GDB_INDEX);
}
catch (const gdb_exception_error &except)
{
diff --git a/gdb/dwarf-index-write.c b/gdb/dwarf-index-write.c
index 8734f99212..7fd9a49270 100644
--- a/gdb/dwarf-index-write.c
+++ b/gdb/dwarf-index-write.c
@@ -1293,11 +1293,13 @@ psyms_seen_size (struct dwarf2_per_objfile *dwarf2_per_objfile)
Return how many bytes were expected to be written into OUT_FILE. */
static size_t
-write_gdbindex (struct dwarf2_per_objfile *dwarf2_per_objfile, FILE *out_file)
+write_gdbindex (struct dwarf2_per_objfile *dwarf2_per_objfile, FILE *out_file,
+ FILE *dwz_out_file)
{
struct objfile *objfile = dwarf2_per_objfile->objfile;
mapped_symtab symtab;
- data_buf cu_list;
+ data_buf main_cu_list;
+ data_buf dwz_cu_list;
/* While we're scanning CU's create a table that maps a psymtab pointer
(which is what addrmap records) to its index (which is what is recorded
@@ -1331,6 +1333,7 @@ write_gdbindex (struct dwarf2_per_objfile *dwarf2_per_objfile, FILE *out_file)
const auto insertpair = cu_index_htab.emplace (psymtab, i);
gdb_assert (insertpair.second);
+ data_buf &cu_list = per_cu->is_dwz ? dwz_cu_list : main_cu_list;
cu_list.append_uint (8, BFD_ENDIAN_LITTLE,
to_underlying (per_cu->sect_off));
cu_list.append_uint (8, BFD_ENDIAN_LITTLE, per_cu->length);
@@ -1370,7 +1373,7 @@ write_gdbindex (struct dwarf2_per_objfile *dwarf2_per_objfile, FILE *out_file)
/* The offset of the CU list from the start of the file. */
contents.append_data (MAYBE_SWAP (total_len));
- total_len += cu_list.size ();
+ total_len += main_cu_list.size ();
/* The offset of the types CU list from the start of the file. */
contents.append_data (MAYBE_SWAP (total_len));
@@ -1391,12 +1394,39 @@ write_gdbindex (struct dwarf2_per_objfile *dwarf2_per_objfile, FILE *out_file)
gdb_assert (contents.size () == size_of_contents);
contents.file_write (out_file);
- cu_list.file_write (out_file);
+ main_cu_list.file_write (out_file);
types_cu_list.file_write (out_file);
addr_vec.file_write (out_file);
symtab_vec.file_write (out_file);
constant_pool.file_write (out_file);
+
+ if (dwz_out_file != NULL)
+ {
+ data_buf dwz_contents;
+ const offset_type dwz_size_of_contents = 6 * sizeof (offset_type);
+ offset_type dwz_total_len = size_of_contents;
+
+ /* The version number. */
+ dwz_contents.append_data (MAYBE_SWAP (8));
+
+ /* The offset of the CU list from the start of the file. */
+ dwz_contents.append_data (MAYBE_SWAP (dwz_total_len));
+ dwz_total_len += dwz_cu_list.size ();
+
+ dwz_contents.append_data (MAYBE_SWAP (dwz_total_len));
+ dwz_contents.append_data (MAYBE_SWAP (dwz_total_len));
+ dwz_contents.append_data (MAYBE_SWAP (dwz_total_len));
+ dwz_contents.append_data (MAYBE_SWAP (dwz_total_len));
+
+ gdb_assert (dwz_contents.size () == dwz_size_of_contents);
+
+ dwz_contents.file_write (dwz_out_file);
+ dwz_cu_list.file_write (dwz_out_file);
+ }
+ else
+ gdb_assert (dwz_cu_list.empty ());
+
return total_len;
}
@@ -1548,6 +1578,7 @@ assert_file_size (FILE *file, const char *filename, size_t expected_size)
void
write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
const char *dir, const char *basename,
+ const char *dwz_basename,
dw_index_kind index_kind)
{
struct objfile *objfile = dwarf2_per_objfile->objfile;
@@ -1587,6 +1618,32 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
unlink_file.emplace (filename_temp.data ());
+ /* Order is important here as well, for the same reason as above. */
+ gdb::optional<std::string> dwz_filename;
+ gdb::optional<gdb::char_vector> dwz_filename_temp;
+ gdb::optional<gdb::unlinker> dwz_unlink_file;
+ gdb::optional<scoped_fd> dwz_out_file_fd;
+ gdb::optional<gdb_file_up> dwz_out_file;
+
+ if (dwz_basename != NULL)
+ {
+ dwz_filename = (std::string (dir) + SLASH_STRING + dwz_basename
+ + (index_kind == dw_index_kind::DEBUG_NAMES
+ ? INDEX5_SUFFIX : INDEX4_SUFFIX));
+ dwz_filename_temp = make_temp_filename (*dwz_filename);
+ dwz_out_file_fd.emplace (gdb_mkostemp_cloexec (dwz_filename_temp->data (),
+ O_BINARY));
+ if (dwz_out_file_fd->get () == -1)
+ perror_with_name (("mkstemp"));
+
+ dwz_out_file = dwz_out_file_fd->to_file ("wb");
+
+ if (*dwz_out_file == nullptr)
+ error (_("Can't open `%s' for writing"), dwz_filename_temp->data ());
+
+ dwz_unlink_file.emplace (dwz_filename_temp->data ());
+ }
+
if (index_kind == dw_index_kind::DEBUG_NAMES)
{
std::string filename_str (std::string (dir) + SLASH_STRING
@@ -1623,7 +1680,8 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
else
{
const size_t total_len
- = write_gdbindex (dwarf2_per_objfile, out_file.get ());
+ = write_gdbindex (dwarf2_per_objfile, out_file.get (),
+ dwz_basename != NULL ? dwz_out_file->get () : NULL);
assert_file_size (out_file.get (), filename_temp.data (), total_len);
}
@@ -1633,7 +1691,18 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
/* Close and move the file in place. */
out_file.reset ();
if (rename (filename_temp.data (), filename.c_str ()) != 0)
+ perror_with_name (("rename"));
+
+ if (dwz_basename != NULL)
+ {
+ /* We want to keep the file. */
+ dwz_unlink_file->keep ();
+
+ /* Close and move the file in place. */
+ dwz_out_file->reset ();
+ if (rename (dwz_filename_temp->data (), dwz_filename->c_str ()) != 0)
perror_with_name (("rename"));
+ }
}
/* Implementation of the `save gdb-index' command.
@@ -1678,8 +1747,14 @@ save_gdb_index_command (const char *arg, int from_tty)
try
{
const char *basename = lbasename (objfile_name (objfile));
+ const dwz_file *dwz = dwarf2_get_dwz_file (dwarf2_per_objfile);
+ const char *dwz_basename = NULL;
+
+ if (dwz != NULL)
+ dwz_basename = lbasename (dwz->filename ());
+
write_psymtabs_to_index (dwarf2_per_objfile, arg, basename,
- index_kind);
+ dwz_basename, index_kind);
}
catch (const gdb_exception_error &except)
{
diff --git a/gdb/dwarf-index-write.h b/gdb/dwarf-index-write.h
index b1d1180c8e..ada6932267 100644
--- a/gdb/dwarf-index-write.h
+++ b/gdb/dwarf-index-write.h
@@ -29,6 +29,6 @@
extern void write_psymtabs_to_index
(struct dwarf2_per_objfile *dwarf2_per_objfile, const char *dir,
- const char *basename, dw_index_kind index_kind);
+ const char *basename, const char *dwz_basename, dw_index_kind index_kind);
#endif /* DWARF_INDEX_WRITE_H */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b5ea9e3cc0..61d47b10d6 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -886,32 +886,6 @@ struct dwp_file
asection **elf_sections = nullptr;
};
-/* This represents a '.dwz' file. */
-
-struct dwz_file
-{
- dwz_file (gdb_bfd_ref_ptr &&bfd)
- : dwz_bfd (std::move (bfd))
- {
- }
-
- /* A dwz file can only contain a few sections. */
- struct dwarf2_section_info abbrev {};
- struct dwarf2_section_info info {};
- struct dwarf2_section_info str {};
- struct dwarf2_section_info line {};
- struct dwarf2_section_info macro {};
- struct dwarf2_section_info gdb_index {};
- struct dwarf2_section_info debug_names {};
-
- /* The dwz's BFD. */
- gdb_bfd_ref_ptr dwz_bfd;
-
- /* If we loaded the index from an external file, this contains the
- resources associated to the open file, memory mapping, etc. */
- std::unique_ptr<index_cache_resource> index_cache_res;
-};
-
/* Struct used to pass misc. parameters to read_die_and_children, et
al. which are used for both .debug_info and .debug_types dies.
All parameters here are unchanging for the life of the call. This
@@ -2672,7 +2646,7 @@ locate_dwz_sections (bfd *abfd, asection *sectp, void *arg)
there is no .gnu_debugaltlink section in the file. Error if there
is such a section but the file cannot be found. */
-static struct dwz_file *
+struct dwz_file *
dwarf2_get_dwz_file (struct dwarf2_per_objfile *dwarf2_per_objfile)
{
const char *filename;
diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
index 34c66167b5..14befd7e79 100644
--- a/gdb/dwarf2read.h
+++ b/gdb/dwarf2read.h
@@ -403,4 +403,38 @@ struct signatured_type
typedef struct signatured_type *sig_type_ptr;
DEF_VEC_P (sig_type_ptr);
+/* This represents a '.dwz' file. */
+
+struct dwz_file
+{
+ dwz_file (gdb_bfd_ref_ptr &&bfd)
+ : dwz_bfd (std::move (bfd))
+ {
+ }
+
+ const char *filename () const
+ {
+ return bfd_get_filename (this->dwz_bfd);
+ }
+
+ /* A dwz file can only contain a few sections. */
+ struct dwarf2_section_info abbrev {};
+ struct dwarf2_section_info info {};
+ struct dwarf2_section_info str {};
+ struct dwarf2_section_info line {};
+ struct dwarf2_section_info macro {};
+ struct dwarf2_section_info gdb_index {};
+ struct dwarf2_section_info debug_names {};
+
+ /* The dwz's BFD. */
+ gdb_bfd_ref_ptr dwz_bfd;
+
+ /* If we loaded the index from an external file, this contains the
+ resources associated to the open file, memory mapping, etc. */
+ std::unique_ptr<index_cache_resource> index_cache_res;
+};
+
+extern struct dwz_file *dwarf2_get_dwz_file
+ (struct dwarf2_per_objfile *dwarf2_per_objfile);
+
#endif /* DWARF2READ_H */
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index.exp b/gdb/testsuite/gdb.dwarf2/gdb-index.exp
index 410e59684f..6fca3c6129 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-index.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index.exp
@@ -34,8 +34,11 @@ if { [prepare_for_testing "failed to prepare" "${testfile}" \
proc add_gdb_index { program } {
set index_file ${program}.gdb-index
+ set dwz ${program}.dwz
+ set dwz_index_file ${dwz}.gdb-index
verbose -log "index_file: ${index_file}"
remote_file host delete ${index_file}
+ remote_file host delete ${dwz_index_file}
gdb_test_no_output "save gdb-index [file dirname ${index_file}]" \
"save gdb-index for file [file tail ${program}]"
@@ -55,6 +58,18 @@ proc add_gdb_index { program } {
if {[run_on_host "objcopy" [gdb_find_objcopy] "--remove-section .gdb_index --add-section .gdb_index=$index_file --set-section-flags .gdb_index=readonly ${program} ${program_with_index}"]} {
return ""
}
+
+ if { [remote_file host exists ${dwz_index_file}] } {
+ # We're modifying $dwz in place, otherwise we'd have to update
+ # .gnu_debugaltlink in $program.
+ set args [join [list "--remove-section .gdb_index" \
+ " --add-section .gdb_index=$dwz_index_file" \
+ " --set-section-flags .gdb_index=readonly $dwz"]]
+ if {[run_on_host "objcopy" [gdb_find_objcopy] "$args"]} {
+ return ""
+ }
+ }
+
return ${program_with_index}
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC, gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs
2019-05-07 14:42 [PATCH][gdb] Write index for dwz -m file Tom de Vries
@ 2019-05-07 16:13 ` Tom de Vries
2019-05-12 19:49 ` Simon Marchi
2019-05-11 2:36 ` [PATCH][gdb] Write index for dwz -m file Simon Marchi
1 sibling, 1 reply; 11+ messages in thread
From: Tom de Vries @ 2019-05-07 16:13 UTC (permalink / raw)
To: gdb-patches, Simon Marchi, Pedro Alves
[-- Attachment #1: Type: text/plain, Size: 337 bytes --]
[ was: Re: [PATCH][gdb] Write index for dwz -m file ]
Hi,
This is a follow-up patch for "[gdb] Write index for dwz -m file".
Any comments on the updated gdb/contrib/gdb-add-index.sh script?
In particular, I'd like some advice on whether I should add shell
variables (as I've done for readelf) for grep, tail and sed.
Thanks,
- Tom
[-- Attachment #2: 0003-gdb-contrib-Fix-gdb-contrib-gdb-add-index.sh-for-dwz-m-ed-execs.patch --]
[-- Type: text/x-patch, Size: 5033 bytes --]
[gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs
Atm gdb-add-index.exp fails with target board cc-with-dwz-m.
Fix this by updating gdb/contrib/gdb-add-index.sh to handle a dwz-m-ed
executable.
Tested on x86_64-linux.
gdb/ChangeLog:
2019-05-07 Tom de Vries <tdevries@suse.de>
PR gdb/24445
* contrib/gdb-add-index.sh: Update to handle dwz-m-ed executable.
---
gdb/contrib/gdb-add-index.sh | 134 +++++++++++++++++++++++++++----------------
1 file changed, 86 insertions(+), 48 deletions(-)
diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
index efaad1dce7..afedce0c84 100755
--- a/gdb/contrib/gdb-add-index.sh
+++ b/gdb/contrib/gdb-add-index.sh
@@ -20,6 +20,7 @@
# If not, or you want others, pass the following in the environment
GDB=${GDB:=gdb}
OBJCOPY=${OBJCOPY:=objcopy}
+READELF=${READELF:=readelf}
myname="${0##*/}"
@@ -43,15 +44,40 @@ fi
dir="${file%/*}"
test "$dir" = "$file" && dir="."
-index4="${file}.gdb-index"
-index5="${file}.debug_names"
-debugstr="${file}.debug_str"
-debugstrmerge="${file}.debug_str.merge"
-debugstrerr="${file}.debug_str.err"
-rm -f $index4 $index5 $debugstr $debugstrmerge $debugstrerr
+dwz_file=""
+if $READELF -S "$file" | grep -q " \.gnu_debugaltlink "; then
+ dwz_file=$($READELF --string-dump=.gnu_debugaltlink "$file" \
+ | grep -A1 "'\.gnu_debugaltlink':" \
+ | tail -n +2 \
+ | sed 's/.*]//')
+ dwz_file=$(echo $dwz_file)
+fi
+
+set_files ()
+{
+ local file="$1"
+
+ index4="${file}.gdb-index"
+ index5="${file}.debug_names"
+ debugstr="${file}.debug_str"
+ debugstrmerge="${file}.debug_str.merge"
+ debugstrerr="${file}.debug_str.err"
+}
+
+tmp_files=
+for f in "$file" "$dwz_file"; do
+ if [ "$f" = "" ]; then
+ continue
+ fi
+ set_files "$file"
+ tmp_files="$tmp_files $index4 $index5 $debugstr $debugstrmerge $debugstrerr"
+done
+
+rm -f $tmp_files
+
# Ensure intermediate index file is removed when we exit.
-trap "rm -f $index4 $index5 $debugstr $debugstrmerge $debugstrerr" 0
+trap "rm -f $tmp_files" 0
$GDB --batch -nx -iex 'set auto-load no' \
-ex "file $file" -ex "save gdb-index $dwarf5 $dir" || {
@@ -67,50 +93,62 @@ $GDB --batch -nx -iex 'set auto-load no' \
# already stripped binary, it's a no-op.
status=0
-if test -f "$index4" -a -f "$index5"; then
- echo "$myname: Both index types were created for $file" 1>&2
- status=1
-elif test -f "$index4" -o -f "$index5"; then
- if test -f "$index4"; then
- index="$index4"
- section=".gdb_index"
- else
- index="$index5"
- section=".debug_names"
- fi
- debugstradd=false
- debugstrupdate=false
- if test -s "$debugstr"; then
- if ! $OBJCOPY --dump-section .debug_str="$debugstrmerge" "$file" /dev/null \
- 2>$debugstrerr; then
- cat >&2 $debugstrerr
- exit 1
- fi
- if grep -q "can't dump section '.debug_str' - it does not exist" \
- $debugstrerr; then
- debugstradd=true
+handle_file ()
+{
+ local file
+ file="$1"
+
+ set_files "$file"
+
+ if test -f "$index4" -a -f "$index5"; then
+ echo "$myname: Both index types were created for $file" 1>&2
+ status=1
+ elif test -f "$index4" -o -f "$index5"; then
+ if test -f "$index4"; then
+ index="$index4"
+ section=".gdb_index"
else
- debugstrupdate=true
- cat >&2 $debugstrerr
+ index="$index5"
+ section=".debug_names"
+ fi
+ debugstradd=false
+ debugstrupdate=false
+ if test -s "$debugstr"; then
+ if ! $OBJCOPY --dump-section .debug_str="$debugstrmerge" "$file" \
+ /dev/null 2>$debugstrerr; then
+ cat >&2 $debugstrerr
+ exit 1
+ fi
+ if grep -q "can't dump section '.debug_str' - it does not exist" \
+ $debugstrerr; then
+ debugstradd=true
+ else
+ debugstrupdate=true
+ cat >&2 $debugstrerr
+ fi
+ cat "$debugstr" >>"$debugstrmerge"
fi
- cat "$debugstr" >>"$debugstrmerge"
- fi
- $OBJCOPY --add-section $section="$index" \
- --set-section-flags $section=readonly \
- $(if $debugstradd; then \
- echo --add-section .debug_str="$debugstrmerge"; \
- echo --set-section-flags .debug_str=readonly; \
- fi; \
- if $debugstrupdate; then \
- echo --update-section .debug_str="$debugstrmerge"; \
- fi) \
- "$file" "$file"
+ $OBJCOPY --add-section $section="$index" \
+ --set-section-flags $section=readonly \
+ $(if $debugstradd; then \
+ echo --add-section .debug_str="$debugstrmerge"; \
+ echo --set-section-flags .debug_str=readonly; \
+ fi; \
+ if $debugstrupdate; then \
+ echo --update-section .debug_str="$debugstrmerge"; \
+ fi) \
+ "$file" "$file"
+
+ status=$?
+ else
+ echo "$myname: No index was created for $file" 1>&2
+ echo "$myname: [Was there no debuginfo? Was there already an index?]" \
+ 1>&2
+ fi
+}
- status=$?
-else
- echo "$myname: No index was created for $file" 1>&2
- echo "$myname: [Was there no debuginfo? Was there already an index?]" 1>&2
-fi
+handle_file "$file"
+handle_file "$dwz_file"
exit $status
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][gdb] Write index for dwz -m file
2019-05-07 14:42 [PATCH][gdb] Write index for dwz -m file Tom de Vries
2019-05-07 16:13 ` [RFC, gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs Tom de Vries
@ 2019-05-11 2:36 ` Simon Marchi
2019-05-13 12:22 ` Tom de Vries
1 sibling, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2019-05-11 2:36 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 2019-05-07 10:42 a.m., Tom de Vries wrote:
> Hi,
>
> With target board cc-with-dwz-m, we run into the following failures:
> ...
> FAIL: gdb.base/index-cache.exp: test_cache_enabled_hit: check index-cache stats
> FAIL: gdb.dwarf2/gdb-index.exp: index used
> FAIL: gdb.dwarf2/gdb-index.exp: index used after symbol reloading
> ...
>
> The problem in both test-cases is that given an executable foo.exe with
> .gnu_debugaltlink file foo.dwz, gdb writes the index for both files into a
> single file.
>
> Fix this by writing the index for foo.dwz to a seperate file. [ Note that
> this does not fix contrib/gdb-add-index.sh. ]
>
> Tested on x86_64-linux with native and cc-with-dwz-m.
>
> The only remaining regressions of cc-with-dwz-m compared to native are:
> - PR24508 (gdb.base/maint.exp)
> - PR24503 (gdb.base/max-depth.exp and gdb.cp/var-tag.exp)
>
> OK for trunk?
>
> Thanks,
> - Tom
The patch sent by Tom was more of a quick prototype I made and attached in
Bugzilla (thanks for updating the test!). I have take the time to clean up
the code a bit, what do you think about the patch below?
From 3e22d9fd561656e3ce86039f5ba4533018055208 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Sun, 28 Apr 2019 23:19:24 -0400
Subject: [PATCH] Write index for dwz -m file
PR 24445 ("dwz multifile index not written to index cache") exposed the
fact that we are not doing things right when we generate an index for an
object file that has is linked to a dwz file. The same happens whether
the index is generated with the index of populating the index cache or
using the save gdb-index command.
The problem can be observed when running these tests with the
cc-with-dwz-m board:
FAIL: gdb.base/index-cache.exp: test_cache_enabled_hit: check index-cache stats
FAIL: gdb.dwarf2/gdb-index.exp: index used
FAIL: gdb.dwarf2/gdb-index.exp: index used after symbol reloading
When generating the index for such file and inspecting the CU list of the
resulting index (with readelf --debug-dump=gdb_index), we can see something
like:
CU table:
[ 0] 0x0 - 0xb9
[ 1] 0x0 - 0x44
This is supposed to be a sorted list of the ranges of all CUs in the
file this index represents, so already having some overlap is a red
flag. It turns out that we save the ranges of CUs coming from both the
main file and the dwz file in the same index.
After digging a little bit, it became quite obvious that the index in
the main file should only list the CUs present in the main file, and a
separate index should be generated for the dwz file, listing the CUs
present in that file.
First, that's what happens if you run dwz on a file that already has a
GDB index embedded. Second, dwarf2read.c has code to read an index from
a dwz file. The index in the dwz file is actually required to be
present, if the main file has an index).
So this patch changes write_psymtabs_to_index to generate an index for
the dwz file, if present. That index only contains a CU list, just like
what the dwz tool does when processing a file that already contains an
index.
Some notes about the implementation:
- The file management (creating a temp file, make sure it's
close/removed on error - in the right order) is a bit heavy in
write_psymtabs_to_index, and I needed to add a third file. I factored
this pattern in a separate class, index_wip_file.
- It became a bit tedious to keep the call to assert_file_size in
write_psymtabs_to_index, write_gdbindex would have had to return two
sizes. Instead, I moved the calls to assert_file_size where the file
is written. The downside is that we lose the filename at this point,
but it was only used for the very improbably case of ftell failing, so
I think it's not a problem.
- The actual writing of the index file is factored out to
write_gdbindex_1, so it can be re-used for both index files.
- While the "save gdb-index" command will now write two .gdb-index
files, this patch does not update the gdb-add-index.sh script, this
will come in a later patch.
gdb/ChangeLog:
YYYY-MM-DD Simon Marchi <simon.marchi@efficios.com>
PR gdb/24445
* dwarf-index-write.h (write_psymtabs_to_index): Add
dwz_basename parameter.
* dwarf-index-write.c (write_gdbindex): Move file writing to
write_gdbindex_1. Change return type void.
(assert_file_size): Move up, remove filename parameter.
(write_gdbindex_1): New function.
(write_debug_names): Change return type to void, call
assert_file_size.
(struct index_wip_file): New struct.
(write_psymtabs_to_index): Add dwz_basename parameter. Move
file logic to index_wip_file. Write index for dwz file if
needed.
(save_gdb_index_command): Pass basename of dwz file, if present.
* dwarf-index-cache.c (index_cache::store): Obtain and pass
build-id of dwz file, if present.
* dwarf2read.c (struct dwz_file): Move to dwarf2read.h.
(dwarf2_get_dwz_file): Likewise.
* dwarf2read.h (struct dwz_file): Move from dwarf2read.c.
(dwarf2_get_dwz_file): Likewise.
gdb/testsuite/ChangeLog:
YYYY-MM-DD Tom de Vries <tdevries@suse.de>
PR gdb/24445
* gdb.dwarf2/gdb-index.exp (add_gdb_index): Update dwz file with
generated index.
---
gdb/dwarf-index-cache.c | 31 ++-
gdb/dwarf-index-write.c | 272 +++++++++++++++----------
gdb/dwarf-index-write.h | 13 +-
gdb/dwarf2read.c | 32 +--
gdb/dwarf2read.h | 38 ++++
gdb/testsuite/gdb.dwarf2/gdb-index.exp | 15 ++
6 files changed, 251 insertions(+), 150 deletions(-)
diff --git a/gdb/dwarf-index-cache.c b/gdb/dwarf-index-cache.c
index 9513891a45fc..c3dc8f91dff0 100644
--- a/gdb/dwarf-index-cache.c
+++ b/gdb/dwarf-index-cache.c
@@ -93,6 +93,7 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
if (!enabled ())
return;
+ /* Get build id of objfile. */
const bfd_build_id *build_id = build_id_bfd_get (obj->obfd);
if (build_id == nullptr)
{
@@ -102,14 +103,35 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
return;
}
+ std::string build_id_str = build_id_to_string (build_id);
+
+ /* Get build id of dwz file, if present. */
+ gdb::optional<std::string> dwz_build_id_str;
+ const dwz_file *dwz = dwarf2_get_dwz_file (dwarf2_per_objfile);
+ const char *dwz_build_id_ptr = NULL;
+
+ if (dwz != nullptr)
+ {
+ const bfd_build_id *dwz_build_id = build_id_bfd_get (dwz->dwz_bfd.get ());
+
+ if (dwz_build_id == nullptr)
+ {
+ if (debug_index_cache)
+ printf_unfiltered ("index cache: dwz objfile %s has no build id\n",
+ dwz->filename ());
+ return;
+ }
+
+ dwz_build_id_str = build_id_to_string (dwz_build_id);
+ dwz_build_id_ptr = dwz_build_id_str->c_str ();
+ }
+
if (m_dir.empty ())
{
warning (_("The index cache directory name is empty, skipping store."));
return;
}
- std::string build_id_str = build_id_to_string (build_id);
-
try
{
/* Try to create the containing directory. */
@@ -122,12 +144,13 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
if (debug_index_cache)
printf_unfiltered ("index cache: writing index cache for objfile %s\n",
- objfile_name (obj));
+ objfile_name (obj));
/* Write the index itself to the directory, using the build id as the
filename. */
write_psymtabs_to_index (dwarf2_per_objfile, m_dir.c_str (),
- build_id_str.c_str (), dw_index_kind::GDB_INDEX);
+ build_id_str.c_str (), dwz_build_id_ptr,
+ dw_index_kind::GDB_INDEX);
}
catch (const gdb_exception_error &except)
{
diff --git a/gdb/dwarf-index-write.c b/gdb/dwarf-index-write.c
index 8734f99212da..9d6ed4471929 100644
--- a/gdb/dwarf-index-write.c
+++ b/gdb/dwarf-index-write.c
@@ -1289,15 +1289,81 @@ psyms_seen_size (struct dwarf2_per_objfile *dwarf2_per_objfile)
return psyms_count / 4;
}
-/* Write new .gdb_index section for OBJFILE into OUT_FILE.
- Return how many bytes were expected to be written into OUT_FILE. */
+/* Assert that FILE's size is EXPECTED_SIZE. Assumes file's seek
+ position is at the end of the file. */
-static size_t
-write_gdbindex (struct dwarf2_per_objfile *dwarf2_per_objfile, FILE *out_file)
+static void
+assert_file_size (FILE *file, size_t expected_size)
+{
+ const auto file_size = ftell (file);
+ if (file_size == -1)
+ perror_with_name (("ftell"));
+ gdb_assert (file_size == expected_size);
+}
+
+/* Write a gdb index file to OUT_FILE from all the sections passed as
+ arguments. */
+
+static void
+write_gdbindex_1 (FILE *out_file,
+ const data_buf &cu_list,
+ const data_buf &types_cu_list,
+ const data_buf &addr_vec,
+ const data_buf &symtab_vec,
+ const data_buf &constant_pool)
+{
+ data_buf contents;
+ const offset_type size_of_header = 6 * sizeof (offset_type);
+ offset_type total_len = size_of_header;
+
+ /* The version number. */
+ contents.append_data (MAYBE_SWAP (8));
+
+ /* The offset of the CU list from the start of the file. */
+ contents.append_data (MAYBE_SWAP (total_len));
+ total_len += cu_list.size ();
+
+ /* The offset of the types CU list from the start of the file. */
+ contents.append_data (MAYBE_SWAP (total_len));
+ total_len += types_cu_list.size ();
+
+ /* The offset of the address table from the start of the file. */
+ contents.append_data (MAYBE_SWAP (total_len));
+ total_len += addr_vec.size ();
+
+ /* The offset of the symbol table from the start of the file. */
+ contents.append_data (MAYBE_SWAP (total_len));
+ total_len += symtab_vec.size ();
+
+ /* The offset of the constant pool from the start of the file. */
+ contents.append_data (MAYBE_SWAP (total_len));
+ total_len += constant_pool.size ();
+
+ gdb_assert (contents.size () == size_of_header);
+
+ contents.file_write (out_file);
+ cu_list.file_write (out_file);
+ types_cu_list.file_write (out_file);
+ addr_vec.file_write (out_file);
+ symtab_vec.file_write (out_file);
+ constant_pool.file_write (out_file);
+
+ assert_file_size (out_file, total_len);
+}
+
+/* Write contents of a .gdb_index section for OBJFILE into OUT_FILE.
+ If OBJFILE has an associated dwz file, write contents of a .gdb_index
+ section for that dwz file into DWZ_OUT_FILE. If OBJFILE does not have an
+ associated dwz file, DWZ_OUT_FILE must be NULL. */
+
+static void
+write_gdbindex (struct dwarf2_per_objfile *dwarf2_per_objfile, FILE *out_file,
+ FILE *dwz_out_file)
{
struct objfile *objfile = dwarf2_per_objfile->objfile;
mapped_symtab symtab;
- data_buf cu_list;
+ data_buf objfile_cu_list;
+ data_buf dwz_cu_list;
/* While we're scanning CU's create a table that maps a psymtab pointer
(which is what addrmap records) to its index (which is what is recorded
@@ -1331,6 +1397,10 @@ write_gdbindex (struct dwarf2_per_objfile *dwarf2_per_objfile, FILE *out_file)
const auto insertpair = cu_index_htab.emplace (psymtab, i);
gdb_assert (insertpair.second);
+ /* The all_comp_units list contains CUs read from the objfile as well as
+ from the eventual dwz file. We need to place the entry in the
+ corresponding index. */
+ data_buf &cu_list = per_cu->is_dwz ? dwz_cu_list : objfile_cu_list;
cu_list.append_uint (8, BFD_ENDIAN_LITTLE,
to_underlying (per_cu->sect_off));
cu_list.append_uint (8, BFD_ENDIAN_LITTLE, per_cu->length);
@@ -1361,43 +1431,13 @@ write_gdbindex (struct dwarf2_per_objfile *dwarf2_per_objfile, FILE *out_file)
data_buf symtab_vec, constant_pool;
write_hash_table (&symtab, symtab_vec, constant_pool);
- data_buf contents;
- const offset_type size_of_contents = 6 * sizeof (offset_type);
- offset_type total_len = size_of_contents;
-
- /* The version number. */
- contents.append_data (MAYBE_SWAP (8));
-
- /* The offset of the CU list from the start of the file. */
- contents.append_data (MAYBE_SWAP (total_len));
- total_len += cu_list.size ();
-
- /* The offset of the types CU list from the start of the file. */
- contents.append_data (MAYBE_SWAP (total_len));
- total_len += types_cu_list.size ();
+ write_gdbindex_1(out_file, objfile_cu_list, types_cu_list, addr_vec,
+ symtab_vec, constant_pool);
- /* The offset of the address table from the start of the file. */
- contents.append_data (MAYBE_SWAP (total_len));
- total_len += addr_vec.size ();
-
- /* The offset of the symbol table from the start of the file. */
- contents.append_data (MAYBE_SWAP (total_len));
- total_len += symtab_vec.size ();
-
- /* The offset of the constant pool from the start of the file. */
- contents.append_data (MAYBE_SWAP (total_len));
- total_len += constant_pool.size ();
-
- gdb_assert (contents.size () == size_of_contents);
-
- contents.file_write (out_file);
- cu_list.file_write (out_file);
- types_cu_list.file_write (out_file);
- addr_vec.file_write (out_file);
- symtab_vec.file_write (out_file);
- constant_pool.file_write (out_file);
-
- return total_len;
+ if (dwz_out_file != NULL)
+ write_gdbindex_1 (dwz_out_file, dwz_cu_list, {}, {}, {}, {});
+ else
+ gdb_assert (dwz_cu_list.empty ());
}
/* DWARF-5 augmentation string for GDB's DW_IDX_GNU_* extension. */
@@ -1407,7 +1447,7 @@ static const gdb_byte dwarf5_gdb_augmentation[] = { 'G', 'D', 'B', 0 };
needed addition to .debug_str section to OUT_FILE_STR. Return how
many bytes were expected to be written into OUT_FILE. */
-static size_t
+static void
write_debug_names (struct dwarf2_per_objfile *dwarf2_per_objfile,
FILE *out_file, FILE *out_file_str)
{
@@ -1528,26 +1568,69 @@ write_debug_names (struct dwarf2_per_objfile *dwarf2_per_objfile,
types_cu_list.file_write (out_file);
nametable.file_write (out_file, out_file_str);
- return expected_bytes;
+ assert_file_size (out_file, expected_bytes);
}
-/* Assert that FILE's size is EXPECTED_SIZE. Assumes file's seek
- position is at the end of the file. */
+/* This represents an index file being written.
-static void
-assert_file_size (FILE *file, const char *filename, size_t expected_size)
+ The data is initially written to a temporary file. When the finalize method
+ is called, the file is closed and moved to its final location.
+
+ On failure (if this object is being destroyed with having called finalize),
+ the temporary file is closed and deleted. */
+
+struct index_wip_file
{
- const auto file_size = ftell (file);
- if (file_size == -1)
- error (_("Can't get `%s' size"), filename);
- gdb_assert (file_size == expected_size);
-}
+ index_wip_file (const char *dir, const char *basename,
+ const char *suffix)
+ {
+ filename = (std::string (dir) + SLASH_STRING + basename
+ + suffix);
+
+ filename_temp = make_temp_filename (filename);
+
+ scoped_fd out_file_fd (gdb_mkostemp_cloexec (filename_temp.data (),
+ O_BINARY));
+ if (out_file_fd.get () == -1)
+ perror_with_name (("mkstemp"));
+
+ out_file = out_file_fd.to_file ("wb");
+
+ if (out_file == nullptr)
+ error (_("Can't open `%s' for writing"), filename_temp.data ());
+
+ unlink_file.emplace (filename_temp.data ());
+ }
+
+ void finalize ()
+ {
+ /* We want to keep the file. */
+ unlink_file->keep ();
+
+ /* Close and move the str file in place. */
+ unlink_file.reset ();
+ if (rename (filename_temp.data (), filename.c_str ()) != 0)
+ perror_with_name (("rename"));
+ }
+
+ std::string filename;
+ gdb::char_vector filename_temp;
+
+ /* Order matters here; we want FILE to be closed before
+ FILENAME_TEMP is unlinked, because on MS-Windows one cannot
+ delete a file that is still open. So, we wrap the unlinker in an
+ optional and emplace it once we know the file name. */
+ gdb::optional<gdb::unlinker> unlink_file;
+
+ gdb_file_up out_file;
+};
/* See dwarf-index-write.h. */
void
write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
const char *dir, const char *basename,
+ const char *dwz_basename,
dw_index_kind index_kind)
{
struct objfile *objfile = dwarf2_per_objfile->objfile;
@@ -1566,74 +1649,33 @@ write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
if (stat (objfile_name (objfile), &st) < 0)
perror_with_name (objfile_name (objfile));
- std::string filename (std::string (dir) + SLASH_STRING + basename
- + (index_kind == dw_index_kind::DEBUG_NAMES
- ? INDEX5_SUFFIX : INDEX4_SUFFIX));
- gdb::char_vector filename_temp = make_temp_filename (filename);
+ const char *index_suffix = (index_kind == dw_index_kind::DEBUG_NAMES
+ ? INDEX5_SUFFIX : INDEX4_SUFFIX);
- /* Order matters here; we want FILE to be closed before
- FILENAME_TEMP is unlinked, because on MS-Windows one cannot
- delete a file that is still open. So, we wrap the unlinker in an
- optional and emplace it once we know the file name. */
- gdb::optional<gdb::unlinker> unlink_file;
- scoped_fd out_file_fd (gdb_mkostemp_cloexec (filename_temp.data (),
- O_BINARY));
- if (out_file_fd.get () == -1)
- perror_with_name (("mkstemp"));
+ index_wip_file objfile_index_wip (dir, basename, index_suffix);
+ gdb::optional<index_wip_file> dwz_index_wip;
- gdb_file_up out_file = out_file_fd.to_file ("wb");
- if (out_file == nullptr)
- error (_("Can't open `%s' for writing"), filename_temp.data ());
-
- unlink_file.emplace (filename_temp.data ());
+ if (dwz_basename != NULL)
+ dwz_index_wip.emplace (dir, dwz_basename, index_suffix);
if (index_kind == dw_index_kind::DEBUG_NAMES)
{
- std::string filename_str (std::string (dir) + SLASH_STRING
- + basename + DEBUG_STR_SUFFIX);
- gdb::char_vector filename_str_temp = make_temp_filename (filename_str);
-
- /* As above, arrange to unlink the file only after the file
- descriptor has been closed. */
- gdb::optional<gdb::unlinker> unlink_file_str;
- scoped_fd out_file_str_fd
- (gdb_mkostemp_cloexec (filename_str_temp.data (), O_BINARY));
- if (out_file_str_fd.get () == -1)
- perror_with_name (("mkstemp"));
-
- gdb_file_up out_file_str = out_file_str_fd.to_file ("wb");
- if (out_file_str == nullptr)
- error (_("Can't open `%s' for writing"), filename_str_temp.data ());
-
- unlink_file_str.emplace (filename_str_temp.data ());
-
- const size_t total_len
- = write_debug_names (dwarf2_per_objfile, out_file.get (),
- out_file_str.get ());
- assert_file_size (out_file.get (), filename_temp.data (), total_len);
-
- /* We want to keep the file .debug_str file too. */
- unlink_file_str->keep ();
-
- /* Close and move the str file in place. */
- out_file_str.reset ();
- if (rename (filename_str_temp.data (), filename_str.c_str ()) != 0)
- perror_with_name (("rename"));
+ index_wip_file str_wip_file (dir, basename, DEBUG_STR_SUFFIX);
+
+ write_debug_names (dwarf2_per_objfile, objfile_index_wip.out_file.get (),
+ str_wip_file.out_file.get ());
+
+ str_wip_file.finalize ();
}
else
- {
- const size_t total_len
- = write_gdbindex (dwarf2_per_objfile, out_file.get ());
- assert_file_size (out_file.get (), filename_temp.data (), total_len);
- }
+ write_gdbindex (dwarf2_per_objfile, objfile_index_wip.out_file.get (),
+ (dwz_index_wip.has_value ()
+ ? dwz_index_wip->out_file.get () : NULL));
- /* We want to keep the file. */
- unlink_file->keep ();
+ objfile_index_wip.finalize ();
- /* Close and move the file in place. */
- out_file.reset ();
- if (rename (filename_temp.data (), filename.c_str ()) != 0)
- perror_with_name (("rename"));
+ if (dwz_index_wip.has_value ())
+ dwz_index_wip->finalize ();
}
/* Implementation of the `save gdb-index' command.
@@ -1678,8 +1720,14 @@ save_gdb_index_command (const char *arg, int from_tty)
try
{
const char *basename = lbasename (objfile_name (objfile));
+ const dwz_file *dwz = dwarf2_get_dwz_file (dwarf2_per_objfile);
+ const char *dwz_basename = NULL;
+
+ if (dwz != NULL)
+ dwz_basename = lbasename (dwz->filename ());
+
write_psymtabs_to_index (dwarf2_per_objfile, arg, basename,
- index_kind);
+ dwz_basename, index_kind);
}
catch (const gdb_exception_error &except)
{
diff --git a/gdb/dwarf-index-write.h b/gdb/dwarf-index-write.h
index b1d1180c8e30..a8874c864385 100644
--- a/gdb/dwarf-index-write.h
+++ b/gdb/dwarf-index-write.h
@@ -23,12 +23,17 @@
#include "symfile.h"
#include "dwarf2read.h"
-/* Create an index file for OBJFILE in the directory DIR. BASENAME is the
- desired filename, minus the extension, which gets added by this function
- based on INDEX_KIND. */
+/* Create index files for OBJFILE in the directory DIR.
+
+ An index file is created for OBJFILE itself, and is created for its
+ associated dwz file, if it has one.
+
+ BASENAME is the desired filename base for OBJFILE's index. An extension
+ derived from INDEX_KIND is added to this base name. DWZ_BASENAME is the
+ same, but for the dwz file's index. */
extern void write_psymtabs_to_index
(struct dwarf2_per_objfile *dwarf2_per_objfile, const char *dir,
- const char *basename, dw_index_kind index_kind);
+ const char *basename, const char *dwz_basename, dw_index_kind index_kind);
#endif /* DWARF_INDEX_WRITE_H */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b29c089606db..3e9e8d1c704a 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -875,32 +875,6 @@ struct dwp_file
asection **elf_sections = nullptr;
};
-/* This represents a '.dwz' file. */
-
-struct dwz_file
-{
- dwz_file (gdb_bfd_ref_ptr &&bfd)
- : dwz_bfd (std::move (bfd))
- {
- }
-
- /* A dwz file can only contain a few sections. */
- struct dwarf2_section_info abbrev {};
- struct dwarf2_section_info info {};
- struct dwarf2_section_info str {};
- struct dwarf2_section_info line {};
- struct dwarf2_section_info macro {};
- struct dwarf2_section_info gdb_index {};
- struct dwarf2_section_info debug_names {};
-
- /* The dwz's BFD. */
- gdb_bfd_ref_ptr dwz_bfd;
-
- /* If we loaded the index from an external file, this contains the
- resources associated to the open file, memory mapping, etc. */
- std::unique_ptr<index_cache_resource> index_cache_res;
-};
-
/* Struct used to pass misc. parameters to read_die_and_children, et
al. which are used for both .debug_info and .debug_types dies.
All parameters here are unchanging for the life of the call. This
@@ -2651,11 +2625,9 @@ locate_dwz_sections (bfd *abfd, asection *sectp, void *arg)
}
}
-/* Open the separate '.dwz' debug file, if needed. Return NULL if
- there is no .gnu_debugaltlink section in the file. Error if there
- is such a section but the file cannot be found. */
+/* See dwarf2read.h. */
-static struct dwz_file *
+struct dwz_file *
dwarf2_get_dwz_file (struct dwarf2_per_objfile *dwarf2_per_objfile)
{
const char *filename;
diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
index 732654348a83..e8658950e362 100644
--- a/gdb/dwarf2read.h
+++ b/gdb/dwarf2read.h
@@ -405,4 +405,42 @@ DEF_VEC_P (sig_type_ptr);
ULONGEST read_unsigned_leb128 (bfd *, const gdb_byte *, unsigned int *);
+/* This represents a '.dwz' file. */
+
+struct dwz_file
+{
+ dwz_file (gdb_bfd_ref_ptr &&bfd)
+ : dwz_bfd (std::move (bfd))
+ {
+ }
+
+ const char *filename () const
+ {
+ return bfd_get_filename (this->dwz_bfd);
+ }
+
+ /* A dwz file can only contain a few sections. */
+ struct dwarf2_section_info abbrev {};
+ struct dwarf2_section_info info {};
+ struct dwarf2_section_info str {};
+ struct dwarf2_section_info line {};
+ struct dwarf2_section_info macro {};
+ struct dwarf2_section_info gdb_index {};
+ struct dwarf2_section_info debug_names {};
+
+ /* The dwz's BFD. */
+ gdb_bfd_ref_ptr dwz_bfd;
+
+ /* If we loaded the index from an external file, this contains the
+ resources associated to the open file, memory mapping, etc. */
+ std::unique_ptr<index_cache_resource> index_cache_res;
+};
+
+/* Open the separate '.dwz' debug file, if needed. Return NULL if
+ there is no .gnu_debugaltlink section in the file. Error if there
+ is such a section but the file cannot be found. */
+
+extern struct dwz_file *dwarf2_get_dwz_file
+ (struct dwarf2_per_objfile *dwarf2_per_objfile);
+
#endif /* DWARF2READ_H */
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index.exp b/gdb/testsuite/gdb.dwarf2/gdb-index.exp
index 410e59684f33..6fca3c612992 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-index.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index.exp
@@ -34,8 +34,11 @@ if { [prepare_for_testing "failed to prepare" "${testfile}" \
proc add_gdb_index { program } {
set index_file ${program}.gdb-index
+ set dwz ${program}.dwz
+ set dwz_index_file ${dwz}.gdb-index
verbose -log "index_file: ${index_file}"
remote_file host delete ${index_file}
+ remote_file host delete ${dwz_index_file}
gdb_test_no_output "save gdb-index [file dirname ${index_file}]" \
"save gdb-index for file [file tail ${program}]"
@@ -55,6 +58,18 @@ proc add_gdb_index { program } {
if {[run_on_host "objcopy" [gdb_find_objcopy] "--remove-section .gdb_index --add-section .gdb_index=$index_file --set-section-flags .gdb_index=readonly ${program} ${program_with_index}"]} {
return ""
}
+
+ if { [remote_file host exists ${dwz_index_file}] } {
+ # We're modifying $dwz in place, otherwise we'd have to update
+ # .gnu_debugaltlink in $program.
+ set args [join [list "--remove-section .gdb_index" \
+ " --add-section .gdb_index=$dwz_index_file" \
+ " --set-section-flags .gdb_index=readonly $dwz"]]
+ if {[run_on_host "objcopy" [gdb_find_objcopy] "$args"]} {
+ return ""
+ }
+ }
+
return ${program_with_index}
}
--
2.21.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC, gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs
2019-05-07 16:13 ` [RFC, gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs Tom de Vries
@ 2019-05-12 19:49 ` Simon Marchi
2019-05-13 10:47 ` Tom de Vries
0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2019-05-12 19:49 UTC (permalink / raw)
To: Tom de Vries, gdb-patches, Pedro Alves
On 2019-05-07 12:13 p.m., Tom de Vries wrote:
> [ was: Re: [PATCH][gdb] Write index for dwz -m file ]
>
>
> Hi,
>
> This is a follow-up patch for "[gdb] Write index for dwz -m file".
>
> Any comments on the updated gdb/contrib/gdb-add-index.sh script?
>
> In particular, I'd like some advice on whether I should add shell
> variables (as I've done for readelf) for grep, tail and sed.
>
> Thanks,
> - Tom
>
Hi Tom,
I think it can be useful to override gdb, readelf and objcopy, as it is likely
that people will want to use specific versions of these (either newer, or
specific to their architectures).
But it's not very likely for grep, tail and sed, as long as we make sure that
we are compatible with the BSD versions of these tools. That would mean making
sure we only use features defined by POSIX.
One case that isn't handled correct by GDB and/or the script (with both my and
your patch applied) is running the script on two executables that share the same
external dwz file. It will fail adding the index to the dwz file the second time.
This use case is kind of important, because the point of having dwz files is to
share it between multiple executables.
This is what I get the second time:
$ GDB="/home/simark/build/binutils-gdb/gdb/gdb --data-directory=/home/simark/build/binutils-gdb/gdb/data-directory" ~/src/binutils-gdb/gdb/contrib/gdb-add-index.sh b
+ objcopy --add-section .gdb_index=shared-debug-info.dwz.gdb-index --set-section-flags .gdb_index=readonly shared-debug-info.dwz shared-debug-info.dwz
objcopy:std9IdCI: can't add section '.gdb_index': file in wrong format
I haven't looked in more details for this problem.
There's a buglet in the clean up code causing the dwz file's index
(shared-debug-info.dwz.gdb-index in my case) to be left in the current directory after
running the script (even successfully). This fixes it:
---
diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
index afedce0c848d..2b3af2e84f71 100755
--- a/gdb/contrib/gdb-add-index.sh
+++ b/gdb/contrib/gdb-add-index.sh
@@ -70,7 +70,7 @@ for f in "$file" "$dwz_file"; do
if [ "$f" = "" ]; then
continue
fi
- set_files "$file"
+ set_files "$f"
tmp_files="$tmp_files $index4 $index5 $debugstr $debugstrmerge $debugstrerr"
done
---
Simon
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC, gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs
2019-05-12 19:49 ` Simon Marchi
@ 2019-05-13 10:47 ` Tom de Vries
2019-06-10 18:41 ` [PATCH, " Tom de Vries
0 siblings, 1 reply; 11+ messages in thread
From: Tom de Vries @ 2019-05-13 10:47 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, Pedro Alves
[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]
On 12-05-19 21:49, Simon Marchi wrote:
> On 2019-05-07 12:13 p.m., Tom de Vries wrote:
>> [ was: Re: [PATCH][gdb] Write index for dwz -m file ]
>>
>>
>> Hi,
>>
>> This is a follow-up patch for "[gdb] Write index for dwz -m file".
>>
>> Any comments on the updated gdb/contrib/gdb-add-index.sh script?
>>
>> In particular, I'd like some advice on whether I should add shell
>> variables (as I've done for readelf) for grep, tail and sed.
>>
>> Thanks,
>> - Tom
>>
>
> Hi Tom,
>
> I think it can be useful to override gdb, readelf and objcopy, as it is likely
> that people will want to use specific versions of these (either newer, or
> specific to their architectures).
>
> But it's not very likely for grep, tail and sed, as long as we make sure that
> we are compatible with the BSD versions of these tools. That would mean making
> sure we only use features defined by POSIX.
>
Ack, thanks for the insight.
> One case that isn't handled correct by GDB and/or the script (with both my and
> your patch applied) is running the script on two executables that share the same
> external dwz file. It will fail adding the index to the dwz file the second time.
> This use case is kind of important, because the point of having dwz files is to
> share it between multiple executables.
>
> This is what I get the second time:
>
> $ GDB="/home/simark/build/binutils-gdb/gdb/gdb --data-directory=/home/simark/build/binutils-gdb/gdb/data-directory" ~/src/binutils-gdb/gdb/contrib/gdb-add-index.sh b
> + objcopy --add-section .gdb_index=shared-debug-info.dwz.gdb-index --set-section-flags .gdb_index=readonly shared-debug-info.dwz shared-debug-info.dwz
> objcopy:std9IdCI: can't add section '.gdb_index': file in wrong format
>
> I haven't looked in more details for this problem.
>
That's fixed now.
[ In a way, it's a known problem. Running gdb-add-index twice on the
same (regular, non-dwz-ed) executable, gives the same kind of error. ]
> There's a buglet in the clean up code causing the dwz file's index
> (shared-debug-info.dwz.gdb-index in my case) to be left in the current directory after
> running the script (even successfully). This fixes it:
>
> ---
> diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
> index afedce0c848d..2b3af2e84f71 100755
> --- a/gdb/contrib/gdb-add-index.sh
> +++ b/gdb/contrib/gdb-add-index.sh
> @@ -70,7 +70,7 @@ for f in "$file" "$dwz_file"; do
> if [ "$f" = "" ]; then
> continue
> fi
> - set_files "$file"
> + set_files "$f"
> tmp_files="$tmp_files $index4 $index5 $debugstr $debugstrmerge $debugstrerr"
> done
Ok, I've incorporated that fix, as well as making the handle_file
$dwz_file conditional on $dwz_file != "".
Updated version attached.
Thanks,
- Tom
[-- Attachment #2: 0003-gdb-contrib-Fix-gdb-contrib-gdb-add-index.sh-for-dwz-m-ed-execs.patch --]
[-- Type: text/x-patch, Size: 5204 bytes --]
[gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs
Atm gdb-add-index.exp fails with target board cc-with-dwz-m.
Fix this by updating gdb/contrib/gdb-add-index.sh to handle a dwz-m-ed
executable.
Tested on x86_64-linux.
gdb/ChangeLog:
2019-05-07 Tom de Vries <tdevries@suse.de>
PR gdb/24445
* contrib/gdb-add-index.sh: Update to handle dwz-m-ed executable.
---
gdb/contrib/gdb-add-index.sh | 138 ++++++++++++++++++++++++++++---------------
1 file changed, 91 insertions(+), 47 deletions(-)
diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
index efaad1dce7..5a37020656 100755
--- a/gdb/contrib/gdb-add-index.sh
+++ b/gdb/contrib/gdb-add-index.sh
@@ -20,6 +20,7 @@
# If not, or you want others, pass the following in the environment
GDB=${GDB:=gdb}
OBJCOPY=${OBJCOPY:=objcopy}
+READELF=${READELF:=readelf}
myname="${0##*/}"
@@ -43,15 +44,44 @@ fi
dir="${file%/*}"
test "$dir" = "$file" && dir="."
-index4="${file}.gdb-index"
-index5="${file}.debug_names"
-debugstr="${file}.debug_str"
-debugstrmerge="${file}.debug_str.merge"
-debugstrerr="${file}.debug_str.err"
-rm -f $index4 $index5 $debugstr $debugstrmerge $debugstrerr
+dwz_file=""
+if $READELF -S "$file" | grep -q " \.gnu_debugaltlink "; then
+ dwz_file=$($READELF --string-dump=.gnu_debugaltlink "$file" \
+ | grep -A1 "'\.gnu_debugaltlink':" \
+ | tail -n +2 \
+ | sed 's/.*]//')
+ dwz_file=$(echo $dwz_file)
+ if $READELF -S "$dwz_file" | grep -E -q " \.(gdb_index|debug_names) "; then
+ # Already has an index, skip it.
+ dwz_file=""
+ fi
+fi
+
+set_files ()
+{
+ local file="$1"
+
+ index4="${file}.gdb-index"
+ index5="${file}.debug_names"
+ debugstr="${file}.debug_str"
+ debugstrmerge="${file}.debug_str.merge"
+ debugstrerr="${file}.debug_str.err"
+}
+
+tmp_files=
+for f in "$file" "$dwz_file"; do
+ if [ "$f" = "" ]; then
+ continue
+ fi
+ set_files "$f"
+ tmp_files="$tmp_files $index4 $index5 $debugstr $debugstrmerge $debugstrerr"
+done
+
+rm -f $tmp_files
+
# Ensure intermediate index file is removed when we exit.
-trap "rm -f $index4 $index5 $debugstr $debugstrmerge $debugstrerr" 0
+trap "rm -f $tmp_files" 0
$GDB --batch -nx -iex 'set auto-load no' \
-ex "file $file" -ex "save gdb-index $dwarf5 $dir" || {
@@ -67,50 +97,64 @@ $GDB --batch -nx -iex 'set auto-load no' \
# already stripped binary, it's a no-op.
status=0
-if test -f "$index4" -a -f "$index5"; then
- echo "$myname: Both index types were created for $file" 1>&2
- status=1
-elif test -f "$index4" -o -f "$index5"; then
- if test -f "$index4"; then
- index="$index4"
- section=".gdb_index"
- else
- index="$index5"
- section=".debug_names"
- fi
- debugstradd=false
- debugstrupdate=false
- if test -s "$debugstr"; then
- if ! $OBJCOPY --dump-section .debug_str="$debugstrmerge" "$file" /dev/null \
- 2>$debugstrerr; then
- cat >&2 $debugstrerr
- exit 1
- fi
- if grep -q "can't dump section '.debug_str' - it does not exist" \
- $debugstrerr; then
- debugstradd=true
+handle_file ()
+{
+ local file
+ file="$1"
+
+ set_files "$file"
+
+ if test -f "$index4" -a -f "$index5"; then
+ echo "$myname: Both index types were created for $file" 1>&2
+ status=1
+ elif test -f "$index4" -o -f "$index5"; then
+ if test -f "$index4"; then
+ index="$index4"
+ section=".gdb_index"
else
- debugstrupdate=true
- cat >&2 $debugstrerr
+ index="$index5"
+ section=".debug_names"
+ fi
+ debugstradd=false
+ debugstrupdate=false
+ if test -s "$debugstr"; then
+ if ! $OBJCOPY --dump-section .debug_str="$debugstrmerge" "$file" \
+ /dev/null 2>$debugstrerr; then
+ cat >&2 $debugstrerr
+ exit 1
+ fi
+ if grep -q "can't dump section '.debug_str' - it does not exist" \
+ $debugstrerr; then
+ debugstradd=true
+ else
+ debugstrupdate=true
+ cat >&2 $debugstrerr
+ fi
+ cat "$debugstr" >>"$debugstrmerge"
fi
- cat "$debugstr" >>"$debugstrmerge"
- fi
- $OBJCOPY --add-section $section="$index" \
- --set-section-flags $section=readonly \
- $(if $debugstradd; then \
- echo --add-section .debug_str="$debugstrmerge"; \
- echo --set-section-flags .debug_str=readonly; \
- fi; \
- if $debugstrupdate; then \
- echo --update-section .debug_str="$debugstrmerge"; \
- fi) \
- "$file" "$file"
+ $OBJCOPY --add-section $section="$index" \
+ --set-section-flags $section=readonly \
+ $(if $debugstradd; then \
+ echo --add-section .debug_str="$debugstrmerge"; \
+ echo --set-section-flags .debug_str=readonly; \
+ fi; \
+ if $debugstrupdate; then \
+ echo --update-section .debug_str="$debugstrmerge"; \
+ fi) \
+ "$file" "$file"
+
+ status=$?
+ else
+ echo "$myname: No index was created for $file" 1>&2
+ echo "$myname: [Was there no debuginfo? Was there already an index?]" \
+ 1>&2
+ fi
+}
- status=$?
-else
- echo "$myname: No index was created for $file" 1>&2
- echo "$myname: [Was there no debuginfo? Was there already an index?]" 1>&2
+handle_file "$file"
+if [ "$dwz_file" != "" ]; then
+ handle_file "$dwz_file"
fi
exit $status
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][gdb] Write index for dwz -m file
2019-05-11 2:36 ` [PATCH][gdb] Write index for dwz -m file Simon Marchi
@ 2019-05-13 12:22 ` Tom de Vries
2019-05-13 13:56 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Tom de Vries @ 2019-05-13 12:22 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 11-05-19 04:36, Simon Marchi wrote:
> @@ -122,12 +144,13 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
>
> if (debug_index_cache)
> printf_unfiltered ("index cache: writing index cache for objfile %s\n",
> - objfile_name (obj));
> + objfile_name (obj));
>
> /* Write the index itself to the directory, using the build id as the
> filename. */
LGTM.
[ I'm not sure if this white-space-only change should be included though. ]
Thanks,
- Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH][gdb] Write index for dwz -m file
2019-05-13 12:22 ` Tom de Vries
@ 2019-05-13 13:56 ` Simon Marchi
2019-06-16 10:43 ` [PING][PATCH][gdb] " Tom de Vries
0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2019-05-13 13:56 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 2019-05-13 8:22 a.m., Tom de Vries wrote:
> On 11-05-19 04:36, Simon Marchi wrote:
>> @@ -122,12 +144,13 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
>>
>> if (debug_index_cache)
>> printf_unfiltered ("index cache: writing index cache for objfile %s\n",
>> - objfile_name (obj));
>> + objfile_name (obj));
>>
>> /* Write the index itself to the directory, using the build id as the
>> filename. */
>
> LGTM.
>
> [ I'm not sure if this white-space-only change should be included though. ]
>
> Thanks,
> - Tom
Hmm probably not, but I am tempted to leave it in, since I am touching the lines around it (and it
fixes the wrong indentation).
I will leave this patch up for review for a week or so, to give the others a chance to comment.
Simon
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH, gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs
2019-05-13 10:47 ` Tom de Vries
@ 2019-06-10 18:41 ` Tom de Vries
2019-06-16 17:41 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Tom de Vries @ 2019-06-10 18:41 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, Pedro Alves
On 13-05-19 12:47, Tom de Vries wrote:
> On 12-05-19 21:49, Simon Marchi wrote:
>> On 2019-05-07 12:13 p.m., Tom de Vries wrote:
>>> [ was: Re: [PATCH][gdb] Write index for dwz -m file ]
>>>
>>>
>>> Hi,
>>>
>>> This is a follow-up patch for "[gdb] Write index for dwz -m file".
>>>
>>> Any comments on the updated gdb/contrib/gdb-add-index.sh script?
>>>
>>> In particular, I'd like some advice on whether I should add shell
>>> variables (as I've done for readelf) for grep, tail and sed.
>>>
>>> Thanks,
>>> - Tom
>>>
>> Hi Tom,
>>
>> I think it can be useful to override gdb, readelf and objcopy, as it is likely
>> that people will want to use specific versions of these (either newer, or
>> specific to their architectures).
>>
>> But it's not very likely for grep, tail and sed, as long as we make sure that
>> we are compatible with the BSD versions of these tools. That would mean making
>> sure we only use features defined by POSIX.
>>
> Ack, thanks for the insight.
>
>> One case that isn't handled correct by GDB and/or the script (with both my and
>> your patch applied) is running the script on two executables that share the same
>> external dwz file. It will fail adding the index to the dwz file the second time.
>> This use case is kind of important, because the point of having dwz files is to
>> share it between multiple executables.
>>
>> This is what I get the second time:
>>
>> $ GDB="/home/simark/build/binutils-gdb/gdb/gdb --data-directory=/home/simark/build/binutils-gdb/gdb/data-directory" ~/src/binutils-gdb/gdb/contrib/gdb-add-index.sh b
>> + objcopy --add-section .gdb_index=shared-debug-info.dwz.gdb-index --set-section-flags .gdb_index=readonly shared-debug-info.dwz shared-debug-info.dwz
>> objcopy:std9IdCI: can't add section '.gdb_index': file in wrong format
>>
>> I haven't looked in more details for this problem.
>>
> That's fixed now.
>
> [ In a way, it's a known problem. Running gdb-add-index twice on the
> same (regular, non-dwz-ed) executable, gives the same kind of error. ]
>
>> There's a buglet in the clean up code causing the dwz file's index
>> (shared-debug-info.dwz.gdb-index in my case) to be left in the current directory after
>> running the script (even successfully). This fixes it:
>>
>> ---
>> diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
>> index afedce0c848d..2b3af2e84f71 100755
>> --- a/gdb/contrib/gdb-add-index.sh
>> +++ b/gdb/contrib/gdb-add-index.sh
>> @@ -70,7 +70,7 @@ for f in "$file" "$dwz_file"; do
>> if [ "$f" = "" ]; then
>> continue
>> fi
>> - set_files "$file"
>> + set_files "$f"
>> tmp_files="$tmp_files $index4 $index5 $debugstr $debugstrmerge $debugstrerr"
>> done
> Ok, I've incorporated that fix, as well as making the handle_file
> $dwz_file conditional on $dwz_file != "".
>
> Updated version attached.
>
OK for trunk?
Thanks,
- Tom
>
> 0003-gdb-contrib-Fix-gdb-contrib-gdb-add-index.sh-for-dwz-m-ed-execs.patch
>
> [gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs
>
> Atm gdb-add-index.exp fails with target board cc-with-dwz-m.
>
> Fix this by updating gdb/contrib/gdb-add-index.sh to handle a dwz-m-ed
> executable.
>
> Tested on x86_64-linux.
>
> gdb/ChangeLog:
>
> 2019-05-07 Tom de Vries <tdevries@suse.de>
>
> PR gdb/24445
> * contrib/gdb-add-index.sh: Update to handle dwz-m-ed executable.
>
> ---
> gdb/contrib/gdb-add-index.sh | 138 ++++++++++++++++++++++++++++---------------
> 1 file changed, 91 insertions(+), 47 deletions(-)
>
> diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
> index efaad1dce7..5a37020656 100755
> --- a/gdb/contrib/gdb-add-index.sh
> +++ b/gdb/contrib/gdb-add-index.sh
> @@ -20,6 +20,7 @@
> # If not, or you want others, pass the following in the environment
> GDB=${GDB:=gdb}
> OBJCOPY=${OBJCOPY:=objcopy}
> +READELF=${READELF:=readelf}
>
> myname="${0##*/}"
>
> @@ -43,15 +44,44 @@ fi
>
> dir="${file%/*}"
> test "$dir" = "$file" && dir="."
> -index4="${file}.gdb-index"
> -index5="${file}.debug_names"
> -debugstr="${file}.debug_str"
> -debugstrmerge="${file}.debug_str.merge"
> -debugstrerr="${file}.debug_str.err"
>
> -rm -f $index4 $index5 $debugstr $debugstrmerge $debugstrerr
> +dwz_file=""
> +if $READELF -S "$file" | grep -q " \.gnu_debugaltlink "; then
> + dwz_file=$($READELF --string-dump=.gnu_debugaltlink "$file" \
> + | grep -A1 "'\.gnu_debugaltlink':" \
> + | tail -n +2 \
> + | sed 's/.*]//')
> + dwz_file=$(echo $dwz_file)
> + if $READELF -S "$dwz_file" | grep -E -q " \.(gdb_index|debug_names) "; then
> + # Already has an index, skip it.
> + dwz_file=""
> + fi
> +fi
> +
> +set_files ()
> +{
> + local file="$1"
> +
> + index4="${file}.gdb-index"
> + index5="${file}.debug_names"
> + debugstr="${file}.debug_str"
> + debugstrmerge="${file}.debug_str.merge"
> + debugstrerr="${file}.debug_str.err"
> +}
> +
> +tmp_files=
> +for f in "$file" "$dwz_file"; do
> + if [ "$f" = "" ]; then
> + continue
> + fi
> + set_files "$f"
> + tmp_files="$tmp_files $index4 $index5 $debugstr $debugstrmerge $debugstrerr"
> +done
> +
> +rm -f $tmp_files
> +
> # Ensure intermediate index file is removed when we exit.
> -trap "rm -f $index4 $index5 $debugstr $debugstrmerge $debugstrerr" 0
> +trap "rm -f $tmp_files" 0
>
> $GDB --batch -nx -iex 'set auto-load no' \
> -ex "file $file" -ex "save gdb-index $dwarf5 $dir" || {
> @@ -67,50 +97,64 @@ $GDB --batch -nx -iex 'set auto-load no' \
> # already stripped binary, it's a no-op.
> status=0
>
> -if test -f "$index4" -a -f "$index5"; then
> - echo "$myname: Both index types were created for $file" 1>&2
> - status=1
> -elif test -f "$index4" -o -f "$index5"; then
> - if test -f "$index4"; then
> - index="$index4"
> - section=".gdb_index"
> - else
> - index="$index5"
> - section=".debug_names"
> - fi
> - debugstradd=false
> - debugstrupdate=false
> - if test -s "$debugstr"; then
> - if ! $OBJCOPY --dump-section .debug_str="$debugstrmerge" "$file" /dev/null \
> - 2>$debugstrerr; then
> - cat >&2 $debugstrerr
> - exit 1
> - fi
> - if grep -q "can't dump section '.debug_str' - it does not exist" \
> - $debugstrerr; then
> - debugstradd=true
> +handle_file ()
> +{
> + local file
> + file="$1"
> +
> + set_files "$file"
> +
> + if test -f "$index4" -a -f "$index5"; then
> + echo "$myname: Both index types were created for $file" 1>&2
> + status=1
> + elif test -f "$index4" -o -f "$index5"; then
> + if test -f "$index4"; then
> + index="$index4"
> + section=".gdb_index"
> else
> - debugstrupdate=true
> - cat >&2 $debugstrerr
> + index="$index5"
> + section=".debug_names"
> + fi
> + debugstradd=false
> + debugstrupdate=false
> + if test -s "$debugstr"; then
> + if ! $OBJCOPY --dump-section .debug_str="$debugstrmerge" "$file" \
> + /dev/null 2>$debugstrerr; then
> + cat >&2 $debugstrerr
> + exit 1
> + fi
> + if grep -q "can't dump section '.debug_str' - it does not exist" \
> + $debugstrerr; then
> + debugstradd=true
> + else
> + debugstrupdate=true
> + cat >&2 $debugstrerr
> + fi
> + cat "$debugstr" >>"$debugstrmerge"
> fi
> - cat "$debugstr" >>"$debugstrmerge"
> - fi
>
> - $OBJCOPY --add-section $section="$index" \
> - --set-section-flags $section=readonly \
> - $(if $debugstradd; then \
> - echo --add-section .debug_str="$debugstrmerge"; \
> - echo --set-section-flags .debug_str=readonly; \
> - fi; \
> - if $debugstrupdate; then \
> - echo --update-section .debug_str="$debugstrmerge"; \
> - fi) \
> - "$file" "$file"
> + $OBJCOPY --add-section $section="$index" \
> + --set-section-flags $section=readonly \
> + $(if $debugstradd; then \
> + echo --add-section .debug_str="$debugstrmerge"; \
> + echo --set-section-flags .debug_str=readonly; \
> + fi; \
> + if $debugstrupdate; then \
> + echo --update-section .debug_str="$debugstrmerge"; \
> + fi) \
> + "$file" "$file"
> +
> + status=$?
> + else
> + echo "$myname: No index was created for $file" 1>&2
> + echo "$myname: [Was there no debuginfo? Was there already an index?]" \
> + 1>&2
> + fi
> +}
>
> - status=$?
> -else
> - echo "$myname: No index was created for $file" 1>&2
> - echo "$myname: [Was there no debuginfo? Was there already an index?]" 1>&2
> +handle_file "$file"
> +if [ "$dwz_file" != "" ]; then
> + handle_file "$dwz_file"
> fi
>
> exit $status
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PING][PATCH][gdb] Write index for dwz -m file
2019-05-13 13:56 ` Simon Marchi
@ 2019-06-16 10:43 ` Tom de Vries
2019-06-16 17:22 ` Simon Marchi
0 siblings, 1 reply; 11+ messages in thread
From: Tom de Vries @ 2019-06-16 10:43 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 13-05-19 15:56, Simon Marchi wrote:
> On 2019-05-13 8:22 a.m., Tom de Vries wrote:
>> On 11-05-19 04:36, Simon Marchi wrote:
>>> @@ -122,12 +144,13 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
>>>
>>> if (debug_index_cache)
>>> printf_unfiltered ("index cache: writing index cache for objfile %s\n",
>>> - objfile_name (obj));
>>> + objfile_name (obj));
>>>
>>> /* Write the index itself to the directory, using the build id as the
>>> filename. */
>>
>> LGTM.
>>
>> [ I'm not sure if this white-space-only change should be included though. ]
>>
>> Thanks,
>> - Tom
>
> Hmm probably not, but I am tempted to leave it in, since I am touching the lines around it (and it
> fixes the wrong indentation).
>
> I will leave this patch up for review for a week or so, to give the others a chance to comment.
>
Ping.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PING][PATCH][gdb] Write index for dwz -m file
2019-06-16 10:43 ` [PING][PATCH][gdb] " Tom de Vries
@ 2019-06-16 17:22 ` Simon Marchi
0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2019-06-16 17:22 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
On 2019-06-16 6:43 a.m., Tom de Vries wrote:
> On 13-05-19 15:56, Simon Marchi wrote:
>> On 2019-05-13 8:22 a.m., Tom de Vries wrote:
>>> On 11-05-19 04:36, Simon Marchi wrote:
>>>> @@ -122,12 +144,13 @@ index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
>>>>
>>>> if (debug_index_cache)
>>>> printf_unfiltered ("index cache: writing index cache for objfile %s\n",
>>>> - objfile_name (obj));
>>>> + objfile_name (obj));
>>>>
>>>> /* Write the index itself to the directory, using the build id as the
>>>> filename. */
>>>
>>> LGTM.
>>>
>>> [ I'm not sure if this white-space-only change should be included though. ]
>>>
>>> Thanks,
>>> - Tom
>>
>> Hmm probably not, but I am tempted to leave it in, since I am touching the lines around it (and it
>> fixes the wrong indentation).
>>
>> I will leave this patch up for review for a week or so, to give the others a chance to comment.
>>
>
> Ping.
>
> Thanks,
> - Tom
Ah, thanks for reminding me. I don't have so much time to spend on GDB these days, so things
easily get forgotten.
I'll give it a last look and push it.
Simon
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH, gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs
2019-06-10 18:41 ` [PATCH, " Tom de Vries
@ 2019-06-16 17:41 ` Simon Marchi
0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2019-06-16 17:41 UTC (permalink / raw)
To: Tom de Vries, gdb-patches, Pedro Alves
On 2019-06-10 2:41 p.m., Tom de Vries wrote:
> On 13-05-19 12:47, Tom de Vries wrote:
>> On 12-05-19 21:49, Simon Marchi wrote:
>>> On 2019-05-07 12:13 p.m., Tom de Vries wrote:
>>>> [ was: Re: [PATCH][gdb] Write index for dwz -m file ]
>>>>
>>>>
>>>> Hi,
>>>>
>>>> This is a follow-up patch for "[gdb] Write index for dwz -m file".
>>>>
>>>> Any comments on the updated gdb/contrib/gdb-add-index.sh script?
>>>>
>>>> In particular, I'd like some advice on whether I should add shell
>>>> variables (as I've done for readelf) for grep, tail and sed.
>>>>
>>>> Thanks,
>>>> - Tom
>>>>
>>> Hi Tom,
>>>
>>> I think it can be useful to override gdb, readelf and objcopy, as it is likely
>>> that people will want to use specific versions of these (either newer, or
>>> specific to their architectures).
>>>
>>> But it's not very likely for grep, tail and sed, as long as we make sure that
>>> we are compatible with the BSD versions of these tools. That would mean making
>>> sure we only use features defined by POSIX.
>>>
>> Ack, thanks for the insight.
>>
>>> One case that isn't handled correct by GDB and/or the script (with both my and
>>> your patch applied) is running the script on two executables that share the same
>>> external dwz file. It will fail adding the index to the dwz file the second time.
>>> This use case is kind of important, because the point of having dwz files is to
>>> share it between multiple executables.
>>>
>>> This is what I get the second time:
>>>
>>> $ GDB="/home/simark/build/binutils-gdb/gdb/gdb --data-directory=/home/simark/build/binutils-gdb/gdb/data-directory" ~/src/binutils-gdb/gdb/contrib/gdb-add-index.sh b
>>> + objcopy --add-section .gdb_index=shared-debug-info.dwz.gdb-index --set-section-flags .gdb_index=readonly shared-debug-info.dwz shared-debug-info.dwz
>>> objcopy:std9IdCI: can't add section '.gdb_index': file in wrong format
>>>
>>> I haven't looked in more details for this problem.
>>>
>> That's fixed now.
>>
>> [ In a way, it's a known problem. Running gdb-add-index twice on the
>> same (regular, non-dwz-ed) executable, gives the same kind of error. ]
>>
>>> There's a buglet in the clean up code causing the dwz file's index
>>> (shared-debug-info.dwz.gdb-index in my case) to be left in the current directory after
>>> running the script (even successfully). This fixes it:
>>>
>>> ---
>>> diff --git a/gdb/contrib/gdb-add-index.sh b/gdb/contrib/gdb-add-index.sh
>>> index afedce0c848d..2b3af2e84f71 100755
>>> --- a/gdb/contrib/gdb-add-index.sh
>>> +++ b/gdb/contrib/gdb-add-index.sh
>>> @@ -70,7 +70,7 @@ for f in "$file" "$dwz_file"; do
>>> if [ "$f" = "" ]; then
>>> continue
>>> fi
>>> - set_files "$file"
>>> + set_files "$f"
>>> tmp_files="$tmp_files $index4 $index5 $debugstr $debugstrmerge $debugstrerr"
>>> done
>> Ok, I've incorporated that fix, as well as making the handle_file
>> $dwz_file conditional on $dwz_file != "".
>>
>> Updated version attached.
>>
>
> OK for trunk?
>
> Thanks,
> - Tom
Hi Tom,
Thanks, this LGTM.
Simon
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-06-16 17:41 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-07 14:42 [PATCH][gdb] Write index for dwz -m file Tom de Vries
2019-05-07 16:13 ` [RFC, gdb/contrib] Fix gdb/contrib/gdb-add-index.sh for dwz-m-ed execs Tom de Vries
2019-05-12 19:49 ` Simon Marchi
2019-05-13 10:47 ` Tom de Vries
2019-06-10 18:41 ` [PATCH, " Tom de Vries
2019-06-16 17:41 ` Simon Marchi
2019-05-11 2:36 ` [PATCH][gdb] Write index for dwz -m file Simon Marchi
2019-05-13 12:22 ` Tom de Vries
2019-05-13 13:56 ` Simon Marchi
2019-06-16 10:43 ` [PING][PATCH][gdb] " Tom de Vries
2019-06-16 17:22 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).