This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 1/2] mingw: update gnulib: prepare the sources
- From: Pedro Alves <palves at redhat dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>, Kai Tietz <ktietz at redhat dot com>
- Cc: gdb-patches at sourceware dot org, Corinna Vinschen <vinschen at redhat dot com>, Nicholas Clifton <nickc at redhat dot com>
- Date: Mon, 23 Mar 2015 16:51:44 +0000
- Subject: Re: [patch 1/2] mingw: update gnulib: prepare the sources
- Authentication-results: sourceware.org; auth=none
- References: <20141222221229 dot GA30575 at host2 dot jankratochvil dot net> <108241234 dot 1319759 dot 1419335690162 dot JavaMail dot zimbra at redhat dot com> <20141224222045 dot GA30482 at host2 dot jankratochvil dot net>
Hi guys,
We really need to move forward with importing a newer gnulib,
for several reasons...
On 12/24/2014 10:20 PM, Jan Kratochvil wrote:
> Hi Kai,
>
> On Tue, 23 Dec 2014 12:54:50 +0100, Kai Tietz wrote:
>>> > > The whole problem is that the gnulib update (in [patch 2/2]) will cause
>>> > > (only) for build_win64 many errors like this one:
>> > Only for win64? This is curious. As headers are shared between 32-bit and
>> > 64-bit, and most part of Win32 API too.
> Yes, only win64 - because the change making struct timeval incompatible with
> select()'s argument
> http://sourceforge.net/p/mingw-w64/mailman/message/29610438/
> has there
> +#ifdef __LP64__
>
>
>> > Well, IMO the real issue here is to include windows.h instead of sys/time.h.
> The patch is now different so I am not sure if it still applies and what is
> the reason for it.
>
> <sys/time.h> includes <winsock2.h> since:
> gnulib:
> https://lists.gnu.org/archive/html/bug-gnulib/2012-01/msg00372.html
> commit f8e84098084b3b53bc6943a5542af1f607ffd477
> Author: Bruno Haible <bruno@clisp.org>
> Date: Sat Jan 28 18:12:10 2012 +0100
> sys_time: Override 'struct timeval' on some native Windows platforms.
>
>
>> > Sadly MS decided to pollute namespace with commonly used names in their
>> > platform-headers. So in the past we tried to avoid to include things like
>> > windows.h in bfd and related headers, and to isolate files, which actually
>> > need to include it (eg windows-unicode stuff in binutils/). To include it
>> > now leads to a general architectural change of binutils' bfd (and other
>> > parts). I am not in general oppose to it, but I am wondering if this is
>> > really wanted/needed.
> Both options are ugly but following the current include/coff/ assumption makes
> the patch smaller.
>
> It builds now (not runtime tested) for mingw64 32-bit and 64-bit.
>
> No regressions on {x86_64,x86_64-m32}-fedora21-linux-gnu.
I don't think this is correct, however.
> --- ./gdb/utils.c 2014-12-24 22:50:08.455459138 +0100
> +++ ./gdb/utils.c 2014-12-24 22:34:08.904812278 +0100
> @@ -37,8 +37,11 @@
> #include <pc.h>
> #endif
>
> -#include <signal.h>
> +/* For struct timeval for timeval-utils.h. */
> +#include <sys/time.h>
> #include "timeval-utils.h"
> +
> +#include <signal.h>
> #include "gdbcmd.h"
> #include "serial.h"
> #include "bfd.h"
> @@ -55,6 +58,7 @@
> #include "top.h"
> #include "main.h"
> #include "solist.h"
> +#include "gdb_timeval.h"
>
> #include "inferior.h" /* for signed_pointer_to_address */
>
For example, further down the file we have:
static int ATTRIBUTE_PRINTF (1, 0)
defaulted_query (const char *ctlstr, const char defchar, va_list args)
{
...
struct timeval prompt_started, prompt_ended, prompt_delta;
...
/* Add time spend in this routine to prompt_for_continue_wait_time. */
gettimeofday (&prompt_ended, NULL);
timeval_sub (&prompt_delta, &prompt_ended, &prompt_started);
timeval_add (&prompt_for_continue_wait_time,
&prompt_for_continue_wait_time, &prompt_delta);
and with this change above:
> -#include <signal.h>
> +/* For struct timeval for timeval-utils.h. */
> +#include <sys/time.h>
> #include "timeval-utils.h"
timeval_sub and timeval_add in "timeval-utils.h" will be prototyped
as if taking gnulib's "struct timeval" replacement, but, those functions
expect to be passed the native "struct timeval", as gnulib's sys/time.h
isn't used when libiberty itself is built.
The only thing gnulib's gettimeofday does is type-convert
the native struct timeval to the replaced type, like this:
int
gettimeofday (struct timeval *restrict tv, void *restrict tz)
{
#undef gettimeofday
...
# if defined timeval /* 'struct timeval' overridden by gnulib? */
# undef timeval
struct timeval otv;
int result = gettimeofday (&otv, (struct timezone *) tz);
if (result == 0)
{
tv->tv_sec = otv.tv_sec;
tv->tv_usec = otv.tv_usec;
}
...
that is, other than POSIX compliance, gnulib's struct timeval
replacement doesn't really give us "real" 64-bit-aware gettimeofday.
So, how about this simpler approach below? Just make sure we
continue using the native struct timeval, as today.
>From 5b35653dbdd66fc91b6222e2b9147fe2f89e0d86 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 23 Mar 2015 15:20:09 +0000
Subject: [PATCH] Prepare for gnulib update
After the last gnulib import (Dec 2012), gnulib upstream started
replacing mingw's 'struct timeval' with a version with 64-bit time_t,
for POSIX compliance:
commit f8e84098084b3b53bc6943a5542af1f607ffd477
Author: Bruno Haible <bruno@clisp.org>
Date: Sat Jan 28 18:12:10 2012 +0100
sys_time: Override 'struct timeval' on some native Windows platforms.
See:
https://lists.gnu.org/archive/html/bug-gnulib/2012-01/msg00372.html
However, that results in conflicts with native Winsock2's 'select':
select()'s argument
http://sourceforge.net/p/mingw-w64/mailman/message/29610438/
... and libiberty's timeval-utils.h timeval_add/timeval_sub, at the
least.
We don't really need the POSIX compliance, so this patch prepares us
to simply not use gnulib's 'struct timeval' replacement once a more
recent gnulib is imported, thus preserving the current behavior, by
adding a sys/time.h wrapper header that undefs gnulib's replacements,
and including that everywhere instead.
The SIZE -> OSIZE change is necessary because newer gnulib's
sys/time.h also includes windows.h/winsock2.h, which defines a
conflicting SIZE symbol.
Cross build-tested mingw-w64 32-bit and 64-bit.
Regtested on x86_64 Fedora 20.
gdb/ChangeLog:
2015-03-23 Pedro Alves <palves@redhat.com>
* Makefile.in (HFILES_NO_SRCDIR): Add common/gdb_sys_time.h.
* common/gdb_sys_time.h: New file.
* dsrec.c: Include gdb_sys_time.h instead of sys/time.h.
* event-loop.c: Likewise.
* gdb_select.h: Likewise.
* gdb_usleep.c: Likewise.
* m32r-rom.c: Likewise.
* maint.c: Likewise.
* mi/mi-main.c: Likewise.
* mi/mi-parse.h: Likewise.
* remote-fileio.c: Likewise.
* remote-m32r-sdi.c: Likewise.
* remote.c: Likewise.
* ser-base.c: Likewise.
* ser-pipe.c: Likewise.
* ser-tcp.c: Likewise.
* ser-unix.c: Likewise.
* symfile.c: Likewise.
* symfile.c: Likewise. Rename OSIZE to SIZE throughout.
* target-memory.c: Include gdb_sys_time.h instead of sys/time.h.
* utils.c: Likewise.
gdb/gdbserver/ChangeLog:
2015-03-23 Pedro Alves <palves@redhat.com>
* debug.c: Include gdb_sys_time.h instead of sys/time.h.
* gdbserver/event-loop.c: Likewise.
* gdbserver/remote-utils.c: Likewise.
* gdbserver/tracepoint.c: Likewise.
---
gdb/Makefile.in | 2 +-
gdb/common/gdb_sys_time.h | 38 ++++++++++++++++++++++++++++++++++++++
gdb/dsrec.c | 2 +-
gdb/event-loop.c | 2 +-
gdb/gdb_select.h | 2 +-
gdb/gdb_usleep.c | 3 +--
gdb/gdbserver/debug.c | 2 +-
gdb/gdbserver/event-loop.c | 2 +-
gdb/gdbserver/remote-utils.c | 2 +-
gdb/gdbserver/tracepoint.c | 2 +-
gdb/m32r-rom.c | 2 +-
gdb/maint.c | 2 +-
gdb/mi/mi-main.c | 2 +-
gdb/mi/mi-parse.h | 2 +-
gdb/remote-fileio.c | 2 +-
gdb/remote-m32r-sdi.c | 2 +-
gdb/remote.c | 2 +-
gdb/ser-base.c | 2 +-
gdb/ser-pipe.c | 2 +-
gdb/ser-tcp.c | 2 +-
gdb/ser-unix.c | 2 +-
gdb/symfile.c | 14 +++++++-------
gdb/target-memory.c | 2 +-
gdb/utils.c | 2 +-
24 files changed, 67 insertions(+), 30 deletions(-)
create mode 100644 gdb/common/gdb_sys_time.h
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index dbace2d..afff3c6 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -968,7 +968,7 @@ i386-linux-nat.h common/common-defs.h common/errors.h common/common-types.h \
common/common-debug.h common/cleanups.h common/gdb_setjmp.h \
common/common-exceptions.h target/target.h common/symbol.h \
common/common-regcache.h fbsd-tdep.h nat/linux-personality.h \
-common/common-remote-fileio.h
+common/common-remote-fileio.h common/gdb_sys_time.h
# Header files that already have srcdir in them, or which are in objdir.
diff --git a/gdb/common/gdb_sys_time.h b/gdb/common/gdb_sys_time.h
new file mode 100644
index 0000000..0f764f8
--- /dev/null
+++ b/gdb/common/gdb_sys_time.h
@@ -0,0 +1,38 @@
+/* Copyright (C) 2015 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef GDB_SYS_TIME_H
+#define GDB_SYS_TIME_H
+
+#include <sys/time.h>
+
+/* On MinGW-w64, gnulib's sys/time.h replaces 'struct timeval' and
+ gettimeofday with versions that support 64-bit time_t, for POSIX
+ compliance. However, the gettimeofday replacement does not ever
+ return time_t values larger than 31-bit, as it simply returns the
+ system's gettimeofday's (signed) 32-bit result as (signed) 64-bit.
+ Because we don't really need the POSIX compliance, and it ends up
+ causing conflicts with other libraries we use that don't use gnulib
+ and thus work with the native struct timeval, such as Winsock2's
+ native 'select' and libiberty, simply undefine away gnulib's
+ replacements. */
+#if GNULIB_defined_struct_timeval
+# undef timeval
+# undef gettimeofday
+#endif
+
+#endif /* #ifndef GDB_SYS_TIME_H */
diff --git a/gdb/dsrec.c b/gdb/dsrec.c
index 82cc968..c4e5352 100644
--- a/gdb/dsrec.c
+++ b/gdb/dsrec.c
@@ -19,7 +19,7 @@
#include "defs.h"
#include "serial.h"
#include "srec.h"
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include <time.h>
#include "gdb_bfd.h"
diff --git a/gdb/event-loop.c b/gdb/event-loop.c
index 79e41fd..7c97fc4 100644
--- a/gdb/event-loop.c
+++ b/gdb/event-loop.c
@@ -31,7 +31,7 @@
#endif
#include <sys/types.h>
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include "gdb_select.h"
#include "observer.h"
diff --git a/gdb/gdb_select.h b/gdb/gdb_select.h
index b608f2a..9748a13 100644
--- a/gdb/gdb_select.h
+++ b/gdb/gdb_select.h
@@ -23,7 +23,7 @@
#ifdef HAVE_SYS_SELECT_H
#include <sys/select.h>
#else
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#endif
#ifdef USE_WIN32API
diff --git a/gdb/gdb_usleep.c b/gdb/gdb_usleep.c
index 22ee6a0..67e4396 100644
--- a/gdb/gdb_usleep.c
+++ b/gdb/gdb_usleep.c
@@ -18,8 +18,7 @@
#include "defs.h"
#include "gdb_usleep.h"
#include "gdb_select.h"
-
-#include <sys/time.h>
+#include "gdb_sys_time.h"
int
gdb_usleep (int usec)
diff --git a/gdb/gdbserver/debug.c b/gdb/gdbserver/debug.c
index 1a1e333..5bbd381 100644
--- a/gdb/gdbserver/debug.c
+++ b/gdb/gdbserver/debug.c
@@ -17,7 +17,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "server.h"
-#include <sys/time.h>
+#include "gdb_sys_time.h"
/* Enable miscellaneous debugging output. The name is historical - it
was originally used to debug LinuxThreads support. */
diff --git a/gdb/gdbserver/event-loop.c b/gdb/gdbserver/event-loop.c
index 08a503f..b171644 100644
--- a/gdb/gdbserver/event-loop.c
+++ b/gdb/gdbserver/event-loop.c
@@ -22,7 +22,7 @@
#include "queue.h"
#include <sys/types.h>
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#ifdef USE_WIN32API
#include <windows.h>
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 1de86be..bc3c3d0 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -51,7 +51,7 @@
#if HAVE_FCNTL_H
#include <fcntl.h>
#endif
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include <unistd.h>
#if HAVE_ARPA_INET_H
#include <arpa/inet.h>
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 1acee8c..48c5710 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -25,7 +25,7 @@
#include <ctype.h>
#include <fcntl.h>
#include <unistd.h>
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include <inttypes.h>
#include <stdint.h>
diff --git a/gdb/m32r-rom.c b/gdb/m32r-rom.c
index a2ac537..d639aa5 100644
--- a/gdb/m32r-rom.c
+++ b/gdb/m32r-rom.c
@@ -31,7 +31,7 @@
#include "command.h"
#include "gdbcmd.h"
#include "symfile.h" /* for generic load */
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include <time.h> /* for time_t */
#include "objfiles.h" /* for ALL_OBJFILES etc. */
#include "inferior.h"
diff --git a/gdb/maint.c b/gdb/maint.c
index 1adea2f..5d7fca5 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -24,7 +24,7 @@
#include "arch-utils.h"
#include <ctype.h>
#include <signal.h>
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include <time.h>
#include "command.h"
#include "gdbcmd.h"
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index acbdb55..85ec281 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -55,7 +55,7 @@
#include "gdbcmd.h"
#include <ctype.h>
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#if defined HAVE_SYS_RESOURCE_H
#include <sys/resource.h>
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index 4c10f92..5900a6f 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -20,7 +20,7 @@
#ifndef MI_PARSE_H
#define MI_PARSE_H
-#include <sys/time.h>
+#include "gdb_sys_time.h"
/* MI parser */
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index 3882321..6af3c4b 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -31,7 +31,7 @@
#include "filestuff.h"
#include <fcntl.h>
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#ifdef __CYGWIN__
#include <sys/cygwin.h> /* For cygwin_conv_path. */
#endif
diff --git a/gdb/remote-m32r-sdi.c b/gdb/remote-m32r-sdi.c
index 01cb5b6..811d069 100644
--- a/gdb/remote-m32r-sdi.c
+++ b/gdb/remote-m32r-sdi.c
@@ -36,7 +36,7 @@
#include <netinet/in.h>
#endif
#include <sys/types.h>
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include <time.h>
#include "gdb_bfd.h"
#include "cli/cli-utils.h"
diff --git a/gdb/remote.c b/gdb/remote.c
index dfa68b3..4c79d49 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -45,7 +45,7 @@
#include "filestuff.h"
#include "rsp-low.h"
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include "event-loop.h"
#include "event-top.h"
diff --git a/gdb/ser-base.c b/gdb/ser-base.c
index 87817c4..48ee98d 100644
--- a/gdb/ser-base.c
+++ b/gdb/ser-base.c
@@ -23,7 +23,7 @@
#include "event-loop.h"
#include "gdb_select.h"
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#ifdef USE_WIN32API
#include <winsock2.h>
#endif
diff --git a/gdb/ser-pipe.c b/gdb/ser-pipe.c
index bf5e4d4..bf8418b 100644
--- a/gdb/ser-pipe.c
+++ b/gdb/ser-pipe.c
@@ -27,7 +27,7 @@
#include <sys/types.h>
#include <sys/socket.h>
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include <fcntl.h>
#include "filestuff.h"
diff --git a/gdb/ser-tcp.c b/gdb/ser-tcp.c
index 9c3dcf4..df7bb11 100644
--- a/gdb/ser-tcp.c
+++ b/gdb/ser-tcp.c
@@ -35,7 +35,7 @@
#include <sys/ioctl.h> /* For FIONBIO. */
#endif
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#ifdef USE_WIN32API
#include <winsock2.h>
diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
index 4125797..6182aba 100644
--- a/gdb/ser-unix.c
+++ b/gdb/ser-unix.c
@@ -26,7 +26,7 @@
#include <sys/types.h>
#include "terminal.h"
#include <sys/socket.h>
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include "gdb_select.h"
#include "gdbcmd.h"
diff --git a/gdb/symfile.c b/gdb/symfile.c
index ba099d3..e4eca2b 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -62,7 +62,7 @@
#include <sys/stat.h>
#include <ctype.h>
#include <time.h>
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include "psymtab.h"
@@ -3544,12 +3544,12 @@ overlay_command (char *args, int from_tty)
In this simple implementation, the target data structures are as follows:
unsigned _novlys; /# number of overlay sections #/
unsigned _ovly_table[_novlys][4] = {
- {VMA, SIZE, LMA, MAPPED}, /# one entry per overlay section #/
+ {VMA, OSIZE, LMA, MAPPED}, /# one entry per overlay section #/
{..., ..., ..., ...},
}
unsigned _novly_regions; /# number of overlay regions #/
unsigned _ovly_region_table[_novly_regions][3] = {
- {VMA, SIZE, MAPPED_TO_LMA}, /# one entry per overlay region #/
+ {VMA, OSIZE, MAPPED_TO_LMA}, /# one entry per overlay region #/
{..., ..., ...},
}
These functions will attempt to update GDB's mappedness state in the
@@ -3567,7 +3567,7 @@ static unsigned cache_novlys = 0;
static CORE_ADDR cache_ovly_table_base = 0;
enum ovly_index
{
- VMA, SIZE, LMA, MAPPED
+ VMA, OSIZE, LMA, MAPPED
};
/* Throw away the cached copy of _ovly_table. */
@@ -3667,14 +3667,14 @@ simple_overlay_update_1 (struct obj_section *osect)
for (i = 0; i < cache_novlys; i++)
if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
&& cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
- /* && cache_ovly_table[i][SIZE] == size */ )
+ /* && cache_ovly_table[i][OSIZE] == size */ )
{
read_target_long_array (cache_ovly_table_base + i * word_size,
(unsigned int *) cache_ovly_table[i],
4, word_size, byte_order);
if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
&& cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
- /* && cache_ovly_table[i][SIZE] == size */ )
+ /* && cache_ovly_table[i][OSIZE] == size */ )
{
osect->ovly_mapped = cache_ovly_table[i][MAPPED];
return 1;
@@ -3740,7 +3740,7 @@ simple_overlay_update (struct obj_section *osect)
for (i = 0; i < cache_novlys; i++)
if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
&& cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
- /* && cache_ovly_table[i][SIZE] == size */ )
+ /* && cache_ovly_table[i][OSIZE] == size */ )
{ /* obj_section matches i'th entry in ovly_table. */
osect->ovly_mapped = cache_ovly_table[i][MAPPED];
break; /* finished with inner for loop: break out. */
diff --git a/gdb/target-memory.c b/gdb/target-memory.c
index 5e5b1d7..177deb6 100644
--- a/gdb/target-memory.c
+++ b/gdb/target-memory.c
@@ -23,7 +23,7 @@
#include "target.h"
#include "memory-map.h"
-#include <sys/time.h>
+#include "gdb_sys_time.h"
static int
compare_block_starting_address (const void *a, const void *b)
diff --git a/gdb/utils.c b/gdb/utils.c
index 7172bba..439e343 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -62,7 +62,7 @@
#include "readline/readline.h"
-#include <sys/time.h>
+#include "gdb_sys_time.h"
#include <time.h>
#include "gdb_usleep.h"
--
1.9.3