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]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent 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