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 seems like we ought to be able to use C here, and, in my opinion,
less assembly is generally better than more. :)

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

Reply via email to