libgloss/arm: report semihosted directories correctly

Message ID 20260814085043.2824706-1-torbjorn.svensson@foss.st.com
State New
Headers
Series libgloss/arm: report semihosted directories correctly |

Commit Message

Torbjorn SVENSSON Aug. 14, 2026, 8:09 a.m. UTC
  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 <torbjorn.svensson@foss.st.com>
---
 libgloss/arm/syscalls.c | 79 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 74 insertions(+), 5 deletions(-)
  

Comments

Jonathan Wakely Aug. 14, 2026, 9:25 a.m. UTC | #1
On Fri, 14 Aug 2026, 09:51 Torbjörn SVENSSON, <torbjorn.svensson@foss.st.com>
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 <torbjorn.svensson@foss.st.com>
> ---
>  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
>
>
  
Torbjorn SVENSSON Aug. 14, 2026, 11:01 a.m. UTC | #2
On 2026-08-14 11:25, Jonathan Wakely wrote:
> 
> 
> On Fri, 14 Aug 2026, 09:51 Torbjörn SVENSSON, <torbjorn.svensson@foss.st.com <mailto:torbjorn.svensson@foss.st.com>> 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 <https://gcc.gnu.org/pipermail/gcc-patches/2026-August/727185.html>
> 
>     Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com <mailto:torbjorn.svensson@foss.st.com>>
>     ---
>       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.

Indeed, this is true.
The reason why I did not bother is that it makes the logic a bit more complex and I think the corner case could be dismissed as working anyway.
Another thing that hit me with your feedback is what would happen when the PC side of the semihosting is running Windows. Is it okay to mix front- and back-slashes or do we need to normalize the path too?
If we normalize to one of them, then should it be front or back?
It's also going to be hard to know what is expected on the other side for the semihosting with this code running on an Arm target...

I do not consider this as a perfect solution, but I cannot come up with something better that would not require an update of the specification and the software on the PC side.

Kind regards,
Torbjörn

> 
> 
>     +    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
>
  
Andreas Schwab Aug. 18, 2026, 8:33 a.m. UTC | #3
On Aug 14 2026, Torbjörn SVENSSON wrote:

> +      st->st_mode |= S_IFCHR;

That doesn't make sense. S_IFMT is a multi-value bitfield, not a
colletion of single bits.
  
Torbjorn SVENSSON Aug. 18, 2026, 8:55 a.m. UTC | #4
On 2026-08-18 10:33, Andreas Schwab wrote:
> On Aug 14 2026, Torbjörn SVENSSON wrote:
> 
>> +      st->st_mode |= S_IFCHR;
> 
> That doesn't make sense. S_IFMT is a multi-value bitfield, not a
> colletion of single bits.
> 

I know that, but what do you want me to do instead?
The bitgroup is always cleared at this stage, but I can change it to

st->st_mode = (st->st_mode & ~S_IFMT) | S_IFCHR;

if that makes more sense (obviously the same for all the other assignments
in the file).

Let me know what you think.

Kind regards,
Torbjörn
  
Torbjorn SVENSSON Aug. 28, 2026, 1:11 p.m. UTC | #5
Gentle ping! :)

Kind regards,
Torbjörn

On 2026-08-14 10:09, Torbjörn SVENSSON 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 <torbjorn.svensson@foss.st.com>
> ---
>   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] != '/')
> +    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;
  

Patch

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] != '/')
+    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;