diff mbox series

gdb/solib-rocm: add support for file URI on Windows

Message ID 20260520172256.629890-1-lancelot.six@amd.com
State New
Headers
Series gdb/solib-rocm: add support for file URI on Windows |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Lancelot SIX May 20, 2026, 5:22 p.m. UTC 
  From: Lancelot Six <lancelot.six@amd.com>

For processes using the ROCm runtime, GPU code objects are reported to
the debugger in the form of a URI (those are available to GDB using the
amd_dbgapi_process_code_object_list function and query the
AMD_DBGAPI_CODE_OBJECT_INFO_URI_NAME property).  Each URI can be of 2
forms:
  - "memory://$PID/mem#offset=$ADDR&size=$SIZE"
  - "file://$PATH#offset=$OFFSET&size=$SIZE"

On the Windows platform, only the "memory" URI form is used at the
moment, but future runtime changes might make it report code objects
using the "file" form.  When using the "file" form, when the runtime
reports an absolute path, the URI will look something like this:

    file:///C:/foo/bar/file.exe#offset=0x123&size=0x321

The decoding scheme currently implemented in
gdb/solib-rocm:rocm_bfd_iovec_open would extract the file path as
"/C:/foo/bar/file.exe", and will eventually hand this path to
solib_open.

Surprisingly enough, solib_open still manages to locate the file
properly.  This is due to the following of code which effectively drops
the leading "/" turning the path into a valid absolute path which can
eventually be opened.

    /* If the search in gdb_sysroot failed, and the path name is
       absolute at this point, make it relative.  (openp will try and open the
       file according to its absolute path otherwise, which is not what we want.)
       Affects subsequent searches for this solib.  */
    if (found_file < 0 && IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname))
      {
        /* First, get rid of any drive letters etc.  */
        while (!IS_TARGET_DIR_SEPARATOR (fskind, *in_pathname))
          in_pathname++;

        /* Next, get rid of all leading dir separators.  */
        while (IS_TARGET_DIR_SEPARATOR (fskind, *in_pathname))
          in_pathname++;
      }

This patch proposes to fix rocm_solib so we properly decode the file
path and give a valid path to solib_open to properly support this
scheme.

Note that this patch only looks for forward slashes "/" in the pattern
matching and not the traditional backslashes (as IS_TARGET_DIR_SEPARATOR
would) as URIs use forward slashes, not backslashes.

Current GDB does not really AMDGPU debugging on Windows (there are still
a couple of missing necessary pieces), but this patch can still be
applied upstream and will eventually be needed.  I have tested this
patch on top of the downstream ROCgdb windows branch[1].  I have also
tested this patch on Linux + gfx1031 on top of master to ensure this
causes no regression.

[1] https://github.com/ROCm/ROCgdb/tree/amd-temp-windows
---
 gdb/solib-rocm.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)


base-commit: edeef5fb9c2dac72899322c66a1132501729bede

Comments

Pedro Alves May 20, 2026, 9:35 p.m. UTC | #1 
On 2026-05-20 18:22, Lancelot SIX wrote:

> Current GDB does not really AMDGPU debugging on Windows (there are still

Some word missing, I guess "support".

> a couple of missing necessary pieces), but this patch can still be
> applied upstream and will eventually be needed.  I have tested this
> patch on top of the downstream ROCgdb windows branch[1].  I have also
> tested this patch on Linux + gfx1031 on top of master to ensure this
> causes no regression.
> 
> [1] https://github.com/ROCm/ROCgdb/tree/amd-temp-windows
> ---
>  gdb/solib-rocm.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/gdb/solib-rocm.c b/gdb/solib-rocm.c
> index d9ae4294c98..d8d36d2f3c0 100644
> --- a/gdb/solib-rocm.c
> +++ b/gdb/solib-rocm.c
> @@ -30,6 +30,7 @@
>  #include "solib.h"
>  #include "solib-svr4.h"
>  #include "symfile.h"
> +#include "filesystem.h"
>  
>  namespace {
>  
> @@ -586,6 +587,21 @@ rocm_bfd_iovec_open (bfd *abfd, inferior *inferior)
>  
>        if (protocol == "file")
>  	{
> +	  /* Windows absolute file path can be encoded with a leading "/" in
> +	     the URI: "file:///C:/Users/foo/bar".  This scheme is not strictly
> +	     standard but widely used (see RFC 8089 Appendix E.2).  


I think this comment has it backwards.  "file:///C:/Users/foo/bar" is actually the standard
form per RFC 8089.  It's the regular "file://<auth>/<path>" syntax where the authority is
empty and the path-absolute happens to begin with a drive letter, so you get a
leading / before "C:".

The non-standard short form discussed by RFC 8089 Appendix E.2 is:

 file:c:/path/to/file

... with no "//" and no leading slash before the drive.

That same appendix states:

 'URIs of the form "file:///c:/path/to/file" are already supported by the "path-absolute" rule.'

These are standard.

The code is still correct, though.  Once you decode the standard URI you do get "/C:/Users/foo/bar",
and you do need to strip the leading / on DOS-based filesystems to turn it back into a real
Windows path.  That's just an artifact of the URI grammar requiring a path-absolute (must
start with /).  See RFC 8089 Appendix D.2.  It's not because the URI form is non-standard.

I'd suggest rewording the comment to something like:

  /* A Windows absolute file path is encoded in a file: URI with a leading
     "/" before the drive letter: "file:///C:/Users/foo/bar".  See grammar in
     RFC 8089 Section 2, the path-absolute production requires the leading "/".
     'path-absolute' is defined by RFC 3986 Section 3.3.
     After decoding, decoded_path would be "/C:/Users/foo/bar", which is not a
     valid Windows path.  Drop the leading "/" as a normalization step.  */

Otherwise LGTM.

Approved-By: Pedro Alves <pedro@palves.net>

Thanks,
Pedro Alves
Lancelot SIX May 27, 2026, 10:33 a.m. UTC | #2 
> I'd suggest rewording the comment to something like:
> 
>    /* A Windows absolute file path is encoded in a file: URI with a leading
>       "/" before the drive letter: "file:///C:/Users/foo/bar".  See grammar in
>       RFC 8089 Section 2, the path-absolute production requires the leading "/".
>       'path-absolute' is defined by RFC 3986 Section 3.3.
>       After decoding, decoded_path would be "/C:/Users/foo/bar", which is not a
>       valid Windows path.  Drop the leading "/" as a normalization step.  */
> 
> Otherwise LGTM.
> 
> Approved-By: Pedro Alves <pedro@palves.net>
> 
> Thanks,
> Pedro Alves
> 

Thanks,

I updated the comment and pushed as 
d623dadc2c4ecd509359015ae5a1ac856ba89b05.

Best,
Lancelot.
diff mbox series

Patch

diff --git a/gdb/solib-rocm.c b/gdb/solib-rocm.c
index d9ae4294c98..d8d36d2f3c0 100644
--- a/gdb/solib-rocm.c
+++ b/gdb/solib-rocm.c
@@ -30,6 +30,7 @@ 
 #include "solib.h"
 #include "solib-svr4.h"
 #include "symfile.h"
+#include "filesystem.h"
 
 namespace {
 
@@ -586,6 +587,21 @@  rocm_bfd_iovec_open (bfd *abfd, inferior *inferior)
 
       if (protocol == "file")
 	{
+	  /* Windows absolute file path can be encoded with a leading "/" in
+	     the URI: "file:///C:/Users/foo/bar".  This scheme is not strictly
+	     standard but widely used (see RFC 8089 Appendix E.2).  At this
+	     stage, decoded_path would be "/C:/Users/foo/bar", which is not a
+	     valid windows path.  Drop the leading "/" as a normalisation step
+	     to get a valid Windows path.  */
+	  if ((effective_target_file_system_kind ()
+	       == file_system_kind_dos_based)
+	      && decoded_path.length () >= 4
+	      && decoded_path[0] == '/'
+	      && c_isalpha (decoded_path[1])
+	      && decoded_path[2] == ':'
+	      && decoded_path[3] == '/')
+	    decoded_path.erase (0, 1);
+
 	  auto info = get_solib_info (inferior);
 	  fileio_error target_errno;
 	  target_fd fd = info->fd_cache.open (decoded_path, &target_errno);