[Crash-utility] Miscellaneous fixes/enhancements to crash 4.0-2.10

[Dave Anderson]

Castor Fu wrote:

> Hi guys:
>
> We're trying to bring in a newer version of crash, and have
> some changes which we've made since going to version 4.0-2.10
> which I'll outline below, and have attached a patch:
>
> 1. Makefile,  help.c, extensions.c, new file kw.c
>       Add a keyword expansion mechanism to the 'extend' command.
>      We did this primarily to let us load different extensions
>      based on the dump file itself or the version of crash.
>
>      +"\n  The named <shared-object> is subjected to a simple keyword expansion",
>      +"  scheme with the following rules:",
>      +"    $(crash_version)       version of crash",
>      +"    $(corestr-<varname>)   string named <varname> taken from the core file",
>      +"    $(builddir)            build directory for the kernel",

I still have no idea what this does?  Can you expand upon how this works,
what does it do, what does it looks like, can it be ignored, etc?  Couldn't
this kind of thing can be self-contained in each extension module?
It just seems to be way too "implementation-specific" for lack of a
better term?

>
>
> 2. defs.h
>      Provide nominal recognition of the LKCD V10 dump format which does
>      not appear to have changed much.
>

I'll leave the LKCD review to those who use it.  But where did I get the impression
that LKCD V10 was supposed to be signficantly different?  Troy?

>
> 3. kernel.c
>      Make compile under gcc 2.x compilers
>      Provide additional information with bad stack addresses.
>

Seems OK -- it's only used in situations where you're trying to resurrect
a backtrace with your own hooks.

>
> 4. lkcd_common.c
>      Handle single-bit errors in LKCD compressed pages by trying out
>      all possible single-bit errors.
>
> 5. lkcd_v7.c
>      If dh_memory_end is set in the dump header, use that to determine the
>      index file size.  (Note that this is not initialized in the main-line
>      LKCD code yet...)
>

Again, taking on blind faith...

>
> 6.  task.c
>      Be more defensive about the value of tc->processor
>

Why is it necessary to do a verify_task() again -- it had to
have gone through verify_task() in order to get here, right?
What exactly did you see that required this?

--- crash-4.0-2.10/task.c       2005-11-07 07:44:06 -08:00
+++ crash-4.0-2.10-new/task.c   2005-11-07 18:57:19 -08:00
@@ -2942,7 +2942,7 @@
                        fprintf(fp, "(SYSRQ)");
                else if (machdep->flags & INIT)
                        fprintf(fp, "(INIT)");
-               else if (kt->cpu_flags[tc->processor] & NMI)
+               else if (verify_task(tc, 2) && kt->cpu_flags[tc->processor] & NMI)
                        fprintf(fp, "(NMI)");
                else if (tc->task == tt->panic_task)
                        fprintf(fp, "(PANIC)");

This I'm presuming is related to the above, but again, what did you
see that requires this?:

@@ -4337,7 +4340,8 @@

         tc = FIRST_CONTEXT();
         for (i = 0; i < RUNNING_TASKS(); i++, tc++) {
-               if (task_has_cpu(tc->task, NULL)) {
+               if (task_has_cpu(tc->task, NULL)
+                    && tc->processor >= 0 && tc->processor < NR_CPUS) {
                        tt->panic_threads[tc->processor] = tc->task;
                        found++;
                }


>
>      Also, try to set the panic string correctly in when decoding dump.

This is LKCD specific anyway, right?

>
>
> 7.  x86.c
>
>      Be a little more liberal deciding that the frame pointer has been included.
>      gcc 3.x can re-schedule code so that instead of
>
>          push bp
>          mov esp, bp
>
>      we sometimes see
>
>          push bp
>          xor eax,eax
>          move esp, bp
>
>      so detect both sequences.

This looks reasonable.  Didn't think anybody built kernels that way anymore...

>
>
> If y'all could consider this for inclusion I would appreciate it.
>
>      -castor