[PATCH] libgloss/arm: report semihosted directories correctly

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