lkcd
[Top] [All Lists]

Re: [PATCH] fix for incomplete backtrace due to wrong esp value + some c

To: Bharata B Rao <bharata@xxxxxxxxxx>
Subject: Re: [PATCH] fix for incomplete backtrace due to wrong esp value + some cleanups
From: "Matt D. Robinson" <yakker@xxxxxxxxxxx>
Date: Wed, 3 Oct 2001 22:57:57 -0700 (PDT)
Cc: <lkcd@xxxxxxxxxxx>
In-reply-to: <20011003155756.A13611@xxxxxxxxxx>
Sender: owner-lkcd@xxxxxxxxxxx
Go ahead and check it in, Bharata.  Again, it was two years
ago, but the __asm__ required then to use a register for
moving %esp at one point, otherwise I wouldn't have used %ecx. :)

One more 'cvs update' ...

--Matt

On Wed, 3 Oct 2001, Bharata B Rao wrote:
|>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>