[Crash-utility] [PATCH 0/4] MIPS support

Mon Jan 12 16:00:08 UTC 2015

----- Original Message -----
> This adds support for analyzing dumps from 32-bit MIPS.
> 
> I've uploaded some sample dumps generated from the QEMU MIPS Malta machine
> (little-endian) at the following location:
> 
>  https://drive.google.com/folderview?id=0B4tMLbMvJ-l6R3J4LWJFc1k0eFU
> 
> Rabin Vincent (4):
>   netdump: Make a helper out of the PPC getregs function
>   Add gdb patch to fix sim/igen build
>   Rename convert() to converts() to avoid conflit with gdb
>   Add support for MIPS
> 
>  Makefile            |   7 +-
>  configure.c         |  48 +++
>  defs.h              | 113 ++++++-
>  gdb-7.6.patch       |  13 +
>  kernel.c            |   6 +-
>  lkcd_vmdump_v2_v3.h |   3 +-
>  mips.c              | 874
>  ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  netdump.c           |  27 +-
>  symbols.c           |   5 +
>  tools.c             |   2 +-
>  10 files changed, 1084 insertions(+), 14 deletions(-)
>  create mode 100644 mips.c

Hello Rabin,

Welcome 32-bit MIPS!  You've done a really nice job with this port.

This makes the 10th architecture supported by the crash utility.  
I don't know what the real-world usage is with respect to little-endian
vs. big-endian 32-bit MIPS, but you got here first.  And in the
future, it should be simple enough to support big-endian MIPS as
well, i.e., in the same manner that both little- and big-endian PPC64
are supported.

I do have a few trivial changes/additions that I'd like you 
to address:

The change for gdb-7.6/sim/igen/Makefile.in is apparently
required because the architectures that have subdirectories
in gdb-7.6/sim build something that's not done otherwise?
Anyway, that's fine, but the name-change for the convert() 
function should be done down there instead of in the top-level
crash sources.  The convert() function has always been an 
exported function in defs.h available to extension modules.  
So it cannot be changed at this point in time without risking 
the breakage of pre-existing modules.

Running cscope from the gdb-7.6 subdirectory on down shows only
this single reference to convert:

  sim/mips/sim-main.h Convert  774 #define Convert(rm,op,from,to) convert (SIM_ARGS, rm, op, from, to)

But it doesn't show any callers to Convert() or convert().  So
it appears to exist as a non-static function in "sim/mips/cp1.c"
that nobody calls?  In any case, please make the convert() name 
change down in the gdb-7.6/sim/mips code, and update gdb-7.6.patch
accordingly.

Also in defs.h, please put the task_struct_thread_reg29 and
task_struct_thread_reg31 offsets at the end of the offset_table
structure.  Again, new entries to that structure should always
be appended to the structure so that it won't break the usage 
of OFFSET() by pre-existing extension modules.  Also, you should
add their offset value displays to dump_offset_table() for use 
by "help -o", and in that function, you can display it next to 
the other task_struct_thread_xxx offsets.

On another note, there should be an understandable and clean 
protection mechanism against running a target=MIPS binary on a 
live host system.  So when attempting to run a target=ARM crash
binary on an x86/x86_64 host, it fails like so:

  $ sudo ./crash

  crash 7.1.0rc18
  Copyright (C) 2002-2014  Red Hat, Inc.
  Copyright (C) 2004, 2005, 2006, 2010  IBM Corporation
  Copyright (C) 1999-2006  Hewlett-Packard Co
  Copyright (C) 2005, 2006, 2011, 2012  Fujitsu Limited
  Copyright (C) 2006, 2007  VA Linux Systems Japan K.K.
  Copyright (C) 2005, 2011  NEC Corporation
  Copyright (C) 1999, 2002, 2007  Silicon Graphics, Inc.
  Copyright (C) 1999, 2000, 2001, 2002  Mission Critical Linux, Inc.
  This program is free software, covered by the GNU General Public License,
  and you are welcome to change it and/or distribute copies of it under
  certain conditions.  Enter "help copying" to see the conditions.
  This program has absolutely no warranty.  Enter "help warranty" for details.

  crash: compiled for the ARM architecture

  $ 

The same thing applies on "target=PPC64" and "target=ARM64" binaries:

  crash: compiled for the PPC64 architecture
  crash: compiled for the ARM64 architecture

With your target=MIPS binary, it shows this confusing error message:

  $ sudo ./crash

  crash 7.1.0rc18
  Copyright (C) 2002-2014  Red Hat, Inc.
  Copyright (C) 2004, 2005, 2006, 2010  IBM Corporation
  Copyright (C) 1999-2006  Hewlett-Packard Co
  Copyright (C) 2005, 2006, 2011, 2012  Fujitsu Limited
  Copyright (C) 2006, 2007  VA Linux Systems Japan K.K.
  Copyright (C) 2005, 2011  NEC Corporation
  Copyright (C) 1999, 2002, 2007  Silicon Graphics, Inc.
  Copyright (C) 1999, 2000, 2001, 2002  Mission Critical Linux, Inc.
  This program is free software, covered by the GNU General Public License,
  and you are welcome to change it and/or distribute copies of it under
  certain conditions.  Enter "help copying" to see the conditions.
  This program has absolutely no warranty.  Enter "help warranty" for details.

  GNU gdb (GDB) 7.6
  Copyright (C) 2013 Free Software Foundation, Inc.
  License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
  This is free software: you are free to change and redistribute it.
  There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
  and "show warranty" for details.
  This GDB was configured as "--host=x86_64-unknown-linux-gnu --target=mipsel-elf-linux"...

  crash: cannot resolve "cpu_data"

  $

It's a small check that can be put at the beginning of mips_init().
I presume that you used ppc.c as a template, so if you'd like, you
can add the same fix there as well.

Lastly (at least at this point), can you update the README[] strings array
in help.c to show that MIPS is a supported architecture, and that it can
be build as a "target=" binary?

With those fixes in place, it should be good to go for crash-7.1.0.

Oh, and thanks very much for the sample vmcores.  I'll keep them around for future
testing.  BTW, the System.map file is completely unnecessary -- is there a reason
you included it?

Thanks,
  Dave