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

----- Original Message ----- 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