diff mbox

Handle loading improper core files gracefully in the mips backend.

Message ID 1452277948-25292-1-git-send-email-lgustavo@codesourcery.com
State New
Headers show

Commit Message

Luis Machado Jan. 8, 2016, 6:32 p.m. UTC 
bz 18964 was reported as a internal error from the mips backend when running
gdb.arch/i386-biarch-core.exp. The backend gets confused and ends up picking
up an invalid combination of 32-bit ISA and 64-bit ABI, resulting, later on,
in an unexpected mismatch between the raw register size for PC (4) and the
pseudo register size for PC (8).

The point that makes the architecture selection go south is when we notice
the core file is 64-bit, so we go with MIPS_ABI_N64, but we are using MIPS16
as the ISA.

The attached patch adds a bit more logic to the incompatibility check during
the architecture initialization.

It doesn't completely solve the problem with this testcase though, but it
gets rid of an ugly internal error and changes things from this:

FAIL: gdb.arch/i386-biarch-core.exp: core-file (GDB internal error)

    === gdb Summary ===

to this:

FAIL: gdb.arch/i386-biarch-core.exp: .text is readable

    === gdb Summary ===

Leaving this failure behind:

x/bx 0x400078^M
0x400078:       Cannot access memory at address 0x400078^M
(gdb) FAIL: gdb.arch/i386-biarch-core.exp: .text is readable

It is not clear to me what is expected of this particular test when executed
for a non-x86 target.

In any case, hardening of the mips backend seems to be an improvement already.

Maciej, do you have more input on additional incompatibilities that we need
to check for?



gdb/ChangeLog:

2016-01-08  Luis Machado  <lgustavo@codesourcery.com>

	bz 18964

	* mips-tdep.c (mips_gdbarch_init): Handle additional
	incompatible ISA/ABI pairs gracefully and return.
---
 gdb/mips-tdep.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

Comments

Maciej W. Rozycki Jan. 9, 2016, 3:02 a.m. UTC | #1 
On Fri, 8 Jan 2016, Luis Machado wrote:

> Maciej, do you have more input on additional incompatibilities that we need
> to check for?

 Why do we get this far in the first place where e_machine != EM_MIPS?  I 
think rather than adding additional checks here (unless they're needed for 
a valid MIPS ELF binary; but that would be a separate problem to fix) we 
should reject such a core file outright.

  Maciej
diff mbox

Patch

diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index c8e12e8..9bdcf96 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -8206,6 +8206,8 @@  mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   enum mips_isa mips_isa;
   int dspacc;
   int dspctl;
+  /* Whether we have an unknown/unsuitable architecture description.  */
+  int arch_is_incompatible = 0;
 
   /* Fill in the OS dependent register numbers and names.  */
   if (info.osabi == GDB_OSABI_IRIX)
@@ -8571,11 +8573,16 @@  mips_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   /* If we have only 32-bit registers, then we can't debug a 64-bit
      ABI.  */
-  if (info.target_desc
-      && tdesc_property (info.target_desc, PROPERTY_GP32) != NULL
-      && mips_abi != MIPS_ABI_EABI32
-      && mips_abi != MIPS_ABI_O32)
+  if (mips_abi != MIPS_ABI_EABI32 && mips_abi != MIPS_ABI_O32
+      && ((info.target_desc
+	   && tdesc_property (info.target_desc, PROPERTY_GP32) != NULL)
+	  || mips_isa == ISA_MIPS16))
+    arch_is_incompatible = 1;
+
+  if (arch_is_incompatible)
     {
+      /* This is an unsuitable combination of ABI/ISA.  Just cleanup and
+	 return.  */
       if (tdesc_data != NULL)
 	tdesc_data_cleanup (tdesc_data);
       return NULL;