From: Lancelot SIX <lancelot.six@amd.com>
To: Tom Tromey <tromey@adacore.com>
Cc: <gdb-patches@sourceware.org>
Subject: Re: [PATCH v2 6/7] Convert solib-rocm to new type-safe gdb_bfd_openr_iovec
Date: Thu, 28 Sep 2023 09:57:54 +0000 [thread overview]
Message-ID: <20230928093346.vqrnmv73wyitf2ai@hpe6u-23> (raw)
In-Reply-To: <20230918-gdb-bfd-vec-v2-6-162c0e9a2bc9@adacore.com>
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
>
next prev parent reply other threads:[~2023-09-28 9:58 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230928093346.vqrnmv73wyitf2ai@hpe6u-23 \
--to=lancelot.six@amd.com \
--cc=gdb-patches@sourceware.org \
--cc=tromey@adacore.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).