[v3] Allow using special files with File I/O functions

Message ID 0102016447dcf9e9-3989bcd9-1272-4a05-93c5-77823c7a0921-000000@eu-west-1.amazonses.com
State New, archived
Headers

Commit Message

Julio Guerra June 28, 2018, 7:27 p.m. UTC
  - Remove the restriction to regular files only and add support for special file
types in the File IO stat structure.
- Define a few more macro definitions of file types such as FIFOs, etc.

The major goal is being able to write advanced embedded testing functions, like:
- using a FIFO between the embedded program and the host, instead of being
  restricted only to the GDB console.
- mocking features based on host's by opening some /dev special files.

2018-06-28  Julio Guerra  <julio@farjump.io>

        * remote-fileio.c (remote_fileio_func_open, remote_fileio_func_stat):
        Allow using File I/O functions open(), stat() and fstat() on special
        files.
        * ../include/gdb/fileio.h: Add macro definitions for special files,
        both for fst_dev and fst_mode fields of struct fst_stat.
        * common/fileio.c (fileio_to_host_mode, fileio_mode_pack): Add new
        special file types in fst_mode's definition.
        (host_to_fileio_stat): Define fst_dev using the new macro definitions
        and according to the file's type.

Signed-off-by: Julio Guerra <julio@farjump.io>
---
 gdb/ChangeLog        | 12 ++++++++
 gdb/common/fileio.c  | 66 +++++++++++++++++++++++++++++++++++---------
 gdb/remote-fileio.c  | 28 ++-----------------
 include/gdb/fileio.h | 19 +++++++++++--
 4 files changed, 83 insertions(+), 42 deletions(-)

-- 
2.18.0
  

Comments

Julio Guerra June 28, 2018, 7:30 p.m. UTC | #1
You can find below the logs of the test I performed using our GDB server with a program embedded in a Raspberry Pi:

 

(gdb) set $st = (struct stat*) malloc(sizeof (struct stat))

(gdb) call stat("/dev/stdout", $st)
$1 = 0x0
(gdb) p *$st
$2 = {
  st_dev = 0x2, 
  st_ino = 0x4, 
  st_mode = 0x2190, 
  st_nlink = 0x1, 
  st_uid = 0x3e8, 
  st_gid = 0x5, 
  st_rdev = 0x8801, 
  st_size = 0x0, 
  st_atime = 0x5b3507e0, 
  st_spare1 = 0x0, 
  st_mtime = 0x5b34fb47, 
  st_spare2 = 0x0, 
  st_ctime = 0x0, 
  st_spare3 = 0x0, 
  st_blksize = 0x0, 
  st_blocks = 0x5b3507e0, 
  st_spare4 = {0x0, 0x0}
}
(gdb) p /o $st->st_mode            
$8 = 020620

(gdb) call stat("myfifo", $st)     
$3 = 0x0
(gdb) p *$st
$4 = {
  st_dev = 0x2, 
  st_ino = 0x4f7c, 
  st_mode = 0x11a4, 
  st_nlink = 0x1, 
  st_uid = 0x3e8, 
  st_gid = 0x3e8, 
  st_rdev = 0x0, 
  st_size = 0x0, 
  st_atime = 0x5b3504ed, 
  st_spare1 = 0x0, 
  st_mtime = 0x5b3504ed, 
  st_spare2 = 0x0, 
  st_ctime = 0x0, 
  st_spare3 = 0x0, 
  st_blksize = 0x0, 
  st_blocks = 0x5b3504ed, 
  st_spare4 = {0x0, 0x0}
}
(gdb) p /o $st->st_mode       
$6 = 010644

(gdb) call stat("/run/dbus/system_bus_socket", $st)  
$9 = 0x0
(gdb) p *$st                                       
$10 = {
  st_dev = 0x2, 
  st_ino = 0x36c7, 
  st_mode = 0xc1b6, 
  st_nlink = 0x1, 
  st_uid = 0x0, 
  st_gid = 0x0, 
  st_rdev = 0x0, 
  st_size = 0x0, 
  st_atime = 0x5b33e995, 
  st_spare1 = 0x0, 
  st_mtime = 0x5b33e995, 
  st_spare2 = 0x0, 
  st_ctime = 0x0, 
  st_spare3 = 0x0, 
  st_blksize = 0x0, 
  st_blocks = 0x5b33e995, 
  st_spare4 = {0x0, 0x0}
}
(gdb) p /o $st->st_mode                            
$11 = 0140666

(gdb) call stat("/tmp", $st)                       
$12 = 0x0
(gdb) p *$st                
$13 = {
  st_dev = 0x2, 
  st_ino = 0x7a5, 
  st_mode = 0x41ff, 
  st_nlink = 0x1, 
  st_uid = 0x0, 
  st_gid = 0x0, 
  st_rdev = 0x0, 
  st_size = 0x0, 
  st_atime = 0x5b350829, 
  st_spare1 = 0x0, 
  st_mtime = 0x5b350829, 
  st_spare2 = 0x0, 
  st_ctime = 0x0, 
  st_spare3 = 0x0, 
  st_blksize = 0x0, 
  st_blocks = 0x5a5a9802, 
  st_spare4 = {0x0, 0x0}
}
(gdb) p /o $st->st_mode     
$14 = 040777

(gdb) call stat("Makefile", $st)
$15 = 0x0
(gdb) p *$st                    
$16 = {
  st_dev = 0x0, 
  st_ino = 0xc6b9, 
  st_mode = 0x81a4, 
  st_nlink = 0x1, 
  st_uid = 0x3e8, 
  st_gid = 0x3e8, 
  st_rdev = 0x0, 
  st_size = 0x0, 
  st_atime = 0x5ae35b23, 
  st_spare1 = 0x0, 
  st_mtime = 0x5b311f46, 
  st_spare2 = 0x0, 
  st_ctime = 0x0, 
  st_spare3 = 0x0, 
  st_blksize = 0x0, 
  st_blocks = 0x5b3500d5, 
  st_spare4 = {0x0, 0x0}
}
(gdb) p /o $st->st_mode         
$17 = 0100644

(gdb) call stat("i dont exist", $st)
$18 = 0xffffffff

(gdb) call fstat(0, $st)
$1 = 0x0
(gdb) p *$st
$3 = {
  st_dev = 0x1, 
  st_ino = 0x0, 
  st_mode = 0x2100, 
  st_nlink = 0x1, 
  st_uid = 0x3e8, 
  st_gid = 0x3e8, 
  st_rdev = 0x0, 
  st_size = 0x0, 
  st_atime = 0x5b35144e, 
  st_spare1 = 0x0, 
  st_mtime = 0x5b35144e, 
  st_spare2 = 0x0, 
  st_ctime = 0x0, 
  st_spare3 = 0x0, 
  st_blksize = 0x0, 
  st_blocks = 0x5b35144e, 
  st_spare4 = {0x0, 0x0}
}

(gdb) call fstat(1, $st)
$4 = 0x0
(gdb) p *$st                                               
$5 = {
  st_dev = 0x1, 
  st_ino = 0x0, 
  st_mode = 0x2080, 
  st_nlink = 0x1, 
  st_uid = 0x3e8, 
  st_gid = 0x3e8, 
  st_rdev = 0x0, 
  st_size = 0x0, 
  st_atime = 0x5b351460, 
  st_spare1 = 0x0, 
  st_mtime = 0x5b351460, 
  st_spare2 = 0x0, 
  st_ctime = 0x0, 
  st_spare3 = 0x0, 
  st_blksize = 0x0, 
  st_blocks = 0x5b351460, 
  st_spare4 = {0x0, 0x0}
}

(gdb) call fstat(2, $st)
$6 = 0x0
(gdb) p *$st            
$7 = {
  st_dev = 0x1, 
  st_ino = 0x0, 
  st_mode = 0x2080, 
  st_nlink = 0x1, 
  st_uid = 0x3e8, 
  st_gid = 0x3e8, 
  st_rdev = 0x0, 
  st_size = 0x0, 
  st_atime = 0x5b35146b, 
  st_spare1 = 0x0, 
  st_mtime = 0x5b35146b, 
  st_spare2 = 0x0, 
  st_ctime = 0x0, 
  st_spare3 = 0x0, 
  st_blksize = 0x0, 
  st_blocks = 0x5b35146b, 
  st_spare4 = {0x0, 0x0}
}

(gdb) call fstat(3, $st)
$8 = 0xffffffff

(gdb) set $fd = open("myfifo", 0)
(gdb) call fstat($fd, $st)       
$10 = 0x0
(gdb) p *$st
$12 = {
  st_dev = 0x2, 
  st_ino = 0x4f7c, 
  st_mode = 0x11a4, 
  st_nlink = 0x1, 
  st_uid = 0x3e8, 
  st_gid = 0x3e8, 
  st_rdev = 0x0, 
  st_size = 0x0, 
  st_atime = 0x5b351511, 
  st_spare1 = 0x0, 
  st_mtime = 0x5b351511, 
  st_spare2 = 0x0, 
  st_ctime = 0x0, 
  st_spare3 = 0x0, 
  st_blksize = 0x0, 
  st_blocks = 0x5b3504ed, 
  st_spare4 = {0x0, 0x0}
}
(gdb) p /o $st->st_mode
$13 = 010644
  
Pedro Alves June 29, 2018, 1:42 p.m. UTC | #2
On 06/28/2018 08:27 PM, Julio Guerra wrote:
> - Remove the restriction to regular files only and add support for special file
> types in the File IO stat structure.
> - Define a few more macro definitions of file types such as FIFOs, etc.
> 
> The major goal is being able to write advanced embedded testing functions, like:
> - using a FIFO between the embedded program and the host, instead of being
>   restricted only to the GDB console.
> - mocking features based on host's by opening some /dev special files.
> 

This needs a GDB manual change, and a NEWS entry.

> 2018-06-28  Julio Guerra  <julio@farjump.io>
> 
>         * remote-fileio.c (remote_fileio_func_open, remote_fileio_func_stat):
>         Allow using File I/O functions open(), stat() and fstat() on special
>         files.
>         * ../include/gdb/fileio.h: Add macro definitions for special files,
>         both for fst_dev and fst_mode fields of struct fst_stat.
>         * common/fileio.c (fileio_to_host_mode, fileio_mode_pack): Add new
>         special file types in fst_mode's definition.
>         (host_to_fileio_stat): Define fst_dev using the new macro definitions
>         and according to the file's type.

Note that include/ has its own ChangeLog file.

>  2018-06-27  Simon Marchi  <simon.marchi@ericsson.com>
>  
>  	* gdb-gdb.py.in (StructMainTypePrettyPrinter) <to_string>: Don't
> diff --git a/gdb/common/fileio.c b/gdb/common/fileio.c
> index 912a7ede3c..9ee78e227c 100644
> --- a/gdb/common/fileio.c
> +++ b/gdb/common/fileio.c
> @@ -119,12 +119,31 @@ fileio_to_host_mode (int fileio_mode, mode_t *mode_p)
>    if (fileio_mode & ~FILEIO_S_SUPPORTED)
>      return -1;
>  
> -  if (fileio_mode & FILEIO_S_IFREG)
> -    mode |= S_IFREG;
> -  if (fileio_mode & FILEIO_S_IFDIR)
> -    mode |= S_IFDIR;
> -  if (fileio_mode & FILEIO_S_IFCHR)
> -    mode |= S_IFCHR;
> +  switch (fileio_mode & FILEIO_S_IFMT)
> +    {
> +    case FILEIO_S_IFSOCK:
> +      *mode_p |= S_IFSOCK;
> +      break;
> +    case FILEIO_S_IFLNK:
> +      mode |= S_IFLNK;
> +      break;
> +    case FILEIO_S_IFREG:
> +      mode |= S_IFREG;
> +      break;
> +    case FILEIO_S_IFBLK:
> +      mode |= S_IFBLK;
> +      break;
> +    case FILEIO_S_IFDIR:
> +      mode |= S_IFDIR;
> +      break;
> +    case FILEIO_S_IFCHR:
> +      mode |= S_IFCHR;
> +      break;
> +    case FILEIO_S_IFIFO:
> +      mode |= S_IFIFO;
> +      break;
> +    }
> +
>    if (fileio_mode & FILEIO_S_IRUSR)
>      mode |= S_IRUSR;
>    if (fileio_mode & FILEIO_S_IWUSR)
> @@ -165,12 +184,31 @@ fileio_mode_pack (mode_t mode)
>  {
>    mode_t tmode = 0;
>  
> -  if (S_ISREG (mode))
> -    tmode |= FILEIO_S_IFREG;
> -  if (S_ISDIR (mode))
> -    tmode |= FILEIO_S_IFDIR;
> -  if (S_ISCHR (mode))
> -    tmode |= FILEIO_S_IFCHR;
> +  switch (mode & S_IFMT)
> +    {
> +    case S_IFSOCK:
> +      tmode |= FILEIO_S_IFSOCK;
> +      break;
> +    case S_IFLNK:
> +      tmode |= FILEIO_S_IFLNK;
> +      break;
> +    case S_IFREG:
> +      tmode |= FILEIO_S_IFREG;
> +      break;
> +    case S_IFBLK:
> +      tmode |= FILEIO_S_IFBLK;
> +      break;
> +    case S_IFDIR:
> +      tmode |= FILEIO_S_IFDIR;
> +      break;
> +    case S_IFCHR:
> +      tmode |= FILEIO_S_IFCHR;
> +      break;
> +    case S_IFIFO:
> +      tmode |= FILEIO_S_IFIFO;
> +      break;
> +    }

I'm not sure whether all these S_FOO macros exist
on all hosts.

Looking at:
 https://www.gnu.org/software/gnulib/manual/html_node/sys_002fstat_002eh.html

gnulib's sys/stat.h (gdb/gnulib/import/sys_stat.in.h) adds some
replacements, but then I'm not sure whether checking S_IFxxx
etc. directly instead of using the S_ISxxx function-style macros
is the right thing to do portability-wise.  It may be S_ISxxx was being
used for a reason?

> +
>    if (mode & S_IRUSR)
>      tmode |= FILEIO_S_IRUSR;
>    if (mode & S_IWUSR)
> @@ -224,8 +262,10 @@ void
>  host_to_fileio_stat (struct stat *st, struct fio_stat *fst)
>  {
>    LONGEST blksize;
> +  long    fst_dev;

We don't align variable names like that.

>  
> -  host_to_fileio_uint ((long) st->st_dev, fst->fst_dev);
> +  fst_dev = S_ISREG(st->st_mode) ? FILEIO_STDEV_FILE : FILEIO_STDEV_SPECIAL;

Missing space in "S_ISREG (".

> +  host_to_fileio_uint (fst_dev, fst->fst_dev);
>    host_to_fileio_uint ((long) st->st_ino, fst->fst_ino);
>    host_to_fileio_mode (st->st_mode, fst->fst_mode);
>    host_to_fileio_uint ((long) st->st_nlink, fst->fst_nlink);
> diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
> index 313da642ea..837081269a 100644
> --- a/gdb/remote-fileio.c
> +++ b/gdb/remote-fileio.c
> @@ -407,24 +407,6 @@ remote_fileio_func_open (remote_target *remote, char *buf)
>        return;
>      }
>  
> -  /* Check if pathname exists and is not a regular file or directory.  If so,
> -     return an appropriate error code.  Same for trying to open directories
> -     for writing.  */

Did you intend to remove the "Same for trying to open directories
for writing." part?

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 18c1915675..19cf5e38a6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@ 
+2018-06-28  Julio Guerra  <julio@farjump.io>
+
+	* remote-fileio.c (remote_fileio_func_open, remote_fileio_func_stat):
+	Allow using File I/O functions open(), stat() and fstat() on special
+	files.
+	* ../include/gdb/fileio.h: Add macro definitions for special files,
+	both for fst_dev and fst_mode fields of struct fst_stat.
+	* common/fileio.c (fileio_to_host_mode, fileio_mode_pack): Add new
+	special file types in fst_mode's definition.
+	(host_to_fileio_stat): Define fst_dev using the new macro definitions
+	and according to the file's type.
+
 2018-06-27  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* gdb-gdb.py.in (StructMainTypePrettyPrinter) <to_string>: Don't
diff --git a/gdb/common/fileio.c b/gdb/common/fileio.c
index 912a7ede3c..9ee78e227c 100644
--- a/gdb/common/fileio.c
+++ b/gdb/common/fileio.c
@@ -119,12 +119,31 @@  fileio_to_host_mode (int fileio_mode, mode_t *mode_p)
   if (fileio_mode & ~FILEIO_S_SUPPORTED)
     return -1;
 
-  if (fileio_mode & FILEIO_S_IFREG)
-    mode |= S_IFREG;
-  if (fileio_mode & FILEIO_S_IFDIR)
-    mode |= S_IFDIR;
-  if (fileio_mode & FILEIO_S_IFCHR)
-    mode |= S_IFCHR;
+  switch (fileio_mode & FILEIO_S_IFMT)
+    {
+    case FILEIO_S_IFSOCK:
+      *mode_p |= S_IFSOCK;
+      break;
+    case FILEIO_S_IFLNK:
+      mode |= S_IFLNK;
+      break;
+    case FILEIO_S_IFREG:
+      mode |= S_IFREG;
+      break;
+    case FILEIO_S_IFBLK:
+      mode |= S_IFBLK;
+      break;
+    case FILEIO_S_IFDIR:
+      mode |= S_IFDIR;
+      break;
+    case FILEIO_S_IFCHR:
+      mode |= S_IFCHR;
+      break;
+    case FILEIO_S_IFIFO:
+      mode |= S_IFIFO;
+      break;
+    }
+
   if (fileio_mode & FILEIO_S_IRUSR)
     mode |= S_IRUSR;
   if (fileio_mode & FILEIO_S_IWUSR)
@@ -165,12 +184,31 @@  fileio_mode_pack (mode_t mode)
 {
   mode_t tmode = 0;
 
-  if (S_ISREG (mode))
-    tmode |= FILEIO_S_IFREG;
-  if (S_ISDIR (mode))
-    tmode |= FILEIO_S_IFDIR;
-  if (S_ISCHR (mode))
-    tmode |= FILEIO_S_IFCHR;
+  switch (mode & S_IFMT)
+    {
+    case S_IFSOCK:
+      tmode |= FILEIO_S_IFSOCK;
+      break;
+    case S_IFLNK:
+      tmode |= FILEIO_S_IFLNK;
+      break;
+    case S_IFREG:
+      tmode |= FILEIO_S_IFREG;
+      break;
+    case S_IFBLK:
+      tmode |= FILEIO_S_IFBLK;
+      break;
+    case S_IFDIR:
+      tmode |= FILEIO_S_IFDIR;
+      break;
+    case S_IFCHR:
+      tmode |= FILEIO_S_IFCHR;
+      break;
+    case S_IFIFO:
+      tmode |= FILEIO_S_IFIFO;
+      break;
+    }
+
   if (mode & S_IRUSR)
     tmode |= FILEIO_S_IRUSR;
   if (mode & S_IWUSR)
@@ -224,8 +262,10 @@  void
 host_to_fileio_stat (struct stat *st, struct fio_stat *fst)
 {
   LONGEST blksize;
+  long    fst_dev;
 
-  host_to_fileio_uint ((long) st->st_dev, fst->fst_dev);
+  fst_dev = S_ISREG(st->st_mode) ? FILEIO_STDEV_FILE : FILEIO_STDEV_SPECIAL;
+  host_to_fileio_uint (fst_dev, fst->fst_dev);
   host_to_fileio_uint ((long) st->st_ino, fst->fst_ino);
   host_to_fileio_mode (st->st_mode, fst->fst_mode);
   host_to_fileio_uint ((long) st->st_nlink, fst->fst_nlink);
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index 313da642ea..837081269a 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -407,24 +407,6 @@  remote_fileio_func_open (remote_target *remote, char *buf)
       return;
     }
 
-  /* Check if pathname exists and is not a regular file or directory.  If so,
-     return an appropriate error code.  Same for trying to open directories
-     for writing.  */
-  if (!stat (pathname, &st))
-    {
-      if (!S_ISREG (st.st_mode) && !S_ISDIR (st.st_mode))
-	{
-	  remote_fileio_reply (remote, -1, FILEIO_ENODEV);
-	  return;
-	}
-      if (S_ISDIR (st.st_mode)
-	  && ((flags & O_WRONLY) == O_WRONLY || (flags & O_RDWR) == O_RDWR))
-	{
-	  remote_fileio_reply (remote, -1, FILEIO_EISDIR);
-	  return;
-	}
-    }
-
   fd = gdb_open_cloexec (pathname, flags, mode);
   if (fd < 0)
     {
@@ -885,16 +867,9 @@  remote_fileio_func_stat (remote_target *remote, char *buf)
       remote_fileio_return_errno (remote, -1);
       return;
     }
-  /* Only operate on regular files and directories.  */
-  if (!ret && !S_ISREG (st.st_mode) && !S_ISDIR (st.st_mode))
-    {
-      remote_fileio_reply (remote, -1, FILEIO_EACCES);
-      return;
-    }
   if (statptr)
     {
       host_to_fileio_stat (&st, &fst);
-      host_to_fileio_uint (0, fst.fst_dev);
 
       errno = target_write_memory (statptr, (gdb_byte *) &fst, sizeof fst);
       if (errno != 0)
@@ -939,7 +914,6 @@  remote_fileio_func_fstat (remote_target *remote, char *buf)
 
   if (fd == FIO_FD_CONSOLE_IN || fd == FIO_FD_CONSOLE_OUT)
     {
-      host_to_fileio_uint (1, fst.fst_dev);
       memset (&st, 0, sizeof (st));
       st.st_mode = S_IFCHR | (fd == FIO_FD_CONSOLE_IN ? S_IRUSR : S_IWUSR);
       st.st_nlink = 1;
@@ -972,6 +946,8 @@  remote_fileio_func_fstat (remote_target *remote, char *buf)
   if (ptrval)
     {
       host_to_fileio_stat (&st, &fst);
+      if (fd == FIO_FD_CONSOLE_IN || fd == FIO_FD_CONSOLE_OUT)
+	host_to_fileio_uint (FILEIO_STDEV_CONSOLE, fst.fst_dev);
 
       errno = target_write_memory (ptrval, (gdb_byte *) &fst, sizeof fst);
       if (errno != 0)
diff --git a/include/gdb/fileio.h b/include/gdb/fileio.h
index 7bb55f579f..ada84c5f10 100644
--- a/include/gdb/fileio.h
+++ b/include/gdb/fileio.h
@@ -37,10 +37,24 @@ 
 				 FILEIO_O_CREAT  | FILEIO_O_TRUNC| \
 				 FILEIO_O_EXCL)
 
+/* Device id values of fst_dev field */
+/* Regular file */
+#define FILEIO_STDEV_FILE           0
+/* GDB's console */
+#define FILEIO_STDEV_CONSOLE        1
+/* Not a regular file nor the console.
+   Bits FILEIO_S_IFMT of fst_mode give the exact file type. */
+#define FILEIO_STDEV_SPECIAL        2
+
 /* mode_t bits */
+#define FILEIO_S_IFSOCK       0140000
+#define FILEIO_S_IFLNK        0120000
 #define FILEIO_S_IFREG        0100000
+#define FILEIO_S_IFBLK         060000
 #define FILEIO_S_IFDIR         040000
 #define FILEIO_S_IFCHR         020000
+#define FILEIO_S_IFIFO         010000
+#define FILEIO_S_IFMT         0170000
 #define FILEIO_S_IRUSR           0400
 #define FILEIO_S_IWUSR           0200
 #define FILEIO_S_IXUSR           0100
@@ -53,9 +67,8 @@ 
 #define FILEIO_S_IWOTH             02
 #define FILEIO_S_IXOTH             01
 #define FILEIO_S_IRWXO             07
-#define FILEIO_S_SUPPORTED         (FILEIO_S_IFREG|FILEIO_S_IFDIR|  \
-				    FILEIO_S_IRWXU|FILEIO_S_IRWXG|  \
-                                    FILEIO_S_IRWXO)
+#define FILEIO_S_SUPPORTED         (FILEIO_S_IFMT | FILEIO_S_IRWXU| \
+				 FILEIO_S_IRWXG | FILEIO_S_IRWXO)
 
 /* lseek(2) flags */
 #define FILEIO_SEEK_SET             0