> 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.
Thanks,
Andrew Fish
> Thanks,
>
> -Jordan
>
>>> On Mon, Jul 11, 2011 at 8:01 PM, <[email protected]> wrote:
>>>> Revision: 12007
>>>> http://edk2.svn.sourceforge.net/edk2/?rev=12007&view=rev
>>>> Author: andrewfish
>>>> Date: 2011-07-12 03:01:34 +0000 (Tue, 12 Jul 2011)
>>>>
>>>> Log Message:
>>>> -----------
>>>> MdePkg: Fix X64 clang compile issues.
>>>>
>>>> Fixed issues with X64 clang, and also make StackSwitch push a zero on the
>>>> new stack to prevent a stack unwind into memory that is no longer valid.
>>>>
>>>> signed-off-by: andrewfish
>>>> reviewed-by: lgao4
>>>> reviewed-by: mdkinney
>>>>
>>>> Modified Paths:
>>>> --------------
>>>> trunk/edk2/MdePkg/Library/BaseLib/BaseLib.inf
>>>> trunk/edk2/MdePkg/Library/BaseLib/X64/SwitchStack.S
>>>> trunk/edk2/MdePkg/Library/BaseLib/X64/Thunk16.S
>>>> trunk/edk2/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>>>>
>>>> Added Paths:
>>>> -----------
>>>> trunk/edk2/MdePkg/Library/BaseLib/Ia32/InternalSwitchStack.S
>>>>
>>>> Modified: trunk/edk2/MdePkg/Library/BaseLib/BaseLib.inf
>>>> ===================================================================
>>>> --- trunk/edk2/MdePkg/Library/BaseLib/BaseLib.inf 2011-07-12
>>>> 02:57:30 UTC (rev 12006)
>>>> +++ trunk/edk2/MdePkg/Library/BaseLib/BaseLib.inf 2011-07-12
>>>> 03:01:34 UTC (rev 12007)
>>>> @@ -277,7 +277,9 @@
>>>> Ia32/DisableCache.S | GCC
>>>>
>>>> Ia32/DivS64x64Remainder.c
>>>> - Ia32/InternalSwitchStack.c
>>>> + Ia32/InternalSwitchStack.c | MSFT
>>>> + Ia32/InternalSwitchStack.c | INTEL
>>>> + Ia32/InternalSwitchStack.S | GCC
>>>> Ia32/Non-existing.c
>>>> Unaligned.c
>>>> X86WriteIdtr.c
>>>>
>>>> Added: trunk/edk2/MdePkg/Library/BaseLib/Ia32/InternalSwitchStack.S
>>>> ===================================================================
>>>> --- trunk/edk2/MdePkg/Library/BaseLib/Ia32/InternalSwitchStack.S
>>>> (rev 0)
>>>> +++ trunk/edk2/MdePkg/Library/BaseLib/Ia32/InternalSwitchStack.S
>>>> 2011-07-12 03:01:34 UTC (rev 12007)
>>>> @@ -0,0 +1,48 @@
>>>> +#------------------------------------------------------------------------------
>>>> +#
>>>> +# Copyright (c) 2006 - 2008, Intel Corporation. All rights reserved.<BR>
>>>> +# Portions copyright (c) 2011, Apple Inc. All rights reserved.
>>>> +# This program and the accompanying materials
>>>> +# are licensed and made available under the terms and conditions of the
>>>> BSD License
>>>> +# which accompanies this distribution. The full text of the license may
>>>> be found at
>>>> +# http://opensource.org/licenses/bsd-license.php.
>>>> +#
>>>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>>>> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
>>>> IMPLIED.
>>>> +#
>>>> +# Module Name:
>>>> +#
>>>> +# InternalSwitchStack.S
>>>> +#
>>>> +# Abstract:
>>>> +#
>>>> +# Implementation of a stack switch on IA-32.
>>>> +#
>>>> +#------------------------------------------------------------------------------
>>>> +
>>>> +ASM_GLOBAL ASM_PFX(InternalSwitchStack)
>>>> +
>>>> +#------------------------------------------------------------------------------
>>>> +# VOID
>>>> +# EFIAPI
>>>> +# InternalSwitchStack (
>>>> +# IN SWITCH_STACK_ENTRY_POINT EntryPoint,
>>>> +# IN VOID *Context1, OPTIONAL
>>>> +# IN VOID *Context2, OPTIONAL
>>>> +# IN VOID *NewStack
>>>> +# );
>>>> +#------------------------------------------------------------------------------
>>>> +ASM_PFX(InternalSwitchStack):
>>>> + pushl %ebp
>>>> + movl %esp, %ebp
>>>> +
>>>> + movl 20(%ebp), %esp # switch stack
>>>> + subl $8, %esp
>>>> +
>>>> + movl 16(%ebp), %eax
>>>> + movl %eax, 4(%esp)
>>>> + movl 12(%ebp), %eax
>>>> + movl %eax, (%esp)
>>>> + pushl $0 # keeps gdb from unwinding stack
>>>> + jmp *8(%ebp) # call and never return
>>>> +
>>>>
>>>> Modified: trunk/edk2/MdePkg/Library/BaseLib/X64/SwitchStack.S
>>>> ===================================================================
>>>> --- trunk/edk2/MdePkg/Library/BaseLib/X64/SwitchStack.S 2011-07-12
>>>> 02:57:30 UTC (rev 12006)
>>>> +++ trunk/edk2/MdePkg/Library/BaseLib/X64/SwitchStack.S 2011-07-12
>>>> 03:01:34 UTC (rev 12007)
>>>> @@ -37,7 +37,10 @@
>>>> #------------------------------------------------------------------------------
>>>> ASM_GLOBAL ASM_PFX(InternalSwitchStack)
>>>> ASM_PFX(InternalSwitchStack):
>>>> - mov %rcx, %rax
>>>> + pushq %rbp
>>>> + movq %rsp, %rbp
>>>> +
>>>> + mov %rcx, %rax // Shift registers for new call
>>>> mov %rdx, %rcx
>>>> mov %r8, %rdx
>>>> #
>>>> @@ -45,4 +48,5 @@
>>>> # in case the callee wishes to spill them.
>>>> #
>>>> lea -0x20(%r9), %rsp
>>>> - call *%rax
>>>> + pushq $0 // stop gdb stack unwind
>>>> + jmp *%rax // call EntryPoint ()
>>>>
>>>> Modified: trunk/edk2/MdePkg/Library/BaseLib/X64/Thunk16.S
>>>> ===================================================================
>>>> --- trunk/edk2/MdePkg/Library/BaseLib/X64/Thunk16.S 2011-07-12
>>>> 02:57:30 UTC (rev 12006)
>>>> +++ trunk/edk2/MdePkg/Library/BaseLib/X64/Thunk16.S 2011-07-12
>>>> 03:01:34 UTC (rev 12007)
>>>> @@ -49,13 +49,18 @@
>>>> .set IA32_REGS_SIZE, 56
>>>>
>>>> .data
>>>> +
>>>> +.set Lm16Size, ASM_PFX(InternalAsmThunk16) - ASM_PFX(m16Start)
>>>> +ASM_PFX(m16Size): .word Lm16Size
>>>> +.set LmThunk16Attr, L_ThunkAttr - ASM_PFX(m16Start)
>>>> +ASM_PFX(mThunk16Attr): .word LmThunk16Attr
>>>> +.set Lm16Gdt, ASM_PFX(NullSeg) - ASM_PFX(m16Start)
>>>> +ASM_PFX(m16Gdt): .word Lm16Gdt
>>>> +.set Lm16GdtrBase, _16GdtrBase - ASM_PFX(m16Start)
>>>> +ASM_PFX(m16GdtrBase): .word Lm16GdtrBase
>>>> +.set LmTransition, _EntryPoint - ASM_PFX(m16Start)
>>>> +ASM_PFX(mTransition): .word LmTransition
>>>>
>>>> -ASM_PFX(m16Size): .word ASM_PFX(InternalAsmThunk16) -
>>>> ASM_PFX(m16Start)
>>>> -ASM_PFX(mThunk16Attr): .word _ThunkAttr - ASM_PFX(m16Start)
>>>> -ASM_PFX(m16Gdt): .word ASM_PFX(NullSeg) - ASM_PFX(m16Start)
>>>> -ASM_PFX(m16GdtrBase): .word _16GdtrBase - ASM_PFX(m16Start)
>>>> -ASM_PFX(mTransition): .word _EntryPoint - ASM_PFX(m16Start)
>>>> -
>>>> .text
>>>>
>>>> ASM_PFX(m16Start):
>>>> @@ -91,7 +96,7 @@
>>>> .byte 0x1e # push ds
>>>> .byte 0x66,0x60 # pushad
>>>> .byte 0x66,0xba # mov edx, imm32
>>>> -_ThunkAttr: .space 4
>>>> +L_ThunkAttr: .space 4
>>>> testb $THUNK_ATTRIBUTE_DISABLE_A20_MASK_INT_15, %dl
>>>> jz L_1
>>>> movl $0x15cd2401,%eax # mov ax, 2401h & int 15h
>>>> @@ -120,7 +125,7 @@
>>>> .byte 0x66,0x2e,0x89,0x87 # mov cs:[bx + (L_64Eip - L_Base)],
>>>> eax
>>>> .word L_64Eip - L_Base
>>>> .byte 0x66,0xb8 # mov eax, imm32
>>>> -SavedCr4: .space 4
>>>> +L_SavedCr4: .long 0
>>>> movq %rax, %cr4
>>>> #
>>>> # rdi in the instruction below is indeed bx in 16-bit code
>>>> @@ -133,15 +138,15 @@
>>>> orb $1,%ah
>>>> wrmsr
>>>> .byte 0x66,0xb8 # mov eax, imm32
>>>> -SavedCr0: .space 4
>>>> +L_SavedCr0: .long
>>>> movq %rax, %cr0
>>>> .byte 0x66,0xea # jmp far cs:L_64Bit
>>>> -L_64Eip: .space 4
>>>> -SavedCs: .space 2
>>>> +L_64Eip: .long 0
>>>> +L_SavedCs: .space 2
>>>> L_64BitCode:
>>>> .byte 0x90
>>>> .byte 0x67,0xbc # mov esp, imm32
>>>> -SavedSp: .space 4 # restore stack
>>>> +L_SavedSp: .long # restore stack
>>>> nop
>>>> ret
>>>>
>>>> @@ -258,19 +263,20 @@
>>>> popq %rcx
>>>> rep
>>>> movsl # copy RegSet
>>>> - lea (SavedCr4 - ASM_PFX(m16Start))(%rdx), %ecx
>>>> + lea (L_SavedCr4 - ASM_PFX(m16Start))(%rdx), %ecx
>>>> movl %edx,%eax # eax <- transition code address
>>>> andl $0xf,%edx
>>>> shll $12,%eax # segment address in high order 16
>>>> bits
>>>> - lea (ASM_PFX(BackFromUserCode) - ASM_PFX(m16Start))(%rdx), %ax
>>>> + .set LBackFromUserCodeDelta, ASM_PFX(BackFromUserCode) -
>>>> ASM_PFX(m16Start)
>>>> + lea (LBackFromUserCodeDelta)(%rdx), %ax
>>>> stosl # [edi] <- return address of user
>>>> code
>>>> sgdt 0x60(%rsp) # save GDT stack in argument space
>>>> movzwq 0x60(%rsp), %r10 # r10 <- GDT limit
>>>> - lea ((ASM_PFX(InternalAsmThunk16) - SavedCr4) + 0xf)(%rcx), %r11
>>>> + lea ((ASM_PFX(InternalAsmThunk16) - L_SavedCr4) + 0xf)(%rcx), %r11
>>>> andq $0xfffffffffffffff0, %r11 # r11 <- 16-byte aligned shadowed
>>>> GDT table in real mode buffer
>>>>
>>>> - movw %r10w, (SavedGdt - SavedCr4)(%rcx) # save the limit of
>>>> shadowed GDT table
>>>> - movq %r11, (SavedGdt - SavedCr4 + 0x2)(%rcx) # save the base
>>>> address of shadowed GDT table
>>>> + movw %r10w, (SavedGdt - L_SavedCr4)(%rcx) # save the limit
>>>> of shadowed GDT table
>>>> + movq %r11, (SavedGdt - L_SavedCr4 + 0x2)(%rcx) # save the base
>>>> address of shadowed GDT table
>>>>
>>>> movq 0x62(%rsp) ,%rsi # rsi <- the original GDT base
>>>> address
>>>> xchg %r10, %rcx # save rcx to r10 and initialize
>>>> rcx to be the limit of GDT table
>>>> @@ -283,7 +289,8 @@
>>>>
>>>> sidt 0x50(%rsp)
>>>> movq %cr0, %rax
>>>> - movl %eax, (SavedCr0 - SavedCr4)(%rcx)
>>>> + .set LSavedCrDelta, L_SavedCr0 - L_SavedCr4
>>>> + movl %eax, (LSavedCrDelta)(%rcx)
>>>> andl $0x7ffffffe,%eax # clear PE, PG bits
>>>> movq %cr4, %rbp
>>>> movl %ebp, (%rcx) # save CR4 in SavedCr4
>>>> @@ -291,17 +298,18 @@
>>>> movl %r8d, %esi # esi <- 16-bit stack segment
>>>> .byte 0x6a, DATA32
>>>> popq %rdx
>>>> - lgdt (_16Gdtr - SavedCr4)(%rcx)
>>>> + lgdt (_16Gdtr - L_SavedCr4)(%rcx)
>>>> movl %edx,%ss
>>>> pushfq
>>>> lea -8(%rdx), %edx
>>>> lea L_RetFromRealMode(%rip), %r8
>>>> pushq %r8
>>>> movl %cs, %r8d
>>>> - movw %r8w, (SavedCs - SavedCr4)(%rcx)
>>>> - movl %esp, (SavedSp - SavedCr4)(%rcx)
>>>> - .byte 0xff, 0x69 # jmp (_EntryPoint -
>>>> SavedCr4)(%rcx)
>>>> - .byte _EntryPoint - SavedCr4
>>>> + movw %r8w, (L_SavedCs - L_SavedCr4)(%rcx)
>>>> + movl %esp, (L_SavedSp - L_SavedCr4)(%rcx)
>>>> + .byte 0xff, 0x69 # jmp (_EntryPoint -
>>>> L_SavedCr4)(%rcx)
>>>> + .set Ltemp1, _EntryPoint - L_SavedCr4
>>>> + .byte Ltemp1
>>>> L_RetFromRealMode:
>>>> popfq
>>>> lgdt 0x60(%rsp) # restore protected mode GDTR
>>>>
>>>> Modified: trunk/edk2/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>>>> ===================================================================
>>>> --- trunk/edk2/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 2011-07-12
>>>> 02:57:30 UTC (rev 12006)
>>>> +++ trunk/edk2/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 2011-07-12
>>>> 03:01:34 UTC (rev 12007)
>>>> @@ -829,6 +829,7 @@
>>>> EFI_IMAGE_RESOURCE_DIRECTORY_ENTRY *ResourceDirectoryEntry;
>>>> EFI_IMAGE_RESOURCE_DIRECTORY_STRING *ResourceDirectoryString;
>>>> EFI_IMAGE_RESOURCE_DATA_ENTRY *ResourceDataEntry;
>>>> + CHAR16 *String;
>>>>
>>>>
>>>> ASSERT (ImageContext != NULL);
>>>> @@ -1209,11 +1210,12 @@
>>>> for (Index = 0; Index < ResourceDirectory->NumberOfNamedEntries;
>>>> Index++) {
>>>> if (ResourceDirectoryEntry->u1.s.NameIsString) {
>>>> ResourceDirectoryString = (EFI_IMAGE_RESOURCE_DIRECTORY_STRING
>>>> *) (Base + ResourceDirectoryEntry->u1.s.NameOffset);
>>>> + String = &ResourceDirectoryString->String[0];
>>>>
>>>> if (ResourceDirectoryString->Length == 3 &&
>>>> - ResourceDirectoryString->String[0] == L'H' &&
>>>> - ResourceDirectoryString->String[1] == L'I' &&
>>>> - ResourceDirectoryString->String[2] == L'I') {
>>>> + String[0] == L'H' &&
>>>> + String[1] == L'I' &&
>>>> + String[2] == L'I') {
>>>> //
>>>> // Resource Type "HII" found
>>>> //
>>>>
>>>>
>>>> This was sent by the SourceForge.net collaborative development platform,
>>>> the world's largest Open Source development site.
>>>>
>>>> ------------------------------------------------------------------------------
>>>> All of the data generated in your IT infrastructure is seriously valuable.
>>>> Why? It contains a definitive record of application performance, security
>>>> threats, fraudulent activity, and more. Splunk takes this data and makes
>>>> sense of it. IT sense. And common sense.
>>>> http://p.sf.net/sfu/splunk-d2d-c2
>>>> _______________________________________________
>>>> edk2-commits mailing list
>>>> [email protected]
>>>> https://lists.sourceforge.net/lists/listinfo/edk2-commits
>>
------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
