From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id BBA37385DC02 for ; Thu, 2 Apr 2020 03:06:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BBA37385DC02 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id C69C21F376; Wed, 1 Apr 2020 23:06:09 -0400 (EDT) Subject: [PATCH] gdb: stop using host-dependent signal numbers in, windows-tdep.c (was: Re: [PATCH 0/7] Add "Windows" OS ABI) To: Simon Marchi , Pedro Alves , gdb-patches@sourceware.org Cc: Jon Turney , Eli Zaretskii References: <20200316170845.184386-1-simon.marchi@polymtl.ca> <4c7333df-41ca-8cf3-0569-db95564b698e@redhat.com> <851468fb-5b6a-e43d-14ae-8ad6fc5d343a@polymtl.ca> From: Simon Marchi Message-ID: Date: Wed, 1 Apr 2020 23:06:09 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <851468fb-5b6a-e43d-14ae-8ad6fc5d343a@polymtl.ca> Content-Type: text/plain; charset=utf-8 Content-Language: en-US-large Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-24.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Apr 2020 03:06:13 -0000 On 2020-04-01 5:56 p.m., Simon Marchi via Gdb-patches wrote: > On 2020-04-01 5:42 p.m., Pedro Alves wrote: >> On 3/16/20 5:08 PM, Simon Marchi via Gdb-patches wrote: >>> This patchset started out as a single patch to have the OS ABI Cygwin >>> applied to Windows x86-64 binaries, here: >>> >>> https://sourceware.org/legacy-ml/gdb-patches/2020-03/msg00195.html >>> >>> with the follow-up here: >>> >>> https://sourceware.org/pipermail/gdb-patches/2020-March/000022.html >>> >>> Eli pointed out that it doesn't make sense for binaries compilied with >>> MinGW to have the Cygwin OS ABI, that there should be separate OS ABIs >>> for Cygwin and non-Cygwin Windows binaries. This already came up in the >>> following bug report: >>> >>> https://sourceware.org/bugzilla/show_bug.cgi?id=21500#add_comment >> >> Hurray. Thanks for doing this. Recently, another spot that could use >> this was added in windows-tdep.c. See: >> >> https://sourceware.org/legacy-ml/gdb-patches/2020-01/msg00048.html >> >> Look for "bummer". > > Ok, I'll see what I can do. > > Simon Here's a patch that does that. Note that I have only built-tested it. I'm so inefficient with Windows that if I wait until I have the time to actually try it, I'll never get it done. So I'm hoping that the Windows gurus can give it a look. Simon >From b08ef225a3fb1030e03ae53ed91fdc6f79550603 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 1 Apr 2020 22:01:54 -0400 Subject: [PATCH] gdb: stop using host-dependent signal numbers in windows-tdep.c The signal enumeration in windows-tdep.c is defined differently whether it is compiled on Cygwin or not. This is problematic, since the code in tdep files is not supposed to be influenced by the host platform (the platform GDB itself runs on). This makes a difference in windows_gdb_signal_to_target. An obvious example of clash is SIGABRT. Let's pretend we are cross-debugging a Cygwin process from a MinGW (non-Cygwin Windows) GDB. If GDB needs to translate the gdb signal number GDB_SIGNAL_ABRT into a target equivalent, it would obtain the MinGW number (22), despite the target being a Cygwin process. Conversely, if debugging a MinGW process from a Cygwin-hosted GDB, GDB_SIGNAL_ABRT would be converted to a Cygwin signal number (6) despite the target being a MinGW process. This is wrong, since we want the result to depend on the target's platform, not GDB's platform. This known flaw was accepted because at the time we had a single OS ABI (called Cygwin) for all Windows binaries (Cygwin ones and non-Cygwin ones). This limitation is now lifted, as we now have separate Windows and Cygwin OS ABIs. This means we are able to detect at runtime whether the binary we are debugging is a Cygwin one or non-Cygwin one. This patch splits the signal enum in two, one for the MinGW flavors and one for Cygwin, removing all the ifdefs that made it depend on the host platform. It then makes two separate gdb_signal_to_target gdbarch methods, that are used according to the OS ABI selected at runtime. There is a bit of re-shuffling needed in how the gdbarch'es are initialized, but nothing major. gdb/ChangeLog: * windows-tdep.h (windows_init_abi): Add comment. (cygwin_init_abi): New declaration. * windows-tdep.c: Split signal enumeration in two, one for Windows and one for Cygwin. (windows_gdb_signal_to_target): Only deal with signal of the Windows OS ABI. (cygwin_gdb_signal_to_target): New function. (windows_init_abi): Rename to windows_init_abi_common, don't set gdb_signal_to_target gdbarch method. Add new new function with this name. (cygwin_init_abi): New function. * amd64-windows-tdep.c (amd64_windows_init_abi_common): Add comment. Don't call windows_init_abi. (amd64_windows_init_abi): Add comment, call windows_init_abi. (amd64_cygwin_init_abi): Add comment, call cygwin_init_abi. * i386-windows-tdep.c (i386_windows_init_abi): Rename to i386_windows_init_abi_common, don't call windows_init_abi. Add a new function of this name. (i386_cygwin_init_abi): New function. (_initialize_i386_windows_tdep): Bind i386_cygwin_init_abi to OS ABI Cygwin. --- gdb/amd64-windows-tdep.c | 10 +- gdb/i386-windows-tdep.c | 27 ++++- gdb/windows-tdep.c | 214 ++++++++++++++++++++++++++------------- gdb/windows-tdep.h | 9 ++ 4 files changed, 181 insertions(+), 79 deletions(-) diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c index 6d5076d1c437..740152b7de8e 100644 --- a/gdb/amd64-windows-tdep.c +++ b/gdb/amd64-windows-tdep.c @@ -1208,6 +1208,8 @@ amd64_windows_auto_wide_charset (void) return "UTF-16"; } +/* Common parts for gdbarch initialization for Windows and Cygwin on AMD64. */ + static void amd64_windows_init_abi_common (gdbarch_info info, struct gdbarch *gdbarch) { @@ -1227,8 +1229,6 @@ amd64_windows_init_abi_common (gdbarch_info info, struct gdbarch *gdbarch) amd64_init_abi (info, gdbarch, amd64_target_description (X86_XSTATE_SSE_MASK, false)); - windows_init_abi (info, gdbarch); - /* Function calls. */ set_gdbarch_push_dummy_call (gdbarch, amd64_windows_push_dummy_call); set_gdbarch_return_value (gdbarch, amd64_windows_return_value); @@ -1241,19 +1241,25 @@ amd64_windows_init_abi_common (gdbarch_info info, struct gdbarch *gdbarch) set_gdbarch_auto_wide_charset (gdbarch, amd64_windows_auto_wide_charset); } +/* gdbarch initialization for Windows on AMD64. */ + static void amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) { amd64_windows_init_abi_common (info, gdbarch); + windows_init_abi (info, gdbarch); /* On Windows, "long"s are only 32bit. */ set_gdbarch_long_bit (gdbarch, 32); } +/* gdbarch initialization for Cygwin on AMD64. */ + static void amd64_cygwin_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) { amd64_windows_init_abi_common (info, gdbarch); + cygwin_init_abi (info, gdbarch); } static gdb_osabi diff --git a/gdb/i386-windows-tdep.c b/gdb/i386-windows-tdep.c index b26731c6bf28..3a07c862f233 100644 --- a/gdb/i386-windows-tdep.c +++ b/gdb/i386-windows-tdep.c @@ -200,13 +200,13 @@ i386_windows_auto_wide_charset (void) return "UTF-16"; } +/* Common parts for gdbarch initialization for Windows and Cygwin on i386. */ + static void -i386_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) +i386_windows_init_abi_common (struct gdbarch_info info, struct gdbarch *gdbarch) { struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); - windows_init_abi (info, gdbarch); - set_gdbarch_skip_trampoline_code (gdbarch, i386_windows_skip_trampoline_code); set_gdbarch_skip_main_prologue (gdbarch, i386_skip_main_prologue); @@ -227,6 +227,24 @@ i386_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) set_gdbarch_auto_wide_charset (gdbarch, i386_windows_auto_wide_charset); } +/* gdbarch initialization for Windows on i386. */ + +static void +i386_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) +{ + i386_windows_init_abi_common (info, gdbarch); + windows_init_abi (info, gdbarch); +} + +/* gdbarch initialization for Cygwin on i386. */ + +static void +i386_cygwin_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) +{ + i386_windows_init_abi_common (info, gdbarch); + cygwin_init_abi (info, gdbarch); +} + static gdb_osabi i386_windows_osabi_sniffer (bfd *abfd) { @@ -270,9 +288,8 @@ _initialize_i386_windows_tdep () gdbarch_register_osabi_sniffer (bfd_arch_i386, bfd_target_elf_flavour, i386_cygwin_core_osabi_sniffer); - /* The Windows and Cygwin OS ABIs are currently equivalent on i386. */ gdbarch_register_osabi (bfd_arch_i386, 0, GDB_OSABI_WINDOWS, i386_windows_init_abi); gdbarch_register_osabi (bfd_arch_i386, 0, GDB_OSABI_CYGWIN, - i386_windows_init_abi); + i386_cygwin_init_abi); } diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c index 0042ea350e80..9c5dfd183bfa 100644 --- a/gdb/windows-tdep.c +++ b/gdb/windows-tdep.c @@ -41,55 +41,69 @@ #define CYGWIN_DLL_NAME "cygwin1.dll" /* Windows signal numbers differ between MinGW flavors and between - those and Cygwin. The below enumeration was gleaned from the - respective headers; the ones marked with MinGW64/Cygwin are defined - only by MinGW64 and Cygwin, not by mingw.org's MinGW. FIXME: We - should really have distinct MinGW vs Cygwin OSABIs, and two - separate enums, selected at runtime. */ + those and Cygwin. The below enumerations were gleaned from the + respective headers. */ + +/* Signal numbers for the various MinGW flavors. The ones marked with + MinGW-w64 are defined by MinGW-w64, not by mingw.org's MinGW. */ enum - { - WINDOWS_SIGHUP = 1, /* MinGW64/Cygwin */ - WINDOWS_SIGINT = 2, - WINDOWS_SIGQUIT = 3, /* MinGW64/Cygwin */ - WINDOWS_SIGILL = 4, - WINDOWS_SIGTRAP = 5, /* MinGW64/Cygwin */ -#ifdef __CYGWIN__ - WINDOWS_SIGABRT = 6, -#else - WINDOWS_SIGIOT = 6, /* MinGW64 */ -#endif - WINDOWS_SIGEMT = 7, /* MinGW64/Cygwin */ - WINDOWS_SIGFPE = 8, - WINDOWS_SIGKILL = 9, /* MinGW64/Cygwin */ - WINDOWS_SIGBUS = 10, /* MinGW64/Cygwin */ - WINDOWS_SIGSEGV = 11, - WINDOWS_SIGSYS = 12, /* MinGW64/Cygwin */ - WINDOWS_SIGPIPE = 13,/* MinGW64/Cygwin */ - WINDOWS_SIGALRM = 14,/* MinGW64/Cygwin */ - WINDOWS_SIGTERM = 15, -#ifdef __CYGWIN__ - WINDOWS_SIGURG = 16, - WINDOWS_SIGSTOP = 17, - WINDOWS_SIGTSTP = 18, - WINDOWS_SIGCONT = 19, - WINDOWS_SIGCHLD = 20, - WINDOWS_SIGTTIN = 21, - WINDOWS_SIGTTOU = 22, - WINDOWS_SIGIO = 23, - WINDOWS_SIGXCPU = 24, - WINDOWS_SIGXFSZ = 25, - WINDOWS_SIGVTALRM = 26, - WINDOWS_SIGPROF = 27, - WINDOWS_SIGWINCH = 28, - WINDOWS_SIGLOST = 29, - WINDOWS_SIGUSR1 = 30, - WINDOWS_SIGUSR2 = 31 -#else - WINDOWS_SIGBREAK = 21, - WINDOWS_SIGABRT = 22 -#endif - }; +{ + WINDOWS_SIGHUP = 1, /* MinGW-w64 */ + WINDOWS_SIGINT = 2, + WINDOWS_SIGQUIT = 3, /* MinGW-w64 */ + WINDOWS_SIGILL = 4, + WINDOWS_SIGTRAP = 5, /* MinGW-w64 */ + WINDOWS_SIGIOT = 6, /* MinGW-w64 */ + WINDOWS_SIGEMT = 7, /* MinGW-w64 */ + WINDOWS_SIGFPE = 8, + WINDOWS_SIGKILL = 9, /* MinGW-w64 */ + WINDOWS_SIGBUS = 10, /* MinGW-w64 */ + WINDOWS_SIGSEGV = 11, + WINDOWS_SIGSYS = 12, /* MinGW-w64 */ + WINDOWS_SIGPIPE = 13, /* MinGW-w64 */ + WINDOWS_SIGALRM = 14, /* MinGW-w64 */ + WINDOWS_SIGTERM = 15, + WINDOWS_SIGBREAK = 21, + WINDOWS_SIGABRT = 22, +}; + +/* Signal numbers for Cygwin. */ + +enum +{ + CYGWIN_SIGHUP = 1, + CYGWIN_SIGINT = 2, + CYGWIN_SIGQUIT = 3, + CYGWIN_SIGILL = 4, + CYGWIN_SIGTRAP = 5, + CYGWIN_SIGABRT = 6, + CYGWIN_SIGEMT = 7, + CYGWIN_SIGFPE = 8, + CYGWIN_SIGKILL = 9, + CYGWIN_SIGBUS = 10, + CYGWIN_SIGSEGV = 11, + CYGWIN_SIGSYS = 12, + CYGWIN_SIGPIPE = 13, + CYGWIN_SIGALRM = 14, + CYGWIN_SIGTERM = 15, + CYGWIN_SIGURG = 16, + CYGWIN_SIGSTOP = 17, + CYGWIN_SIGTSTP = 18, + CYGWIN_SIGCONT = 19, + CYGWIN_SIGCHLD = 20, + CYGWIN_SIGTTIN = 21, + CYGWIN_SIGTTOU = 22, + CYGWIN_SIGIO = 23, + CYGWIN_SIGXCPU = 24, + CYGWIN_SIGXFSZ = 25, + CYGWIN_SIGVTALRM = 26, + CYGWIN_SIGPROF = 27, + CYGWIN_SIGWINCH = 28, + CYGWIN_SIGLOST = 29, + CYGWIN_SIGUSR1 = 30, + CYGWIN_SIGUSR2 = 31, +}; struct cmd_list_element *info_w32_cmdlist; @@ -607,7 +621,7 @@ init_w32_command_list (void) } } -/* Implementation of `gdbarch_gdb_signal_to_target'. */ +/* Implementation of `gdbarch_gdb_signal_to_target' for Windows. */ static int windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal) @@ -646,40 +660,81 @@ windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal) return WINDOWS_SIGALRM; case GDB_SIGNAL_TERM: return WINDOWS_SIGTERM; -#ifdef __CYGWIN__ + } + return -1; +} + +/* Implementation of `gdbarch_gdb_signal_to_target' for Cygwin. */ + +static int +cygwin_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal) +{ + switch (signal) + { + case GDB_SIGNAL_0: + return 0; + case GDB_SIGNAL_HUP: + return CYGWIN_SIGHUP; + case GDB_SIGNAL_INT: + return CYGWIN_SIGINT; + case GDB_SIGNAL_QUIT: + return CYGWIN_SIGQUIT; + case GDB_SIGNAL_ILL: + return CYGWIN_SIGILL; + case GDB_SIGNAL_TRAP: + return CYGWIN_SIGTRAP; + case GDB_SIGNAL_ABRT: + return CYGWIN_SIGABRT; + case GDB_SIGNAL_EMT: + return CYGWIN_SIGEMT; + case GDB_SIGNAL_FPE: + return CYGWIN_SIGFPE; + case GDB_SIGNAL_KILL: + return CYGWIN_SIGKILL; + case GDB_SIGNAL_BUS: + return CYGWIN_SIGBUS; + case GDB_SIGNAL_SEGV: + return CYGWIN_SIGSEGV; + case GDB_SIGNAL_SYS: + return CYGWIN_SIGSYS; + case GDB_SIGNAL_PIPE: + return CYGWIN_SIGPIPE; + case GDB_SIGNAL_ALRM: + return CYGWIN_SIGALRM; + case GDB_SIGNAL_TERM: + return CYGWIN_SIGTERM; case GDB_SIGNAL_URG: - return WINDOWS_SIGURG; + return CYGWIN_SIGURG; case GDB_SIGNAL_STOP: - return WINDOWS_SIGSTOP; + return CYGWIN_SIGSTOP; case GDB_SIGNAL_TSTP: - return WINDOWS_SIGTSTP; + return CYGWIN_SIGTSTP; case GDB_SIGNAL_CONT: - return WINDOWS_SIGCONT; + return CYGWIN_SIGCONT; case GDB_SIGNAL_CHLD: - return WINDOWS_SIGCHLD; + return CYGWIN_SIGCHLD; case GDB_SIGNAL_TTIN: - return WINDOWS_SIGTTIN; + return CYGWIN_SIGTTIN; case GDB_SIGNAL_TTOU: - return WINDOWS_SIGTTOU; + return CYGWIN_SIGTTOU; case GDB_SIGNAL_IO: - return WINDOWS_SIGIO; + return CYGWIN_SIGIO; case GDB_SIGNAL_XCPU: - return WINDOWS_SIGXCPU; + return CYGWIN_SIGXCPU; case GDB_SIGNAL_XFSZ: - return WINDOWS_SIGXFSZ; + return CYGWIN_SIGXFSZ; case GDB_SIGNAL_VTALRM: - return WINDOWS_SIGVTALRM; + return CYGWIN_SIGVTALRM; case GDB_SIGNAL_PROF: - return WINDOWS_SIGPROF; + return CYGWIN_SIGPROF; case GDB_SIGNAL_WINCH: - return WINDOWS_SIGWINCH; + return CYGWIN_SIGWINCH; case GDB_SIGNAL_PWR: - return WINDOWS_SIGLOST; + return CYGWIN_SIGLOST; case GDB_SIGNAL_USR1: - return WINDOWS_SIGUSR1; + return CYGWIN_SIGUSR1; case GDB_SIGNAL_USR2: - return WINDOWS_SIGUSR2; -#endif /* __CYGWIN__ */ + return CYGWIN_SIGUSR2; } return -1; } @@ -865,11 +920,11 @@ windows_solib_create_inferior_hook (int from_tty) static struct target_so_ops windows_so_ops; -/* To be called from the various GDB_OSABI_CYGWIN handlers for the - various Windows architectures and machine types. */ +/* Common parts for gdbarch initialization for the Windows and Cygwin OS + ABIs. */ -void -windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) +static void +windows_init_abi_common (struct gdbarch_info info, struct gdbarch *gdbarch) { set_gdbarch_wchar_bit (gdbarch, 16); set_gdbarch_wchar_signed (gdbarch, 0); @@ -881,8 +936,6 @@ windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) set_gdbarch_iterate_over_objfiles_in_search_order (gdbarch, windows_iterate_over_objfiles_in_search_order); - set_gdbarch_gdb_signal_to_target (gdbarch, windows_gdb_signal_to_target); - windows_so_ops = solib_target_so_ops; windows_so_ops.solib_create_inferior_hook = windows_solib_create_inferior_hook; @@ -891,6 +944,23 @@ windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) set_gdbarch_get_siginfo_type (gdbarch, windows_get_siginfo_type); } +/* See windows-tdep.h. */ +void +windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) +{ + windows_init_abi_common (info, gdbarch); + set_gdbarch_gdb_signal_to_target (gdbarch, windows_gdb_signal_to_target); +} + +/* See windows-tdep.h. */ + +void +cygwin_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) +{ + windows_init_abi_common (info, gdbarch); + set_gdbarch_gdb_signal_to_target (gdbarch, cygwin_gdb_signal_to_target); +} + /* Implementation of `tlb' variable. */ static const struct internalvar_funcs tlb_funcs = diff --git a/gdb/windows-tdep.h b/gdb/windows-tdep.h index f2dc4260469d..cd7717bd9174 100644 --- a/gdb/windows-tdep.h +++ b/gdb/windows-tdep.h @@ -31,9 +31,18 @@ extern void windows_xfer_shared_library (const char* so_name, struct gdbarch *gdbarch, struct obstack *obstack); +/* To be called from the various GDB_OSABI_WINDOWS handlers for the + various Windows architectures and machine types. */ + extern void windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch); +/* To be called from the various GDB_OSABI_CYGWIN handlers for the + various Windows architectures and machine types. */ + +extern void cygwin_init_abi (struct gdbarch_info info, + struct gdbarch *gdbarch); + /* Return true if the Portable Executable behind ABFD uses the Cygwin dll (cygwin1.dll). */ -- 2.26.0