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

Pedro Alves
2020-06-19T23:05:58Z

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