lkcd
[Top] [All Lists]

[PATCH] fix for incomplete backtrace due to wrong esp value + some clean

To: lkcd@xxxxxxxxxxx
Subject: [PATCH] fix for incomplete backtrace due to wrong esp value + some cleanups
From: Bharata B Rao <bharata@xxxxxxxxxx>
Date: Wed, 3 Oct 2001 15:57:56 +0530
Reply-to: bharata@xxxxxxxxxx
Sender: owner-lkcd@xxxxxxxxxxx
User-agent: Mutt/1.2.5i
Matt,
Here is a patch for dump_base.c and dump_i386.c which does the following:

1. When dump is triggerred from kernel traps(and when regs pointer is valid),
the current method of getting esp value is not correct.
Currently, esp is collected as dha->dha_regs.esp = (unsigned long) (regs + 1);
Here you go down the stack by entire pt_regs structure. In case of kernel
mode traps, since stack switch doesn't happen, the esp and xss members of
pt_regs won't be filled up by the processor. So here you are going down the
stack by additional 8 bytes. This has been fixed in the patch below.

I have seen that this eliminates the TRACE ERROR that is observed when
bt command is used in lcrash.

2. Move the arch specific inline assembler code in dump_base.c to dump_i386.c

3. Since collecting of esp/eip happens twice move it into 
__dump_save_panic_regs(), which is called twice.

Let me know if I can commit these into cvs.

Regards,
Bharata.
-- 

drivers/dump/dump_base.c
--- lkcd_cvs/2.4/drivers/dump/dump_base.c       Wed Sep 26 15:29:25 2001
+++ dump_base.c Wed Oct  3 15:37:37 2001
@@ -912,16 +912,7 @@
                 * So for that reason, we save the eip/esp
                 * now so we can re-build the trace later.
                 */
-               __asm__ __volatile__("
-                       pushl %%ecx\n
-                       movl  %%esp, %%ecx\n
-                       movl  %%ecx, %0\n
-                       popl  %%ecx\n"
-                       : "=g" (dump_header_asm.dha_esp)
-               );
-               __asm__ __volatile__("pushl  %ecx\n");
                __dump_save_panic_regs(&dump_header_asm);
-               __asm__ __volatile__("popl   %ecx\n");
 #endif
 
                /* update header to disk for the last time */


drivers/dump/dump_i386.c
--- lkcd_cvs/2.4/drivers/dump/dump_i386.c       Tue Sep 25 10:47:03 2001
+++ dump_i386.c Wed Oct  3 15:35:34 2001
@@ -34,23 +34,20 @@
 /*
  * Name: __dump_save_panic_regs()
  * Func: Save the EIP (really the RA).  We may pass an argument later.
+ *      Save ESP also here. 
  */
-void
+inline void 
 __dump_save_panic_regs(dump_header_asm_t *dha)
 {
+       __asm__ __volatile__("movl  %%esp, %0\n"
+               : "=r" (dha->dha_esp));
        /* hate to do this, but ... */
 #ifdef CONFIG_FRAME_POINTER
-       __asm__ __volatile__("
-               movl  4(%%esp), %%ecx\n
-               movl  %%ecx, %0\n"
-               : "=g" (dha->dha_eip)
-       );
+       __asm__ __volatile__("movl  4(%%esp), %0\n"
+               : "=r" (dha->dha_eip));
 #else
-       __asm__ __volatile__("
-               movl  (%%esp), %%ecx\n
-               movl  %%ecx, %0\n"
-               : "=g" (dha->dha_eip)
-       );
+       __asm__ __volatile__("movl  (%%esp), %0\n"
+               : "=r" (dha->dha_eip));
 #endif
 }
 
@@ -62,20 +59,11 @@
 __dump_configure_header(dump_header_asm_t *dha, struct pt_regs *regs)
 {
        /* save the dump specific esp/eip */
-       __asm__ __volatile__("
-               pushl %%ecx\n
-               movl  %%esp, %%ecx\n
-               movl  %%ecx, %0\n
-               popl  %%ecx\n"
-               : "=g" (dha->dha_esp)
-       );
-       __asm__ __volatile__("pushl  %ecx\n");
        __dump_save_panic_regs(dha);
-       __asm__ __volatile__("popl   %ecx\n");
 
        /* one final check -- modify if we're in user mode */
        if ((regs) && (!user_mode(regs))) {
-               dha->dha_regs.esp = (unsigned long) (regs + 1);
+               dha->dha_regs.esp = (unsigned long) &(regs->esp);
        }
 
        return (1);


<Prev in Thread] Current Thread [Next in Thread>