[PING]: [PATCH] libgloss/arm: report semihosted directories correctly
Torbjorn SVENSSON
torbjorn.svensson@foss.st.com
Fri Aug 28 13:11:07 GMT 2026
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;
More information about the Newlib
mailing list