public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Andrew Fish via groups.io" <afish=apple.com@groups.io>
To: edk2-devel-groups-io <devel@edk2.groups.io>,
	"Desimone, Nathaniel L" <nathaniel.l.desimone@intel.com>
Cc: Ray Ni <ray.ni@intel.com>,
	Mike Kinney <michael.d.kinney@intel.com>,
	Chasel Chiu <chasel.chiu@intel.com>
Subject: Re: [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
Date: Sat, 23 Sep 2023 08:16:54 -0700	[thread overview]
Message-ID: <BA0E7E8B-390D-49E6-98D7-4C530086DEDB@apple.com> (raw)
In-Reply-To: <20230922224923.1978-1-nathaniel.l.desimone@intel.com>

Very cool. Thank You!

Thanks,

Andrew Fish

> On Sep 22, 2023, at 3:49 PM, Nate DeSimone <nathaniel.l.desimone@intel.com> wrote:
> 
> The Visual Studio Windows debugger will only load symbols for PE/COFF images
> that Windows is aware of. Therefore, to enable source level debugging, all
> PEI/DXE modules must be loaded via LoadLibrary() or LoadLibraryEx() and the
> the instance in memory created by LoadLibrary() must be the one that is
> actually executed.
> 
> The current source level debug implementation in EmulatorPkg for Windows is
> inherited from the old Nt32Pkg. This implementation makes the assumption that
> all PEI/DXE modules have a DLL export tables with a symbol named
> InitializeDriver. Therefore, this source level debug implementation requires
> all modules to be linked in a non-PI spec defined manner. Support for adding
> the InitializeDriver symbol was removed in EmulatorPkg, which broke source
> level debugging.
> 
> To fix this, the source level debugging implementation has been modified to
> use the PE/COFF entry point directly. This brings the implementation into
> compliance with the PI spec and should work with any PEIM/DXE driver.
> Implementing this requires parsing the in-memory instance of the PE/COFF image
> created by Windows to find the entrypoint and since PEIMs/DXE drivers are not
> garunteed to have 4KB aligned sections, it also requires explicit configuration
> of the page table using VirtualProtect().
> 
> With this fix, the debugging experience is now so good it is unprecedented!
> In Visual Studio Code, add the following to launch.json:
> 
> {
> "version": "0.2.0",
> "configurations": [
>   {
>     "name": "EmulatorPkg Launch",
>     "type": "cppvsdbg",
>     "request": "launch",
>     "program": "${workspaceFolder}/<path_to_build>/Build/EmulatorX64/DEBUG_<tool_chain>/X64/WinHost",
>     "args": [],
>     "stopAtEntry": false,
>     "cwd": "${workspaceFolder}/<path_to_build>/Build/EmulatorX64/DEBUG_<tool_chain>/X64/",
>     "environment": [],
>     "console": false,
>   }
> ]
> }
> 
> Make modifications to the above template as nessesary and build EmulatorPkg.
> Now, just add breakpoints directly in Visual Studio Code the way you would with
> any other software project. When you start the debugger, it will halt at the
> breakpoint automatically without any extra configuration required.
> 
> Cc: Andrew Fish <afish@apple.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> ---
> EmulatorPkg/Win/Host/WinHost.c | 206 +++++++++++++++++++++++++++++----
> 1 file changed, 182 insertions(+), 24 deletions(-)
> 
> diff --git a/EmulatorPkg/Win/Host/WinHost.c b/EmulatorPkg/Win/Host/WinHost.c
> index 193a947fbd..e414da6c55 100644
> --- a/EmulatorPkg/Win/Host/WinHost.c
> +++ b/EmulatorPkg/Win/Host/WinHost.c
> @@ -8,7 +8,7 @@
>  This code produces 128 K of temporary memory for the SEC stack by directly
>  allocate memory space with ReadWrite and Execute attribute.
> 
> -Copyright (c) 2006 - 2022, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.<BR>
> (C) Copyright 2016-2020 Hewlett Packard Enterprise Development LP<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
> **/
> @@ -977,7 +977,7 @@ AddModHandle (
>  for (Index = 0; Index < mPdbNameModHandleArraySize; Index++, Array++) {
>    if (Array->PdbPointer == NULL) {
>      //
> -      // Make a copy of the stirng and store the ModHandle
> +      // Make a copy of the string and store the ModHandle
>      //
>      Handle            = GetProcessHeap ();
>      Size              = AsciiStrLen (ImageContext->PdbPointer) + 1;
> @@ -1056,26 +1056,45 @@ RemoveModHandle (
>  return NULL;
> }
> 
> +typedef struct {
> +  UINTN   Base;
> +  UINT32  Size;
> +  UINT32  Flags;
> +} IMAGE_SECTION_DATA;
> +
> VOID
> EFIAPI
> PeCoffLoaderRelocateImageExtraAction (
>  IN OUT PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
>  )
> {
> -  EFI_STATUS  Status;
> -  VOID        *DllEntryPoint;
> -  CHAR16      *DllFileName;
> -  HMODULE     Library;
> -  UINTN       Index;
> +  EFI_STATUS                          Status;
> +  VOID                                *DllEntryPoint;
> +  CHAR16                              *DllFileName;
> +  HMODULE                             Library;
> +  UINTN                               Index;
> +  PE_COFF_LOADER_IMAGE_CONTEXT        PeCoffImageContext;
> +  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
> +  EFI_IMAGE_SECTION_HEADER            *FirstSection;
> +  EFI_IMAGE_SECTION_HEADER            *Section;
> +  IMAGE_SECTION_DATA                  *SectionData;
> +  UINTN                               NumberOfSections;
> +  UINTN                               Base;
> +  UINTN                               End;
> +  UINTN                               RegionBase;
> +  UINTN                               RegionSize;
> +  UINT32                              Flags;
> +  DWORD                               NewProtection;
> +  DWORD                               OldProtection;
> 
>  ASSERT (ImageContext != NULL);
>  //
> -  // If we load our own PE COFF images the Windows debugger can not source
> -  //  level debug our code. If a valid PDB pointer exists use it to load
> -  //  the *.dll file as a library using Windows* APIs. This allows
> -  //  source level debug. The image is still loaded and relocated
> -  //  in the Framework memory space like on a real system (by the code above),
> -  //  but the entry point points into the DLL loaded by the code below.
> +  // If we load our own PE/COFF images the Windows debugger can not source
> +  // level debug our code. If a valid PDB pointer exists use it to load
> +  // the *.dll file as a library using Windows* APIs. This allows
> +  // source level debug. The image is still loaded and relocated
> +  // in the Framework memory space like on a real system (by the code above),
> +  // but the entry point points into the DLL loaded by the code below.
>  //
> 
>  DllEntryPoint = NULL;
> @@ -1106,27 +1125,166 @@ PeCoffLoaderRelocateImageExtraAction (
>    }
> 
>    //
> -    // Replace .PDB with .DLL on the filename
> +    // Replace .PDB with .DLL in the filename
>    //
>    DllFileName[Index - 3] = 'D';
>    DllFileName[Index - 2] = 'L';
>    DllFileName[Index - 1] = 'L';
> 
>    //
> -    // Load the .DLL file into the user process's address space for source
> -    // level debug
> +    // Load the .DLL file into the process's address space for source level
> +    // debug.
> +    //
> +    // EFI modules use the PE32 entry point for a different purpose than
> +    // Windows. For Windows DLLs, the PE entry point is used for the DllMain()
> +    // function. DllMain() has a very specific purpose; it initializes runtime
> +    // libraries, instance data, and thread local storage. LoadLibrary()/
> +    // LoadLibraryEx() will run the PE32 entry point and assume it to be a
> +    // DllMain() implementation by default. By passing the
> +    // DONT_RESOLVE_DLL_REFERENCES argument to LoadLibraryEx(), the execution
> +    // of the entry point as a DllMain() function will be suppressed. This
> +    // also prevents other modules that are referenced by the DLL from being
> +    // loaded. We use LoadLibraryEx() to create a copy of the PE32
> +    // image that the OS (and therefore the debugger) is aware of.
> +    // Source level debugging is the only reason to do this.
>    //
>    Library = LoadLibraryEx (DllFileName, NULL, DONT_RESOLVE_DLL_REFERENCES);
>    if (Library != NULL) {
>      //
> -      // InitializeDriver is the entry point we put in all our EFI DLL's. The
> -      // DONT_RESOLVE_DLL_REFERENCES argument to LoadLIbraryEx() suppresses the
> -      // normal DLL entry point of DllMain, and prevents other modules that are
> -      // referenced in side the DllFileName from being loaded. There is no error
> -      // checking as the we can point to the PE32 image loaded by Tiano. This
> -      // step is only needed for source level debugging
> +      // Parse the PE32 image loaded by the OS and find the entry point
>      //
> -      DllEntryPoint = (VOID *)(UINTN)GetProcAddress (Library, "InitializeDriver");
> +      ZeroMem (&PeCoffImageContext, sizeof (PeCoffImageContext));
> +      PeCoffImageContext.Handle = Library;
> +      PeCoffImageContext.ImageRead = PeCoffLoaderImageReadFromMemory;
> +      Status = PeCoffLoaderGetImageInfo (&PeCoffImageContext);
> +      if (EFI_ERROR (Status) || (PeCoffImageContext.ImageError != IMAGE_ERROR_SUCCESS)) {
> +        SecPrint ("DLL is not a valid PE/COFF image.\n\r");
> +        FreeLibrary (Library);
> +        Library = NULL;
> +      } else {
> +        Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)Library + (UINTN)PeCoffImageContext.PeCoffHeaderOffset);
> +        if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +          //
> +          // Use PE32 offset
> +          //
> +          DllEntryPoint = (VOID *) ((UINTN)Library + (UINTN)Hdr.Pe32->OptionalHeader.AddressOfEntryPoint);
> +        } else {
> +          //
> +          // Use PE32+ offset
> +          //
> +          DllEntryPoint = (VOID *) ((UINTN)Library + (UINTN)Hdr.Pe32Plus->OptionalHeader.AddressOfEntryPoint);
> +        }
> +        //
> +        // Now we need to configure memory access for the copy of the PE32 image
> +        // loaded by the OS.
> +        //
> +        // Most Windows DLLs are linked with sections 4KB aligned but EFI
> +        // modules are not to reduce size. Because of this we need to compute
> +        // the union of memory access attributes and explicitly configure
> +        // each page.
> +        //
> +        FirstSection = (EFI_IMAGE_SECTION_HEADER *)(
> +                                            (UINTN)Library +
> +                                            PeCoffImageContext.PeCoffHeaderOffset +
> +                                            sizeof (UINT32) +
> +                                            sizeof (EFI_IMAGE_FILE_HEADER) +
> +                                            Hdr.Pe32->FileHeader.SizeOfOptionalHeader
> +                                            );
> +        NumberOfSections = (UINTN)(Hdr.Pe32->FileHeader.NumberOfSections);
> +        Section = FirstSection;
> +        SectionData = malloc (NumberOfSections * sizeof (IMAGE_SECTION_DATA));
> +        if (SectionData == NULL) {
> +          FreeLibrary (Library);
> +          Library = NULL;
> +          DllEntryPoint = NULL;
> +        }
> +        ZeroMem (SectionData, NumberOfSections * sizeof (IMAGE_SECTION_DATA));
> +        //
> +        // Extract the section data from the PE32 image
> +        //
> +        for (Index = 0; Index < NumberOfSections; Index++) {
> +          SectionData[Index].Base = (UINTN)Library + Section->VirtualAddress;
> +          SectionData[Index].Size = Section->Misc.VirtualSize;
> +          if (SectionData[Index].Size == 0) {
> +            SectionData[Index].Size = Section->SizeOfRawData;
> +          }
> +          SectionData[Index].Flags = (Section->Characteristics &
> +                                      (EFI_IMAGE_SCN_MEM_EXECUTE | EFI_IMAGE_SCN_MEM_WRITE));
> +          Section += 1;
> +        }
> +        //
> +        // Loop over every DWORD in memory and compute the union of the memory
> +        // access bits.
> +        //
> +        End = (UINTN)Library + (UINTN)PeCoffImageContext.ImageSize;
> +        RegionBase = (UINTN)Library;
> +        RegionSize = 0;
> +        Flags = 0;
> +        for (Base = (UINTN)Library + sizeof (UINT32); Base < End; Base += sizeof (UINT32)) {
> +          for (Index = 0; Index < NumberOfSections; Index++) {
> +            if (SectionData[Index].Base <= Base &&
> +                (SectionData[Index].Base + SectionData[Index].Size) > Base) {
> +              Flags |= SectionData[Index].Flags;
> +            }
> +          }
> +          //
> +          // When a new page is reached configure the memory access for the
> +          // previous page.
> +          //
> +          if (Base % SIZE_4KB == 0) {
> +            RegionSize += SIZE_4KB;
> +            if ((Flags & EFI_IMAGE_SCN_MEM_WRITE) == EFI_IMAGE_SCN_MEM_WRITE) {
> +              if ((Flags & EFI_IMAGE_SCN_MEM_EXECUTE) == EFI_IMAGE_SCN_MEM_EXECUTE) {
> +                NewProtection = PAGE_EXECUTE_READWRITE;
> +              } else {
> +                NewProtection = PAGE_READWRITE;
> +              }
> +            } else {
> +              if ((Flags & EFI_IMAGE_SCN_MEM_EXECUTE) == EFI_IMAGE_SCN_MEM_EXECUTE) {
> +                NewProtection = PAGE_EXECUTE_READ;
> +              } else {
> +                NewProtection = PAGE_READONLY;
> +              }
> +            }
> +            if (!VirtualProtect ((LPVOID)RegionBase, (SIZE_T) RegionSize, NewProtection, &OldProtection)) {
> +              SecPrint ("Setting PE32 Section Access Failed\n\r");
> +              FreeLibrary (Library);
> +              free (SectionData);
> +              Library = NULL;
> +              DllEntryPoint = NULL;
> +              break;
> +            }
> +            Flags = 0;
> +            RegionBase = Base;
> +            RegionSize = 0;
> +          }
> +        }
> +        free (SectionData);
> +        //
> +        // Configure the last partial page
> +        //
> +        if (Library != NULL && (End - RegionBase) > 0) {
> +          if ((Flags & EFI_IMAGE_SCN_MEM_WRITE) == EFI_IMAGE_SCN_MEM_WRITE) {
> +            if ((Flags & EFI_IMAGE_SCN_MEM_EXECUTE) == EFI_IMAGE_SCN_MEM_EXECUTE) {
> +              NewProtection = PAGE_EXECUTE_READWRITE;
> +            } else {
> +              NewProtection = PAGE_READWRITE;
> +            }
> +          } else {
> +            if ((Flags & EFI_IMAGE_SCN_MEM_EXECUTE) == EFI_IMAGE_SCN_MEM_EXECUTE) {
> +              NewProtection = PAGE_EXECUTE_READ;
> +            } else {
> +              NewProtection = PAGE_READONLY;
> +            }
> +          }
> +          if (!VirtualProtect ((LPVOID)RegionBase, (SIZE_T) (End - RegionBase), NewProtection, &OldProtection)) {
> +            SecPrint ("Setting PE32 Section Access Failed\n\r");
> +            FreeLibrary (Library);
> +            Library = NULL;
> +            DllEntryPoint = NULL;
> +          }
> +        }
> +      }
>    }
> 
>    if ((Library != NULL) && (DllEntryPoint != NULL)) {
> @@ -1142,7 +1300,7 @@ PeCoffLoaderRelocateImageExtraAction (
>        // This DLL is not already loaded, so source level debugging is supported.
>        //
>        ImageContext->EntryPoint = (EFI_PHYSICAL_ADDRESS)(UINTN)DllEntryPoint;
> -        SecPrint ("LoadLibraryEx (\n\r  %S,\n\r  NULL, DONT_RESOLVE_DLL_REFERENCES)\n\r", DllFileName);
> +        SecPrint ("LoadLibraryEx (\n\r  %S,\n\r  NULL, DONT_RESOLVE_DLL_REFERENCES) @ 0x%X\n\r", DllFileName, (int) (UINTN) Library);
>      }
>    } else {
>      SecPrint ("WARNING: No source level debug %S. \n\r", DllFileName);
> -- 
> 2.39.2.windows.1
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109025): https://edk2.groups.io/g/devel/message/109025
Mute This Topic: https://groups.io/mt/101531560/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2023-09-23 15:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-22 22:49 [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows Nate DeSimone
2023-09-23 15:16 ` Andrew Fish via groups.io [this message]
2023-09-26 20:07 ` Michael D Kinney
2023-09-26 20:32   ` Nate DeSimone
2023-09-26 21:28     ` Michael D Kinney
2023-09-26 21:54       ` Nate DeSimone
2023-09-26 22:06         ` Michael D Kinney
2023-09-27  3:45 ` Ni, Ray
     [not found]   ` <MW4PR11MB582123DDAAD0FED98300C3C9CDC1A@MW4PR11MB5821.namprd11.prod.outlook.com>
2023-09-28 22:19     ` Nate DeSimone
2023-09-29  3:30       ` Ni, Ray

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BA0E7E8B-390D-49E6-98D7-4C530086DEDB@apple.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox