arch/sim: avoid host-call being interrupted before getting errno

Sim use coroutine base on one thread in host to do switch context. but if we allow switch context with in one API (host-API and errno get), maybe the switch context from coroutine cause re-enter host-API call. Make the errno get behavior not work as expected.

Signed-off-by: guanyi3 <guanyi3@xiaomi.com>

arch/sim/src/sim/posix/sim_hostfs.c

@@ -37,24 +37,12 @@
 #include <errno.h>
 
 #include "hostfs.h"
+#include "sim_internal.h"
 
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
-/****************************************************************************
- * Name: host_errno_convert
- ****************************************************************************/
-
-static int host_errno_convert(int errcode)
-{
-  /* Provide a common interface, which should have different conversions
-   * on different platforms.
-   */
-
-  return errcode;
-}
-
 /****************************************************************************
  * Name: host_stat_convert
  ****************************************************************************/
@@ -206,13 +194,7 @@ int host_open(const char *pathname, int flags, int mode)
       mapflags |= O_DIRECTORY;
     }
 
-  int ret = open(pathname, mapflags, mode);
-  if (ret == -1)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(open, pathname, mapflags, mode);
 }
 
 /****************************************************************************
@@ -223,13 +205,7 @@ int host_close(int fd)
 {
   /* Just call the close routine */
 
-  int ret = close(fd);
-  if (ret == -1)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(close, fd);
 }
 
 /****************************************************************************
@@ -240,13 +216,7 @@ nuttx_ssize_t host_read(int fd, void *buf, nuttx_size_t count)
 {
   /* Just call the read routine */
 
-  nuttx_ssize_t ret = read(fd, buf, count);
-  if (ret == -1)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(read, fd, buf, count);
 }
 
 /****************************************************************************
@@ -257,13 +227,7 @@ nuttx_ssize_t host_write(int fd, const void *buf, nuttx_size_t count)
 {
   /* Just call the write routine */
 
-  nuttx_ssize_t ret = write(fd, buf, count);
-  if (ret == -1)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(write, fd, buf, count);
 }
 
 /****************************************************************************
@@ -275,13 +239,7 @@ nuttx_off_t host_lseek(int fd, nuttx_off_t pos, nuttx_off_t offset,
 {
   /* Just call the lseek routine */
 
-  nuttx_off_t ret = lseek(fd, offset, whence);
-  if (ret == (nuttx_off_t)-1)
-    {
-      ret =  host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(lseek, fd, offset, whence);
 }
 
 /****************************************************************************
@@ -292,13 +250,7 @@ int host_ioctl(int fd, int request, unsigned long arg)
 {
   /* Just call the ioctl routine */
 
-  int ret = ioctl(fd, request, arg);
-  if (ret < 0)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(ioctl, fd, request, arg);
 }
 
 /****************************************************************************
@@ -318,13 +270,7 @@ void host_sync(int fd)
 
 int host_dup(int fd)
 {
-  int ret = dup(fd);
-  if (ret < 0)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(dup, fd);
 }
 
 /****************************************************************************
@@ -338,11 +284,7 @@ int host_fstat(int fd, struct nuttx_stat_s *buf)
 
   /* Call the host's stat routine */
 
-  ret = fstat(fd, &hostbuf);
-  if (ret < 0)
-    {
-      ret = host_errno_convert(-errno);
-    }
+  ret = host_uninterruptible_errno(fstat, fd, &hostbuf);
 
   /* Map the return values */
 
@@ -361,19 +303,19 @@ int host_fchstat(int fd, const struct nuttx_stat_s *buf, int flags)
 
   if (flags & NUTTX_CH_STAT_MODE)
     {
-      ret = fchmod(fd, buf->st_mode);
+      ret = host_uninterruptible_errno(fchmod, fd, buf->st_mode);
       if (ret < 0)
         {
-          return host_errno_convert(-errno);
+          return ret;
         }
     }
 
   if (flags & (NUTTX_CH_STAT_UID | NUTTX_CH_STAT_GID))
     {
-      ret = fchown(fd, buf->st_uid, buf->st_gid);
+      ret = host_uninterruptible_errno(fchown, fd, buf->st_uid, buf->st_gid);
       if (ret < 0)
         {
-          return host_errno_convert(-errno);
+          return ret;
         }
     }
 
@@ -401,10 +343,10 @@ int host_fchstat(int fd, const struct nuttx_stat_s *buf, int flags)
           times[1].tv_nsec = UTIME_OMIT;
         }
 
-      ret = futimens(fd, times);
+      ret = host_uninterruptible_errno(futimens, fd, times);
       if (ret < 0)
         {
-          return host_errno_convert(-errno);
+          return ret;
         }
     }
 
@@ -417,13 +359,7 @@ int host_fchstat(int fd, const struct nuttx_stat_s *buf, int flags)
 
 int host_ftruncate(int fd, nuttx_off_t length)
 {
-  int ret = ftruncate(fd, length);
-  if (ret < 0)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(ftruncate, fd, length);
 }
 
 /****************************************************************************
@@ -513,13 +449,7 @@ void host_rewinddir(void *dirp)
 
 int host_closedir(void *dirp)
 {
-  int ret = closedir(dirp);
-  if (ret < 0)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(closedir, dirp);
 }
 
 /****************************************************************************
@@ -533,11 +463,7 @@ int host_statfs(const char *path, struct nuttx_statfs_s *buf)
 
   /* Call the host's statfs routine */
 
-  ret = statvfs(path, &hostbuf);
-  if (ret < 0)
-    {
-      ret = host_errno_convert(-errno);
-    }
+  ret = host_uninterruptible_errno(statvfs, path, &hostbuf);
 
   /* Map the struct statfs value */
 
@@ -559,13 +485,7 @@ int host_statfs(const char *path, struct nuttx_statfs_s *buf)
 
 int host_unlink(const char *pathname)
 {
-  int ret = unlink(pathname);
-  if (ret < 0)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(unlink, pathname);
 }
 
 /****************************************************************************
@@ -576,13 +496,7 @@ int host_mkdir(const char *pathname, int mode)
 {
   /* Just call the host's mkdir routine */
 
-  int ret = mkdir(pathname, mode);
-  if (ret < 0)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(mkdir, pathname, mode);
 }
 
 /****************************************************************************
@@ -591,13 +505,7 @@ int host_mkdir(const char *pathname, int mode)
 
 int host_rmdir(const char *pathname)
 {
-  int ret = rmdir(pathname);
-  if (ret < 0)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(rmdir, pathname);
 }
 
 /****************************************************************************
@@ -606,13 +514,7 @@ int host_rmdir(const char *pathname)
 
 int host_rename(const char *oldpath, const char *newpath)
 {
-  int ret = rename(oldpath, newpath);
-  if (ret < 0)
-    {
-      ret = host_errno_convert(-errno);
-    }
-
-  return ret;
+  return host_uninterruptible_errno(rename, oldpath, newpath);
 }
 
 /****************************************************************************
@@ -626,11 +528,7 @@ int host_stat(const char *path, struct nuttx_stat_s *buf)
 
   /* Call the host's stat routine */
 
-  ret = stat(path, &hostbuf);
-  if (ret < 0)
-    {
-      ret = host_errno_convert(-errno);
-    }
+  ret = host_uninterruptible_errno(stat, path, &hostbuf);
 
   /* Map the return values */
 
@@ -649,19 +547,20 @@ int host_chstat(const char *path, const struct nuttx_stat_s *buf, int flags)
 
   if (flags & NUTTX_CH_STAT_MODE)
     {
-      ret = chmod(path, buf->st_mode);
+      ret = host_uninterruptible_errno(chmod, path, buf->st_mode);
       if (ret < 0)
         {
-          return host_errno_convert(-errno);
+          return ret;
         }
     }
 
   if (flags & (NUTTX_CH_STAT_UID | NUTTX_CH_STAT_GID))
     {
-      ret = chown(path, buf->st_uid, buf->st_gid);
+      ret = host_uninterruptible_errno(chown, path,
+                                       buf->st_uid, buf->st_gid);
       if (ret < 0)
         {
-          return host_errno_convert(-errno);
+          return ret;
         }
     }
 
@@ -689,10 +588,10 @@ int host_chstat(const char *path, const struct nuttx_stat_s *buf, int flags)
           times[1].tv_nsec = UTIME_OMIT;
         }
 
-      ret = utimensat(AT_FDCWD, path, times, 0);
+      ret = host_uninterruptible_errno(utimensat, AT_FDCWD, path, times, 0);
       if (ret < 0)
         {
-          return host_errno_convert(-errno);
+          return ret;
         }
     }
 

arch/sim/src/sim/posix/sim_hostmisc.c

@@ -128,32 +128,38 @@ int host_system(char *buf, size_t len, const char *fmt, ...)
   int ret;
   char cmd[512];
   va_list vars;
+  uint64_t flags;
 
   va_start(vars, fmt);
-  ret = vsnprintf(cmd, sizeof(cmd), fmt, vars);
+  ret = host_uninterruptible_errno(vsnprintf, cmd, sizeof(cmd), fmt, vars);
   va_end(vars);
   if (ret <= 0 || ret > sizeof(cmd))
     {
-      return ret < 0 ? -errno : -EINVAL;
+      return ret < 0 ? ret : -EINVAL;
     }
 
   if (buf == NULL)
     {
-      ret = host_uninterruptible(system, cmd);
+      ret = host_uninterruptible_errno(system, cmd);
     }
   else
     {
+      flags = up_irq_save();
       fp = host_uninterruptible(popen, cmd, "r");
       if (fp == NULL)
         {
-          return -errno;
+          ret = -errno;
+          up_irq_restore(flags);
+          return ret;
         }
 
-      ret = host_uninterruptible(fread, buf, 1, len, fp);
+      up_irq_restore(flags);
+
+      ret = host_uninterruptible_errno(fread, buf, 1, len, fp);
       host_uninterruptible(pclose, fp);
     }
 
-  return ret < 0 ? -errno : ret;
+  return ret;
 }
 
 /****************************************************************************
@@ -223,6 +229,6 @@ int host_waitpid(pid_t pid)
 {
   int status;
 
-  pid = host_uninterruptible(waitpid, pid, &status, 0);
-  return pid < 0 ? -errno : status;
+  pid = host_uninterruptible_errno(waitpid, pid, &status, 0);
+  return pid < 0 ? pid : status;
 }

arch/sim/src/sim/posix/sim_hostuart.c

@@ -96,15 +96,15 @@ void host_uart_start(void)
 {
   /* Get the current stdin terminal mode */
 
-  tcgetattr(0, &g_cooked);
+  host_uninterruptible_no_return(tcgetattr, 0, &g_cooked);
 
   /* Put stdin into raw mode */
 
-  setrawmode(0);
+  host_uninterruptible_no_return(setrawmode, 0);
 
   /* Restore the original terminal mode before exit */
 
-  atexit(restoremode);
+  host_uninterruptible_no_return(atexit, restoremode);
 }
 
 /****************************************************************************
@@ -115,16 +115,12 @@ int host_uart_open(const char *pathname)
 {
   int fd;
 
-  fd = open(pathname, O_RDWR | O_NONBLOCK);
+  fd = host_uninterruptible_errno(open, pathname, O_RDWR | O_NONBLOCK);
   if (fd >= 0)
     {
       /* keep raw mode */
 
-      setrawmode(fd);
-    }
-  else
-    {
-      fd = -errno;
+      host_uninterruptible_no_return(setrawmode, fd);
     }
 
   return fd;
@@ -136,7 +132,7 @@ int host_uart_open(const char *pathname)
 
 void host_uart_close(int fd)
 {
-  close(fd);
+  host_uninterruptible(close, fd);
 }
 
 /****************************************************************************
@@ -149,11 +145,11 @@ int host_uart_puts(int fd, const char *buf, size_t size)
 
   do
     {
-      ret = write(fd, buf, size);
+      ret = host_uninterruptible_errno(write, fd, buf, size);
     }
-  while (ret < 0 && errno == EINTR);
+  while (ret < 0 && ret == -EINTR);
 
-  return ret < 0 ? -errno : ret;
+  return ret;
 }
 
 /****************************************************************************
@@ -166,11 +162,11 @@ int host_uart_gets(int fd, char *buf, size_t size)
 
   do
     {
-      ret = read(fd, buf, size);
+      ret = host_uninterruptible_errno(read, fd, buf, size);
     }
-  while (ret < 0 && errno == EINTR);
+  while (ret < 0 && ret == -EINTR);
 
-  return ret < 0 ? -errno : ret;
+  return ret;
 }
 
 /****************************************************************************
@@ -182,12 +178,8 @@ int host_uart_getcflag(int fd, unsigned int *cflag)
   struct termios t;
   int ret;
 
-  ret = tcgetattr(fd, &t);
-  if (ret < 0)
-    {
-      ret = -errno;
-    }
-  else
+  ret = host_uninterruptible_errno(tcgetattr, fd, &t);
+  if (ret >= 0)
     {
       *cflag = t.c_cflag;
     }
@@ -204,16 +196,11 @@ int host_uart_setcflag(int fd, unsigned int cflag)
   struct termios t;
   int ret;
 
-  ret = tcgetattr(fd, &t);
+  ret = host_uninterruptible_errno(tcgetattr, fd, &t);
   if (!ret)
     {
       t.c_cflag = cflag;
-      ret = tcsetattr(fd, TCSANOW, &t);
-    }
-
-  if (ret < 0)
-    {
-      ret = -errno;
+      ret = host_uninterruptible_errno(tcsetattr, fd, TCSANOW, &t);
     }
 
   return ret;
@@ -229,7 +216,7 @@ bool host_uart_checkin(int fd)
 
   pfd.fd     = fd;
   pfd.events = POLLIN;
-  return poll(&pfd, 1, 0) == 1;
+  return host_uninterruptible(poll, &pfd, 1, 0) == 1;
 }
 
 /****************************************************************************
@@ -242,7 +229,7 @@ bool host_uart_checkout(int fd)
 
   pfd.fd     = fd;
   pfd.events = POLLOUT;
-  return poll(&pfd, 1, 0) == 1;
+  return host_uninterruptible(poll, &pfd, 1, 0) == 1;
 }
 
 /****************************************************************************

arch/sim/src/sim/sim_internal.h

@@ -107,6 +107,12 @@
 #define sim_savestate(regs) sim_copyfullstate(regs, up_current_regs())
 #define sim_restorestate(regs) up_set_current_regs(regs)
 
+/* Provide a common interface, which should have different conversions
+ * on different platforms.
+ */
+
+#define host_errno_convert(errcode) (errcode)
+
 #define sim_saveusercontext(saveregs, ret)                      \
     do                                                          \
       {                                                         \
@@ -134,8 +140,6 @@
 
 #define host_uninterruptible(func, ...)                         \
     ({                                                          \
-        extern uint64_t up_irq_save(void);                      \
-        extern void up_irq_restore(uint64_t flags);             \
         uint64_t flags_ = up_irq_save();                        \
         typeof(func(__VA_ARGS__)) ret_ = func(__VA_ARGS__);     \
         up_irq_restore(flags_);                                 \
@@ -145,14 +149,24 @@
 #define host_uninterruptible_no_return(func, ...)               \
     do                                                          \
       {                                                         \
-        extern uint64_t up_irq_save(void);                      \
-        extern void up_irq_restore(uint64_t flags);             \
         uint64_t flags_ = up_irq_save();                        \
         func(__VA_ARGS__);                                      \
         up_irq_restore(flags_);                                 \
       }                                                         \
     while (0)
 
+#define host_uninterruptible_errno(func, ...)                   \
+    ({                                                          \
+        uint64_t flags_ = up_irq_save();                        \
+        typeof(func(__VA_ARGS__)) ret_ = func(__VA_ARGS__);     \
+        if (ret_ < 0)                                           \
+          {                                                     \
+            ret_ = host_errno_convert(-errno);                  \
+          }                                                     \
+        up_irq_restore(flags_);                                 \
+        ret_;                                                   \
+    })
+
 /* File System Definitions **************************************************/
 
 /* These definitions characterize the compressed filesystem image */
@@ -204,6 +218,9 @@ extern char **g_argv;
  * Public Function Prototypes
  ****************************************************************************/
 
+uint64_t up_irq_save(void);
+void up_irq_restore(uint64_t flags);
+
 /* Context switching */
 
 void sim_copyfullstate(xcpt_reg_t *dest, xcpt_reg_t *src);