Re: [PATCH] libgloss/arm: report semihosted directories correctly

On Fri, 14 Aug 2026, 09:51 Torbjörn SVENSSON, <[email protected]>
wrote:

> In GCC r17-2048-gcc195f7b11a406, support for resolving /etc/localtime
> symlinks
> was added.  As a consequense, when using semihosting for arm-none-eabi, the
> resolution fails due to that semihosting unconditionally consider all
> paths as
> character devices.  This commit tries to work around that by inferring as
> much
> stat details as possible from available APIs in the semihosting
> specification.
>
> With this change, I see that the failure is resolved, but there are a few
> tests that have been marked as xfail that now passes.  I will follow up
> with a patch for GCC as soon as this change is merged.
>
> These are the changes when testing with GCC r17-2833-g32657f29f91871.
>
> Without patch:
> PASS: 27_io/basic_filebuf/sgetn/char/1-in.cc  -std=gnu++20 (test for
> excess errors)
> XFAIL: 27_io/basic_filebuf/sgetn/char/1-in.cc  -std=gnu++20 execution test
> PASS: 27_io/basic_filebuf/sgetn/char/1-io.cc  -std=gnu++20 (test for
> excess errors)
> XFAIL: 27_io/basic_filebuf/sgetn/char/1-io.cc  -std=gnu++20 execution test
> PASS: 27_io/basic_filebuf/sgetn/char/2-in.cc  -std=gnu++20 (test for
> excess errors)
> XFAIL: 27_io/basic_filebuf/sgetn/char/2-in.cc  -std=gnu++20 execution test
> PASS: 27_io/basic_filebuf/sgetn/char/2-io.cc  -std=gnu++20 (test for
> excess errors)
> XFAIL: 27_io/basic_filebuf/sgetn/char/2-io.cc  -std=gnu++20 execution test
> PASS: 27_io/basic_filebuf/underflow/wchar_t/11603.cc  -std=gnu++20 (test
> for excess errors)
> FAIL: 27_io/basic_filebuf/underflow/wchar_t/11603.cc  -std=gnu++20
> execution test
> PASS: 27_io/basic_istream/readsome/char/6746-2.cc  -std=gnu++20 (test for
> excess errors)
> XFAIL: 27_io/basic_istream/readsome/char/6746-2.cc  -std=gnu++20 execution
> test
> PASS: 27_io/basic_istream/readsome/wchar_t/6746-2.cc  -std=gnu++20 (test
> for excess errors)
> XFAIL: 27_io/basic_istream/readsome/wchar_t/6746-2.cc  -std=gnu++20
> execution test
> PASS: std/time/tzdb/1.cc  -std=gnu++20 (test for excess errors)
> FAIL: std/time/tzdb/1.cc  -std=gnu++20 execution test
> PASS: std/time/zoned_time/custom.cc  -std=gnu++20 (test for excess errors)
> FAIL: std/time/zoned_time/custom.cc  -std=gnu++20 execution test
>
> With patch:
> PASS: 27_io/basic_filebuf/sgetn/char/1-in.cc  -std=gnu++20 (test for
> excess errors)
> XPASS: 27_io/basic_filebuf/sgetn/char/1-in.cc  -std=gnu++20 execution test
> PASS: 27_io/basic_filebuf/sgetn/char/1-io.cc  -std=gnu++20 (test for
> excess errors)
> XPASS: 27_io/basic_filebuf/sgetn/char/1-io.cc  -std=gnu++20 execution test
> PASS: 27_io/basic_filebuf/sgetn/char/2-in.cc  -std=gnu++20 (test for
> excess errors)
> XPASS: 27_io/basic_filebuf/sgetn/char/2-in.cc  -std=gnu++20 execution test
> PASS: 27_io/basic_filebuf/sgetn/char/2-io.cc  -std=gnu++20 (test for
> excess errors)
> XPASS: 27_io/basic_filebuf/sgetn/char/2-io.cc  -std=gnu++20 execution test
> PASS: 27_io/basic_filebuf/underflow/wchar_t/11603.cc  -std=gnu++20 (test
> for excess errors)
> PASS: 27_io/basic_filebuf/underflow/wchar_t/11603.cc  -std=gnu++20
> execution test
> PASS: 27_io/basic_istream/readsome/char/6746-2.cc  -std=gnu++20 (test for
> excess errors)
> XPASS: 27_io/basic_istream/readsome/char/6746-2.cc  -std=gnu++20 execution
> test
> PASS: 27_io/basic_istream/readsome/wchar_t/6746-2.cc  -std=gnu++20 (test
> for excess errors)
> XPASS: 27_io/basic_istream/readsome/wchar_t/6746-2.cc  -std=gnu++20
> execution test
> PASS: std/time/tzdb/1.cc  -std=gnu++20 (test for excess errors)
> PASS: std/time/tzdb/1.cc  -std=gnu++20 execution test
> PASS: std/time/zoned_time/custom.cc  -std=gnu++20 (test for excess errors)
> PASS: std/time/zoned_time/custom.cc  -std=gnu++20 execution test
>
>
> These is also an implementation choise to be made.
> Would it be prefered to use stack (like I do in this patch) or would it be
> better to allocated the temporary string in path_is_dir on the heap using
> malloc?  Regardless of solution, semihosting is expensive, so I don't think
> it really matters what solution is used. I choose stack based for the first
> version as it does not pull in any extra functions.
>
> Ok to push to master as-is or should I change to a heap based
> implementation?
>
> Kind regards,
> Torbjörn
>
> --
>
> Arm semihosting does not provide a real stat operation, so _stat has to
> infer file information from the operations that are available.
>
> The previous code always treated an opened path as a regular file, and
> _swistat then added S_IFCHR unconditionally.  That meant directories were
> not reported as directories, and the file type bits could end up
> describing more than one kind of file at once.
>
> Use _isatty to identify character devices, use SYS_FLEN for regular
> files, and probe "path/." so _stat can recognize directories.
>
> This gives callers such as std::filesystem::canonical enough information
> to handle semihosted paths correctly.
>
> Discovered, and reported, in
> https://gcc.gnu.org/pipermail/gcc-patches/2026-August/727185.html
>
> Signed-off-by: Torbjörn SVENSSON <[email protected]>
> ---
>  libgloss/arm/syscalls.c | 79 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 74 insertions(+), 5 deletions(-)
>
> diff --git a/libgloss/arm/syscalls.c b/libgloss/arm/syscalls.c
> index 710a741ee..162059fc0 100644
> --- a/libgloss/arm/syscalls.c
> +++ b/libgloss/arm/syscalls.c
> @@ -15,6 +15,7 @@
>  #include <reent.h>
>  #include <unistd.h>
>  #include <sys/wait.h>
> +#include <limits.h>
>  #include "swi.h"
>
>  /* Forward prototypes.  */
> @@ -46,6 +47,7 @@ void  initialise_monitor_handles (void);
>  static int     checkerror      (int);
>  static int     error           (int);
>  static int     get_errno       (void);
> +static int     path_is_dir     (const char *);
>
>  /* Semihosting utilities.  */
>  static void initialise_semihosting_exts (void);
> @@ -346,6 +348,46 @@ checkerror (int result)
>    return result;
>  }
>
> +/* Check if the given path is likely a directory. */
> +static int
> +path_is_dir (const char *path)
> +{
> +  size_t path_len;
> +  size_t dir_len;
> +  char dir_path[PATH_MAX + 3];
> +  int fd;
> +
> +  path_len = strlen (path);
> +  if (path_len == 0)
> +    return 0;
> +
> +  if (path_len + 3 > sizeof (dir_path))
> +    {
> +      errno = ENAMETOOLONG;
> +      return -1;
> +    }
> +
> +  /* Build a new string that ends with "/.". */
> +  memcpy (dir_path, path, path_len);
> +  dir_len = path_len;
> +  if (dir_path[dir_len - 1] != '/')
>

Just an observation...

This check means you will add /. to a path ending with /.. which is
redundant, but I think that's OK.

If strlen(dir_path) == sizeof(dir_path) and it ends with /.. then there's
no need to append anything, but this function will return ENAMETOOLONG.
Again, I think that's OK. It's an edge case and the path is already longer
than PATH_MAX.


+    dir_path[dir_len++] = '/';
> +  dir_path[dir_len++] = '.';
> +  dir_path[dir_len] = '\0';
> +
> +  /* Try to open the directory. */
> +  fd = _open (dir_path, O_RDONLY);
> +
> +  /* Error means that path either does not exist
> +   * or is not a directory. */
> +  if (fd == -1)
> +    return 0;
> +
> +  /* Not interested in the error.  */
> +  _close (fd);
> +  return 1;
> +}
> +
>  /* fh, is a valid internal file handle.
>     ptr, is a null terminated string.
>     len, is the length in bytes to read.
> @@ -738,10 +780,14 @@ _swistat (int fd, struct stat * st)
>        return -1;
>      }
>
> -  /* Always assume a character device,
> -     with 1024 byte blocks. */
> -  st->st_mode |= S_IFCHR;
>    st->st_blksize = 1024;
> +  if (_isatty (fd))
> +    {
> +      /* Assume character device. */
> +      st->st_mode |= S_IFCHR;
> +      return 0;
> +    }
> +
>  #ifdef ARM_RDI_MONITOR
>    res = checkerror (do_AngelSWI (AngelSWI_Reason_FLen, &pfd->handle));
>  #else
> @@ -752,7 +798,19 @@ _swistat (int fd, struct stat * st)
>    checkerror (res);
>  #endif
>    if (res == -1)
> -    return -1;
> +    {
> +      if (errno == EISDIR)
> +       {
> +         /* Path is a directory. */
> +         st->st_mode |= S_IFDIR;
> +         return 0;
> +       }
> +      return -1;
> +    }
> +
> +  /* Assume regular file. */
> +  st->st_mode |= S_IFREG;
> +
>    /* Return the file size. */
>    st->st_size = res;
>    return 0;
> @@ -770,12 +828,23 @@ _stat (const char *fname, struct stat *st)
>  {
>    int fd, res;
>    memset (st, 0, sizeof (* st));
> +  res = path_is_dir (fname);
> +  if (res == 1)
> +    {
> +      st->st_mode = S_IFDIR | S_IREAD | S_IEXEC;
> +      st->st_blksize = 1024;
> +      return 0;
> +    }
> +  else if (res == -1)
> +    return -1;
> +
>    /* The best we can do is try to open the file readonly.  If it exists,
>       then we can guess a few things about it.  */
>    if ((fd = _open (fname, O_RDONLY)) == -1)
>      return -1;
> -  st->st_mode |= S_IFREG | S_IREAD;
>    res = _swistat (fd, st);
> +  if (res == 0)
> +    st->st_mode |= S_IREAD;
>    /* Not interested in the error.  */
>    _close (fd);
>    return res;
> --
> 2.43.0
>
>