> On Nov 3, 2014, at 8:58 AM, Andrew Fish <[email protected]> wrote: > > >> On Oct 31, 2014, at 2:36 PM, Jordan Justen <[email protected]> wrote: >> >> On Fri, Oct 31, 2014 at 1:18 PM, Andrew Fish <[email protected]> wrote: >>> >>>> On Oct 31, 2014, at 12:38 PM, Jordan Justen <[email protected]> wrote: >>>> >>>> Andrew, >>>> >>>> Back in February 2011, you added this assembly file to (apparently) >>>> work around a gdb bug. Do you think it is still required? (Did the bug >>>> get fixed?) >>>> >>> >>> It is not a bug. It is how the stack unwind logic works in gdb/lldb. For >>> clang the default is to emit frame pointers, so this means you can always >>> walk the stack frame. So for example an exception handler lib can print the >>> stack trace for the exception. >>> >> >> I see. Sort of. >> > > The stack walking code looks for the frame pointer and the return address > stored on the stack. A return address of zero means stop the walk. > >>>> It seems like we ought to be able to use C here, and, in my opinion, >>>> less assembly is generally better than more. :) >>>> >>> >>> It is probably possible to adjust the stack frame in the C code to be >>> compatible. >> >> I guess what confuses me is that C functions generally work, so what >> was it about this C routine that messes things up? (Yeah, I know. I >> guess it is not surprising that there is a snag when switching >> stacks... :) >> > > The stack just needs to be in the correct state. > >> So, if I change to the C code, then I'd probably see backtrace issues >> with gdb in EmulatorPkg? Do you think this is CLANG specific, or >> should GCC also show the issue? >> > > It will show up in any IA32/X64 compiler that emits a frame pointer. The > caller pushes the return address and the callee saves the frame pointer next > to it. > > pushl %ebp > movl %esp, %ebp > … > popl %ebp > retl > > >> Do you have ideas for tweaking the C code to fix it there? >> > > It may be as simple as simulating the push of zero on the stack that is > handed into LongJump. I’ll try to find some time to take a look. >
Jordan, Sorry for the delay (I wanted to verify it on a real system before I answered). I figured out how we could retire the InternalSwitchStack.S, and use InternalSwitchStack.c in all cases for IA32. PUSH is decrement ESP by 4 and place data at [ESP]. Thus the 1st push after the LongJump happens at NewStack - 16. It looks like this code reserves space for the return from the call (that never happens, and it is just junk on the stack) JumpBuffer.Esp = (UINTN)NewStack - sizeof (VOID*); JumpBuffer.Esp -= sizeof (Context1) + sizeof (Context2); I verified via a lldb the state of the system: (void *) NewStack = 0x88d0a000 (BASE_LIBRARY_JUMP_BUFFER) JumpBuffer = (Ebx = 0xff71c1b8, Esi = 0x00000400, Edi = 0xff71c1f8, Ebp = 0x00000004, Esp = 0x88d09ff4, Eip = 0xffe06a10) Adding this line of code will terminate the stack unwind: // Terminate stack unwind for compilers that support frame pointers. ((VOID**)JumpBuffer.Esp)[0] = 0; It looks like this location is allocated, but not written to, so this should be a low risk change. Thanks, Andrew Fish Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Andrew Fish <[email protected]> Current version: https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdePkg/Library/BaseLib/Ia32/InternalSwitchStack.c VOID EFIAPI InternalSwitchStack ( IN SWITCH_STACK_ENTRY_POINT EntryPoint, IN VOID *Context1, OPTIONAL IN VOID *Context2, OPTIONAL IN VOID *NewStack, IN VA_LIST Marker ) { BASE_LIBRARY_JUMP_BUFFER JumpBuffer; JumpBuffer.Eip = (UINTN)EntryPoint; JumpBuffer.Esp = (UINTN)NewStack - sizeof (VOID*); JumpBuffer.Esp -= sizeof (Context1) + sizeof (Context2); ((VOID**)JumpBuffer.Esp)[1] = Context1; ((VOID**)JumpBuffer.Esp)[2] = Context2; LongJump (&JumpBuffer, (UINTN)-1); } ------------------------------------------------------------------------------ Comprehensive Server Monitoring with Site24x7. Monitor 10 servers for $9/Month. Get alerted through email, SMS, voice calls or mobile push notifications. Take corrective actions from your mobile device. http://pubads.g.doubleclick.net/gampad/clk?id=154624111&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
