public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
@ 2023-09-22 22:49 Nate DeSimone
  2023-09-23 15:16 ` Andrew Fish via groups.io
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Nate DeSimone @ 2023-09-22 22:49 UTC (permalink / raw)
  To: devel; +Cc: Andrew Fish, Ray Ni, Michael D Kinney, Chasel Chiu

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 (#109019): https://edk2.groups.io/g/devel/message/109019
Mute This Topic: https://groups.io/mt/101531560/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
  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
  2023-09-26 20:07 ` Michael D Kinney
  2023-09-27  3:45 ` Ni, Ray
  2 siblings, 0 replies; 10+ messages in thread
From: Andrew Fish via groups.io @ 2023-09-23 15:16 UTC (permalink / raw)
  To: edk2-devel-groups-io, Desimone, Nathaniel L
  Cc: Ray Ni, Mike Kinney, Chasel Chiu

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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
  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
@ 2023-09-26 20:07 ` Michael D Kinney
  2023-09-26 20:32   ` Nate DeSimone
  2023-09-27  3:45 ` Ni, Ray
  2 siblings, 1 reply; 10+ messages in thread
From: Michael D Kinney @ 2023-09-26 20:07 UTC (permalink / raw)
  To: Desimone, Nathaniel L, devel@edk2.groups.io
  Cc: Andrew Fish, Ni, Ray, Chiu, Chasel, Kinney, Michael D

Hi Nate,

I am able to do source level debug of EmulatorPkg using VS Code today.

What scenarios are broken?

I do know that the DLL based approach would only allow a single
instance of the module to be loaded and debugged.  If, for example,
a driver is loaded more than once from the UEFI Shell in the
EmulatorPkg env, the 2nd driver would use the first DLL which 
does not match the PI spec behavior.

It also appears that this change can support PE/COFF images 
that do not have sections that are 4KB aligned and handles
the page protection settings for the user mode application
env.  Is that correct?

Mike

> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Friday, September 22, 2023 3:49 PM
> To: devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>
> Subject: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> 
> 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_chai
> n>/X64/WinHost",
>       "args": [],
>       "stopAtEntry": false,
>       "cwd":
> "${workspaceFolder}/<path_to_build>/Build/EmulatorX64/DEBUG_<tool_chai
> n>/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 (#109088): https://edk2.groups.io/g/devel/message/109088
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
  2023-09-26 20:07 ` Michael D Kinney
@ 2023-09-26 20:32   ` Nate DeSimone
  2023-09-26 21:28     ` Michael D Kinney
  0 siblings, 1 reply; 10+ messages in thread
From: Nate DeSimone @ 2023-09-26 20:32 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io
  Cc: Andrew Fish, Ni, Ray, Chiu, Chasel

Hi Mike,

Source level debug with VS Code does indeed work today with gdb or lldb. This change makes the Visual Studio Windows debugger work as well.

You are correct that if the same DLL is loaded more than once that this method cannot perform source level debug on the second instance; but this change won't break that scenario either. If the same DLL occurs twice, then we will use the PE/COFF image loaded by either the PEI core or DXE core instead of the one loaded by Windows. This means that the second instance of the DLL will not be source level debug-able by Visual Studio; but PI-spec compliance is maintained. This behavior is unchanged from the original code.

Yes, this code enables PE/COFF images that do not have sections that are 4KB aligned. It will setup page protection for user mode. Without this change you must turn off the NX bit when using the Visual Studio Windows debugger.

Thanks,
Nate

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Tuesday, September 26, 2023 1:08 PM
To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; devel@edk2.groups.io
Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows

Hi Nate,

I am able to do source level debug of EmulatorPkg using VS Code today.

What scenarios are broken?

I do know that the DLL based approach would only allow a single instance of the module to be loaded and debugged.  If, for example, a driver is loaded more than once from the UEFI Shell in the EmulatorPkg env, the 2nd driver would use the first DLL which does not match the PI spec behavior.

It also appears that this change can support PE/COFF images that do not have sections that are 4KB aligned and handles the page protection settings for the user mode application env.  Is that correct?

Mike

> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Friday, September 22, 2023 3:49 PM
> To: devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Kinney, 
> Michael D <michael.d.kinney@intel.com>; Chiu, Chasel 
> <chasel.chiu@intel.com>
> Subject: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> 
> 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_chai
> n>/X64/WinHost",
>       "args": [],
>       "stopAtEntry": false,
>       "cwd":
> "${workspaceFolder}/<path_to_build>/Build/EmulatorX64/DEBUG_<tool_chai
> n>/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 (#109089): https://edk2.groups.io/g/devel/message/109089
Mute This Topic: https://groups.io/mt/101531560/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
  2023-09-26 20:32   ` Nate DeSimone
@ 2023-09-26 21:28     ` Michael D Kinney
  2023-09-26 21:54       ` Nate DeSimone
  0 siblings, 1 reply; 10+ messages in thread
From: Michael D Kinney @ 2023-09-26 21:28 UTC (permalink / raw)
  To: Desimone, Nathaniel L, devel@edk2.groups.io
  Cc: Andrew Fish, Ni, Ray, Chiu, Chasel, Kinney, Michael D

I have VS Code on Windows working today using the PDB based symbol file.

The following lines in the EmulatorPkg DSC file force 4KB alignment and the DLL export of the InitializeDriver symbol

https://github.com/tianocore/edk2/blob/bf0bdacdd6f6cdd2e9ac5db14b6daf19a5a5bd57/EmulatorPkg/EmulatorPkg.dsc#L497

  MSFT:*_*_*_DLINK_FLAGS     = /ALIGN:4096 /FILEALIGN:4096 /SUBSYSTEM:CONSOLE
  MSFT:DEBUG_*_*_DLINK_FLAGS = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
  MSFT:NOOPT_*_*_DLINK_FLAGS = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000


I think this is why it is working for me without this change.

Are you suggesting that with this change these can be removed?

Mike

> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Tuesday, September 26, 2023 1:32 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>
> Subject: RE: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> 
> Hi Mike,
> 
> Source level debug with VS Code does indeed work today with gdb or
> lldb. This change makes the Visual Studio Windows debugger work as
> well.
> 
> You are correct that if the same DLL is loaded more than once that
> this method cannot perform source level debug on the second instance;
> but this change won't break that scenario either. If the same DLL
> occurs twice, then we will use the PE/COFF image loaded by either the
> PEI core or DXE core instead of the one loaded by Windows. This means
> that the second instance of the DLL will not be source level debug-
> able by Visual Studio; but PI-spec compliance is maintained. This
> behavior is unchanged from the original code.
> 
> Yes, this code enables PE/COFF images that do not have sections that
> are 4KB aligned. It will setup page protection for user mode. Without
> this change you must turn off the NX bit when using the Visual Studio
> Windows debugger.
> 
> Thanks,
> Nate
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, September 26, 2023 1:08 PM
> To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>;
> devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> 
> Hi Nate,
> 
> I am able to do source level debug of EmulatorPkg using VS Code today.
> 
> What scenarios are broken?
> 
> I do know that the DLL based approach would only allow a single
> instance of the module to be loaded and debugged.  If, for example, a
> driver is loaded more than once from the UEFI Shell in the EmulatorPkg
> env, the 2nd driver would use the first DLL which does not match the
> PI spec behavior.
> 
> It also appears that this change can support PE/COFF images that do
> not have sections that are 4KB aligned and handles the page protection
> settings for the user mode application env.  Is that correct?
> 
> Mike
> 
> > -----Original Message-----
> > From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> > Sent: Friday, September 22, 2023 3:49 PM
> > To: devel@edk2.groups.io
> > Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>;
> Kinney,
> > Michael D <michael.d.kinney@intel.com>; Chiu, Chasel
> > <chasel.chiu@intel.com>
> > Subject: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> >
> > 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_chai
> > n>/X64/WinHost",
> >       "args": [],
> >       "stopAtEntry": false,
> >       "cwd":
> >
> "${workspaceFolder}/<path_to_build>/Build/EmulatorX64/DEBUG_<tool_chai
> > n>/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 (#109093): https://edk2.groups.io/g/devel/message/109093
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
  2023-09-26 21:28     ` Michael D Kinney
@ 2023-09-26 21:54       ` Nate DeSimone
  2023-09-26 22:06         ` Michael D Kinney
  0 siblings, 1 reply; 10+ messages in thread
From: Nate DeSimone @ 2023-09-26 21:54 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io
  Cc: Andrew Fish, Ni, Ray, Chiu, Chasel

Hi Mike,

Yes, we can delete those lines now 😊. I have those lines commented out in my copy of EmulatorPkg. I found this because I have been trying to use EmulatorPkg to source level debug drivers that I built separately via one of the edk2-platform builds.

Thanks,
Nate

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Tuesday, September 26, 2023 2:29 PM
To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; devel@edk2.groups.io
Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows

I have VS Code on Windows working today using the PDB based symbol file.

The following lines in the EmulatorPkg DSC file force 4KB alignment and the DLL export of the InitializeDriver symbol

https://github.com/tianocore/edk2/blob/bf0bdacdd6f6cdd2e9ac5db14b6daf19a5a5bd57/EmulatorPkg/EmulatorPkg.dsc#L497

  MSFT:*_*_*_DLINK_FLAGS     = /ALIGN:4096 /FILEALIGN:4096 /SUBSYSTEM:CONSOLE
  MSFT:DEBUG_*_*_DLINK_FLAGS = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
  MSFT:NOOPT_*_*_DLINK_FLAGS = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000


I think this is why it is working for me without this change.

Are you suggesting that with this change these can be removed?

Mike

> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Tuesday, September 26, 2023 1:32 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; 
> devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Chiu, 
> Chasel <chasel.chiu@intel.com>
> Subject: RE: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> 
> Hi Mike,
> 
> Source level debug with VS Code does indeed work today with gdb or 
> lldb. This change makes the Visual Studio Windows debugger work as 
> well.
> 
> You are correct that if the same DLL is loaded more than once that 
> this method cannot perform source level debug on the second instance; 
> but this change won't break that scenario either. If the same DLL 
> occurs twice, then we will use the PE/COFF image loaded by either the 
> PEI core or DXE core instead of the one loaded by Windows. This means 
> that the second instance of the DLL will not be source level debug- 
> able by Visual Studio; but PI-spec compliance is maintained. This 
> behavior is unchanged from the original code.
> 
> Yes, this code enables PE/COFF images that do not have sections that 
> are 4KB aligned. It will setup page protection for user mode. Without 
> this change you must turn off the NX bit when using the Visual Studio 
> Windows debugger.
> 
> Thanks,
> Nate
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, September 26, 2023 1:08 PM
> To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; 
> devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Chiu, 
> Chasel <chasel.chiu@intel.com>; Kinney, Michael D 
> <michael.d.kinney@intel.com>
> Subject: RE: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> 
> Hi Nate,
> 
> I am able to do source level debug of EmulatorPkg using VS Code today.
> 
> What scenarios are broken?
> 
> I do know that the DLL based approach would only allow a single 
> instance of the module to be loaded and debugged.  If, for example, a 
> driver is loaded more than once from the UEFI Shell in the EmulatorPkg 
> env, the 2nd driver would use the first DLL which does not match the 
> PI spec behavior.
> 
> It also appears that this change can support PE/COFF images that do 
> not have sections that are 4KB aligned and handles the page protection 
> settings for the user mode application env.  Is that correct?
> 
> Mike
> 
> > -----Original Message-----
> > From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> > Sent: Friday, September 22, 2023 3:49 PM
> > To: devel@edk2.groups.io
> > Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>;
> Kinney,
> > Michael D <michael.d.kinney@intel.com>; Chiu, Chasel 
> > <chasel.chiu@intel.com>
> > Subject: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> >
> > 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_chai
> > n>/X64/WinHost",
> >       "args": [],
> >       "stopAtEntry": false,
> >       "cwd":
> >
> "${workspaceFolder}/<path_to_build>/Build/EmulatorX64/DEBUG_<tool_chai
> > n>/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 (#109095): https://edk2.groups.io/g/devel/message/109095
Mute This Topic: https://groups.io/mt/101531560/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
  2023-09-26 21:54       ` Nate DeSimone
@ 2023-09-26 22:06         ` Michael D Kinney
  0 siblings, 0 replies; 10+ messages in thread
From: Michael D Kinney @ 2023-09-26 22:06 UTC (permalink / raw)
  To: Desimone, Nathaniel L, devel@edk2.groups.io
  Cc: Andrew Fish, Ni, Ray, Chiu, Chasel, Kinney, Michael D

Hi Nate,

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

That makes complete sense now on the use case when building 
drivers or apps outside EmulatorPkg DSC file and wanting
to support source level debug.  In the past, I have had to
add components to EmulatorPkg DSC file.

Thank you for adding feature to support source level debug
of components built outside EmulatorPkg.

Thanks,

Mike

> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Tuesday, September 26, 2023 2:55 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>
> Subject: RE: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> 
> Hi Mike,
> 
> Yes, we can delete those lines now 😊. I have those lines commented
> out in my copy of EmulatorPkg. I found this because I have been trying
> to use EmulatorPkg to source level debug drivers that I built
> separately via one of the edk2-platform builds.
> 
> Thanks,
> Nate
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, September 26, 2023 2:29 PM
> To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>;
> devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> 
> I have VS Code on Windows working today using the PDB based symbol
> file.
> 
> The following lines in the EmulatorPkg DSC file force 4KB alignment
> and the DLL export of the InitializeDriver symbol
> 
> https://github.com/tianocore/edk2/blob/bf0bdacdd6f6cdd2e9ac5db14b6daf1
> 9a5a5bd57/EmulatorPkg/EmulatorPkg.dsc#L497
> 
>   MSFT:*_*_*_DLINK_FLAGS     = /ALIGN:4096 /FILEALIGN:4096
> /SUBSYSTEM:CONSOLE
>   MSFT:DEBUG_*_*_DLINK_FLAGS =
> /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
>   MSFT:NOOPT_*_*_DLINK_FLAGS =
> /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
> 
> 
> I think this is why it is working for me without this change.
> 
> Are you suggesting that with this change these can be removed?
> 
> Mike
> 
> > -----Original Message-----
> > From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> > Sent: Tuesday, September 26, 2023 1:32 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Chiu,
> > Chasel <chasel.chiu@intel.com>
> > Subject: RE: [PATCH v1] EmulatorPkg: Fix Source Level Debug on
> Windows
> >
> > Hi Mike,
> >
> > Source level debug with VS Code does indeed work today with gdb or
> > lldb. This change makes the Visual Studio Windows debugger work as
> > well.
> >
> > You are correct that if the same DLL is loaded more than once that
> > this method cannot perform source level debug on the second
> instance;
> > but this change won't break that scenario either. If the same DLL
> > occurs twice, then we will use the PE/COFF image loaded by either
> the
> > PEI core or DXE core instead of the one loaded by Windows. This
> means
> > that the second instance of the DLL will not be source level debug-
> > able by Visual Studio; but PI-spec compliance is maintained. This
> > behavior is unchanged from the original code.
> >
> > Yes, this code enables PE/COFF images that do not have sections that
> > are 4KB aligned. It will setup page protection for user mode.
> Without
> > this change you must turn off the NX bit when using the Visual
> Studio
> > Windows debugger.
> >
> > Thanks,
> > Nate
> >
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Tuesday, September 26, 2023 1:08 PM
> > To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>;
> > devel@edk2.groups.io
> > Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Chiu,
> > Chasel <chasel.chiu@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: RE: [PATCH v1] EmulatorPkg: Fix Source Level Debug on
> Windows
> >
> > Hi Nate,
> >
> > I am able to do source level debug of EmulatorPkg using VS Code
> today.
> >
> > What scenarios are broken?
> >
> > I do know that the DLL based approach would only allow a single
> > instance of the module to be loaded and debugged.  If, for example,
> a
> > driver is loaded more than once from the UEFI Shell in the
> EmulatorPkg
> > env, the 2nd driver would use the first DLL which does not match the
> > PI spec behavior.
> >
> > It also appears that this change can support PE/COFF images that do
> > not have sections that are 4KB aligned and handles the page
> protection
> > settings for the user mode application env.  Is that correct?
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> > > Sent: Friday, September 22, 2023 3:49 PM
> > > To: devel@edk2.groups.io
> > > Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>;
> > Kinney,
> > > Michael D <michael.d.kinney@intel.com>; Chiu, Chasel
> > > <chasel.chiu@intel.com>
> > > Subject: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> > >
> > > 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_chai
> > > n>/X64/WinHost",
> > >       "args": [],
> > >       "stopAtEntry": false,
> > >       "cwd":
> > >
> >
> "${workspaceFolder}/<path_to_build>/Build/EmulatorX64/DEBUG_<tool_chai
> > > n>/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 (#109096): https://edk2.groups.io/g/devel/message/109096
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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
  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
  2023-09-26 20:07 ` Michael D Kinney
@ 2023-09-27  3:45 ` Ni, Ray
       [not found]   ` <MW4PR11MB582123DDAAD0FED98300C3C9CDC1A@MW4PR11MB5821.namprd11.prod.outlook.com>
  2 siblings, 1 reply; 10+ messages in thread
From: Ni, Ray @ 2023-09-27  3:45 UTC (permalink / raw)
  To: Desimone, Nathaniel L, devel@edk2.groups.io
  Cc: Andrew Fish, Kinney, Michael D, Chiu, Chasel

[-- Attachment #1: Type: text/plain, Size: 16569 bytes --]

Nate,

Thanks for the great patch! Minor comments:

  1.  Regarding the DllEntryPoint, what's the difference between below line in your patch and the original code that calls GetProcAddress()?
     *   DllEntryPoint = (VOID *) ((UINTN)Library + (UINTN)Hdr.Pe32Plus->OptionalHeader.AddressOfEntryPoint);
  2.  Does it avoid relying on each driver exporting its entrypoint with a fixed symbol name "InitializeDriver"?
Does the new DllEntryPoint equal to the original one retrieved from GetProcAddress()?
What else benifits? (Just curious)
Can it be in a separate patch so that future readers can easily understand the purpose of the change?
  3.  It seems the patch assumes the handle returned from LoadLibrary() is the address of the loaded DLL in memory. I tried to find in MSDN if any doc supports this assumption but failed. Can you provide any?
I am ok if the assumption is based on the current LoadLibrary() implementation. But can you please explicitly mention that assumption in comments?

Thanks,
Ray
________________________________
From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Sent: Saturday, September 23, 2023 6:49 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Andrew Fish <afish@apple.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
Subject: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows

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 (#109100): https://edk2.groups.io/g/devel/message/109100
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]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 34670 bytes --]

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
       [not found]   ` <MW4PR11MB582123DDAAD0FED98300C3C9CDC1A@MW4PR11MB5821.namprd11.prod.outlook.com>
@ 2023-09-28 22:19     ` Nate DeSimone
  2023-09-29  3:30       ` Ni, Ray
  0 siblings, 1 reply; 10+ messages in thread
From: Nate DeSimone @ 2023-09-28 22:19 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io
  Cc: Andrew Fish, Kinney, Michael D, Chiu, Chasel

Hi Ray,

Responses inline below.

Thanks,
Nate

> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, September 26, 2023 8:46 PM
> To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>;
> devel@edk2.groups.io
> Cc: Andrew Fish <afish@apple.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
> Subject: Re: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> 
> Nate,
> 
> Thanks for the great patch! Minor comments:
>
> 1. Regarding the DllEntryPoint, what's the difference between below line in
> your patch and the original code that calls GetProcAddress()?
> a. DllEntryPoint = (VOID *) ((UINTN)Library + (UINTN)Hdr.Pe32Plus-
> >OptionalHeader.AddressOfEntryPoint);

The new code reads the driver's entry point from the PE32 optional header's AddressOfEntryPoint, the old code reads the driver's entry point by locating the .edata section and enumerating the entries in the export table until one with the name "InitializeDriver" is found.

> 2. Does it avoid relying on each driver exporting its entrypoint with a fixed
> symbol name "InitializeDriver"?

Yes.

> Does the new DllEntryPoint equal to the original one retrieved from
> GetProcAddress()?

Assuming the following line is added to [BuildOptions]:

  MSFT:DEBUG_*_*_DLINK_FLAGS = /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT)

Then yes, they will be identical. But in the general case, there is no specification level requirement and therefore no guarantee that this export entry will exist, or that it will point to the same location.

> What else benifits? (Just curious)

Section 2.1.1 of the UEFI specification states the following:

<snip>
Once the image is loaded into memory, and the appropriate
fix-ups have been performed, control is transferred to a loaded image at the AddressOfEntryPoint
reference according to the normal indirect calling conventions of applications based on supported 32-bit,
64-bit, or 128-bit processors. All other linkage to and from an UEFI image is done programmatically.
</snip>

Therefore, adding this patch will make EmulatorPkg's source level debug support compliant with the UEFI specification.

> Can it be in a separate patch so that future readers can easily understand the
> purpose of the change?

I would not recommend it for bisect-ability reasons because this EmulatorPkg will crash if the page table fixups are not done. It appears that the act of calling GetProcAddress() also causes the OS to set up the page table correctly.

> 3. It seems the patch assumes the handle returned from LoadLibrary() is the
> address of the loaded DLL in memory. I tried to find in MSDN if any doc
> supports this assumption but failed. Can you provide any?
> I am ok if the assumption is based on the current LoadLibrary()
> implementation. But can you please explicitly mention that assumption in
> comments?

It is very buried, but MSDN does explicitly state that the HMODULE data type will always contain the load address of that module. It is discussed here in the remarks section: https://learn.microsoft.com/en-us/windows/win32/api/psapi/ns-psapi-moduleinfo

> 
> Thanks,
> Ray
> ________________________________________
> From: Desimone, Nathaniel L <mailto:nathaniel.l.desimone@intel.com>
> Sent: Saturday, September 23, 2023 6:49 AM
> To: mailto:devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> Cc: Andrew Fish <mailto:afish@apple.com>; Ni, Ray
> <mailto:ray.ni@intel.com>; Kinney, Michael D
> <mailto:michael.d.kinney@intel.com>; Chiu, Chasel
> <mailto:chasel.chiu@intel.com>
> Subject: [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
> 
> 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_cha
> in>/X64/WinHost",
>       "args": [],
>       "stopAtEntry": false,
>       "cwd":
> "${workspaceFolder}/<path_to_build>/Build/EmulatorX64/DEBUG_<tool_cha
> in>/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 <mailto:afish@apple.com>
> Cc: Ray Ni <mailto:ray.ni@intel.com>
> Cc: Michael D Kinney <mailto:michael.d.kinney@intel.com>
> Cc: Chasel Chiu <mailto:chasel.chiu@intel.com>
> Signed-off-by: Nate DeSimone <mailto: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 (#109181): https://edk2.groups.io/g/devel/message/109181
Mute This Topic: https://groups.io/mt/101531560/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [edk2-devel] [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows
  2023-09-28 22:19     ` Nate DeSimone
@ 2023-09-29  3:30       ` Ni, Ray
  0 siblings, 0 replies; 10+ messages in thread
From: Ni, Ray @ 2023-09-29  3:30 UTC (permalink / raw)
  To: Nate DeSimone, devel

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

Thank you Nate!
Reviewed-by: Ray Ni <ray.ni@intel.com>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109182): https://edk2.groups.io/g/devel/message/109182
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]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 928 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-09-29  3:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox