[PING^2]: [PATCH] libgloss/arm: report semihosted directories correctly
Torbjorn SVENSSON
torbjorn.svensson@foss.st.com
Mon Sep 7 15:37:05 GMT 2026
Another gentle ping! :)
Kind regards,
Torbjörn
On 2026-08-28 15:11, Torbjorn SVENSSON wrote:
> 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