* [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe
@ 2023-09-18 14:52 Tom Tromey
2023-09-18 14:52 ` [PATCH v2 1/7] Introduce type-safe variant of gdb_bfd_openr_iovec Tom Tromey
` (7 more replies)
0 siblings, 8 replies; 11+ messages in thread
From: Tom Tromey @ 2023-09-18 14:52 UTC (permalink / raw)
To: gdb-patches; +Cc: Lancelot Six
This series rewrites gdb_bfd_openr_iovec to be type-safe.
Note that I can't really test the solib-rocm.c changes, this was done
as a best effort.
Regression tested on x86-64 Fedora 36.
---
Changes in v2:
- Use static_cast
- Applied fix to the rocm patch
- Link to v1: https://inbox.sourceware.org/gdb-patches/20230824-gdb-bfd-vec-v1-0-850e4e907ed1@adacore.com
---
Tom Tromey (7):
Introduce type-safe variant of gdb_bfd_openr_iovec
Small constructor change to target_buffer
Convert mem_bfd_iovec to new type-safe gdb_bfd_openr_iovec
Convert target fileio to new type-safe gdb_bfd_openr_iovec
Convert minidebug to new type-safe gdb_bfd_openr_iovec
Convert solib-rocm to new type-safe gdb_bfd_openr_iovec
Remove old gdb_bfd_openr_iovec
gdb/gdb_bfd.c | 211 +++++++++++++++++++++++++++----------------------------
gdb/gdb_bfd.h | 43 ++++++++----
gdb/minidebug.c | 98 +++++++++++---------------
gdb/solib-rocm.c | 66 +++++------------
4 files changed, 191 insertions(+), 227 deletions(-)
---
base-commit: cf2ab5ef0b716dea512d85276c484fce758fa5d4
change-id: 20230824-gdb-bfd-vec-ba7f6c1139bb
Best regards,
--
Tom Tromey <tromey@adacore.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/7] Introduce type-safe variant of gdb_bfd_openr_iovec
2023-09-18 14:52 [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Tom Tromey
@ 2023-09-18 14:52 ` Tom Tromey
2023-09-18 14:52 ` [PATCH v2 2/7] Small constructor change to target_buffer Tom Tromey
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2023-09-18 14:52 UTC (permalink / raw)
To: gdb-patches; +Cc: Lancelot Six
This patch adds a new, type-safe variant of gdb_bfd_openr_iovec. In
this approach, the underlying user data is simply an object, the
callbacks are methods, and the "open" function is a function view.
Nothing uses this new code yet.
Reviewed-By: Lancelot Six <lancelot.six@amd.com>
---
gdb/gdb_bfd.c | 42 ++++++++++++++++++++++++++++++++++++++++++
gdb/gdb_bfd.h | 31 +++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+)
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 3765561cbe9..de7ecaea630 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -907,6 +907,48 @@ gdb_bfd_openw (const char *filename, const char *target)
/* See gdb_bfd.h. */
+gdb_bfd_ref_ptr
+gdb_bfd_openr_iovec (const char *filename, const char *target,
+ gdb_iovec_opener_ftype open_fn)
+{
+ auto do_open = [] (bfd *nbfd, void *closure) -> void *
+ {
+ auto real_opener = static_cast<gdb_iovec_opener_ftype *> (closure);
+ return (*real_opener) (nbfd);
+ };
+
+ auto read_trampoline = [] (bfd *nbfd, void *stream, void *buf,
+ file_ptr nbytes, file_ptr offset) -> file_ptr
+ {
+ gdb_bfd_iovec_base *obj = static_cast<gdb_bfd_iovec_base *> (stream);
+ return obj->read (nbfd, buf, nbytes, offset);
+ };
+
+ auto stat_trampoline = [] (struct bfd *abfd, void *stream,
+ struct stat *sb) -> int
+ {
+ gdb_bfd_iovec_base *obj = static_cast<gdb_bfd_iovec_base *> (stream);
+ return obj->stat (abfd, sb);
+ };
+
+ auto close_trampoline = [] (struct bfd *nbfd, void *stream) -> int
+ {
+ gdb_bfd_iovec_base *obj = static_cast<gdb_bfd_iovec_base *> (stream);
+ delete obj;
+ /* Success. */
+ return 0;
+ };
+
+ bfd *result = bfd_openr_iovec (filename, target,
+ do_open, &open_fn,
+ read_trampoline, close_trampoline,
+ stat_trampoline);
+
+ return gdb_bfd_ref_ptr::new_reference (result);
+}
+
+/* See gdb_bfd.h. */
+
gdb_bfd_ref_ptr
gdb_bfd_openr_iovec (const char *filename, const char *target,
void *(*open_func) (struct bfd *nbfd,
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index d15b1106d9a..ae374f5d7ae 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -22,6 +22,7 @@
#include "registry.h"
#include "gdbsupport/byte-vector.h"
+#include "gdbsupport/function-view.h"
#include "gdbsupport/gdb_ref_ptr.h"
#include "gdbsupport/iterator-range.h"
#include "gdbsupport/next-iterator.h"
@@ -150,6 +151,36 @@ gdb_bfd_ref_ptr gdb_bfd_openr (const char *, const char *);
gdb_bfd_ref_ptr gdb_bfd_openw (const char *, const char *);
+/* The base class for BFD "iovec" implementations. This is used by
+ gdb_bfd_openr_iovec and enables better type safety. */
+
+class gdb_bfd_iovec_base
+{
+protected:
+
+ gdb_bfd_iovec_base () = default;
+
+public:
+
+ virtual ~gdb_bfd_iovec_base () = default;
+
+ /* The "read" callback. */
+ virtual file_ptr read (bfd *abfd, void *buffer, file_ptr nbytes,
+ file_ptr offset) = 0;
+
+ /* The "stat" callback. */
+ virtual int stat (struct bfd *abfd, struct stat *sb) = 0;
+};
+
+/* The type of the function used to open a new iovec-based BFD. */
+using gdb_iovec_opener_ftype
+ = gdb::function_view<gdb_bfd_iovec_base * (bfd *)>;
+
+/* A type-safe wrapper for bfd_openr_iovec. */
+
+gdb_bfd_ref_ptr gdb_bfd_openr_iovec (const char *filename, const char *target,
+ gdb_iovec_opener_ftype open_fn);
+
/* A wrapper for bfd_openr_iovec that initializes the gdb-specific
reference count. */
--
2.40.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/7] Small constructor change to target_buffer
2023-09-18 14:52 [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Tom Tromey
2023-09-18 14:52 ` [PATCH v2 1/7] Introduce type-safe variant of gdb_bfd_openr_iovec Tom Tromey
@ 2023-09-18 14:52 ` Tom Tromey
2023-09-18 14:52 ` [PATCH v2 3/7] Convert mem_bfd_iovec to new type-safe gdb_bfd_openr_iovec Tom Tromey
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2023-09-18 14:52 UTC (permalink / raw)
To: gdb-patches
This changes the target_buffer constructor to initialize m_filename
rather than assign to it.
---
gdb/gdb_bfd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index de7ecaea630..6b64e92b48e 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -226,10 +226,10 @@ struct target_buffer
target memory. */
target_buffer (CORE_ADDR base, ULONGEST size)
: m_base (base),
- m_size (size)
+ m_size (size),
+ m_filename (xstrprintf ("<in-memory@%s>",
+ core_addr_to_string_nz (m_base)))
{
- m_filename
- = xstrprintf ("<in-memory@%s>", core_addr_to_string_nz (m_base));
}
/* Return the size of the in-memory BFD file. */
--
2.40.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 3/7] Convert mem_bfd_iovec to new type-safe gdb_bfd_openr_iovec
2023-09-18 14:52 [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Tom Tromey
2023-09-18 14:52 ` [PATCH v2 1/7] Introduce type-safe variant of gdb_bfd_openr_iovec Tom Tromey
2023-09-18 14:52 ` [PATCH v2 2/7] Small constructor change to target_buffer Tom Tromey
@ 2023-09-18 14:52 ` Tom Tromey
2023-09-18 14:52 ` [PATCH v2 4/7] Convert target fileio " Tom Tromey
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2023-09-18 14:52 UTC (permalink / raw)
To: gdb-patches
This converts the mem_bfd_iovec / target_buffer code to use the new
type-safe gdb_bfd_openr_iovec.
---
gdb/gdb_bfd.c | 61 ++++++++++++++++++++---------------------------------------
1 file changed, 20 insertions(+), 41 deletions(-)
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 6b64e92b48e..e331703d4b1 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -220,7 +220,7 @@ gdb_bfd_has_target_filename (struct bfd *abfd)
/* For `gdb_bfd_open_from_target_memory`. An object that manages the
details of a BFD in target memory. */
-struct target_buffer
+struct target_buffer : public gdb_bfd_iovec_base
{
/* Constructor. BASE and SIZE define where the BFD can be found in
target memory. */
@@ -245,6 +245,11 @@ struct target_buffer
const char *filename () const
{ return m_filename.get (); }
+ file_ptr read (bfd *abfd, void *buffer, file_ptr nbytes,
+ file_ptr offset) override;
+
+ int stat (struct bfd *abfd, struct stat *sb) override;
+
private:
/* The base address of the in-memory BFD file. */
CORE_ADDR m_base;
@@ -256,47 +261,23 @@ struct target_buffer
gdb::unique_xmalloc_ptr<char> m_filename;
};
-/* For `gdb_bfd_open_from_target_memory`. Opening the file is a no-op. */
-
-static void *
-mem_bfd_iovec_open (struct bfd *abfd, void *open_closure)
-{
- return open_closure;
-}
-
-/* For `gdb_bfd_open_from_target_memory`. Closing the file is just freeing the
- base/size pair on our side. */
-
-static int
-mem_bfd_iovec_close (struct bfd *abfd, void *stream)
-{
- struct target_buffer *buffer = (target_buffer *) stream;
- delete buffer;
-
- /* Zero means success. */
- return 0;
-}
-
/* For `gdb_bfd_open_from_target_memory`. For reading the file, we just need to
pass through to target_read_memory and fix up the arguments and return
values. */
-static file_ptr
-mem_bfd_iovec_pread (struct bfd *abfd, void *stream, void *buf,
+file_ptr
+target_buffer::read (struct bfd *abfd, void *buf,
file_ptr nbytes, file_ptr offset)
{
- struct target_buffer *buffer = (struct target_buffer *) stream;
-
/* If this read will read all of the file, limit it to just the rest. */
- if (offset + nbytes > buffer->size ())
- nbytes = buffer->size () - offset;
+ if (offset + nbytes > size ())
+ nbytes = size () - offset;
/* If there are no more bytes left, we've reached EOF. */
if (nbytes == 0)
return 0;
- int err
- = target_read_memory (buffer->base () + offset, (gdb_byte *) buf, nbytes);
+ int err = target_read_memory (base () + offset, (gdb_byte *) buf, nbytes);
if (err)
return -1;
@@ -306,13 +287,11 @@ mem_bfd_iovec_pread (struct bfd *abfd, void *stream, void *buf,
/* For `gdb_bfd_open_from_target_memory`. For statting the file, we only
support the st_size attribute. */
-static int
-mem_bfd_iovec_stat (struct bfd *abfd, void *stream, struct stat *sb)
+int
+target_buffer::stat (struct bfd *abfd, struct stat *sb)
{
- struct target_buffer *buffer = (struct target_buffer*) stream;
-
memset (sb, 0, sizeof (struct stat));
- sb->st_size = buffer->size ();
+ sb->st_size = size ();
return 0;
}
@@ -322,14 +301,14 @@ gdb_bfd_ref_ptr
gdb_bfd_open_from_target_memory (CORE_ADDR addr, ULONGEST size,
const char *target)
{
- struct target_buffer *buffer = new target_buffer (addr, size);
+ std::unique_ptr<target_buffer> buffer
+ = gdb::make_unique<target_buffer> (addr, size);
return gdb_bfd_openr_iovec (buffer->filename (), target,
- mem_bfd_iovec_open,
- buffer,
- mem_bfd_iovec_pread,
- mem_bfd_iovec_close,
- mem_bfd_iovec_stat);
+ [&] (bfd *nbfd)
+ {
+ return buffer.release ();
+ });
}
/* bfd_openr_iovec OPEN_CLOSURE data for gdb_bfd_open. */
--
2.40.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 4/7] Convert target fileio to new type-safe gdb_bfd_openr_iovec
2023-09-18 14:52 [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Tom Tromey
` (2 preceding siblings ...)
2023-09-18 14:52 ` [PATCH v2 3/7] Convert mem_bfd_iovec to new type-safe gdb_bfd_openr_iovec Tom Tromey
@ 2023-09-18 14:52 ` Tom Tromey
2023-09-18 14:52 ` [PATCH v2 5/7] Convert minidebug " Tom Tromey
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2023-09-18 14:52 UTC (permalink / raw)
To: gdb-patches
This converts the target fileio BFD iovec implementation to use the
new type-safe gdb_bfd_openr_iovec.
---
gdb/gdb_bfd.c | 97 ++++++++++++++++++++++++++++++-----------------------------
1 file changed, 50 insertions(+), 47 deletions(-)
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index e331703d4b1..2f489a4f210 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -311,30 +311,48 @@ gdb_bfd_open_from_target_memory (CORE_ADDR addr, ULONGEST size,
});
}
-/* bfd_openr_iovec OPEN_CLOSURE data for gdb_bfd_open. */
-struct gdb_bfd_open_closure
+/* An object that manages the underlying stream for a BFD, using
+ target file I/O. */
+
+struct target_fileio_stream : public gdb_bfd_iovec_base
{
- inferior *inf;
- bool warn_if_slow;
+ target_fileio_stream (bfd *nbfd, int fd)
+ : m_bfd (nbfd),
+ m_fd (fd)
+ {
+ }
+
+ ~target_fileio_stream ();
+
+ file_ptr read (bfd *abfd, void *buffer, file_ptr nbytes,
+ file_ptr offset) override;
+
+ int stat (struct bfd *abfd, struct stat *sb) override;
+
+private:
+
+ /* The BFD. Saved for the destructor. */
+ bfd *m_bfd;
+
+ /* The file descriptor. */
+ int m_fd;
};
-/* Wrapper for target_fileio_open suitable for passing as the
- OPEN_FUNC argument to gdb_bfd_openr_iovec. */
+/* Wrapper for target_fileio_open suitable for use as a helper
+ function for gdb_bfd_openr_iovec. */
-static void *
-gdb_bfd_iovec_fileio_open (struct bfd *abfd, void *open_closure)
+static target_fileio_stream *
+gdb_bfd_iovec_fileio_open (struct bfd *abfd, inferior *inf, bool warn_if_slow)
{
const char *filename = bfd_get_filename (abfd);
int fd;
fileio_error target_errno;
- int *stream;
- gdb_bfd_open_closure *oclosure = (gdb_bfd_open_closure *) open_closure;
gdb_assert (is_target_filename (filename));
- fd = target_fileio_open (oclosure->inf,
+ fd = target_fileio_open (inf,
filename + strlen (TARGET_SYSROOT_PREFIX),
- FILEIO_O_RDONLY, 0, oclosure->warn_if_slow,
+ FILEIO_O_RDONLY, 0, warn_if_slow,
&target_errno);
if (fd == -1)
{
@@ -343,19 +361,15 @@ gdb_bfd_iovec_fileio_open (struct bfd *abfd, void *open_closure)
return NULL;
}
- stream = XCNEW (int);
- *stream = fd;
- return stream;
+ return new target_fileio_stream (abfd, fd);
}
-/* Wrapper for target_fileio_pread suitable for passing as the
- PREAD_FUNC argument to gdb_bfd_openr_iovec. */
+/* Wrapper for target_fileio_pread. */
-static file_ptr
-gdb_bfd_iovec_fileio_pread (struct bfd *abfd, void *stream, void *buf,
+file_ptr
+target_fileio_stream::read (struct bfd *abfd, void *buf,
file_ptr nbytes, file_ptr offset)
{
- int fd = *(int *) stream;
fileio_error target_errno;
file_ptr pos, bytes;
@@ -364,7 +378,7 @@ gdb_bfd_iovec_fileio_pread (struct bfd *abfd, void *stream, void *buf,
{
QUIT;
- bytes = target_fileio_pread (fd, (gdb_byte *) buf + pos,
+ bytes = target_fileio_pread (m_fd, (gdb_byte *) buf + pos,
nbytes - pos, offset + pos,
&target_errno);
if (bytes == 0)
@@ -392,46 +406,35 @@ gdb_bfd_close_warning (const char *name, const char *reason)
warning (_("cannot close \"%s\": %s"), name, reason);
}
-/* Wrapper for target_fileio_close suitable for passing as the
- CLOSE_FUNC argument to gdb_bfd_openr_iovec. */
+/* Wrapper for target_fileio_close. */
-static int
-gdb_bfd_iovec_fileio_close (struct bfd *abfd, void *stream)
+target_fileio_stream::~target_fileio_stream ()
{
- int fd = *(int *) stream;
fileio_error target_errno;
- xfree (stream);
-
/* Ignore errors on close. These may happen with remote
targets if the connection has already been torn down. */
try
{
- target_fileio_close (fd, &target_errno);
+ target_fileio_close (m_fd, &target_errno);
}
catch (const gdb_exception &ex)
{
/* Also avoid crossing exceptions over bfd. */
- gdb_bfd_close_warning (bfd_get_filename (abfd),
+ gdb_bfd_close_warning (bfd_get_filename (m_bfd),
ex.message->c_str ());
}
-
- /* Zero means success. */
- return 0;
}
-/* Wrapper for target_fileio_fstat suitable for passing as the
- STAT_FUNC argument to gdb_bfd_openr_iovec. */
+/* Wrapper for target_fileio_fstat. */
-static int
-gdb_bfd_iovec_fileio_fstat (struct bfd *abfd, void *stream,
- struct stat *sb)
+int
+target_fileio_stream::stat (struct bfd *abfd, struct stat *sb)
{
- int fd = *(int *) stream;
fileio_error target_errno;
int result;
- result = target_fileio_fstat (fd, sb, &target_errno);
+ result = target_fileio_fstat (m_fd, sb, &target_errno);
if (result == -1)
{
errno = fileio_error_to_host (target_errno);
@@ -482,13 +485,13 @@ gdb_bfd_open (const char *name, const char *target, int fd,
{
gdb_assert (fd == -1);
- gdb_bfd_open_closure open_closure { current_inferior (), warn_if_slow };
- return gdb_bfd_openr_iovec (name, target,
- gdb_bfd_iovec_fileio_open,
- &open_closure,
- gdb_bfd_iovec_fileio_pread,
- gdb_bfd_iovec_fileio_close,
- gdb_bfd_iovec_fileio_fstat);
+ auto open = [&] (bfd *nbfd) -> gdb_bfd_iovec_base *
+ {
+ return gdb_bfd_iovec_fileio_open (nbfd, current_inferior (),
+ warn_if_slow);
+ };
+
+ return gdb_bfd_openr_iovec (name, target, open);
}
name += strlen (TARGET_SYSROOT_PREFIX);
--
2.40.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 5/7] Convert minidebug to new type-safe gdb_bfd_openr_iovec
2023-09-18 14:52 [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Tom Tromey
` (3 preceding siblings ...)
2023-09-18 14:52 ` [PATCH v2 4/7] Convert target fileio " Tom Tromey
@ 2023-09-18 14:52 ` Tom Tromey
2023-09-18 14:52 ` [PATCH v2 6/7] Convert solib-rocm " Tom Tromey
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2023-09-18 14:52 UTC (permalink / raw)
To: gdb-patches
This converts the minidebug BFD iovec implementation to the new
type-safe gdb_bfd_openr_iovec.
---
gdb/minidebug.c | 98 +++++++++++++++++++++++----------------------------------
1 file changed, 40 insertions(+), 58 deletions(-)
diff --git a/gdb/minidebug.c b/gdb/minidebug.c
index 979e569e639..e2b60fb959c 100644
--- a/gdb/minidebug.c
+++ b/gdb/minidebug.c
@@ -57,7 +57,7 @@ static lzma_allocator gdb_lzma_allocator = { alloc_lzma, free_lzma, NULL };
a section. This keeps only the last decompressed block in memory
to allow larger data without using to much memory. */
-struct gdb_lzma_stream
+struct gdb_lzma_stream : public gdb_bfd_iovec_base
{
/* Section of input BFD from which we are decoding data. */
asection *section = nullptr;
@@ -69,19 +69,25 @@ struct gdb_lzma_stream
bfd_size_type data_start = 0;
bfd_size_type data_end = 0;
gdb::byte_vector data;
-};
-/* bfd_openr_iovec OPEN_P implementation for
- find_separate_debug_file_in_section. OPEN_CLOSURE is 'asection *'
- of the section to decompress.
- Return 'struct gdb_lzma_stream *' must be freed by caller by delete,
- together with its INDEX lzma data. */
+ ~gdb_lzma_stream ()
+ {
+ lzma_index_end (index, &gdb_lzma_allocator);
+ }
-static void *
-lzma_open (struct bfd *nbfd, void *open_closure)
+ file_ptr read (bfd *abfd, void *buffer, file_ptr nbytes,
+ file_ptr offset) override;
+
+ int stat (struct bfd *abfd, struct stat *sb) override;
+};
+
+/* bfd_openr_iovec implementation helper for
+ find_separate_debug_file_in_section. */
+
+static gdb_lzma_stream *
+lzma_open (struct bfd *nbfd, asection *section)
{
- asection *section = (asection *) open_closure;
bfd_size_type size, offset;
lzma_stream_flags options;
gdb_byte footer[LZMA_STREAM_HEADER_SIZE];
@@ -127,15 +133,13 @@ lzma_open (struct bfd *nbfd, void *open_closure)
return lstream;
}
-/* bfd_openr_iovec PREAD_P implementation for
- find_separate_debug_file_in_section. Passed STREAM
- is 'struct gdb_lzma_stream *'. */
+/* bfd_openr_iovec read implementation for
+ find_separate_debug_file_in_section. */
-static file_ptr
-lzma_pread (struct bfd *nbfd, void *stream, void *buf, file_ptr nbytes,
- file_ptr offset)
+file_ptr
+gdb_lzma_stream::read (struct bfd *nbfd, void *buf, file_ptr nbytes,
+ file_ptr offset)
{
- struct gdb_lzma_stream *lstream = (struct gdb_lzma_stream *) stream;
bfd_size_type chunk_size;
lzma_index_iter iter;
file_ptr block_offset;
@@ -147,12 +151,9 @@ lzma_pread (struct bfd *nbfd, void *stream, void *buf, file_ptr nbytes,
res = 0;
while (nbytes > 0)
{
- if (lstream->data.empty ()
- || lstream->data_start > offset || offset >= lstream->data_end)
+ if (data.empty () || data_start > offset || offset >= data_end)
{
- asection *section = lstream->section;
-
- lzma_index_iter_init (&iter, lstream->index);
+ lzma_index_iter_init (&iter, index);
if (lzma_index_iter_locate (&iter, offset))
break;
@@ -184,15 +185,14 @@ lzma_pread (struct bfd *nbfd, void *stream, void *buf, file_ptr nbytes,
!= LZMA_OK)
break;
- lstream->data = std::move (uncompressed);
- lstream->data_start = iter.block.uncompressed_file_offset;
- lstream->data_end = (iter.block.uncompressed_file_offset
- + iter.block.uncompressed_size);
+ data = std::move (uncompressed);
+ data_start = iter.block.uncompressed_file_offset;
+ data_end = (iter.block.uncompressed_file_offset
+ + iter.block.uncompressed_size);
}
- chunk_size = std::min (nbytes, (file_ptr) lstream->data_end - offset);
- memcpy (buf, lstream->data.data () + offset - lstream->data_start,
- chunk_size);
+ chunk_size = std::min (nbytes, (file_ptr) data_end - offset);
+ memcpy (buf, data.data () + offset - data_start, chunk_size);
buf = (gdb_byte *) buf + chunk_size;
offset += chunk_size;
nbytes -= chunk_size;
@@ -202,36 +202,14 @@ lzma_pread (struct bfd *nbfd, void *stream, void *buf, file_ptr nbytes,
return res;
}
-/* bfd_openr_iovec CLOSE_P implementation for
- find_separate_debug_file_in_section. Passed STREAM
- is 'struct gdb_lzma_stream *'. */
-
-static int
-lzma_close (struct bfd *nbfd,
- void *stream)
-{
- struct gdb_lzma_stream *lstream = (struct gdb_lzma_stream *) stream;
-
- lzma_index_end (lstream->index, &gdb_lzma_allocator);
- delete lstream;
-
- /* Zero means success. */
- return 0;
-}
-
-/* bfd_openr_iovec STAT_P implementation for
- find_separate_debug_file_in_section. Passed STREAM
- is 'struct gdb_lzma_stream *'. */
+/* bfd_openr_iovec stat implementation for
+ find_separate_debug_file_in_section. */
-static int
-lzma_stat (struct bfd *abfd,
- void *stream,
- struct stat *sb)
+int
+gdb_lzma_stream::stat (struct bfd *abfd, struct stat *sb)
{
- struct gdb_lzma_stream *lstream = (struct gdb_lzma_stream *) stream;
-
memset (sb, 0, sizeof (struct stat));
- sb->st_size = lzma_index_uncompressed_size (lstream->index);
+ sb->st_size = lzma_index_uncompressed_size (index);
return 0;
}
@@ -265,8 +243,12 @@ find_separate_debug_file_in_section (struct objfile *objfile)
std::string filename = string_printf (_(".gnu_debugdata for %s"),
objfile_name (objfile));
- abfd = gdb_bfd_openr_iovec (filename.c_str (), gnutarget, lzma_open,
- section, lzma_pread, lzma_close, lzma_stat);
+ auto open = [&] (bfd *nbfd) -> gdb_lzma_stream *
+ {
+ return lzma_open (nbfd, section);
+ };
+
+ abfd = gdb_bfd_openr_iovec (filename.c_str (), gnutarget, open);
if (abfd == NULL)
return NULL;
--
2.40.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 6/7] Convert solib-rocm to new type-safe gdb_bfd_openr_iovec
2023-09-18 14:52 [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Tom Tromey
` (4 preceding siblings ...)
2023-09-18 14:52 ` [PATCH v2 5/7] Convert minidebug " Tom Tromey
@ 2023-09-18 14:52 ` Tom Tromey
2023-09-28 9:57 ` Lancelot SIX
2023-09-18 14:52 ` [PATCH v2 7/7] Remove old gdb_bfd_openr_iovec Tom Tromey
2023-09-28 10:02 ` [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Lancelot SIX
7 siblings, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2023-09-18 14:52 UTC (permalink / raw)
To: gdb-patches; +Cc: Lancelot Six
This converts the solib-rocm BFD iovec implementations to the new
type-safe gdb_bfd_openr_iovec. They were already essentially using
this approach, just without the type-safe wrapper.
Thanks to Lancelot Six for testing and fixing this patch.
Co-Authored-By: Lancelot Six <lancelot.six@amd.com>
---
gdb/solib-rocm.c | 66 ++++++++++++++++----------------------------------------
1 file changed, 18 insertions(+), 48 deletions(-)
diff --git a/gdb/solib-rocm.c b/gdb/solib-rocm.c
index 56c210e9fa5..6b84f09e88b 100644
--- a/gdb/solib-rocm.c
+++ b/gdb/solib-rocm.c
@@ -268,24 +268,13 @@ namespace {
/* Interface to interact with a ROCm code object stream. */
-struct rocm_code_object_stream
+struct rocm_code_object_stream : public gdb_bfd_iovec_base
{
DISABLE_COPY_AND_ASSIGN (rocm_code_object_stream);
- /* Copy SIZE bytes from the underlying objfile storage starting at OFFSET
- into the user provided buffer BUF.
+ int stat (bfd *abfd, struct stat *sb) final override;
- Return the number of bytes actually copied (might be inferior to SIZE if
- the end of the stream is reached). */
- virtual file_ptr read (void *buf, file_ptr size, file_ptr offset) = 0;
-
- /* Retrieve file information in SB.
-
- Return 0 on success. On failure, set the appropriate bfd error number
- (using bfd_set_error) and return -1. */
- int stat (struct stat *sb);
-
- virtual ~rocm_code_object_stream () = default;
+ ~rocm_code_object_stream () override = default;
protected:
rocm_code_object_stream () = default;
@@ -298,7 +287,7 @@ struct rocm_code_object_stream
};
int
-rocm_code_object_stream::stat (struct stat *sb)
+rocm_code_object_stream::stat (bfd *, struct stat *sb)
{
const LONGEST size = this->size ();
if (size == -1)
@@ -319,7 +308,8 @@ struct rocm_code_object_stream_file final : rocm_code_object_stream
rocm_code_object_stream_file (inferior *inf, int fd, ULONGEST offset,
ULONGEST size);
- file_ptr read (void *buf, file_ptr size, file_ptr offset) override;
+ file_ptr read (bfd *abfd, void *buf, file_ptr size,
+ file_ptr offset) override;
LONGEST size () override;
@@ -348,7 +338,7 @@ rocm_code_object_stream_file::rocm_code_object_stream_file
}
file_ptr
-rocm_code_object_stream_file::read (void *buf, file_ptr size,
+rocm_code_object_stream_file::read (bfd *, void *buf, file_ptr size,
file_ptr offset)
{
fileio_error target_errno;
@@ -423,7 +413,8 @@ struct rocm_code_object_stream_memory final : public rocm_code_object_stream
rocm_code_object_stream_memory (gdb::byte_vector buffer);
- file_ptr read (void *buf, file_ptr size, file_ptr offset) override;
+ file_ptr read (bfd *abfd, void *buf, file_ptr size,
+ file_ptr offset) override;
protected:
@@ -445,7 +436,7 @@ rocm_code_object_stream_memory::rocm_code_object_stream_memory
}
file_ptr
-rocm_code_object_stream_memory::read (void *buf, file_ptr size,
+rocm_code_object_stream_memory::read (bfd *, void *buf, file_ptr size,
file_ptr offset)
{
if (size > m_objfile_image.size () - offset)
@@ -457,8 +448,8 @@ rocm_code_object_stream_memory::read (void *buf, file_ptr size,
} /* anonymous namespace */
-static void *
-rocm_bfd_iovec_open (bfd *abfd, void *inferior_void)
+static gdb_bfd_iovec_base *
+rocm_bfd_iovec_open (bfd *abfd, inferior *inferior)
{
gdb::string_view uri (bfd_get_filename (abfd));
gdb::string_view protocol_delim = "://";
@@ -522,7 +513,6 @@ rocm_bfd_iovec_open (bfd *abfd, void *inferior_void)
{
ULONGEST offset = 0;
ULONGEST size = 0;
- inferior *inferior = static_cast<struct inferior *> (inferior_void);
auto try_strtoulst = [] (gdb::string_view v)
{
@@ -607,28 +597,6 @@ rocm_bfd_iovec_open (bfd *abfd, void *inferior_void)
}
}
-static int
-rocm_bfd_iovec_close (bfd *nbfd, void *data)
-{
- delete static_cast<rocm_code_object_stream *> (data);
-
- return 0;
-}
-
-static file_ptr
-rocm_bfd_iovec_pread (bfd *abfd, void *data, void *buf, file_ptr size,
- file_ptr offset)
-{
- return static_cast<rocm_code_object_stream *> (data)->read (buf, size,
- offset);
-}
-
-static int
-rocm_bfd_iovec_stat (bfd *abfd, void *data, struct stat *sb)
-{
- return static_cast<rocm_code_object_stream *> (data)->stat (sb);
-}
-
static gdb_bfd_ref_ptr
rocm_solib_bfd_open (const char *pathname)
{
@@ -636,10 +604,12 @@ rocm_solib_bfd_open (const char *pathname)
if (strstr (pathname, "://") == nullptr)
return svr4_so_ops.bfd_open (pathname);
- gdb_bfd_ref_ptr abfd
- = gdb_bfd_openr_iovec (pathname, "elf64-amdgcn", rocm_bfd_iovec_open,
- current_inferior (), rocm_bfd_iovec_pread,
- rocm_bfd_iovec_close, rocm_bfd_iovec_stat);
+ auto open = [] (bfd *nbfd) -> gdb_bfd_iovec_base *
+ {
+ return rocm_bfd_iovec_open (nbfd, current_inferior ());
+ };
+
+ gdb_bfd_ref_ptr abfd = gdb_bfd_openr_iovec (pathname, "elf64-amdgcn", open);
if (abfd == nullptr)
error (_("Could not open `%s' as an executable file: %s"), pathname,
--
2.40.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 7/7] Remove old gdb_bfd_openr_iovec
2023-09-18 14:52 [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Tom Tromey
` (5 preceding siblings ...)
2023-09-18 14:52 ` [PATCH v2 6/7] Convert solib-rocm " Tom Tromey
@ 2023-09-18 14:52 ` Tom Tromey
2023-09-28 10:02 ` [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Lancelot SIX
7 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2023-09-18 14:52 UTC (permalink / raw)
To: gdb-patches
This removes the old gdb_bfd_openr_iovec entirely. I think any new
code should use the type-safe approach.
---
gdb/gdb_bfd.c | 25 -------------------------
gdb/gdb_bfd.h | 18 ------------------
2 files changed, 43 deletions(-)
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 2f489a4f210..217753cf914 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -931,31 +931,6 @@ gdb_bfd_openr_iovec (const char *filename, const char *target,
/* See gdb_bfd.h. */
-gdb_bfd_ref_ptr
-gdb_bfd_openr_iovec (const char *filename, const char *target,
- void *(*open_func) (struct bfd *nbfd,
- void *open_closure),
- void *open_closure,
- file_ptr (*pread_func) (struct bfd *nbfd,
- void *stream,
- void *buf,
- file_ptr nbytes,
- file_ptr offset),
- int (*close_func) (struct bfd *nbfd,
- void *stream),
- int (*stat_func) (struct bfd *abfd,
- void *stream,
- struct stat *sb))
-{
- bfd *result = bfd_openr_iovec (filename, target,
- open_func, open_closure,
- pread_func, close_func, stat_func);
-
- return gdb_bfd_ref_ptr::new_reference (result);
-}
-
-/* See gdb_bfd.h. */
-
void
gdb_bfd_mark_parent (bfd *child, bfd *parent)
{
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index ae374f5d7ae..604365b61b1 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -181,24 +181,6 @@ using gdb_iovec_opener_ftype
gdb_bfd_ref_ptr gdb_bfd_openr_iovec (const char *filename, const char *target,
gdb_iovec_opener_ftype open_fn);
-/* A wrapper for bfd_openr_iovec that initializes the gdb-specific
- reference count. */
-
-gdb_bfd_ref_ptr gdb_bfd_openr_iovec (const char *filename, const char *target,
- void *(*open_func) (struct bfd *nbfd,
- void *open_closure),
- void *open_closure,
- file_ptr (*pread_func) (struct bfd *nbfd,
- void *stream,
- void *buf,
- file_ptr nbytes,
- file_ptr offset),
- int (*close_func) (struct bfd *nbfd,
- void *stream),
- int (*stat_func) (struct bfd *abfd,
- void *stream,
- struct stat *sb));
-
/* A wrapper for bfd_openr_next_archived_file that initializes the
gdb-specific reference count. */
--
2.40.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 6/7] Convert solib-rocm to new type-safe gdb_bfd_openr_iovec
2023-09-18 14:52 ` [PATCH v2 6/7] Convert solib-rocm " Tom Tromey
@ 2023-09-28 9:57 ` Lancelot SIX
0 siblings, 0 replies; 11+ messages in thread
From: Lancelot SIX @ 2023-09-28 9:57 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Hi Tom,
I went through this patch and tested it. I have also tested it against
the downstream ROCgdb, and everything seems good te me.
It looks good to me, so
Acked-by: Lancelot Six <lancelot.six@amd.com>
Thanks for doing this.
Best,
Lancelot.
On Mon, Sep 18, 2023 at 08:52:51AM -0600, Tom Tromey wrote:
> This converts the solib-rocm BFD iovec implementations to the new
> type-safe gdb_bfd_openr_iovec. They were already essentially using
> this approach, just without the type-safe wrapper.
>
> Thanks to Lancelot Six for testing and fixing this patch.
>
> Co-Authored-By: Lancelot Six <lancelot.six@amd.com>
> ---
> gdb/solib-rocm.c | 66 ++++++++++++++++----------------------------------------
> 1 file changed, 18 insertions(+), 48 deletions(-)
>
> diff --git a/gdb/solib-rocm.c b/gdb/solib-rocm.c
> index 56c210e9fa5..6b84f09e88b 100644
> --- a/gdb/solib-rocm.c
> +++ b/gdb/solib-rocm.c
> @@ -268,24 +268,13 @@ namespace {
>
> /* Interface to interact with a ROCm code object stream. */
>
> -struct rocm_code_object_stream
> +struct rocm_code_object_stream : public gdb_bfd_iovec_base
> {
> DISABLE_COPY_AND_ASSIGN (rocm_code_object_stream);
>
> - /* Copy SIZE bytes from the underlying objfile storage starting at OFFSET
> - into the user provided buffer BUF.
> + int stat (bfd *abfd, struct stat *sb) final override;
>
> - Return the number of bytes actually copied (might be inferior to SIZE if
> - the end of the stream is reached). */
> - virtual file_ptr read (void *buf, file_ptr size, file_ptr offset) = 0;
> -
> - /* Retrieve file information in SB.
> -
> - Return 0 on success. On failure, set the appropriate bfd error number
> - (using bfd_set_error) and return -1. */
> - int stat (struct stat *sb);
> -
> - virtual ~rocm_code_object_stream () = default;
> + ~rocm_code_object_stream () override = default;
>
> protected:
> rocm_code_object_stream () = default;
> @@ -298,7 +287,7 @@ struct rocm_code_object_stream
> };
>
> int
> -rocm_code_object_stream::stat (struct stat *sb)
> +rocm_code_object_stream::stat (bfd *, struct stat *sb)
> {
> const LONGEST size = this->size ();
> if (size == -1)
> @@ -319,7 +308,8 @@ struct rocm_code_object_stream_file final : rocm_code_object_stream
> rocm_code_object_stream_file (inferior *inf, int fd, ULONGEST offset,
> ULONGEST size);
>
> - file_ptr read (void *buf, file_ptr size, file_ptr offset) override;
> + file_ptr read (bfd *abfd, void *buf, file_ptr size,
> + file_ptr offset) override;
>
> LONGEST size () override;
>
> @@ -348,7 +338,7 @@ rocm_code_object_stream_file::rocm_code_object_stream_file
> }
>
> file_ptr
> -rocm_code_object_stream_file::read (void *buf, file_ptr size,
> +rocm_code_object_stream_file::read (bfd *, void *buf, file_ptr size,
> file_ptr offset)
> {
> fileio_error target_errno;
> @@ -423,7 +413,8 @@ struct rocm_code_object_stream_memory final : public rocm_code_object_stream
>
> rocm_code_object_stream_memory (gdb::byte_vector buffer);
>
> - file_ptr read (void *buf, file_ptr size, file_ptr offset) override;
> + file_ptr read (bfd *abfd, void *buf, file_ptr size,
> + file_ptr offset) override;
>
> protected:
>
> @@ -445,7 +436,7 @@ rocm_code_object_stream_memory::rocm_code_object_stream_memory
> }
>
> file_ptr
> -rocm_code_object_stream_memory::read (void *buf, file_ptr size,
> +rocm_code_object_stream_memory::read (bfd *, void *buf, file_ptr size,
> file_ptr offset)
> {
> if (size > m_objfile_image.size () - offset)
> @@ -457,8 +448,8 @@ rocm_code_object_stream_memory::read (void *buf, file_ptr size,
>
> } /* anonymous namespace */
>
> -static void *
> -rocm_bfd_iovec_open (bfd *abfd, void *inferior_void)
> +static gdb_bfd_iovec_base *
> +rocm_bfd_iovec_open (bfd *abfd, inferior *inferior)
> {
> gdb::string_view uri (bfd_get_filename (abfd));
> gdb::string_view protocol_delim = "://";
> @@ -522,7 +513,6 @@ rocm_bfd_iovec_open (bfd *abfd, void *inferior_void)
> {
> ULONGEST offset = 0;
> ULONGEST size = 0;
> - inferior *inferior = static_cast<struct inferior *> (inferior_void);
>
> auto try_strtoulst = [] (gdb::string_view v)
> {
> @@ -607,28 +597,6 @@ rocm_bfd_iovec_open (bfd *abfd, void *inferior_void)
> }
> }
>
> -static int
> -rocm_bfd_iovec_close (bfd *nbfd, void *data)
> -{
> - delete static_cast<rocm_code_object_stream *> (data);
> -
> - return 0;
> -}
> -
> -static file_ptr
> -rocm_bfd_iovec_pread (bfd *abfd, void *data, void *buf, file_ptr size,
> - file_ptr offset)
> -{
> - return static_cast<rocm_code_object_stream *> (data)->read (buf, size,
> - offset);
> -}
> -
> -static int
> -rocm_bfd_iovec_stat (bfd *abfd, void *data, struct stat *sb)
> -{
> - return static_cast<rocm_code_object_stream *> (data)->stat (sb);
> -}
> -
> static gdb_bfd_ref_ptr
> rocm_solib_bfd_open (const char *pathname)
> {
> @@ -636,10 +604,12 @@ rocm_solib_bfd_open (const char *pathname)
> if (strstr (pathname, "://") == nullptr)
> return svr4_so_ops.bfd_open (pathname);
>
> - gdb_bfd_ref_ptr abfd
> - = gdb_bfd_openr_iovec (pathname, "elf64-amdgcn", rocm_bfd_iovec_open,
> - current_inferior (), rocm_bfd_iovec_pread,
> - rocm_bfd_iovec_close, rocm_bfd_iovec_stat);
> + auto open = [] (bfd *nbfd) -> gdb_bfd_iovec_base *
> + {
> + return rocm_bfd_iovec_open (nbfd, current_inferior ());
> + };
> +
> + gdb_bfd_ref_ptr abfd = gdb_bfd_openr_iovec (pathname, "elf64-amdgcn", open);
>
> if (abfd == nullptr)
> error (_("Could not open `%s' as an executable file: %s"), pathname,
>
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe
2023-09-18 14:52 [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Tom Tromey
` (6 preceding siblings ...)
2023-09-18 14:52 ` [PATCH v2 7/7] Remove old gdb_bfd_openr_iovec Tom Tromey
@ 2023-09-28 10:02 ` Lancelot SIX
2023-09-28 16:45 ` Tom Tromey
7 siblings, 1 reply; 11+ messages in thread
From: Lancelot SIX @ 2023-09-28 10:02 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
Hi Tom,
I went through the series and tested it locally (native boald against
master and downstream ROCgdb).
Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Best,
Lancelot.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe
2023-09-28 10:02 ` [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Lancelot SIX
@ 2023-09-28 16:45 ` Tom Tromey
0 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2023-09-28 16:45 UTC (permalink / raw)
To: Lancelot SIX; +Cc: Tom Tromey, gdb-patches
>>>>> "Lancelot" == Lancelot SIX <lancelot.six@amd.com> writes:
Lancelot> Hi Tom,
Lancelot> I went through the series and tested it locally (native boald against
Lancelot> master and downstream ROCgdb).
Lancelot> Reviewed-By: Lancelot Six <lancelot.six@amd.com>
Thank you, I'm going to check this in.
Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-09-28 16:45 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-18 14:52 [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Tom Tromey
2023-09-18 14:52 ` [PATCH v2 1/7] Introduce type-safe variant of gdb_bfd_openr_iovec Tom Tromey
2023-09-18 14:52 ` [PATCH v2 2/7] Small constructor change to target_buffer Tom Tromey
2023-09-18 14:52 ` [PATCH v2 3/7] Convert mem_bfd_iovec to new type-safe gdb_bfd_openr_iovec Tom Tromey
2023-09-18 14:52 ` [PATCH v2 4/7] Convert target fileio " Tom Tromey
2023-09-18 14:52 ` [PATCH v2 5/7] Convert minidebug " Tom Tromey
2023-09-18 14:52 ` [PATCH v2 6/7] Convert solib-rocm " Tom Tromey
2023-09-28 9:57 ` Lancelot SIX
2023-09-18 14:52 ` [PATCH v2 7/7] Remove old gdb_bfd_openr_iovec Tom Tromey
2023-09-28 10:02 ` [PATCH v2 0/7] Rewrite gdb_bfd_openr_iovec to be type-safe Lancelot SIX
2023-09-28 16:45 ` Tom Tromey
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).