diff mbox

i386-tdep: Verify architecture before proceeding with `set/show mpx'

Message ID alpine.DEB.2.00.1610171554200.31859@tp.orcam.me.uk
State Committed
Headers

Commit Message

Maciej W. Rozycki Oct. 17, 2016, 3:15 p.m. UTC 
  Make sure the architecture is `bfd_arch_i386' before handling the `set 
mpx' and `show mpx' commands, avoiding the issue with `i386_mpx_enabled' 
interpreting `gdbarch->tdep' according to the `struct gdbarch_tdep' 
definition in i386-tdep.h while indeed in a multi-target configuration
it may have a different layout and cause GDB to crash or at least 
misbehave.

	gdb/
	* i386-tdep.c (i386_mpx_info_bounds): Make sure the architecture
	is `bfd_arch_i386' before proceeding.
	(i386_mpx_set_bounds): Likewise.
---
Hi,

 Noticed in `mips-mti-linux-gnu' `--enable-targets=all' regression testing 
with an upcoming change which modifies the mips-tdep.h definition of 
`struct gdbarch_tdep', showing up as:

(gdb) PASS: gdb.base/default.exp: info stack
info set
ada print-signatures:  Whether the output of formal and return types for functions in the overloads selection menu is activated is on.
[...]
mipsfpu:  The MIPS floating-point coprocessor is set automatically (currently double-precision)
ERROR: Process no longer exists
UNRESOLVED: gdb.base/default.exp: info set

which is a segfault due to the retrieved numerical value of `tdep->tdesc' 
being 4 in `i386_mpx_enabled'.  With the change in place, the test case 
now passes, with the expected message produced:

(gdb) PASS: gdb.base/default.exp: info stack
info set
ada print-signatures:  Whether the output of formal and return types for functions in the overloads selection menu is activated is on.
[...]
mipsfpu:  The MIPS floating-point coprocessor is set automatically (currently double-precision)
mpx bound:  Intel Memory Protection Extensions not supported on this target.
multiple-symbols:  How the debugger handles ambiguities in expressions is "all".
[...]
write:  Writing into executable and core files is off.
(gdb) PASS: gdb.base/default.exp: info set

I've decided to quit from `i386_mpx_info_bounds' and `i386_mpx_set_bounds' 
right away for code clarity rather than burying the condition within 
`i386_mpx_enabled'.

 OK to apply?

  Maciej

gdb-i386-mpx-arch.diff

Comments

Pedro Alves Oct. 17, 2016, 3:49 p.m. UTC | #1 
On 10/17/2016 04:15 PM, Maciej W. Rozycki wrote:

> I've decided to quit from `i386_mpx_info_bounds' and `i386_mpx_set_bounds' 
> right away for code clarity rather than burying the condition within 
> `i386_mpx_enabled'.
> 
>  OK to apply?

OK.

Thanks,
Pedro Alves
Maciej W. Rozycki Oct. 18, 2016, 3:51 a.m. UTC | #2 
On Mon, 17 Oct 2016, Pedro Alves wrote:

> OK.

 Committed, thanks.

  Maciej
diff mbox

Patch

Index: binutils/gdb/i386-tdep.c
===================================================================
--- binutils.orig/gdb/i386-tdep.c	2016-10-05 00:58:08.000000000 +0100
+++ binutils/gdb/i386-tdep.c	2016-10-17 06:28:19.719738724 +0100
@@ -8857,7 +8857,8 @@  i386_mpx_info_bounds (char *args, int fr
   struct gdbarch *gdbarch = get_current_arch ();
   struct type *data_ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
 
-  if (!i386_mpx_enabled ())
+  if (gdbarch_bfd_arch_info (gdbarch)->arch != bfd_arch_i386
+      || !i386_mpx_enabled ())
     {
       printf_unfiltered (_("Intel Memory Protection Extensions not "
 			   "supported on this target.\n"));
@@ -8900,7 +8901,8 @@  i386_mpx_set_bounds (char *args, int fro
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   struct type *data_ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
 
-  if (!i386_mpx_enabled ())
+  if (gdbarch_bfd_arch_info (gdbarch)->arch != bfd_arch_i386
+      || !i386_mpx_enabled ())
     error (_("Intel Memory Protection Extensions not supported\
  on this target."));