Re: [PATCH 2/3] Consistently use BFD's time

[Pedro Alves <palves@redhat.com>]

On 6/19/20 9:24 PM, Tom Tromey wrote:
> Tom> I rebased the stat branch and I'll integrate my patches into it today.
> 
> I couldn't make this work.  The previous patch worked because, at the
> time, common-defs.h could include bfd.h relatively early.  That's no
> longer the case, and after trying a number of things I found that
> perhaps this double inclusion interferes with some system headers
> somehow. 

> I'm not sure what to do.  Maybe I can try namespacing just sys/stat.h?

Can you clarify what do you see not work?  I tried rebasing it
here, and it seems to work.  It builds cleanly on GNU/Linux and
mingw32-w64 (both -m64 and -m32).  I've pasted the resulting patch below.

I've also added an #error in case something pulls in sys/stat.h before
we get to defs.h.  I think that if that happens due to some include
in common-defs.h pulling sys/stat.h (say, libiberty.h ever exports some
API using struct stat), then it would likely be exposing a problem
similar to the bfd.h one, in that we should be calling such an API
with the stat that libiberty was built against.  Hopefully that won't
happen -- external interfaces using "struct stat" are a really bad idea,
as this whole thread shows...

I think I'd still like to try to understand why the --avoid=largefile
solution doesn't work though.

From fd8d185820e572dcff6fbfe2e65cd5b79cef86f3 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 19 Jun 2020 23:09:10 +0100
Subject: [PATCH] Handle different "struct stat" between GDB and BFD

---
 gdb/defs.h    | 19 +++++++++++++++++++
 gdb/gdb_bfd.c | 45 +++++++++++++++++++++++++++++++++++++++++----
 gdb/gdb_bfd.h |  2 +-
 gdb/jit.c     |  4 ++--
 gdb/symfile.c |  2 +-
 5 files changed, 64 insertions(+), 8 deletions(-)

diff --git a/gdb/defs.h b/gdb/defs.h
index a75511158a4..b8723dccd12 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -34,8 +34,27 @@
 #undef PACKAGE_TARNAME
 
 #include <config.h>
+
+/* Gnulib may replace struct stat with its own version.  Bfd does not
+   use gnulib, so when we call into bfd, we must use the system struct
+   stat.  */
+
+#ifdef _GL_SYS_STAT_H
+# error "gnulib's <sys/stat.h> included too early"
+#endif
+
+#define __need_system_sys_stat_h 1
+
+#include <sys/stat.h>
+
 #include "bfd.h"
 
+typedef struct stat sys_stat;
+
+#undef __need_system_sys_stat_h
+
+#include <sys/stat.h>
+
 #include <sys/types.h>
 #include <limits.h>
 
diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 25e0178a8b8..6026a6b7dbe 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -66,7 +66,7 @@ struct gdb_bfd_data
       needs_relocations (0),
       crc_computed (0)
   {
-    struct stat buf;
+    sys_stat buf;
 
     if (bfd_stat (abfd, &buf) == 0)
       {
@@ -361,24 +361,61 @@ gdb_bfd_iovec_fileio_close (struct bfd *abfd, void *stream)
   return 0;
 }
 
+/* Convert between a struct stat (potentially a gnulib replacement)
+   and a sys_stat (the system's struct stat).  */
+
+static sys_stat
+to_sys_stat (struct stat *st)
+{
+  sys_stat sst {};
+
+#define COPY(FIELD) \
+  sst.FIELD = st->FIELD
+
+  COPY (st_dev);
+  COPY (st_ino);
+  COPY (st_mode);
+  COPY (st_nlink);
+  COPY (st_uid);
+  COPY (st_gid);
+  COPY (st_rdev);
+
+  /* Check for overflow?  */
+  COPY (st_size);
+
+  // Should probably check _GL_WINDOWS_STAT_TIMESPEC and refer to
+  // st_atim, etc. instead.
+  COPY (st_atime);
+  COPY (st_mtime);
+  COPY (st_ctime);
+
+#undef COPY
+
+  return sst;
+}
+
 /* Wrapper for target_fileio_fstat suitable for passing as the
    STAT_FUNC argument to gdb_bfd_openr_iovec.  */
 
 static int
 gdb_bfd_iovec_fileio_fstat (struct bfd *abfd, void *stream,
-			    struct stat *sb)
+			    sys_stat *sb)
 {
   int fd = *(int *) stream;
   int target_errno;
   int result;
 
-  result = target_fileio_fstat (fd, sb, &target_errno);
+  struct stat st;
+
+  result = target_fileio_fstat (fd, &st, &target_errno);
   if (result == -1)
     {
       errno = fileio_errno_to_host (target_errno);
       bfd_set_error (bfd_error_system_call);
     }
 
+  *sb = to_sys_stat (&st);
+
   return result;
 }
 
@@ -826,7 +863,7 @@ gdb_bfd_openr_iovec (const char *filename, const char *target,
 					void *stream),
 		     int (*stat_func) (struct bfd *abfd,
 				       void *stream,
-				       struct stat *sb))
+				       sys_stat *sb))
 {
   bfd *result = bfd_openr_iovec (filename, target,
 				 open_func, open_closure,
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index a941b79fe73..8eb44f5ea20 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -158,7 +158,7 @@ gdb_bfd_ref_ptr gdb_bfd_openr_iovec (const char *filename, const char *target,
 							void *stream),
 				     int (*stat_func) (struct bfd *abfd,
 						       void *stream,
-						       struct stat *sb));
+						       sys_stat *sb));
 
 /* A wrapper for bfd_openr_next_archived_file that initializes the
    gdb-specific reference count.  */
diff --git a/gdb/jit.c b/gdb/jit.c
index 1b5ef46469e..bb59dafb126 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -124,11 +124,11 @@ mem_bfd_iovec_pread (struct bfd *abfd, void *stream, void *buf,
 /* For statting the file, we only support the st_size attribute.  */
 
 static int
-mem_bfd_iovec_stat (struct bfd *abfd, void *stream, struct stat *sb)
+mem_bfd_iovec_stat (struct bfd *abfd, void *stream, sys_stat *sb)
 {
   struct target_buffer *buffer = (struct target_buffer*) stream;
 
-  memset (sb, 0, sizeof (struct stat));
+  memset (sb, 0, sizeof (sys_stat));
   sb->st_size = buffer->size;
   return 0;
 }
diff --git a/gdb/symfile.c b/gdb/symfile.c
index b29f864b373..a00f04697ad 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1257,7 +1257,7 @@ separate_debug_file_exists (const std::string &name, unsigned long crc,
 {
   unsigned long file_crc;
   int file_crc_p;
-  struct stat parent_stat, abfd_stat;
+  sys_stat parent_stat, abfd_stat;
   int verified_as_different;
 
   /* Find a separate debug info file as if symbols would be present in

base-commit: 87f83f20023bf366c14ec4e0fd307948d96caaee
-- 
2.14.5