<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">
Nate,</div>
<div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">
<br>
</div>
<div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof">
Thanks for the great patch! Minor comments:</div>
<div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof ContentPasted0">
<ol data-editing-info="{"orderedStyleType":1,"unorderedStyleType":1}" data-listchain="__List_Chain_188">
<li style="list-style-type: "1. ";"><span>Regarding the DllEntryPoint, what's the difference between below line in your patch and the original code that calls GetProcAddress()?</span></li><ol style="list-style-type: lower-alpha;">
<li style="display: block;">DllEntryPoint = (VOID *) ((UINTN)Library + (UINTN)Hdr.Pe32Plus->OptionalHeader.AddressOfEntryPoint);</li></ol>
<li style="display: block; list-style-type: "2. "; font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; color: rgb(36, 36, 36);">
Does it avoid relying on each driver exporting its entrypoint with a fixed symbol name "<span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; text-align: start; display: inline !important; color: rgb(36, 36, 36); background-color: rgb(255, 255, 255);" class="ContentPasted1">InitializeDriver"?</span>
<div style="list-style-type: "2. "; font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; color: rgb(36, 36, 36);">
<span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; text-align: start; display: inline !important; color: rgb(36, 36, 36); background-color: rgb(255, 255, 255);" class="ContentPasted1">Does
the new DllEntryPoint equal to the original one retrieved from GetProcAddress()?</span></div>
<div style="list-style-type: "2. "; font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; color: rgb(36, 36, 36);">
<span style="font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; text-align: start; display: inline !important; color: rgb(36, 36, 36); background-color: rgb(255, 255, 255);" class="ContentPasted1">What
else benifits? (Just curious)</span></div>
<div style="list-style-type: "2. "; font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; color: rgb(36, 36, 36);">
Can it be in a separate patch so that future readers can easily understand the purpose of the change?</div>
</li><li style="list-style-type: "2. ";">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?
<div style="list-style-type: "3. ";">I am ok if the assumption is based on the current LoadLibrary() implementation. But can you please explicitly mention that assumption in comments?</div>
</li></ol>
<div><br>
<span></span></div>
</div>
<div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);" class="elementToProof ContentPasted0">
<span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">Thanks,</span><br>
</div>
<div class="elementToProof">
<div id="Signature">
<div style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
Ray</div>
</div>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com><br>
<b>Sent:</b> Saturday, September 23, 2023 6:49 AM<br>
<b>To:</b> devel@edk2.groups.io <devel@edk2.groups.io><br>
<b>Cc:</b> 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><br>
<b>Subject:</b> [PATCH v1] EmulatorPkg: Fix Source Level Debug on Windows</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">The Visual Studio Windows debugger will only load symbols for PE/COFF images<br>
that Windows is aware of. Therefore, to enable source level debugging, all<br>
PEI/DXE modules must be loaded via LoadLibrary() or LoadLibraryEx() and the<br>
the instance in memory created by LoadLibrary() must be the one that is<br>
actually executed.<br>
<br>
The current source level debug implementation in EmulatorPkg for Windows is<br>
inherited from the old Nt32Pkg. This implementation makes the assumption that<br>
all PEI/DXE modules have a DLL export tables with a symbol named<br>
InitializeDriver. Therefore, this source level debug implementation requires<br>
all modules to be linked in a non-PI spec defined manner. Support for adding<br>
the InitializeDriver symbol was removed in EmulatorPkg, which broke source<br>
level debugging.<br>
<br>
To fix this, the source level debugging implementation has been modified to<br>
use the PE/COFF entry point directly. This brings the implementation into<br>
compliance with the PI spec and should work with any PEIM/DXE driver.<br>
Implementing this requires parsing the in-memory instance of the PE/COFF image<br>
created by Windows to find the entrypoint and since PEIMs/DXE drivers are not<br>
garunteed to have 4KB aligned sections, it also requires explicit configuration<br>
of the page table using VirtualProtect().<br>
<br>
With this fix, the debugging experience is now so good it is unprecedented!<br>
In Visual Studio Code, add the following to launch.json:<br>
<br>
{<br>
"version": "0.2.0",<br>
"configurations": [<br>
{<br>
"name": "EmulatorPkg Launch",<br>
"type": "cppvsdbg",<br>
"request": "launch",<br>
"program": "${workspaceFolder}/<path_to_build>/Build/EmulatorX64/DEBUG_<tool_chain>/X64/WinHost",<br>
"args": [],<br>
"stopAtEntry": false,<br>
"cwd": "${workspaceFolder}/<path_to_build>/Build/EmulatorX64/DEBUG_<tool_chain>/X64/",<br>
"environment": [],<br>
"console": false,<br>
}<br>
]<br>
}<br>
<br>
Make modifications to the above template as nessesary and build EmulatorPkg.<br>
Now, just add breakpoints directly in Visual Studio Code the way you would with<br>
any other software project. When you start the debugger, it will halt at the<br>
breakpoint automatically without any extra configuration required.<br>
<br>
Cc: Andrew Fish <afish@apple.com><br>
Cc: Ray Ni <ray.ni@intel.com><br>
Cc: Michael D Kinney <michael.d.kinney@intel.com><br>
Cc: Chasel Chiu <chasel.chiu@intel.com><br>
Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com><br>
---<br>
EmulatorPkg/Win/Host/WinHost.c | 206 +++++++++++++++++++++++++++++----<br>
1 file changed, 182 insertions(+), 24 deletions(-)<br>
<br>
diff --git a/EmulatorPkg/Win/Host/WinHost.c b/EmulatorPkg/Win/Host/WinHost.c<br>
index 193a947fbd..e414da6c55 100644<br>
--- a/EmulatorPkg/Win/Host/WinHost.c<br>
+++ b/EmulatorPkg/Win/Host/WinHost.c<br>
@@ -8,7 +8,7 @@<br>
This code produces 128 K of temporary memory for the SEC stack by directly<br>
allocate memory space with ReadWrite and Execute attribute.<br>
<br>
-Copyright (c) 2006 - 2022, Intel Corporation. All rights reserved.<BR><br>
+Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.<BR><br>
(C) Copyright 2016-2020 Hewlett Packard Enterprise Development LP<BR><br>
SPDX-License-Identifier: BSD-2-Clause-Patent<br>
**/<br>
@@ -977,7 +977,7 @@ AddModHandle (<br>
for (Index = 0; Index < mPdbNameModHandleArraySize; Index++, Array++) {<br>
if (Array->PdbPointer == NULL) {<br>
//<br>
- // Make a copy of the stirng and store the ModHandle<br>
+ // Make a copy of the string and store the ModHandle<br>
//<br>
Handle = GetProcessHeap ();<br>
Size = AsciiStrLen (ImageContext->PdbPointer) + 1;<br>
@@ -1056,26 +1056,45 @@ RemoveModHandle (<br>
return NULL;<br>
}<br>
<br>
+typedef struct {<br>
+ UINTN Base;<br>
+ UINT32 Size;<br>
+ UINT32 Flags;<br>
+} IMAGE_SECTION_DATA;<br>
+<br>
VOID<br>
EFIAPI<br>
PeCoffLoaderRelocateImageExtraAction (<br>
IN OUT PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext<br>
)<br>
{<br>
- EFI_STATUS Status;<br>
- VOID *DllEntryPoint;<br>
- CHAR16 *DllFileName;<br>
- HMODULE Library;<br>
- UINTN Index;<br>
+ EFI_STATUS Status;<br>
+ VOID *DllEntryPoint;<br>
+ CHAR16 *DllFileName;<br>
+ HMODULE Library;<br>
+ UINTN Index;<br>
+ PE_COFF_LOADER_IMAGE_CONTEXT PeCoffImageContext;<br>
+ EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;<br>
+ EFI_IMAGE_SECTION_HEADER *FirstSection;<br>
+ EFI_IMAGE_SECTION_HEADER *Section;<br>
+ IMAGE_SECTION_DATA *SectionData;<br>
+ UINTN NumberOfSections;<br>
+ UINTN Base;<br>
+ UINTN End;<br>
+ UINTN RegionBase;<br>
+ UINTN RegionSize;<br>
+ UINT32 Flags;<br>
+ DWORD NewProtection;<br>
+ DWORD OldProtection;<br>
<br>
ASSERT (ImageContext != NULL);<br>
//<br>
- // If we load our own PE COFF images the Windows debugger can not source<br>
- // level debug our code. If a valid PDB pointer exists use it to load<br>
- // the *.dll file as a library using Windows* APIs. This allows<br>
- // source level debug. The image is still loaded and relocated<br>
- // in the Framework memory space like on a real system (by the code above),<br>
- // but the entry point points into the DLL loaded by the code below.<br>
+ // If we load our own PE/COFF images the Windows debugger can not source<br>
+ // level debug our code. If a valid PDB pointer exists use it to load<br>
+ // the *.dll file as a library using Windows* APIs. This allows<br>
+ // source level debug. The image is still loaded and relocated<br>
+ // in the Framework memory space like on a real system (by the code above),<br>
+ // but the entry point points into the DLL loaded by the code below.<br>
//<br>
<br>
DllEntryPoint = NULL;<br>
@@ -1106,27 +1125,166 @@ PeCoffLoaderRelocateImageExtraAction (<br>
}<br>
<br>
//<br>
- // Replace .PDB with .DLL on the filename<br>
+ // Replace .PDB with .DLL in the filename<br>
//<br>
DllFileName[Index - 3] = 'D';<br>
DllFileName[Index - 2] = 'L';<br>
DllFileName[Index - 1] = 'L';<br>
<br>
//<br>
- // Load the .DLL file into the user process's address space for source<br>
- // level debug<br>
+ // Load the .DLL file into the process's address space for source level<br>
+ // debug.<br>
+ //<br>
+ // EFI modules use the PE32 entry point for a different purpose than<br>
+ // Windows. For Windows DLLs, the PE entry point is used for the DllMain()<br>
+ // function. DllMain() has a very specific purpose; it initializes runtime<br>
+ // libraries, instance data, and thread local storage. LoadLibrary()/<br>
+ // LoadLibraryEx() will run the PE32 entry point and assume it to be a<br>
+ // DllMain() implementation by default. By passing the<br>
+ // DONT_RESOLVE_DLL_REFERENCES argument to LoadLibraryEx(), the execution<br>
+ // of the entry point as a DllMain() function will be suppressed. This<br>
+ // also prevents other modules that are referenced by the DLL from being<br>
+ // loaded. We use LoadLibraryEx() to create a copy of the PE32<br>
+ // image that the OS (and therefore the debugger) is aware of.<br>
+ // Source level debugging is the only reason to do this.<br>
//<br>
Library = LoadLibraryEx (DllFileName, NULL, DONT_RESOLVE_DLL_REFERENCES);<br>
if (Library != NULL) {<br>
//<br>
- // InitializeDriver is the entry point we put in all our EFI DLL's. The<br>
- // DONT_RESOLVE_DLL_REFERENCES argument to LoadLIbraryEx() suppresses the<br>
- // normal DLL entry point of DllMain, and prevents other modules that are<br>
- // referenced in side the DllFileName from being loaded. There is no error<br>
- // checking as the we can point to the PE32 image loaded by Tiano. This<br>
- // step is only needed for source level debugging<br>
+ // Parse the PE32 image loaded by the OS and find the entry point<br>
//<br>
- DllEntryPoint = (VOID *)(UINTN)GetProcAddress (Library, "InitializeDriver");<br>
+ ZeroMem (&PeCoffImageContext, sizeof (PeCoffImageContext));<br>
+ PeCoffImageContext.Handle = Library;<br>
+ PeCoffImageContext.ImageRead = PeCoffLoaderImageReadFromMemory;<br>
+ Status = PeCoffLoaderGetImageInfo (&PeCoffImageContext);<br>
+ if (EFI_ERROR (Status) || (PeCoffImageContext.ImageError != IMAGE_ERROR_SUCCESS)) {<br>
+ SecPrint ("DLL is not a valid PE/COFF image.\n\r");<br>
+ FreeLibrary (Library);<br>
+ Library = NULL;<br>
+ } else {<br>
+ Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)Library + (UINTN)PeCoffImageContext.PeCoffHeaderOffset);<br>
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {<br>
+ //<br>
+ // Use PE32 offset<br>
+ //<br>
+ DllEntryPoint = (VOID *) ((UINTN)Library + (UINTN)Hdr.Pe32->OptionalHeader.AddressOfEntryPoint);<br>
+ } else {<br>
+ //<br>
+ // Use PE32+ offset<br>
+ //<br>
+ DllEntryPoint = (VOID *) ((UINTN)Library + (UINTN)Hdr.Pe32Plus->OptionalHeader.AddressOfEntryPoint);<br>
+ }<br>
+ //<br>
+ // Now we need to configure memory access for the copy of the PE32 image<br>
+ // loaded by the OS.<br>
+ //<br>
+ // Most Windows DLLs are linked with sections 4KB aligned but EFI<br>
+ // modules are not to reduce size. Because of this we need to compute<br>
+ // the union of memory access attributes and explicitly configure<br>
+ // each page.<br>
+ //<br>
+ FirstSection = (EFI_IMAGE_SECTION_HEADER *)(<br>
+ (UINTN)Library +<br>
+ PeCoffImageContext.PeCoffHeaderOffset +<br>
+ sizeof (UINT32) +<br>
+ sizeof (EFI_IMAGE_FILE_HEADER) +<br>
+ Hdr.Pe32->FileHeader.SizeOfOptionalHeader<br>
+ );<br>
+ NumberOfSections = (UINTN)(Hdr.Pe32->FileHeader.NumberOfSections);<br>
+ Section = FirstSection;<br>
+ SectionData = malloc (NumberOfSections * sizeof (IMAGE_SECTION_DATA));<br>
+ if (SectionData == NULL) {<br>
+ FreeLibrary (Library);<br>
+ Library = NULL;<br>
+ DllEntryPoint = NULL;<br>
+ }<br>
+ ZeroMem (SectionData, NumberOfSections * sizeof (IMAGE_SECTION_DATA));<br>
+ //<br>
+ // Extract the section data from the PE32 image<br>
+ //<br>
+ for (Index = 0; Index < NumberOfSections; Index++) {<br>
+ SectionData[Index].Base = (UINTN)Library + Section->VirtualAddress;<br>
+ SectionData[Index].Size = Section->Misc.VirtualSize;<br>
+ if (SectionData[Index].Size == 0) {<br>
+ SectionData[Index].Size = Section->SizeOfRawData;<br>
+ }<br>
+ SectionData[Index].Flags = (Section->Characteristics &<br>
+ (EFI_IMAGE_SCN_MEM_EXECUTE | EFI_IMAGE_SCN_MEM_WRITE));<br>
+ Section += 1;<br>
+ }<br>
+ //<br>
+ // Loop over every DWORD in memory and compute the union of the memory<br>
+ // access bits.<br>
+ //<br>
+ End = (UINTN)Library + (UINTN)PeCoffImageContext.ImageSize;<br>
+ RegionBase = (UINTN)Library;<br>
+ RegionSize = 0;<br>
+ Flags = 0;<br>
+ for (Base = (UINTN)Library + sizeof (UINT32); Base < End; Base += sizeof (UINT32)) {<br>
+ for (Index = 0; Index < NumberOfSections; Index++) {<br>
+ if (SectionData[Index].Base <= Base &&<br>
+ (SectionData[Index].Base + SectionData[Index].Size) > Base) {<br>
+ Flags |= SectionData[Index].Flags;<br>
+ }<br>
+ }<br>
+ //<br>
+ // When a new page is reached configure the memory access for the<br>
+ // previous page.<br>
+ //<br>
+ if (Base % SIZE_4KB == 0) {<br>
+ RegionSize += SIZE_4KB;<br>
+ if ((Flags & EFI_IMAGE_SCN_MEM_WRITE) == EFI_IMAGE_SCN_MEM_WRITE) {<br>
+ if ((Flags & EFI_IMAGE_SCN_MEM_EXECUTE) == EFI_IMAGE_SCN_MEM_EXECUTE) {<br>
+ NewProtection = PAGE_EXECUTE_READWRITE;<br>
+ } else {<br>
+ NewProtection = PAGE_READWRITE;<br>
+ }<br>
+ } else {<br>
+ if ((Flags & EFI_IMAGE_SCN_MEM_EXECUTE) == EFI_IMAGE_SCN_MEM_EXECUTE) {<br>
+ NewProtection = PAGE_EXECUTE_READ;<br>
+ } else {<br>
+ NewProtection = PAGE_READONLY;<br>
+ }<br>
+ }<br>
+ if (!VirtualProtect ((LPVOID)RegionBase, (SIZE_T) RegionSize, NewProtection, &OldProtection)) {<br>
+ SecPrint ("Setting PE32 Section Access Failed\n\r");<br>
+ FreeLibrary (Library);<br>
+ free (SectionData);<br>
+ Library = NULL;<br>
+ DllEntryPoint = NULL;<br>
+ break;<br>
+ }<br>
+ Flags = 0;<br>
+ RegionBase = Base;<br>
+ RegionSize = 0;<br>
+ }<br>
+ }<br>
+ free (SectionData);<br>
+ //<br>
+ // Configure the last partial page<br>
+ //<br>
+ if (Library != NULL && (End - RegionBase) > 0) {<br>
+ if ((Flags & EFI_IMAGE_SCN_MEM_WRITE) == EFI_IMAGE_SCN_MEM_WRITE) {<br>
+ if ((Flags & EFI_IMAGE_SCN_MEM_EXECUTE) == EFI_IMAGE_SCN_MEM_EXECUTE) {<br>
+ NewProtection = PAGE_EXECUTE_READWRITE;<br>
+ } else {<br>
+ NewProtection = PAGE_READWRITE;<br>
+ }<br>
+ } else {<br>
+ if ((Flags & EFI_IMAGE_SCN_MEM_EXECUTE) == EFI_IMAGE_SCN_MEM_EXECUTE) {<br>
+ NewProtection = PAGE_EXECUTE_READ;<br>
+ } else {<br>
+ NewProtection = PAGE_READONLY;<br>
+ }<br>
+ }<br>
+ if (!VirtualProtect ((LPVOID)RegionBase, (SIZE_T) (End - RegionBase), NewProtection, &OldProtection)) {<br>
+ SecPrint ("Setting PE32 Section Access Failed\n\r");<br>
+ FreeLibrary (Library);<br>
+ Library = NULL;<br>
+ DllEntryPoint = NULL;<br>
+ }<br>
+ }<br>
+ }<br>
}<br>
<br>
if ((Library != NULL) && (DllEntryPoint != NULL)) {<br>
@@ -1142,7 +1300,7 @@ PeCoffLoaderRelocateImageExtraAction (<br>
// This DLL is not already loaded, so source level debugging is supported.<br>
//<br>
ImageContext->EntryPoint = (EFI_PHYSICAL_ADDRESS)(UINTN)DllEntryPoint;<br>
- SecPrint ("LoadLibraryEx (\n\r %S,\n\r NULL, DONT_RESOLVE_DLL_REFERENCES)\n\r", DllFileName);<br>
+ SecPrint ("LoadLibraryEx (\n\r %S,\n\r NULL, DONT_RESOLVE_DLL_REFERENCES) @ 0x%X\n\r", DllFileName, (int) (UINTN) Library);<br>
}<br>
} else {<br>
SecPrint ("WARNING: No source level debug %S. \n\r", DllFileName);<br>
-- <br>
2.39.2.windows.1<br>
<br>
</div>
</span></font></div>
</body>
</html>
<div width="1" style="color:white;clear:both">_._,_._,_</div>
<hr>
Groups.io Links:<p>
You receive all messages sent to this group.
<p>
<a target="_blank" href="https://edk2.groups.io/g/devel/message/109100">View/Reply Online (#109100)</a> |
|
<a target="_blank" href="https://groups.io/mt/101531560/1813853">Mute This Topic</a>
| <a href="https://edk2.groups.io/g/devel/post">New Topic</a>
<br>
<a href="https://edk2.groups.io/g/devel/editsub/1813853">Your Subscription</a> |
<a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> |
<a href="https://edk2.groups.io/g/devel/leave/3943202/1813853/130120423/xyzzy">Unsubscribe</a>
[edk2-devel-archive@redhat.com]<br>
<div width="1" style="color:white;clear:both">_._,_._,_</div>