* [PATCH RFC 3/5] Make index reading functions more modular
2018-05-09 21:27 [PATCH RFC 0/5] Add a DWARF index cache Simon Marchi
2018-05-09 21:27 ` [PATCH RFC 5/5] Add " Simon Marchi
2018-05-09 21:27 ` [PATCH RFC 4/5] Introduce scoped_mmapped_file Simon Marchi
@ 2018-05-09 21:27 ` Simon Marchi
2018-05-10 21:02 ` Tom Tromey
2018-05-09 21:27 ` [PATCH RFC 2/5] Remove mapped_index::total_size Simon Marchi
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2018-05-09 21:27 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
The read_gdb_index_from_section and read_debug_names_from_section
functions read the index content, as their names state, from sections of
object files. A following patch will make it possible to read index
content from standalone files.
This patch therefore decouples the code that reads the index content
from object file sections from the code that processes that content.
Functions dwarf2_read_gdb_index and dwarf2_read_debug_names receive
callbacks that are responsible for providing the index contents (for
both the main file and the potential dwz file).
gdb/ChangeLog:
* dwarf2read.c (read_gdb_index_from_section): Rename to...
(read_gdb_index_from_buffer): ... this. Remove section
parameter, add buffer parameter.
(get_gdb_index_contents_ftype,
get_gdb_index_contents_dwz_ftype): New typedefs.
(dwarf2_read_gdb_index): Add callback parameters to get the
index contents.
(read_debug_names_from_section): Rename to...
(read_debug_names_from_buffer): ... this. Remove section
parameter, add buffer and abfd parameters.
(get_debug_names_contents_ftype,
get_debug_names_contents_dwz_ftype): New typedefs.
(dwarf2_read_debug_names): Add callbacks parameters to get index
contents.
(get_gdb_index_contents_from_section): New.
(get_debug_names_contents_from_section): New.
(dwarf2_initialize_objfile): Update calls to
dwarf2_read_debug_names and dwarf2_read_gdb_index.
---
gdb/dwarf2read.c | 207 ++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 136 insertions(+), 71 deletions(-)
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 1c72818..bc92756 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3406,8 +3406,8 @@ find_slot_in_mapped_hash (struct mapped_index *index, const char *name,
}
}
-/* A helper function that reads the .gdb_index from SECTION and fills
- in MAP. FILENAME is the name of the file containing the section;
+/* A helper function that reads the .gdb_index from BUFFER and fills
+ in MAP. FILENAME is the name of the file containing the data;
it is used for error reporting. DEPRECATED_OK is true if it is
ok to use deprecated sections.
@@ -3415,37 +3415,23 @@ find_slot_in_mapped_hash (struct mapped_index *index, const char *name,
out parameters that are filled in with information about the CU and
TU lists in the section.
- Returns 1 if all went well, 0 otherwise. */
+ Returns true if all went well, false otherwise. */
static bool
-read_gdb_index_from_section (struct objfile *objfile,
- const char *filename,
- bool deprecated_ok,
- struct dwarf2_section_info *section,
- struct mapped_index *map,
- const gdb_byte **cu_list,
- offset_type *cu_list_elements,
- const gdb_byte **types_list,
- offset_type *types_list_elements)
-{
- const gdb_byte *addr;
- offset_type version;
- offset_type *metadata;
- int i;
-
- if (dwarf2_section_empty_p (section))
- return 0;
+read_gdb_index_from_buffer (struct objfile *objfile,
+ const char *filename,
+ bool deprecated_ok,
+ gdb::array_view<const gdb_byte> buffer,
+ struct mapped_index *map,
+ const gdb_byte **cu_list,
+ offset_type *cu_list_elements,
+ const gdb_byte **types_list,
+ offset_type *types_list_elements)
+{
+ const gdb_byte *addr = &buffer[0];
- /* Older elfutils strip versions could keep the section in the main
- executable while splitting it for the separate debug info file. */
- if ((get_section_flags (section) & SEC_HAS_CONTENTS) == 0)
- return 0;
-
- dwarf2_read_section (objfile, section);
-
- addr = section->buffer;
/* Version check. */
- version = MAYBE_SWAP (*(offset_type *) addr);
+ offset_type version = MAYBE_SWAP (*(offset_type *) addr);
/* Versions earlier than 3 emitted every copy of a psymbol. This
causes the index to behave very poorly for certain requests. Version 3
contained incomplete addrmap. So, it seems better to just ignore such
@@ -3498,9 +3484,9 @@ to use the section anyway."),
map->version = version;
- metadata = (offset_type *) (addr + sizeof (offset_type));
+ offset_type *metadata = (offset_type *) (addr + sizeof (offset_type));
- i = 0;
+ int i = 0;
*cu_list = addr + MAYBE_SWAP (metadata[i]);
*cu_list_elements = ((MAYBE_SWAP (metadata[i + 1]) - MAYBE_SWAP (metadata[i]))
/ 8);
@@ -3531,11 +3517,23 @@ to use the section anyway."),
return 1;
}
+/* Callback types for dwarf2_read_gdb_index. */
+
+typedef gdb::function_view
+ <gdb::array_view<const gdb_byte>(objfile *, dwarf2_per_objfile *)>
+ get_gdb_index_contents_ftype;
+typedef gdb::function_view
+ <gdb::array_view<const gdb_byte>(objfile *, dwz_file *)>
+ get_gdb_index_contents_dwz_ftype;
+
/* Read .gdb_index. If everything went ok, initialize the "quick"
elements of all the CUs and return 1. Otherwise, return 0. */
static int
-dwarf2_read_gdb_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
+dwarf2_read_gdb_index
+ (struct dwarf2_per_objfile *dwarf2_per_objfile,
+ get_gdb_index_contents_ftype get_gdb_index_contents,
+ get_gdb_index_contents_dwz_ftype get_gdb_index_contents_dwz)
{
struct mapped_index local_map, *map;
const gdb_byte *cu_list, *types_list, *dwz_list = NULL;
@@ -3543,11 +3541,17 @@ dwarf2_read_gdb_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
struct dwz_file *dwz;
struct objfile *objfile = dwarf2_per_objfile->objfile;
- if (!read_gdb_index_from_section (objfile, objfile_name (objfile),
- use_deprecated_index_sections,
- &dwarf2_per_objfile->gdb_index, &local_map,
- &cu_list, &cu_list_elements, &types_list,
- &types_list_elements))
+ gdb::array_view<const gdb_byte> main_index_contents
+ = get_gdb_index_contents (objfile, dwarf2_per_objfile);
+
+ if (main_index_contents.empty ())
+ return 0;
+
+ if (!read_gdb_index_from_buffer (objfile, objfile_name (objfile),
+ use_deprecated_index_sections,
+ main_index_contents, &local_map, &cu_list,
+ &cu_list_elements, &types_list,
+ &types_list_elements))
return 0;
/* Don't use the index if it's empty. */
@@ -3563,12 +3567,18 @@ dwarf2_read_gdb_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
const gdb_byte *dwz_types_ignore;
offset_type dwz_types_elements_ignore;
- if (!read_gdb_index_from_section (objfile,
- bfd_get_filename (dwz->dwz_bfd), 1,
- &dwz->gdb_index, &dwz_map,
- &dwz_list, &dwz_list_elements,
- &dwz_types_ignore,
- &dwz_types_elements_ignore))
+ gdb::array_view<const gdb_byte> dwz_index_content
+ = get_gdb_index_contents_dwz (objfile, dwz);
+
+ if (dwz_index_content.empty ())
+ return 0;
+
+ if (!read_gdb_index_from_buffer (objfile,
+ bfd_get_filename (dwz->dwz_bfd), 1,
+ dwz_index_content, &dwz_map,
+ &dwz_list, &dwz_list_elements,
+ &dwz_types_ignore,
+ &dwz_types_elements_ignore))
{
warning (_("could not read '.gdb_index' section from %s; skipping"),
bfd_get_filename (dwz->dwz_bfd));
@@ -5357,40 +5367,28 @@ static const gdb_byte dwarf5_augmentation[] = { 'G', 'D', 'B', 0 };
Returns true if all went well, false otherwise. */
static bool
-read_debug_names_from_section (struct objfile *objfile,
- const char *filename,
- struct dwarf2_section_info *section,
- mapped_debug_names &map)
+read_debug_names_from_buffer (struct objfile *objfile,
+ const char *filename,
+ gdb::array_view<const gdb_byte> buffer,
+ bfd *abfd, mapped_debug_names &map)
{
- if (dwarf2_section_empty_p (section))
- return false;
-
- /* Older elfutils strip versions could keep the section in the main
- executable while splitting it for the separate debug info file. */
- if ((get_section_flags (section) & SEC_HAS_CONTENTS) == 0)
- return false;
-
- dwarf2_read_section (objfile, section);
+ const gdb_byte *addr = &buffer[0];
map.dwarf5_byte_order = gdbarch_byte_order (get_objfile_arch (objfile));
- const gdb_byte *addr = section->buffer;
-
- bfd *const abfd = get_section_bfd_owner (section);
-
unsigned int bytes_read;
LONGEST length = read_initial_length (abfd, addr, &bytes_read);
addr += bytes_read;
map.dwarf5_is_dwarf64 = bytes_read != 4;
map.offset_size = map.dwarf5_is_dwarf64 ? 8 : 4;
- if (bytes_read + length != section->size)
+ if (bytes_read + length != buffer.size ())
{
/* There may be multiple per-CU indices. */
warning (_("Section .debug_names in %s length %s does not match "
"section length %s, ignoring .debug_names."),
filename, plongest (bytes_read + length),
- pulongest (section->size));
+ pulongest (buffer.size ()));
return false;
}
@@ -5592,19 +5590,37 @@ create_cus_from_debug_names (struct dwarf2_per_objfile *dwarf2_per_objfile,
true /* is_dwz */);
}
+/* Callback types for dwarf2_read_debug_names. */
+
+typedef gdb::function_view
+ <gdb::array_view<const gdb_byte>(objfile *, dwarf2_per_objfile *)>
+ get_debug_names_contents_ftype;
+typedef gdb::function_view
+ <gdb::array_view<const gdb_byte>(objfile *, dwz_file *)>
+ get_debug_names_contents_dwz_ftype;
+
/* Read .debug_names. If everything went ok, initialize the "quick"
elements of all the CUs and return true. Otherwise, return false. */
static bool
-dwarf2_read_debug_names (struct dwarf2_per_objfile *dwarf2_per_objfile)
+dwarf2_read_debug_names
+ (struct dwarf2_per_objfile *dwarf2_per_objfile,
+ get_debug_names_contents_ftype get_debug_names_contents,
+ get_debug_names_contents_dwz_ftype get_debug_names_contents_dwz)
{
mapped_debug_names local_map (dwarf2_per_objfile);
mapped_debug_names dwz_map (dwarf2_per_objfile);
struct objfile *objfile = dwarf2_per_objfile->objfile;
- if (!read_debug_names_from_section (objfile, objfile_name (objfile),
- &dwarf2_per_objfile->debug_names,
- local_map))
+ gdb::array_view<const gdb_byte> main_debug_names_content
+ = get_debug_names_contents (objfile, dwarf2_per_objfile);
+
+ if (main_debug_names_content.empty ())
+ return false;
+
+ if (!read_debug_names_from_buffer (objfile, objfile_name (objfile),
+ main_debug_names_content, objfile->obfd,
+ local_map))
return false;
/* Don't use the index if it's empty. */
@@ -5616,9 +5632,16 @@ dwarf2_read_debug_names (struct dwarf2_per_objfile *dwarf2_per_objfile)
dwz_file *dwz = dwarf2_get_dwz_file (dwarf2_per_objfile);
if (dwz != NULL)
{
- if (!read_debug_names_from_section (objfile,
- bfd_get_filename (dwz->dwz_bfd),
- &dwz->debug_names, dwz_map))
+ gdb::array_view<const gdb_byte> dwz_debug_names_content
+ = get_debug_names_contents_dwz (objfile, dwz);
+
+ if (dwz_debug_names_content.empty ())
+ return false;
+
+ if (!read_debug_names_from_buffer (objfile,
+ bfd_get_filename (dwz->dwz_bfd),
+ dwz_debug_names_content, dwz->dwz_bfd,
+ dwz_map))
{
warning (_("could not read '.debug_names' section from %s; skipping"),
bfd_get_filename (dwz->dwz_bfd));
@@ -6166,6 +6189,44 @@ const struct quick_symbol_functions dwarf2_debug_names_functions =
dw2_map_symbol_filenames
};
+template <typename T>
+static gdb::array_view<const gdb_byte>
+get_gdb_index_contents_from_section (objfile *obj, T *section_owner)
+{
+ dwarf2_section_info *section = §ion_owner->gdb_index;
+
+ if (dwarf2_section_empty_p (section))
+ return {};
+
+ /* Older elfutils strip versions could keep the section in the main
+ executable while splitting it for the separate debug info file. */
+ if ((get_section_flags (section) & SEC_HAS_CONTENTS) == 0)
+ return {};
+
+ dwarf2_read_section (obj, section);
+
+ return {section->buffer, section->size};
+}
+
+template<typename T>
+static gdb::array_view<const gdb_byte>
+get_debug_names_contents_from_section (objfile *obj, T *section_owner)
+{
+ dwarf2_section_info *section = §ion_owner->debug_names;
+
+ if (dwarf2_section_empty_p (section))
+ return {};
+
+ /* Older elfutils strip versions could keep the section in the main
+ executable while splitting it for the separate debug info file. */
+ if ((get_section_flags (section) & SEC_HAS_CONTENTS) == 0)
+ return {};
+
+ dwarf2_read_section (obj, section);
+
+ return {section->buffer, section->size};
+}
+
/* See symfile.h. */
bool
@@ -6203,13 +6264,17 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
return true;
}
- if (dwarf2_read_debug_names (dwarf2_per_objfile))
+ if (dwarf2_read_debug_names (dwarf2_per_objfile,
+ get_debug_names_contents_from_section<struct dwarf2_per_objfile>,
+ get_debug_names_contents_from_section<dwz_file>))
{
*index_kind = dw_index_kind::DEBUG_NAMES;
return true;
}
- if (dwarf2_read_gdb_index (dwarf2_per_objfile))
+ if (dwarf2_read_gdb_index (dwarf2_per_objfile,
+ get_gdb_index_contents_from_section<struct dwarf2_per_objfile>,
+ get_gdb_index_contents_from_section<dwz_file>))
{
*index_kind = dw_index_kind::GDB_INDEX;
return true;
--
2.7.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RFC 4/5] Introduce scoped_mmapped_file
2018-05-09 21:27 [PATCH RFC 0/5] Add a DWARF index cache Simon Marchi
2018-05-09 21:27 ` [PATCH RFC 5/5] Add " Simon Marchi
@ 2018-05-09 21:27 ` Simon Marchi
2018-05-10 21:04 ` Tom Tromey
2018-05-09 21:27 ` [PATCH RFC 3/5] Make index reading functions more modular Simon Marchi
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2018-05-09 21:27 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
We already have scoped_mmap, which can is a thin RAII layer over mmap.
If one simply wants to mmap an entire file for reading, it takes a bit
of boilerplate. This patch introduces the scoped_mmapped_file class to
make this easier.
gdb/ChangeLog:
* common/scoped_mmapped_file.h: New file.
* common/scoped_fd.h (class scoped_fd) <destroy>: New method.
<reset>: New method.
<~scoped_fd>: Use destroy.
* unittests/scoped_mmapped_file-selftests.c: New file.
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
unittests/scoped_mmapped_file-selftests.c.
---
gdb/Makefile.in | 1 +
gdb/common/scoped_fd.h | 17 ++++-
gdb/common/scoped_mmapped_file.h | 70 ++++++++++++++++++++
gdb/unittests/scoped_mmapped_file-selftests.c | 95 +++++++++++++++++++++++++++
4 files changed, 181 insertions(+), 2 deletions(-)
create mode 100644 gdb/common/scoped_mmapped_file.h
create mode 100644 gdb/unittests/scoped_mmapped_file-selftests.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 87d74a7..4e302c8 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -429,6 +429,7 @@ SUBDIR_UNITTESTS_SRCS = \
unittests/rsp-low-selftests.c \
unittests/scoped_fd-selftests.c \
unittests/scoped_mmap-selftests.c \
+ unittests/scoped_mmapped_file-selftests.c \
unittests/scoped_restore-selftests.c \
unittests/string_view-selftests.c \
unittests/tracepoint-selftests.c \
diff --git a/gdb/common/scoped_fd.h b/gdb/common/scoped_fd.h
index a6a8ab9..574c26e 100644
--- a/gdb/common/scoped_fd.h
+++ b/gdb/common/scoped_fd.h
@@ -34,8 +34,7 @@ public:
explicit scoped_fd (int fd = -1) noexcept : m_fd (fd) {}
~scoped_fd ()
{
- if (m_fd >= 0)
- close (m_fd);
+ destroy ();
}
DISABLE_COPY_AND_ASSIGN (scoped_fd);
@@ -52,7 +51,21 @@ public:
return m_fd;
}
+ void reset (int new_fd)
+ {
+ destroy ();
+
+ m_fd = new_fd;
+ }
+
private:
+
+ void destroy ()
+ {
+ if (m_fd >= 0)
+ close (m_fd);
+ }
+
int m_fd;
};
diff --git a/gdb/common/scoped_mmapped_file.h b/gdb/common/scoped_mmapped_file.h
new file mode 100644
index 0000000..18bc26a
--- /dev/null
+++ b/gdb/common/scoped_mmapped_file.h
@@ -0,0 +1,70 @@
+/* scoped_mmapped_file, automatically map/unmap entire files.
+
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+#ifndef SCOPED_MMAPPED_FILE_H
+#define SCOPED_MMAPPED_FILE_H
+
+#if defined(HAVE_SYS_MMAN_H)
+
+#include "scoped_mmap.h"
+#include "scoped_fd.h"
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+/* A smart-pointer-like class to automatically mmap an entire file. */
+
+class scoped_mmapped_file
+{
+public:
+
+ scoped_mmapped_file () = default;
+
+ /* Map FILENAME in memory. Throw an error if anything goes wrong. */
+ scoped_mmapped_file (const char *filename)
+ {
+ m_fd.reset (open (filename, 0, O_RDONLY));
+ if (m_fd.get () < 0)
+ perror_with_name ("open");
+
+ off_t size = lseek (m_fd.get (), 0, SEEK_END);
+ if (size < 0)
+ perror_with_name ("lseek");
+
+ /* We can't map an empty file. */
+ if (size == 0)
+ error (_("file to mmap is empty"));
+
+ m_mmap.reset (nullptr, size, PROT_READ, MAP_PRIVATE, m_fd.get (), 0);
+ if (m_mmap.get () == MAP_FAILED)
+ perror_with_name ("mmap");
+ }
+
+ size_t size () const noexcept { return m_mmap.size (); }
+ void *get () const noexcept { return m_mmap.get (); }
+
+private:
+ scoped_mmap m_mmap;
+ scoped_fd m_fd;
+};
+
+#endif /* defined(HAVE_SYS_MMAN_H) */
+
+#endif /* SCOPED_MMAPPED_FILE_H */
diff --git a/gdb/unittests/scoped_mmapped_file-selftests.c b/gdb/unittests/scoped_mmapped_file-selftests.c
new file mode 100644
index 0000000..8d8f517
--- /dev/null
+++ b/gdb/unittests/scoped_mmapped_file-selftests.c
@@ -0,0 +1,95 @@
+/* Self tests for scoped_mmapped_file for GDB, the GNU debugger.
+
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+#include "defs.h"
+#include "selftest.h"
+#include "common/scoped_mmapped_file.h"
+
+#if defined (HAVE_SYS_MMAN_H)
+
+namespace selftests {
+namespace scoped_mmapped_file {
+
+static void
+test_destroy ()
+{
+ char filename[] = "scoped_mmapped_file-selftest-XXXXXX";
+ int fd = mkstemp (filename);
+ SELF_CHECK (fd >= 0);
+
+ write (fd, "Hello!", 7);
+ close (fd);
+
+ {
+ ::scoped_mmapped_file f (filename);
+
+ SELF_CHECK (f.get () != MAP_FAILED);
+ SELF_CHECK (f.size () == 7);
+ SELF_CHECK (0 == strcmp ((char *) f.get (), "Hello!"));
+ }
+
+ unlink (filename);
+}
+
+static void
+test_default_constructor ()
+{
+ ::scoped_mmapped_file f;
+
+ SELF_CHECK (f.get () == MAP_FAILED);
+ SELF_CHECK (f.size () == 0);
+}
+
+static void
+test_invalid_filename ()
+{
+ bool threw = false;
+
+ try {
+ ::scoped_mmapped_file f ("/this/file/should/not/exist");
+ } catch (gdb_exception &e) {
+ threw = true;
+ }
+
+ SELF_CHECK (threw);
+}
+
+static void
+run_tests ()
+{
+ test_destroy ();
+ test_default_constructor ();
+ test_invalid_filename ();
+}
+
+} /* namespace scoped_mmapped_file */
+} /* namespace selftests */
+
+#endif /* defined (HAVE_SYS_MMAN_H) */
+
+void
+_initialize_scoped_mmapped_file_selftests ()
+{
+#if defined (HAVE_SYS_MMAN_H)
+ selftests::register_test ("scoped_mmapped_file",
+ selftests::scoped_mmapped_file::run_tests);
+#endif /* defined (HAVE_SYS_MMAN_H) */
+}
+
+
--
2.7.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RFC 1/5] Rename some functions, index -> gdb_index
2018-05-09 21:27 [PATCH RFC 0/5] Add a DWARF index cache Simon Marchi
` (3 preceding siblings ...)
2018-05-09 21:27 ` [PATCH RFC 2/5] Remove mapped_index::total_size Simon Marchi
@ 2018-05-09 21:27 ` Simon Marchi
2018-05-10 20:40 ` Tom Tromey
2018-05-09 21:57 ` [PATCH RFC 0/5] Add a DWARF index cache Simon Marchi
5 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2018-05-09 21:27 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
Since we now have two index formats, DWARF5/debug_names and gdb_index, I
wanted to rename some functions to make it clear that they deal with the
gdb_index format specifically.
gdb/ChangeLog:
* dwarf2read.c (read_index_from_section): Rename to...
(read_gdb_index_from_section): ... this, update all callers.
(dwarf2_read_index): Rename to...
(dwarf2_read_gdb_index): ... this, update all callers.
---
gdb/dwarf2read.c | 44 ++++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 26ec5ef..823b498 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3421,15 +3421,15 @@ find_slot_in_mapped_hash (struct mapped_index *index, const char *name,
Returns 1 if all went well, 0 otherwise. */
static bool
-read_index_from_section (struct objfile *objfile,
- const char *filename,
- bool deprecated_ok,
- struct dwarf2_section_info *section,
- struct mapped_index *map,
- const gdb_byte **cu_list,
- offset_type *cu_list_elements,
- const gdb_byte **types_list,
- offset_type *types_list_elements)
+read_gdb_index_from_section (struct objfile *objfile,
+ const char *filename,
+ bool deprecated_ok,
+ struct dwarf2_section_info *section,
+ struct mapped_index *map,
+ const gdb_byte **cu_list,
+ offset_type *cu_list_elements,
+ const gdb_byte **types_list,
+ offset_type *types_list_elements)
{
const gdb_byte *addr;
offset_type version;
@@ -3539,7 +3539,7 @@ to use the section anyway."),
elements of all the CUs and return 1. Otherwise, return 0. */
static int
-dwarf2_read_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
+dwarf2_read_gdb_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
{
struct mapped_index local_map, *map;
const gdb_byte *cu_list, *types_list, *dwz_list = NULL;
@@ -3547,11 +3547,11 @@ dwarf2_read_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
struct dwz_file *dwz;
struct objfile *objfile = dwarf2_per_objfile->objfile;
- if (!read_index_from_section (objfile, objfile_name (objfile),
- use_deprecated_index_sections,
- &dwarf2_per_objfile->gdb_index, &local_map,
- &cu_list, &cu_list_elements,
- &types_list, &types_list_elements))
+ if (!read_gdb_index_from_section (objfile, objfile_name (objfile),
+ use_deprecated_index_sections,
+ &dwarf2_per_objfile->gdb_index, &local_map,
+ &cu_list, &cu_list_elements, &types_list,
+ &types_list_elements))
return 0;
/* Don't use the index if it's empty. */
@@ -3567,12 +3567,12 @@ dwarf2_read_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
const gdb_byte *dwz_types_ignore;
offset_type dwz_types_elements_ignore;
- if (!read_index_from_section (objfile, bfd_get_filename (dwz->dwz_bfd),
- 1,
- &dwz->gdb_index, &dwz_map,
- &dwz_list, &dwz_list_elements,
- &dwz_types_ignore,
- &dwz_types_elements_ignore))
+ if (!read_gdb_index_from_section (objfile,
+ bfd_get_filename (dwz->dwz_bfd), 1,
+ &dwz->gdb_index, &dwz_map,
+ &dwz_list, &dwz_list_elements,
+ &dwz_types_ignore,
+ &dwz_types_elements_ignore))
{
warning (_("could not read '.gdb_index' section from %s; skipping"),
bfd_get_filename (dwz->dwz_bfd));
@@ -6213,7 +6213,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
return true;
}
- if (dwarf2_read_index (dwarf2_per_objfile))
+ if (dwarf2_read_gdb_index (dwarf2_per_objfile))
{
*index_kind = dw_index_kind::GDB_INDEX;
return true;
--
2.7.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RFC 5/5] Add DWARF index cache
2018-05-09 21:27 [PATCH RFC 0/5] Add a DWARF index cache Simon Marchi
@ 2018-05-09 21:27 ` Simon Marchi
2018-05-10 22:24 ` Tom Tromey
2018-05-09 21:27 ` [PATCH RFC 4/5] Introduce scoped_mmapped_file Simon Marchi
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Simon Marchi @ 2018-05-09 21:27 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
GDB can generate indexes for DWARF debug information, which, when
integrated in the original binary, can speed up loading object files.
This can be done using the gdb-add-index script or directly by the
linker itself. However, not many people know about this. And even
among those who do, because it requires additional steps, I don't know a
lot of people who actually go through that trouble.
To help make using the DWARF index more transparent, this patch
introduces a DWARF index cache. When enabled, loading an index-less
binary in GDB will automatically save an index file in ~/.cache/gdb.
When loading that same object file again, the index file will be looked
up and used to load the DWARF index. You therefore get the benefit of
the DWARF index without having to do additional manual steps or modify
your build system. When an index section is already present in the
file, GDB will prefer that one over looking up the cache.
When doing my edit-compile-debug cycle, I often debug multiple times the
same build, so the cache helps reducing the load time of the debug
sessions after the first one.
Here are random thoughts/comments in not particular order:
- The saved index file is exactly the same as the output of the "save
gdb-index" command. It is therefore the exact same content that would
be found in the .gdb_index or .debug_names section. We just leave it
as a standalone file instead of merging it in the binary.
- The cache is just a directory with files named after the object
file's build-id. It is not possible to save/load the index for an
object file without build-id in the cache. Note that Clang does not
add build-ids by default. They can be added with -Wl,--build-id.
- There is a setting (set index-cache format) to make GDB save the index
in the cache in the DWARF5 format rather than GDB's own format.
However, as of this patch, GDB can only read the GDB index format from
the cache. The difference with the DWARF5 format is that we can
generate an addendum to the .debug_str section that you're supposed to
integrate to the original binary. This complicates a little bit
loading the data from the cached index files, so I would work on this
after receiving initial comments.
- The size taken up by ~/.cache/gdb is not limited. I was thinking we
could add configurable limit (like ccache does), but that would come
after. Also, maybe a command to flush the cache.
- The cache is disabled by default. I think once it's been out there
and tested for a while, it could be turned on by default, so that
everybody can enjoy it.
- The cache location was made to follow the XDG specification: if the
XDG_CACHE_HOME environment variable, it is used, otherwise it falls
back to ~/.cache/gdb. It is possible to change it using "set
index-cache directory". On other OSes than GNU/Linux, ~/.cache may
not be the best place to put such data. On macOS it should probably
default to ~/Library/Caches/... On Windows, %LocalAppData%/... I
don't intend to do this part, but further patches are welcome.
- I think that we need to be careful that multiple instances of GDB
don't interfere with each other (not far fetched at all if you run GDB
in some automated script) and the cache is always coherent (either the
file is not found, or if it is found and entirely valid). Writing the
file directly to its final location seems like a recipe for failure.
One GDB could read a file in the index while it is being written by
another GDB. To mitigate this, I made write_psymtabs_to_index write
to temporary files and rename them once it's done. Two GDB instances
writing the index for the same file should not step on each other's
toes (the last file to be renamed will stay). A GDB looking up a file
will only see a complete file or no file. Also, if GDB crashes while
generating the index file, it will leave a work-in-progress file, but
it won't be picked up by other instances looking up in the cache.
gdb/ChangeLog:
* common/pathstuff.h (get_standard_cache_dir): New.
* common/pathstuff.c (get_standard_cache_dir): New.
* build-id.h (build_id_to_string): New.
* dwarf-index-common.h (INDEX4_SUFFIX, INDEX5_SUFFIX,
DEBUG_STR_SUFFIX): Move to here.
* dwarf-index-write.c (INDEX4_SUFFIX, INDEX5_SUFFIX,
DEBUG_STR_SUFFIX): Move from there.
(write_psymtabs_to_index): Make non-static, add basename
parameter. Write to temporary files, rename when done.
(save_gdb_index_command): Adjust call to
write_psymtabs_to_index.
* dwarf2read.h (dwarf2_per_objfile) <index_cache_res>: New
field.
* dwarf2read.c (dwz_file) <index_cache_res>: New field.
(get_gdb_index_contents_from_cache): New.
(get_gdb_index_contents_from_cache_dwz): New.
(dwarf2_initialize_objfile): Read index from cache.
(dwarf2_build_psymtabs): Save to index.
* dwarf-index-cache.h: New file.
* dwarf-index-cache.c: New file.
* dwarf-index-write.h: New file.
gdb/testsuite/ChangeLog:
* boards/index-cache-gdb.exp: New file.
* gdb.dwarf2/index-cache.exp: New file.
* gdb.dwarf2/index-cache.c: New file.
* gdb.base/maint.exp: Check if we are using the index cache.
---
gdb/Makefile.in | 2 +
gdb/build-id.h | 11 +
gdb/common/pathstuff.c | 16 ++
gdb/common/pathstuff.h | 10 +
gdb/dwarf-index-cache.c | 421 +++++++++++++++++++++++++++++++++
gdb/dwarf-index-cache.h | 86 +++++++
gdb/dwarf-index-common.h | 5 +
gdb/dwarf-index-write.c | 81 +++++--
gdb/dwarf-index-write.h | 34 +++
gdb/dwarf2read.c | 46 ++++
gdb/dwarf2read.h | 5 +
gdb/testsuite/boards/local-board.exp | 1 +
gdb/testsuite/gdb.base/index-cache.c | 23 ++
gdb/testsuite/gdb.base/index-cache.exp | 204 ++++++++++++++++
gdb/testsuite/gdb.base/maint.exp | 23 ++
gdb/testsuite/lib/mi-support.exp | 16 +-
16 files changed, 953 insertions(+), 31 deletions(-)
create mode 100644 gdb/dwarf-index-cache.c
create mode 100644 gdb/dwarf-index-cache.h
create mode 100644 gdb/dwarf-index-write.h
create mode 100644 gdb/testsuite/gdb.base/index-cache.c
create mode 100644 gdb/testsuite/gdb.base/index-cache.exp
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 4e302c8..97beade 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -994,6 +994,7 @@ COMMON_SFILES = \
disasm.c \
disasm-selftests.c \
dummy-frame.c \
+ dwarf-index-cache.c \
dwarf-index-common.c \
dwarf-index-write.c \
dwarf2-frame.c \
@@ -1217,6 +1218,7 @@ HFILES_NO_SRCDIR = \
dictionary.h \
disasm.h \
dummy-frame.h \
+ dwarf-index-cache.h \
dwarf-index-common.h \
dwarf2-frame.h \
dwarf2-frame-tailcall.h \
diff --git a/gdb/build-id.h b/gdb/build-id.h
index 15fb609..796e5ac 100644
--- a/gdb/build-id.h
+++ b/gdb/build-id.h
@@ -21,6 +21,7 @@
#define BUILD_ID_H
#include "gdb_bfd.h"
+#include "common/rsp-low.h"
/* Locate NT_GNU_BUILD_ID from ABFD and return its content. */
@@ -47,4 +48,14 @@ extern gdb_bfd_ref_ptr build_id_to_debug_bfd (size_t build_id_len,
extern std::string find_separate_debug_file_by_buildid
(struct objfile *objfile);
+/* Return an hex-string representation of BUILD_ID. */
+
+static inline std::string
+build_id_to_string (const bfd_build_id *build_id)
+{
+ gdb_assert (build_id != NULL);
+
+ return bin2hex (build_id->data, build_id->size);
+}
+
#endif /* BUILD_ID_H */
diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index 8c4093f..b64d43a 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -158,3 +158,19 @@ contains_dir_separator (const char *path)
return false;
}
+
+/* See common/pathstuff.h. */
+
+std::string
+get_standard_cache_dir ()
+{
+ char *xdg_cache_home = getenv ("XDG_CACHE_HOME");
+ if (xdg_cache_home != NULL)
+ return string_printf ("%s/gdb", xdg_cache_home);
+
+ char *home = getenv ("HOME");
+ if (home != NULL)
+ return string_printf ("%s/.cache/gdb", home);
+
+ return {};
+}
diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
index 9f26127..264700f 100644
--- a/gdb/common/pathstuff.h
+++ b/gdb/common/pathstuff.h
@@ -50,4 +50,14 @@ extern gdb::unique_xmalloc_ptr<char> gdb_abspath (const char *path);
extern bool contains_dir_separator (const char *path);
+/* Get the usual user cache directory for the current platform.
+
+ On Linux, it follows the XDG Base Directory specification: use
+ $XDG_CACHE_HOME/gdb if the XDG_CACHE_HOME environment variable is defined,
+ otherwise $HOME/.cache.
+
+ Return an empty string if neither XDG_CACHE_HOME or HOME are defined. */
+
+extern std::string get_standard_cache_dir ();
+
#endif /* PATHSTUFF_H */
diff --git a/gdb/dwarf-index-cache.c b/gdb/dwarf-index-cache.c
new file mode 100644
index 0000000..c7b0b3a
--- /dev/null
+++ b/gdb/dwarf-index-cache.c
@@ -0,0 +1,421 @@
+/* Caching of GDB/DWARF index files.
+
+ Copyright (C) 1994-2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+#include "defs.h"
+#include "dwarf-index-cache.h"
+
+#include "build-id.h"
+#include "cli/cli-cmds.h"
+#include "command.h"
+#include "common/pathstuff.h"
+#include "common/scoped_mmapped_file.h"
+#include "dwarf-index-write.h"
+#include "dwarf2read.h"
+#include "objfiles.h"
+#include "selftest.h"
+#include <string>
+#include <stdlib.h>
+
+static int debug_index_cache = 0;
+static char *index_cache_directory = NULL;
+index_cache global_index_cache;
+
+/* Index format to use for the index cache. */
+static const char index_format_gdb[] = "gdb";
+static const char index_format_dwarf_5[] = "dwarf-5";
+static const char *index_formats[] = {
+ index_format_gdb,
+ index_format_dwarf_5,
+ nullptr,
+};
+static const char *index_cache_format_ = index_format_gdb;
+
+/* set/show index-cache commands. */
+
+static cmd_list_element *set_index_cache_prefix_list;
+static cmd_list_element *show_index_cache_prefix_list;
+
+/* Return the index format to use when writing files to the cache. */
+
+static dw_index_kind
+index_cache_format ()
+{
+ if (index_cache_format_ == index_format_gdb)
+ return dw_index_kind::GDB_INDEX;
+ else if (index_cache_format_ == index_format_dwarf_5)
+ return dw_index_kind::DEBUG_NAMES;
+ else
+ gdb_assert_not_reached (_("Invalid value for index cache format."));
+}
+
+
+/* A cheap (as in low-quality) recursive mkdir. Try to create all the parents
+ directories up to DIR and DIR itself. Stop if we hit an error along the way.
+ There is no attempt to remove created directories in case of failure. */
+
+static void
+mkdir_recursive (const char *dir)
+{
+ /* This function only deals with absolute paths. */
+ gdb_assert (dir[0] == '/');
+
+ gdb::unique_xmalloc_ptr<char> holder (xstrdup (dir));
+ char * const start = holder.get ();
+ char *component_start = start;
+ char *component_end = start;
+
+ while (1)
+ {
+ /* Find the beginning of the next component. */
+ while (*component_start == '/')
+ component_start++;
+
+ /* Are we done? */
+ if (*component_start == '\0')
+ return;
+
+ /* Find the slash or null-terminator after this component. */
+ component_end = component_start;
+ while (*component_end != '/' && *component_end != '\0')
+ component_end++;
+
+ /* Temporarily replace the slash with a null terminator, so we can create
+ the directory up to this component. */
+ char saved_char = *component_end;
+ *component_end = '\0';
+
+ /* If we get EEXIST and the existing path is a directory, then we're
+ happy. If it exists, but it's a regular file and this is not the last
+ component, we'll fail at the next component. If this is the last
+ component, the caller will fail with ENOTDIR when trying to
+ open/create a file under that path. */
+ if (mkdir (start, 0700) != 0)
+ if (errno != EEXIST)
+ return;
+
+ /* Restore the overwritten char. */
+ *component_end = saved_char;
+ component_start = component_end;
+ }
+}
+
+index_cache_resource::~index_cache_resource () = default;
+
+void
+index_cache::set_directory (std::string &&dir)
+{
+ gdb_assert (!dir.empty ());
+
+ m_dir = std::move (dir);
+
+ if (debug_index_cache)
+ printf_unfiltered ("index cache: now using directory %s\n", m_dir.c_str ());
+}
+
+void
+index_cache::enable ()
+{
+ if (debug_index_cache)
+ printf_unfiltered ("index cache: enabling (%s)\n", m_dir.c_str ());
+
+ m_enabled = true;
+}
+
+void
+index_cache::disable ()
+{
+ if (debug_index_cache)
+ printf_unfiltered ("index cache: disabling\n");
+
+ m_enabled = false;
+}
+
+/* See index-cache.h. */
+
+void
+index_cache::store (struct dwarf2_per_objfile *dwarf2_per_objfile)
+{
+ objfile *obj = dwarf2_per_objfile->objfile;
+
+ if (!enabled ())
+ return;
+
+ const bfd_build_id *build_id = build_id_bfd_get (obj->obfd);
+ if (build_id == nullptr)
+ {
+ if (debug_index_cache)
+ printf_unfiltered ("index cache: objfile %s has no build id\n",
+ objfile_name (obj));
+ return;
+ }
+
+ std::string build_id_str = build_id_to_string (build_id);
+
+ TRY
+ {
+ mkdir_recursive (m_dir.c_str ());
+
+ if (debug_index_cache)
+ printf_unfiltered ("index cache: writing index cache for objfile %s\n",
+ objfile_name (obj));
+
+ write_psymtabs_to_index (dwarf2_per_objfile, m_dir.c_str (),
+ build_id_str.c_str (), index_cache_format ());
+ }
+ CATCH (except, RETURN_MASK_ERROR)
+ {
+ if (debug_index_cache)
+ printf_unfiltered ("index cache: couldn't store index cache for objfile "
+ "%s: %s", objfile_name (obj), except.message);
+ }
+ END_CATCH
+}
+
+#if HAVE_SYS_MMAN_H
+
+struct index_cache_resource_mmap final : index_cache_resource
+{
+ index_cache_resource_mmap (const char *filename)
+ : mapping (filename)
+ {}
+
+ scoped_mmapped_file mapping;
+};
+
+gdb::array_view<const gdb_byte>
+index_cache::lookup_gdb_index (const bfd_build_id *build_id,
+ std::unique_ptr<index_cache_resource> *resource)
+{
+ if (!enabled ())
+ return {};
+
+ std::string filename = make_index_filename (build_id, INDEX4_SUFFIX);
+
+ TRY
+ {
+ if (debug_index_cache)
+ printf_unfiltered ("index cache: trying to read %s\n",
+ filename.c_str ());
+
+ index_cache_resource_mmap *mmap_resource
+ = new index_cache_resource_mmap (filename.c_str ());
+
+ resource->reset (mmap_resource);
+
+ return gdb::array_view<const gdb_byte>
+ ((const gdb_byte *) mmap_resource->mapping.get (),
+ mmap_resource->mapping.size ());
+ }
+ CATCH (except, RETURN_MASK_ERROR)
+ {
+ if (debug_index_cache)
+ printf_unfiltered ("index cache: couldn't read %s: %s\n",
+ filename.c_str (), except.message);
+ }
+ END_CATCH
+
+ return {};
+}
+
+#else /* !HAVE_SYS_MMAN_H */
+
+gdb::array_view<const gdb_byte>
+index_cache::lookup_gdb_index (const bfd_build_id *build_id,
+ std::unique_ptr<index_cache_resource> *resource)
+{
+ return {};
+}
+
+#endif
+
+/* See index-cache.h. */
+
+std::string
+index_cache::make_index_filename (const bfd_build_id *build_id,
+ const char *suffix) const
+{
+ std::string build_id_str = build_id_to_string (build_id);
+
+ return string_printf ("%s%s%s%s", m_dir.c_str (), SLASH_STRING,
+ build_id_str.c_str (), suffix);
+}
+
+static void
+set_index_cache_command (const char *arg, int from_tty)
+{
+ printf_unfiltered ("\
+Missing arguments. See \"help set index-cache\" for help.\n");
+}
+
+static bool in_show_index_cache_command = false;
+
+static void
+show_index_cache_command (const char *arg, int from_tty)
+{
+ auto restore_flag = make_scoped_restore (&in_show_index_cache_command, true);
+
+ cmd_show_list (show_index_cache_prefix_list, from_tty, "");
+
+ printf_unfiltered ("\n");
+ printf_unfiltered ("The index cache is currently %s.\n",
+ global_index_cache.enabled () ? "enabled" : "disabled");
+}
+
+static void
+set_index_cache_on_command (const char *arg, int from_tty)
+{
+ global_index_cache.enable ();
+}
+
+static void
+set_index_cache_off_command (const char *arg, int from_tty)
+{
+ global_index_cache.disable ();
+}
+
+static void
+set_index_cache_directory_command (const char *arg, int from_tty,
+ cmd_list_element *element)
+{
+ global_index_cache.set_directory (index_cache_directory);
+}
+
+static void
+show_index_cache_stats_command (const char *arg, int from_tty)
+{
+ const char *indent = "";
+
+ /* If this command is invoked through "show index-cache", make the display a
+ bit nicer. */
+ if (in_show_index_cache_command)
+ {
+ indent = " ";
+ printf_unfiltered ("\n");
+ }
+
+ printf_unfiltered ("%s Cache hits (this session): %u\n",
+ indent, global_index_cache.n_hits ());
+ printf_unfiltered ("%sCache misses (this session): %u\n",
+ indent, global_index_cache.n_misses ());
+}
+
+#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
+namespace selftests
+{
+static bool
+create_dir_and_check (const char *dir)
+{
+ mkdir_recursive (dir);
+
+ struct stat st;
+ if (stat (dir, &st) != 0)
+ perror_with_name ("stat");
+
+ return (st.st_mode & S_IFDIR) != 0;
+}
+
+static void
+test_mkdir_recursive ()
+{
+ char base[] = "/tmp/gdb-selftests-XXXXXX";
+
+ if (mkdtemp (base) == NULL)
+ perror_with_name ("mkdtemp");
+
+ std::string dir = string_printf ("%s/a/b", base);
+ SELF_CHECK (create_dir_and_check (dir.c_str ()));
+
+ dir = string_printf ("%s/a/b/c//d/e/", base);
+ SELF_CHECK (create_dir_and_check (dir.c_str ()));
+}
+}
+#endif /* GDB_SELF_TEST && defined (HAVE_MKDTEMP) */
+
+void
+_initialize_index_cache ()
+{
+ /* Set the default index cache directory. */
+ std::string cache_dir = get_standard_cache_dir ();
+ if (!cache_dir.empty ())
+ {
+ index_cache_directory = xstrdup (cache_dir.c_str ());
+ global_index_cache.set_directory (std::move (cache_dir));
+ }
+ else
+ {
+ /* Err... I guess /tmp is better than nothing. */
+ index_cache_directory = xstrdup ("/tmp");
+ global_index_cache.set_directory ("/tmp");
+ }
+
+ /* set index-cache */
+ add_prefix_cmd ("index-cache", class_files, set_index_cache_command,
+ _("Set index-cache options"), &set_index_cache_prefix_list,
+ "set index-cache ", false, &setlist);
+
+ /* show index-cache */
+ add_prefix_cmd ("index-cache", class_files, show_index_cache_command,
+ _("Show index-cache options"), &show_index_cache_prefix_list,
+ "show index-cache ", false, &showlist);
+
+ /* set index-cache on */
+ add_cmd ("on", class_files, set_index_cache_on_command,
+ _("Enable the index cache."), &set_index_cache_prefix_list);
+
+ /* set index-cache off */
+ add_cmd ("off", class_files, set_index_cache_off_command,
+ _("Disable the index cache."), &set_index_cache_prefix_list);
+
+ /* set index-cache format */
+ add_setshow_enum_cmd ("format", class_files, index_formats,
+ &index_cache_format_, "\
+Set the index format to use for the index cache.", "\
+Show the index format to use for the index cache.", "\
+Index format to use for the index cache.", nullptr, nullptr,
+ &set_index_cache_prefix_list,
+ &show_index_cache_prefix_list);
+
+ /* set index-cache directory */
+ add_setshow_filename_cmd ("directory", class_files, &index_cache_directory,
+ _("Set the directory of the index cache."),
+ _("Show the directory of the index cache."),
+ NULL,
+ set_index_cache_directory_command, NULL,
+ &set_index_cache_prefix_list,
+ &show_index_cache_prefix_list);
+
+ /* show index-cache stats */
+ add_cmd ("stats", class_files, show_index_cache_stats_command,
+ _("Show some stats about the index cache."),
+ &show_index_cache_prefix_list);
+
+ /* set debug index-cache */
+ add_setshow_boolean_cmd ("index-cache", class_maintenance,
+ &debug_index_cache,
+ _("Set display of index-cache debug messages."),
+ _("Show display of index-cache debug messages."),
+ _("\
+When non-zero, debugging output for the index cache is displayed."),
+ NULL, NULL,
+ &setdebuglist, &showdebuglist);
+
+#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
+ selftests::register_test ("mkdir_recursive", selftests::test_mkdir_recursive);
+#endif
+}
diff --git a/gdb/dwarf-index-cache.h b/gdb/dwarf-index-cache.h
new file mode 100644
index 0000000..907d3f5
--- /dev/null
+++ b/gdb/dwarf-index-cache.h
@@ -0,0 +1,86 @@
+/* Caching of GDB/DWARF index files.
+
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+#ifndef DWARF_INDEX_CACHE_H
+#define DWARF_INDEX_CACHE_H
+
+#include "dwarf-index-common.h"
+#include "common/array-view.h"
+#include "symfile.h"
+
+struct index_cache_resource
+{
+ virtual ~index_cache_resource () = 0;
+};
+
+class index_cache
+{
+public:
+ void set_directory (std::string &&dir);
+
+ bool enabled () const
+ {
+ return m_enabled;
+ }
+
+ void enable ();
+ void disable ();
+
+ /* Store an index for the specified object file in the cache. */
+ void store (struct dwarf2_per_objfile *dwarf2_per_objfile);
+
+ gdb::array_view<const gdb_byte>
+ lookup_gdb_index (const bfd_build_id *build_id,
+ std::unique_ptr<index_cache_resource> *resource);
+
+ unsigned int n_hits () const
+ { return m_n_hits; }
+
+ void hit ()
+ {
+ if (enabled ())
+ m_n_hits++;
+ }
+
+ unsigned int n_misses () const
+ { return m_n_misses; }
+
+ void miss ()
+ {
+ if (enabled ())
+ m_n_misses++;
+ }
+
+private:
+
+ std::string make_index_filename (const bfd_build_id *build_id,
+ const char *suffix) const;
+
+ std::string m_dir;
+
+ bool m_enabled = false;
+
+ /* Number of cache hits and misses during this GDB session. */
+ unsigned int m_n_hits = 0;
+ unsigned int m_n_misses = 0;
+};
+
+extern index_cache global_index_cache;
+
+#endif /* DWARF_INDEX_CACHE_H */
diff --git a/gdb/dwarf-index-common.h b/gdb/dwarf-index-common.h
index 32774c7..742eba6 100644
--- a/gdb/dwarf-index-common.h
+++ b/gdb/dwarf-index-common.h
@@ -20,6 +20,11 @@
#ifndef DWARF_INDEX_COMMON_H
#define DWARF_INDEX_COMMON_H
+/* The suffix for an index file. */
+#define INDEX4_SUFFIX ".gdb-index"
+#define INDEX5_SUFFIX ".debug_names"
+#define DEBUG_STR_SUFFIX ".debug_str"
+
/* All offsets in the index are of this type. It must be
architecture-independent. */
typedef uint32_t offset_type;
diff --git a/gdb/dwarf-index-write.c b/gdb/dwarf-index-write.c
index a5e196d..31f9487 100644
--- a/gdb/dwarf-index-write.c
+++ b/gdb/dwarf-index-write.c
@@ -24,6 +24,7 @@
#include "common/byte-vector.h"
#include "common/filestuff.h"
#include "common/gdb_unlinker.h"
+#include "common/scoped_fd.h"
#include "complaints.h"
#include "dwarf-index-common.h"
#include "dwarf2.h"
@@ -39,11 +40,6 @@
#include <unordered_map>
#include <unordered_set>
-/* The suffix for an index file. */
-#define INDEX4_SUFFIX ".gdb-index"
-#define INDEX5_SUFFIX ".debug_names"
-#define DEBUG_STR_SUFFIX ".debug_str"
-
/* Ensure only legit values are used. */
#define DW2_GDB_INDEX_SYMBOL_STATIC_SET_VALUE(cu_index, value) \
do { \
@@ -1546,11 +1542,11 @@ assert_file_size (FILE *file, const char *filename, size_t expected_size)
gdb_assert (file_size == expected_size);
}
-/* Create an index file for OBJFILE in the directory DIR. */
+/* See dwarf-index-write.h. */
-static void
+void
write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
- const char *dir,
+ const char *dir, const char *basename,
dw_index_kind index_kind)
{
struct objfile *objfile = dwarf2_per_objfile->objfile;
@@ -1568,50 +1564,83 @@ 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
- + lbasename (objfile_name (objfile))
+ /* Make a filename suitable to pass to mkstemp based on F (e.g.
+ /tmp/foo -> /tmp/foo-XXXXXX). */
+ auto make_temp_filename = [] (const std::string &f) -> gdb::char_vector
+ {
+ gdb::char_vector filename_temp (f.length () + 8);
+ strcpy (filename_temp.data (), f.c_str ());
+ strcat (filename_temp.data () + f.size (), "-XXXXXX");
+ return filename_temp;
+ };
+
+ 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);
- FILE *out_file = gdb_fopen_cloexec (filename.c_str (), "wb").release ();
- if (!out_file)
- error (_("Can't open `%s' for writing"), filename.c_str ());
+ scoped_fd out_file_fd (mkstemp (filename_temp.data ()));
+ if (out_file_fd.get () == -1)
+ perror_with_name ("mkstemp");
- /* Order matters here; we want FILE to be closed before FILENAME is
+ FILE *out_file = gdb_fopen_cloexec (filename_temp.data (), "wb").release ();
+ if (out_file == nullptr)
+ error (_("Can't open `%s' for writing"), filename_temp.data ());
+
+ /* 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. (Don't call anything here that might throw until
- file_closer is created.) */
- gdb::unlinker unlink_file (filename.c_str ());
+ file_closer is created.) We don't need OUT_FILE_FD anymore, so we might
+ as well close it now. */
+ out_file_fd.reset (-1);
+ gdb::unlinker unlink_file (filename_temp.data ());
gdb_file_up close_out_file (out_file);
if (index_kind == dw_index_kind::DEBUG_NAMES)
{
std::string filename_str (std::string (dir) + SLASH_STRING
- + lbasename (objfile_name (objfile))
- + DEBUG_STR_SUFFIX);
+ + basename + DEBUG_STR_SUFFIX);
+ gdb::char_vector filename_str_temp = make_temp_filename (filename_str);
+
+ scoped_fd out_file_str_fd (mkstemp (filename_str_temp.data ()));
+ if (out_file_str_fd.get () == -1)
+ perror_with_name ("mkstemp");
+
FILE *out_file_str
- = gdb_fopen_cloexec (filename_str.c_str (), "wb").release ();
- if (!out_file_str)
- error (_("Can't open `%s' for writing"), filename_str.c_str ());
- gdb::unlinker unlink_file_str (filename_str.c_str ());
+ = gdb_fopen_cloexec (filename_str_temp.data (), "wb").release ();
+ if (out_file_str == nullptr)
+ error (_("Can't open `%s' for writing"), filename_str_temp.data ());
+
+ out_file_str_fd.reset (-1);
+ gdb::unlinker unlink_file_str (filename_str_temp.data ());
gdb_file_up close_out_file_str (out_file_str);
const size_t total_len
= write_debug_names (dwarf2_per_objfile, out_file, out_file_str);
- assert_file_size (out_file, filename.c_str (), total_len);
+ assert_file_size (out_file, 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. */
+ close_out_file_str.reset ();
+ if (rename (filename_str_temp.data (), filename_str.c_str ()))
+ perror_with_name ("rename");
}
else
{
const size_t total_len
= write_gdbindex (dwarf2_per_objfile, out_file);
- assert_file_size (out_file, filename.c_str (), total_len);
+ assert_file_size (out_file, filename_temp.data (), total_len);
}
/* We want to keep the file. */
unlink_file.keep ();
+
+ /* Close and move the file in place. */
+ close_out_file.reset ();
+ if (rename (filename_temp.data (), filename.c_str ()))
+ perror_with_name ("rename");
}
/* Implementation of the `save gdb-index' command.
@@ -1656,7 +1685,9 @@ save_gdb_index_command (const char *arg, int from_tty)
{
TRY
{
- write_psymtabs_to_index (dwarf2_per_objfile, arg, index_kind);
+ const char *basename = lbasename (objfile_name (objfile));
+ write_psymtabs_to_index (dwarf2_per_objfile, arg, basename,
+ index_kind);
}
CATCH (except, RETURN_MASK_ERROR)
{
diff --git a/gdb/dwarf-index-write.h b/gdb/dwarf-index-write.h
new file mode 100644
index 0000000..bfba95a
--- /dev/null
+++ b/gdb/dwarf-index-write.h
@@ -0,0 +1,34 @@
+/* DWARF index writing support for GDB.
+
+ Copyright (C) 2018 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+#ifndef DWARF_INDEX_WRITE_H
+#define DWARF_INDEX_WRITE_H
+
+#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. */
+
+void write_psymtabs_to_index (struct dwarf2_per_objfile *dwarf2_per_objfile,
+ const char *dir, const char *basename,
+ dw_index_kind index_kind);
+
+#endif /* DWARF_INDEX_WRITE_H */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index bc92756..eca7310 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -30,6 +30,7 @@
#include "defs.h"
#include "dwarf2read.h"
+#include "dwarf-index-cache.h"
#include "dwarf-index-common.h"
#include "bfd.h"
#include "elf-bfd.h"
@@ -850,6 +851,10 @@ struct dwz_file
/* The dwz's BFD. */
bfd *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
@@ -6227,6 +6232,32 @@ get_debug_names_contents_from_section (objfile *obj, T *section_owner)
return {section->buffer, section->size};
}
+/* Lookup the index cache for the contents of the index associated to
+ DWARF2_OBJ. */
+
+static gdb::array_view<const gdb_byte>
+get_gdb_index_contents_from_cache (objfile *obj, dwarf2_per_objfile *dwarf2_obj)
+{
+ const bfd_build_id *build_id = build_id_bfd_get (obj->obfd);
+ if (build_id == nullptr)
+ return {};
+
+ return global_index_cache.lookup_gdb_index (build_id,
+ &dwarf2_obj->index_cache_res);
+}
+
+/* Same as the above, but for DWZ. */
+
+static gdb::array_view<const gdb_byte>
+get_gdb_index_contents_from_cache_dwz (objfile *obj, dwz_file *dwz)
+{
+ const bfd_build_id *build_id = build_id_bfd_get (dwz->dwz_bfd);
+ if (build_id == nullptr)
+ return {};
+
+ return global_index_cache.lookup_gdb_index (build_id, &dwz->index_cache_res);
+}
+
/* See symfile.h. */
bool
@@ -6264,6 +6295,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
return true;
}
+ /* First, try to read the index from a section of the object file... */
if (dwarf2_read_debug_names (dwarf2_per_objfile,
get_debug_names_contents_from_section<struct dwarf2_per_objfile>,
get_debug_names_contents_from_section<dwz_file>))
@@ -6280,6 +6312,17 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
return true;
}
+ /* ... otherwise, try to find the index in the index cache. */
+ if (dwarf2_read_gdb_index (dwarf2_per_objfile,
+ get_gdb_index_contents_from_cache,
+ get_gdb_index_contents_from_cache_dwz))
+ {
+ global_index_cache.hit ();
+ *index_kind = dw_index_kind::GDB_INDEX;
+ return true;
+ }
+
+ global_index_cache.miss ();
return false;
}
@@ -6305,6 +6348,9 @@ dwarf2_build_psymtabs (struct objfile *objfile)
psymtab_discarder psymtabs (objfile);
dwarf2_build_psymtabs_hard (dwarf2_per_objfile);
psymtabs.keep ();
+
+ /* (maybe) store an index in the cache. */
+ global_index_cache.store (dwarf2_per_objfile);
}
CATCH (except, RETURN_MASK_ERROR)
{
diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
index 8e6c41d..0f9ccc6 100644
--- a/gdb/dwarf2read.h
+++ b/gdb/dwarf2read.h
@@ -20,6 +20,7 @@
#ifndef DWARF2READ_H
#define DWARF2READ_H
+#include "dwarf-index-cache.h"
#include "filename-seen-cache.h"
#include "gdb_obstack.h"
@@ -241,6 +242,10 @@ public:
/* Table containing all filenames. This is an optional because the
table is lazily constructed on first access. */
gdb::optional<filename_seen_cache> filenames_cache;
+
+ /* 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;
};
/* Get the dwarf2_per_objfile associated to OBJFILE. */
diff --git a/gdb/testsuite/boards/local-board.exp b/gdb/testsuite/boards/local-board.exp
index df9e5bf..b00f11f 100644
--- a/gdb/testsuite/boards/local-board.exp
+++ b/gdb/testsuite/boards/local-board.exp
@@ -21,4 +21,5 @@ global board
global board_info
# Remove any target variant specifications from the name.
set baseboard [lindex [split $board "/"] 0]
+puts "NAME IS $baseboard"
set board_info($baseboard,isremote) 0
diff --git a/gdb/testsuite/gdb.base/index-cache.c b/gdb/testsuite/gdb.base/index-cache.c
new file mode 100644
index 0000000..460d70e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/index-cache.c
@@ -0,0 +1,23 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 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/>. */
+
+int
+main ()
+{
+ return 0;
+}
+
diff --git a/gdb/testsuite/gdb.base/index-cache.exp b/gdb/testsuite/gdb.base/index-cache.exp
new file mode 100644
index 0000000..fcada77
--- /dev/null
+++ b/gdb/testsuite/gdb.base/index-cache.exp
@@ -0,0 +1,204 @@
+# Copyright 2018 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/>.
+
+# This test checks that the index-cache feature generates the expected files at
+# the expected location.
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+ return
+}
+
+# List the files in DIR on the host (where GDB-under-test runs).
+# Return a list of two elements:
+# - 0 on success, -1 on failure
+# - the list of files on success, empty on failure
+
+proc ls_host { dir } {
+ lassign [remote_exec host ls "-1 $dir"] ret output
+
+ if { $ret != 0 } {
+ fail "failed to list files on host in $dir"
+ return -1
+ }
+
+ # ls -1 returns a list separated by \r\n. split will return a bunch of
+ # empty entries (it treats a sequence of split characters as separate
+ # fields, plus there is a \r\n at the end of the result). Ignore empty
+ # list elements.
+ set filtered {}
+ set files [split $output \r\n]
+
+ foreach file $files {
+ if { $file != "" } {
+ lappend filtered $file
+ }
+ }
+
+ return "0 $filtered"
+}
+
+# Execute "show index-cache stats" and verify the output against expected
+# values.
+
+proc check_cache_stats { expected_hits expected_misses } {
+ set re [multi_line \
+ " Cache hits .this session.: $expected_hits" \
+ "Cache misses .this session.: $expected_misses" \
+ ]
+
+ gdb_test "show index-cache stats" $re "check index-cache stats"
+}
+
+# Run CODE using a fresh GDB configured based on the other parameters.
+
+proc run_test_with_flags { index_format cache_dir cache_enabled code } {
+ global GDBFLAGS testfile
+
+ save_vars { GDBFLAGS } {
+ set GDBFLAGS "$GDBFLAGS -iex \"set index-cache format $index_format\""
+ set GDBFLAGS "$GDBFLAGS -iex \"set index-cache directory $cache_dir\""
+ set GDBFLAGS "$GDBFLAGS -iex \"set index-cache $cache_enabled\""
+
+ clean_restart ${testfile}
+
+ uplevel 1 $code
+ }
+}
+
+# Test administrative stuff.
+
+proc_with_prefix test_basic_stuff { } {
+ global testfile
+
+ clean_restart ${testfile}
+
+ # Check that the index cache is disabled by default.
+ gdb_test \
+ "show index-cache" \
+ " is currently disabled." \
+ "index-cache is disabled by default"
+
+ # Test that we can enable it and "show index-cache" reflects that.
+ gdb_test_no_output "set index-cache on" "enable index cache"
+ gdb_test \
+ "show index-cache" \
+ " is currently enabled." \
+ "index-cache is now enabled"
+
+ # Test the "set/show index-cache directory" commands.
+ gdb_test "set index-cache directory" "Argument required.*" "set index-cache directory without arg"
+ gdb_test_no_output "set index-cache directory /tmp" "change the index cache directory"
+ gdb_test \
+ "show index-cache directory" \
+ "The directory of the index cache is \"/tmp\"." \
+ "show index cache directory"
+}
+
+# Test loading a binary with the cache disabled. No file should be created.
+
+proc_with_prefix test_cache_disabled { index_format cache_dir } {
+ lassign [ls_host $cache_dir] ret files_before
+
+ run_test_with_flags $index_format $cache_dir off {
+ lassign [ls_host $cache_dir] ret files_after
+
+ set nfiles_created [expr [llength $files_after] - [llength $files_before]]
+ gdb_assert "$nfiles_created == 0" "no files were created"
+
+ check_cache_stats 0 0
+ }
+}
+
+# Test with the cache enabled, we expect to have exactly one file created.
+
+proc_with_prefix test_cache_enabled_miss { index_format cache_dir } {
+ global testfile
+
+ lassign [ls_host $cache_dir] ret files_before
+
+ run_test_with_flags $index_format $cache_dir on {
+
+ lassign [ls_host $cache_dir] ret files_after
+ set nfiles_created [expr [llength $files_after] - [llength $files_before]]
+ gdb_assert "$nfiles_created > 0" "at least one file was created"
+
+ set build_id [get_build_id [standard_output_file ${testfile}]]
+ if { $build_id == "" } {
+ fail "couldn't get executable build id"
+ return
+ }
+
+ if { $index_format == "gdb" } {
+ set expected_created_file [list "${build_id}.gdb-index"]
+ } else {
+ set expected_created_file [list "${build_id}.debug_names"]
+ }
+
+ set found_idx [lsearch -exact $files_after $expected_created_file]
+ gdb_assert "$found_idx >= 0" "expected file is there"
+
+ remote_exec host rm "-f $cache_dir/$expected_created_file"
+
+ check_cache_stats 0 1
+ }
+}
+
+
+# Test with the cache enabled, this time we should have one file (the
+# same), but one cache read hit.
+
+proc_with_prefix test_cache_enabled_hit { index_format cache_dir } {
+ # Just to populate the cache.
+ run_test_with_flags $index_format $cache_dir on {}
+
+ lassign [ls_host $cache_dir] ret files_before
+
+ run_test_with_flags $index_format $cache_dir on {
+ lassign [ls_host $cache_dir] ret files_after
+ set nfiles_created [expr [llength $files_after] - [llength $files_before]]
+ gdb_assert "$nfiles_created == 0" "no files were created"
+
+ # Reading DWARF5 from cache is not implemented yet.
+ if { $index_format == "dwarf-5" } {
+ setup_xfail "*-*-*"
+ }
+ check_cache_stats 1 0
+ }
+}
+
+test_basic_stuff
+
+foreach_with_prefix index_format { "gdb" "dwarf-5" } {
+ # The cache dir should be on the host (possibly remote), so we can't use the
+ # standard output directory for that (it's on the build machine).
+ lassign [remote_exec host mktemp -d] ret cache_dir
+
+ if { $ret != 0 } {
+ fail "couldn't create temporary cache dir"
+ return
+ }
+
+ # The ouput of mktemp contains an end of line, remove it.
+ set cache_dir [string trimright $cache_dir \r\n]
+
+ test_cache_disabled $index_format $cache_dir
+ test_cache_enabled_miss $index_format $cache_dir
+ test_cache_enabled_hit $index_format $cache_dir
+
+ # Test again with the cache disabled, now that it is populated.
+ test_cache_disabled $index_format $cache_dir
+}
diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index aefaeb0..e3a708f 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -123,6 +123,29 @@ gdb_test_multiple "maint info sections .gdb_index .debug_names" "check for .gdb_
}
}
+# We are using the index if the index cache is enabled and there are not cache
+# misses.
+set index_cache_misses -1
+gdb_test_multiple "show index-cache stats" "check index cache stats" {
+ -re ".*Cache misses \\(this session\\): (\\d+)\r\n.*$gdb_prompt $" {
+ set index_cache_misses $expect_out(1,string)
+ }
+}
+
+set using_index_cache 0
+gdb_test_multiple "show index-cache" "check index cache status" {
+ -re ".*is currently disabled.\r\n$gdb_prompt $" {
+ set using_index_cache 0
+ }
+ -re ".*is currently enabled.\r\n$gdb_prompt $" {
+ set using_index_cache 1
+ }
+}
+
+if { $index_cache_misses == 0 && $using_index_cache } {
+ set have_gdb_index 1
+}
+
#
# this command does not produce any output
# unless there is some problem with the symtabs and psymtabs
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 851e490..883015d 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -327,8 +327,10 @@ proc default_mi_gdb_start { args } {
# Overridable function. You can override this function in your
# baseboard file.
#
-proc mi_gdb_start { args } {
- return [eval default_mi_gdb_start $args]
+if { [info procs mi_gdb_start] == "" } {
+ proc mi_gdb_start { args } {
+ return [eval default_mi_gdb_start $args]
+ }
}
# Many of the tests depend on setting breakpoints at various places and
@@ -645,11 +647,13 @@ proc mi_gdb_target_load { } {
# load a file into the debugger.
# return a -1 if anything goes wrong.
#
-proc mi_gdb_load { arg } {
- if { $arg != "" } {
- return [mi_gdb_file_cmd $arg]
+if { [info procs mi_gdb_load] == "" } {
+ proc mi_gdb_load { arg } {
+ if { $arg != "" } {
+ return [mi_gdb_file_cmd $arg]
+ }
+ return 0
}
- return 0
}
# mi_gdb_test COMMAND PATTERN MESSAGE [IPATTERN] -- send a command to gdb;
--
2.7.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RFC 0/5] Add a DWARF index cache
@ 2018-05-09 21:27 Simon Marchi
2018-05-09 21:27 ` [PATCH RFC 5/5] Add " Simon Marchi
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Simon Marchi @ 2018-05-09 21:27 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
I made a quick and dirty prototype a while ago where GDB would automatically
save and lookup DWARF index files. A few people told me that it was quite
useful, so I'm trying to make it suitable for merging. I'm sending it as an
RFC first to get comments on the feature and the approach, and because there
are probably a few loose ends. Patches 1-4 are mostly cleanup or preparatory,
the final one contains the actual feature. The commit message of that last
patch also contains more details.
Simon Marchi (5):
Rename some functions, index -> gdb_index
Remove mapped_index::total_size
Make index reading functions more modular
Introduce scoped_mmapped_file
Add DWARF index cache
gdb/Makefile.in | 3 +
gdb/build-id.h | 11 +
gdb/common/pathstuff.c | 16 +
gdb/common/pathstuff.h | 10 +
gdb/common/scoped_fd.h | 17 +-
gdb/common/scoped_mmapped_file.h | 70 +++++
gdb/dwarf-index-cache.c | 421 ++++++++++++++++++++++++++
gdb/dwarf-index-cache.h | 86 ++++++
gdb/dwarf-index-common.h | 5 +
gdb/dwarf-index-write.c | 81 +++--
gdb/dwarf-index-write.h | 34 +++
gdb/dwarf2read.c | 257 +++++++++++-----
gdb/dwarf2read.h | 5 +
gdb/testsuite/boards/local-board.exp | 1 +
gdb/testsuite/gdb.base/index-cache.c | 23 ++
gdb/testsuite/gdb.base/index-cache.exp | 204 +++++++++++++
gdb/testsuite/gdb.base/maint.exp | 23 ++
gdb/testsuite/lib/mi-support.exp | 16 +-
gdb/unittests/scoped_mmapped_file-selftests.c | 95 ++++++
19 files changed, 1270 insertions(+), 108 deletions(-)
create mode 100644 gdb/common/scoped_mmapped_file.h
create mode 100644 gdb/dwarf-index-cache.c
create mode 100644 gdb/dwarf-index-cache.h
create mode 100644 gdb/dwarf-index-write.h
create mode 100644 gdb/testsuite/gdb.base/index-cache.c
create mode 100644 gdb/testsuite/gdb.base/index-cache.exp
create mode 100644 gdb/unittests/scoped_mmapped_file-selftests.c
--
2.7.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RFC 2/5] Remove mapped_index::total_size
2018-05-09 21:27 [PATCH RFC 0/5] Add a DWARF index cache Simon Marchi
` (2 preceding siblings ...)
2018-05-09 21:27 ` [PATCH RFC 3/5] Make index reading functions more modular Simon Marchi
@ 2018-05-09 21:27 ` Simon Marchi
2018-05-09 23:08 ` Simon Marchi
2018-05-10 20:54 ` Tom Tromey
2018-05-09 21:27 ` [PATCH RFC 1/5] Rename some functions, index -> gdb_index Simon Marchi
2018-05-09 21:57 ` [PATCH RFC 0/5] Add a DWARF index cache Simon Marchi
5 siblings, 2 replies; 20+ messages in thread
From: Simon Marchi @ 2018-05-09 21:27 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
It is unused.
gdb/ChangeLog:
* dwarf2read.c (mapped_index) <total_size>: Remove.
---
gdb/dwarf2read.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 823b498..1c72818 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -197,9 +197,6 @@ struct mapped_index final : public mapped_index_base
/* Index data format version. */
int version;
- /* The total length of the buffer. */
- off_t total_size;
-
/* The address table data. */
gdb::array_view<const gdb_byte> address_table;
@@ -3500,7 +3497,6 @@ to use the section anyway."),
return 0;
map->version = version;
- map->total_size = section->size;
metadata = (offset_type *) (addr + sizeof (offset_type));
--
2.7.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RFC 0/5] Add a DWARF index cache
2018-05-09 21:27 [PATCH RFC 0/5] Add a DWARF index cache Simon Marchi
` (4 preceding siblings ...)
2018-05-09 21:27 ` [PATCH RFC 1/5] Rename some functions, index -> gdb_index Simon Marchi
@ 2018-05-09 21:57 ` Simon Marchi
5 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2018-05-09 21:57 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
I made a quick and dirty prototype a while ago where GDB would automatically
save and lookup DWARF index files. A few people told me that it was quite
useful, so I'm trying to make it suitable for merging. I'm sending it as an
RFC first to get comments on the feature and the approach, and because there
are probably a few loose ends. Patches 1-4 are mostly cleanup or preparatory,
the final one contains the actual feature. The commit message of that last
patch also contains more details.
Simon Marchi (5):
Rename some functions, index -> gdb_index
Remove mapped_index::total_size
Make index reading functions more modular
Introduce scoped_mmapped_file
Add DWARF index cache
gdb/Makefile.in | 3 +
gdb/build-id.h | 11 +
gdb/common/pathstuff.c | 16 +
gdb/common/pathstuff.h | 10 +
gdb/common/scoped_fd.h | 17 +-
gdb/common/scoped_mmapped_file.h | 70 +++++
gdb/dwarf-index-cache.c | 421 ++++++++++++++++++++++++++
gdb/dwarf-index-cache.h | 86 ++++++
gdb/dwarf-index-common.h | 5 +
gdb/dwarf-index-write.c | 81 +++--
gdb/dwarf-index-write.h | 34 +++
gdb/dwarf2read.c | 257 +++++++++++-----
gdb/dwarf2read.h | 5 +
gdb/testsuite/boards/local-board.exp | 1 +
gdb/testsuite/gdb.base/index-cache.c | 23 ++
gdb/testsuite/gdb.base/index-cache.exp | 204 +++++++++++++
gdb/testsuite/gdb.base/maint.exp | 23 ++
gdb/testsuite/lib/mi-support.exp | 16 +-
gdb/unittests/scoped_mmapped_file-selftests.c | 95 ++++++
19 files changed, 1270 insertions(+), 108 deletions(-)
create mode 100644 gdb/common/scoped_mmapped_file.h
create mode 100644 gdb/dwarf-index-cache.c
create mode 100644 gdb/dwarf-index-cache.h
create mode 100644 gdb/dwarf-index-write.h
create mode 100644 gdb/testsuite/gdb.base/index-cache.c
create mode 100644 gdb/testsuite/gdb.base/index-cache.exp
create mode 100644 gdb/unittests/scoped_mmapped_file-selftests.c
--
2.7.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH RFC 2/5] Remove mapped_index::total_size
2018-05-09 21:27 ` [PATCH RFC 2/5] Remove mapped_index::total_size Simon Marchi
@ 2018-05-09 23:08 ` Simon Marchi
2018-05-10 20:54 ` Tom Tromey
1 sibling, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2018-05-09 23:08 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
It is unused.
gdb/ChangeLog:
* dwarf2read.c (mapped_index) <total_size>: Remove.
---
gdb/dwarf2read.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 823b498..1c72818 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -197,9 +197,6 @@ struct mapped_index final : public mapped_index_base
/* Index data format version. */
int version;
- /* The total length of the buffer. */
- off_t total_size;
-
/* The address table data. */
gdb::array_view<const gdb_byte> address_table;
@@ -3500,7 +3497,6 @@ to use the section anyway."),
return 0;
map->version = version;
- map->total_size = section->size;
metadata = (offset_type *) (addr + sizeof (offset_type));
--
2.7.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 1/5] Rename some functions, index -> gdb_index
2018-05-09 21:27 ` [PATCH RFC 1/5] Rename some functions, index -> gdb_index Simon Marchi
@ 2018-05-10 20:40 ` Tom Tromey
2018-06-12 2:03 ` Simon Marchi
0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2018-05-10 20:40 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> * dwarf2read.c (read_index_from_section): Rename to...
Simon> (read_gdb_index_from_section): ... this, update all callers.
Simon> (dwarf2_read_index): Rename to...
Simon> (dwarf2_read_gdb_index): ... this, update all callers.
This seems reasonable to me.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 2/5] Remove mapped_index::total_size
2018-05-09 21:27 ` [PATCH RFC 2/5] Remove mapped_index::total_size Simon Marchi
2018-05-09 23:08 ` Simon Marchi
@ 2018-05-10 20:54 ` Tom Tromey
2018-05-18 20:26 ` Simon Marchi
1 sibling, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2018-05-10 20:54 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> It is unused.
Simon> gdb/ChangeLog:
Simon> * dwarf2read.c (mapped_index) <total_size>: Remove.
IMO this qualifies as obvious.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 3/5] Make index reading functions more modular
2018-05-09 21:27 ` [PATCH RFC 3/5] Make index reading functions more modular Simon Marchi
@ 2018-05-10 21:02 ` Tom Tromey
2018-05-10 21:18 ` Simon Marchi
0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2018-05-10 21:02 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> The read_gdb_index_from_section and read_debug_names_from_section
Simon> functions read the index content, as their names state, from sections of
Simon> object files. A following patch will make it possible to read index
Simon> content from standalone files.
Simon> This patch therefore decouples the code that reads the index content
Simon> from object file sections from the code that processes that content.
Simon> Functions dwarf2_read_gdb_index and dwarf2_read_debug_names receive
Simon> callbacks that are responsible for providing the index contents (for
Simon> both the main file and the potential dwz file).
This callback style makes the code harder to read and understand; but
other than that this seems fine.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 4/5] Introduce scoped_mmapped_file
2018-05-09 21:27 ` [PATCH RFC 4/5] Introduce scoped_mmapped_file Simon Marchi
@ 2018-05-10 21:04 ` Tom Tromey
2018-05-10 21:27 ` Simon Marchi
0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2018-05-10 21:04 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> + void reset (int new_fd)
Simon> + {
Simon> + destroy ();
Simon> +
Simon> + m_fd = new_fd;
Simon> + }
I was wondering if this should only destroy() when new_fd!=m_fd.
That way self-resetting would not cause a bug.
Simon> + /* Map FILENAME in memory. Throw an error if anything goes wrong. */
Simon> + scoped_mmapped_file (const char *filename)
Simon> + {
Simon> + m_fd.reset (open (filename, 0, O_RDONLY));
... however, then I thought perhaps this question could be avoided by
just initializing m_fd directly, like:
scoped_mmapped_file (const char *filename)
: m_fd (open (filename, 0, O_RDONLY))
Then "reset" wouldn't be needed at all.
Also I think that should be "open (filename, O_RDONLY)" -- no "0, ".
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 3/5] Make index reading functions more modular
2018-05-10 21:02 ` Tom Tromey
@ 2018-05-10 21:18 ` Simon Marchi
0 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2018-05-10 21:18 UTC (permalink / raw)
To: Tom Tromey; +Cc: Simon Marchi, gdb-patches
On 2018-05-10 16:54, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
> Simon> The read_gdb_index_from_section and
> read_debug_names_from_section
> Simon> functions read the index content, as their names state, from
> sections of
> Simon> object files. A following patch will make it possible to read
> index
> Simon> content from standalone files.
>
> Simon> This patch therefore decouples the code that reads the index
> content
> Simon> from object file sections from the code that processes that
> content.
> Simon> Functions dwarf2_read_gdb_index and dwarf2_read_debug_names
> receive
> Simon> callbacks that are responsible for providing the index contents
> (for
> Simon> both the main file and the potential dwz file).
>
> This callback style makes the code harder to read and understand; but
> other than that this seems fine.
Indeed, I'm open to suggestions :).
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 4/5] Introduce scoped_mmapped_file
2018-05-10 21:04 ` Tom Tromey
@ 2018-05-10 21:27 ` Simon Marchi
2018-05-12 15:49 ` Tom Tromey
2018-06-13 1:36 ` Simon Marchi
0 siblings, 2 replies; 20+ messages in thread
From: Simon Marchi @ 2018-05-10 21:27 UTC (permalink / raw)
To: Tom Tromey; +Cc: Simon Marchi, gdb-patches
On 2018-05-10 16:59, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
> Simon> + void reset (int new_fd)
> Simon> + {
> Simon> + destroy ();
> Simon> +
> Simon> + m_fd = new_fd;
> Simon> + }
>
> I was wondering if this should only destroy() when new_fd!=m_fd.
> That way self-resetting would not cause a bug.
It probably should, indeed.
> Simon> + /* Map FILENAME in memory. Throw an error if anything goes
> wrong. */
> Simon> + scoped_mmapped_file (const char *filename)
> Simon> + {
> Simon> + m_fd.reset (open (filename, 0, O_RDONLY));
>
> ... however, then I thought perhaps this question could be avoided by
> just initializing m_fd directly, like:
>
> scoped_mmapped_file (const char *filename)
> : m_fd (open (filename, 0, O_RDONLY))
>
> Then "reset" wouldn't be needed at all.
I can do that, but I am using reset() in the following patch anyway.
Though if you find a way of avoiding that usage, then we can remove the
method.
> Also I think that should be "open (filename, O_RDONLY)" -- no "0, ".
Right, thanks.
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 5/5] Add DWARF index cache
2018-05-09 21:27 ` [PATCH RFC 5/5] Add " Simon Marchi
@ 2018-05-10 22:24 ` Tom Tromey
2018-07-09 20:56 ` Simon Marchi
0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2018-05-10 22:24 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
Simon> To help make using the DWARF index more transparent, this patch
Simon> introduces a DWARF index cache. When enabled, loading an index-less
Simon> binary in GDB will automatically save an index file in ~/.cache/gdb.
Simon> When loading that same object file again, the index file will be looked
Simon> up and used to load the DWARF index.
Thanks for doing this. I like this feature.
I read through the patch, but I didn't read the tests. Some notes
below.
I'm sure you know, since it is an RFC, but the patch needs some more
comments here and there.
I have a wacky idea, too -- I'm testing a patch to move psymtabs out of
the objfile obstack and into a "self managed" object. With this in
place, and because psymtabs are effectively immutable, it seems to me
that the index writing could be done in a background thread. (I haven't
looked to see how the DWARF 5 index stuff works, I only really know
about .gdb_index, so this may not apply there.)
Simon> - The cache is just a directory with files named after the object
Simon> file's build-id. It is not possible to save/load the index for an
Simon> object file without build-id in the cache. Note that Clang does not
Simon> add build-ids by default.
One thing that would be nice is also having some way to associate file
names with the build-ids. Then gdb could notice when a build-id is
stale and unlink that cache entry.
For example you could have a symlink whose name is the some mangled form
of the objfile name, and whose target is the cache file. Then if gdb
sees an objfile with that name but with a different link target, it can
remove the link target. This might be a pain to implement in a non-racy
way, though.
I wonder if gdb should write out a short README into that directory when
it is first created.
Simon> - The size taken up by ~/.cache/gdb is not limited. I was thinking we
Simon> could add configurable limit (like ccache does), but that would come
Simon> after. Also, maybe a command to flush the cache.
Seems reasonable.
Simon> +/* A cheap (as in low-quality) recursive mkdir. Try to create all the parents
Simon> + directories up to DIR and DIR itself. Stop if we hit an error along the way.
Simon> + There is no attempt to remove created directories in case of failure. */
Simon> +
Simon> +static void
Simon> +mkdir_recursive (const char *dir)
gnulib has a 'mkdir-p' module which could perhaps be used instead.
Simon> +index_cache_resource::~index_cache_resource () = default;
Simon> +struct index_cache_resource
Simon> +{
Simon> + virtual ~index_cache_resource () = 0;
Simon> +};
I don't understand how to reconcile these two things - probably just
another bit of C++ knowledge I am missing. Would you mind explaining
it?
Simon> +std::string
Simon> +index_cache::make_index_filename (const bfd_build_id *build_id,
Simon> + const char *suffix) const
Simon> +{
Simon> + std::string build_id_str = build_id_to_string (build_id);
Simon> +
Simon> + return string_printf ("%s%s%s%s", m_dir.c_str (), SLASH_STRING,
Simon> + build_id_str.c_str (), suffix);
I think this particular one would be clearer just using "+".
Simon> + /* Err... I guess /tmp is better than nothing. */
Simon> + index_cache_directory = xstrdup ("/tmp");
Simon> + global_index_cache.set_directory ("/tmp");
I tend to think nothing would be better. Using /tmp means that gdb will
be reading files with (sometimes) predictable names that could be
created by anybody. Better, IMO, to issue some sort of warning and
disable the feature.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 4/5] Introduce scoped_mmapped_file
2018-05-10 21:27 ` Simon Marchi
@ 2018-05-12 15:49 ` Tom Tromey
2018-06-13 1:36 ` Simon Marchi
1 sibling, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2018-05-12 15:49 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi, gdb-patches
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
>> ... however, then I thought perhaps this question could be avoided by
>> just initializing m_fd directly, like:
>>
>> scoped_mmapped_file (const char *filename)
>> : m_fd (open (filename, 0, O_RDONLY))
>>
>> Then "reset" wouldn't be needed at all.
Simon> I can do that, but I am using reset() in the following patch anyway.
Oops, I didn't notice that. FWIW I think it's fine to have a reset
method.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 2/5] Remove mapped_index::total_size
2018-05-10 20:54 ` Tom Tromey
@ 2018-05-18 20:26 ` Simon Marchi
0 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2018-05-18 20:26 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2018-05-10 04:33 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
> Simon> It is unused.
> Simon> gdb/ChangeLog:
>
> Simon> * dwarf2read.c (mapped_index) <total_size>: Remove.
>
> IMO this qualifies as obvious.
>
> Tom
>
Right, I pushed it on its own.
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 1/5] Rename some functions, index -> gdb_index
2018-05-10 20:40 ` Tom Tromey
@ 2018-06-12 2:03 ` Simon Marchi
0 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2018-06-12 2:03 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi; +Cc: gdb-patches
On 2018-05-10 04:33 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
> Simon> * dwarf2read.c (read_index_from_section): Rename to...
> Simon> (read_gdb_index_from_section): ... this, update all callers.
> Simon> (dwarf2_read_index): Rename to...
> Simon> (dwarf2_read_gdb_index): ... this, update all callers.
>
> This seems reasonable to me.
Thanks, I pushed this one too by itself.
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 4/5] Introduce scoped_mmapped_file
2018-05-10 21:27 ` Simon Marchi
2018-05-12 15:49 ` Tom Tromey
@ 2018-06-13 1:36 ` Simon Marchi
1 sibling, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2018-06-13 1:36 UTC (permalink / raw)
To: Tom Tromey; +Cc: Simon Marchi, gdb-patches
On 2018-05-10 05:10 PM, Simon Marchi wrote:
> On 2018-05-10 16:59, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>>
>> Simon> + void reset (int new_fd)
>> Simon> + {
>> Simon> + destroy ();
>> Simon> +
>> Simon> + m_fd = new_fd;
>> Simon> + }
>>
>> I was wondering if this should only destroy() when new_fd!=m_fd.
>> That way self-resetting would not cause a bug.
>
> It probably should, indeed.
I looked into this a bit more, and I think I'll leave it like this. This is
how unique_ptr works (at least libstdc++'s and libc++'s implementations of it),
and that's how our other scoped_* implementations work (scoped_mmap amd ref_ptr).
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH RFC 5/5] Add DWARF index cache
2018-05-10 22:24 ` Tom Tromey
@ 2018-07-09 20:56 ` Simon Marchi
0 siblings, 0 replies; 20+ messages in thread
From: Simon Marchi @ 2018-07-09 20:56 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 2018-05-10 05:27 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
> Simon> To help make using the DWARF index more transparent, this patch
> Simon> introduces a DWARF index cache. When enabled, loading an index-less
> Simon> binary in GDB will automatically save an index file in ~/.cache/gdb.
> Simon> When loading that same object file again, the index file will be looked
> Simon> up and used to load the DWARF index.
>
> Thanks for doing this. I like this feature.
>
> I read through the patch, but I didn't read the tests. Some notes
> below.
>
> I'm sure you know, since it is an RFC, but the patch needs some more
> comments here and there.
I didn't realize it at the time, but reading it now, I concur. I added some,
hopefully it helps.
> I have a wacky idea, too -- I'm testing a patch to move psymtabs out of
> the objfile obstack and into a "self managed" object. With this in
> place, and because psymtabs are effectively immutable, it seems to me
> that the index writing could be done in a background thread. (I haven't
> looked to see how the DWARF 5 index stuff works, I only really know
> about .gdb_index, so this may not apply there.)
Yes, sounds like a good idea.
> Simon> - The cache is just a directory with files named after the object
> Simon> file's build-id. It is not possible to save/load the index for an
> Simon> object file without build-id in the cache. Note that Clang does not
> Simon> add build-ids by default.
>
> One thing that would be nice is also having some way to associate file
> names with the build-ids. Then gdb could notice when a build-id is
> stale and unlink that cache entry.
>
> For example you could have a symlink whose name is the some mangled form
> of the objfile name, and whose target is the cache file. Then if gdb
> sees an objfile with that name but with a different link target, it can
> remove the link target. This might be a pain to implement in a non-racy
> way, though.
Good idea, but as you said it could be racy. There needs to be more thought
put into this, so I'll leave this for later.
> I wonder if gdb should write out a short README into that directory when
> it is first created.
Good idea. In particular, it could say what these files are used for, and say
that it's perfectly safe to delete them if they take too much space (and refer
to the GDB manual about how to manage the cache size limit, eventually).
I'll keep this idea in mind for later.
> Simon> +/* A cheap (as in low-quality) recursive mkdir. Try to create all the parents
> Simon> + directories up to DIR and DIR itself. Stop if we hit an error along the way.
> Simon> + There is no attempt to remove created directories in case of failure. */
> Simon> +
> Simon> +static void
> Simon> +mkdir_recursive (const char *dir)
>
> gnulib has a 'mkdir-p' module which could perhaps be used instead.
One problem of the 'mkdir-p' module is that it pulls the xmalloc module, and we end
up with multiple definitions of xmalloc, xrealloc, etc. However, the mkdir-p module
uses the mkancesdirs module internally, which is simpler and would probably be enough
for our needs. However, I couldn't quite figure out how to use the savewd argument...
Also, the mkancesdirs module is not C++-friendly yet, so there would need to be some
patches to gnulib first, then we would need to update our gnulib import. I would leave
it as-is for the initial submission to keep it simple, it can always be improved later.
> Simon> +index_cache_resource::~index_cache_resource () = default;
>
> Simon> +struct index_cache_resource
> Simon> +{
> Simon> + virtual ~index_cache_resource () = 0;
> Simon> +};
>
> I don't understand how to reconcile these two things - probably just
> another bit of C++ knowledge I am missing. Would you mind explaining
> it?
The "= 0" part means the method (or destructor, in this case) is pure. This
is to prevent somebody instantiating this class directly.
The "= default" one is to ask the compiler to generate the default version
of the destructor for this class. If you remove it, you'll get some
undefined reference to "~index_cache_resource". I think it's equivalent
to
index_cache_resource::~index_cache_resource () {}
but more C++11-y. We often see "= default" used directly in the class
definition, but AFAIK it's not possible to put both "= 0" and "= default"
on the same declaration, hence the need to place it in the .c file.
>
> Simon> +std::string
> Simon> +index_cache::make_index_filename (const bfd_build_id *build_id,
> Simon> + const char *suffix) const
> Simon> +{
> Simon> + std::string build_id_str = build_id_to_string (build_id);
> Simon> +
> Simon> + return string_printf ("%s%s%s%s", m_dir.c_str (), SLASH_STRING,
> Simon> + build_id_str.c_str (), suffix);
>
> I think this particular one would be clearer just using "+".
Done.
> Simon> + /* Err... I guess /tmp is better than nothing. */
> Simon> + index_cache_directory = xstrdup ("/tmp");
> Simon> + global_index_cache.set_directory ("/tmp");
>
> I tend to think nothing would be better. Using /tmp means that gdb will
> be reading files with (sometimes) predictable names that could be
> created by anybody. Better, IMO, to issue some sort of warning and
> disable the feature.
Done.
Thanks for looking into it! Since I didn't receive too many negative comments,
I'll send a non-RFC version soon.
Simon
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-07-09 20:56 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-09 21:27 [PATCH RFC 0/5] Add a DWARF index cache Simon Marchi
2018-05-09 21:27 ` [PATCH RFC 5/5] Add " Simon Marchi
2018-05-10 22:24 ` Tom Tromey
2018-07-09 20:56 ` Simon Marchi
2018-05-09 21:27 ` [PATCH RFC 4/5] Introduce scoped_mmapped_file Simon Marchi
2018-05-10 21:04 ` Tom Tromey
2018-05-10 21:27 ` Simon Marchi
2018-05-12 15:49 ` Tom Tromey
2018-06-13 1:36 ` Simon Marchi
2018-05-09 21:27 ` [PATCH RFC 3/5] Make index reading functions more modular Simon Marchi
2018-05-10 21:02 ` Tom Tromey
2018-05-10 21:18 ` Simon Marchi
2018-05-09 21:27 ` [PATCH RFC 2/5] Remove mapped_index::total_size Simon Marchi
2018-05-09 23:08 ` Simon Marchi
2018-05-10 20:54 ` Tom Tromey
2018-05-18 20:26 ` Simon Marchi
2018-05-09 21:27 ` [PATCH RFC 1/5] Rename some functions, index -> gdb_index Simon Marchi
2018-05-10 20:40 ` Tom Tromey
2018-06-12 2:03 ` Simon Marchi
2018-05-09 21:57 ` [PATCH RFC 0/5] Add a DWARF index cache 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).