Mike, Thanks for fixing this regression issue. I also did a comparison between this and the Nt32 accordingly code. They are almost the same.
I also noticed your unit test steps in Bugzilla and the behavior is expected. I agree and also suggest that this fix to be included in the coming stable tag release. Reviewed-by: Ray Ni <ray...@intel.com> > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D > Kinney > Sent: Thursday, August 22, 2019 10:36 AM > To: devel@edk2.groups.io > Cc: Justen, Jordan L <jordan.l.jus...@intel.com>; Ni, Ray <ray...@intel.com>; > Andrew Fish <af...@apple.com>; Tim Lewis <tim.le...@insyde.com> > Subject: [edk2-devel] [Patch][edk2-stable201908 1/2] EmulatorPkg/Win/Host: > Fix image unload regression > > https://bugzilla.tianocore.org/show_bug.cgi?id=2104 > > When UEFI Applications or UEFI Drivers are unloaded, the > PeCoffLoaderUnloadImageExtraAction() needs to unload the image using > FreeLibrary() if the image was successfully loaded using LoadLibrrayEx(). > > This is a regression from the Nt32Pkg that supported unloading applications > and > drivers as well as loading the same application or driver multiple times. > > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Cc: Andrew Fish <af...@apple.com> > Cc: Tim Lewis <tim.le...@insyde.com> > Signed-off-by: Michael D Kinney <michael.d.kin...@intel.com> > --- > EmulatorPkg/Win/Host/WinHost.c | 167 > +++++++++++++++++++++++++++++++-- > 1 file changed, 161 insertions(+), 6 deletions(-) > > diff --git a/EmulatorPkg/Win/Host/WinHost.c > b/EmulatorPkg/Win/Host/WinHost.c index dd52075f98..9c6acac279 100644 > --- a/EmulatorPkg/Win/Host/WinHost.c > +++ b/EmulatorPkg/Win/Host/WinHost.c > @@ -19,6 +19,25 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #define SE_TIME_ZONE_NAME TEXT("SeTimeZonePrivilege") > #endif > > +// > +// The growth size for array of module handle entries // #define > +MAX_PDB_NAME_TO_MOD_HANDLE_ARRAY_SIZE 0x100 > + > +// > +// Module handle entry structure > +// > +typedef struct { > + CHAR8 *PdbPointer; > + VOID *ModHandle; > +} PDB_NAME_TO_MOD_HANDLE; > + > +// > +// An Array to hold the module handles > +// > +PDB_NAME_TO_MOD_HANDLE *mPdbNameModHandleArray = NULL; > +UINTN mPdbNameModHandleArraySize = 0; > + > // > // Default information about where the FD is located. > // This array gets filled in with information from PcdWinNtFirmwareVolume > @@ -840,6 +859,120 @@ Returns: > return Count; > } > > +/** > + Store the ModHandle in an array indexed by the Pdb File name. > + The ModHandle is needed to unload the image. > + @param ImageContext - Input data returned from PE Laoder Library. Used to > find the > + .PDB file name of the PE Image. > + @param ModHandle - Returned from LoadLibraryEx() and stored for call to > + FreeLibrary(). > + @return return EFI_SUCCESS when ModHandle was stored. > +--*/ > +EFI_STATUS > +AddModHandle ( > + IN PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext, > + IN VOID *ModHandle > + ) > + > +{ > + UINTN Index; > + PDB_NAME_TO_MOD_HANDLE *Array; > + UINTN PreviousSize; > + PDB_NAME_TO_MOD_HANDLE *TempArray; > + HANDLE Handle; > + UINTN Size; > + > + // > + // Return EFI_ALREADY_STARTED if this DLL has already been loaded // > + Array = mPdbNameModHandleArray; for (Index = 0; Index < > + mPdbNameModHandleArraySize; Index++, Array++) { > + if (Array->PdbPointer != NULL && Array->ModHandle == ModHandle) { > + return EFI_ALREADY_STARTED; > + } > + } > + > + Array = mPdbNameModHandleArray; > + for (Index = 0; Index < mPdbNameModHandleArraySize; Index++, Array++) { > + if (Array->PdbPointer == NULL) { > + // > + // Make a copy of the stirng and store the ModHandle > + // > + Handle = GetProcessHeap (); > + Size = AsciiStrLen (ImageContext->PdbPointer) + 1; > + Array->PdbPointer = HeapAlloc ( Handle, HEAP_ZERO_MEMORY, Size); > + ASSERT (Array->PdbPointer != NULL); > + > + AsciiStrCpyS (Array->PdbPointer, Size, ImageContext->PdbPointer); > + Array->ModHandle = ModHandle; > + return EFI_SUCCESS; > + } > + } > + > + // > + // No free space in mPdbNameModHandleArray so grow it by // > + MAX_PDB_NAME_TO_MOD_HANDLE_ARRAY_SIZE entires. > + // > + PreviousSize = mPdbNameModHandleArraySize * sizeof > + (PDB_NAME_TO_MOD_HANDLE); mPdbNameModHandleArraySize += > + MAX_PDB_NAME_TO_MOD_HANDLE_ARRAY_SIZE; > + // > + // re-allocate a new buffer and copy the old values to the new locaiton. > + // > + TempArray = HeapAlloc (GetProcessHeap (), > + HEAP_ZERO_MEMORY, > + mPdbNameModHandleArraySize * sizeof > (PDB_NAME_TO_MOD_HANDLE) > + ); > + > + CopyMem ((VOID *) (UINTN) TempArray, (VOID *) > + (UINTN)mPdbNameModHandleArray, PreviousSize); > + > + HeapFree (GetProcessHeap (), 0, mPdbNameModHandleArray); > + > + mPdbNameModHandleArray = TempArray; > + if (mPdbNameModHandleArray == NULL) { > + ASSERT (FALSE); > + return EFI_OUT_OF_RESOURCES; > + } > + > + return AddModHandle (ImageContext, ModHandle); } > + > +/** > + Return the ModHandle and delete the entry in the array. > + @param ImageContext - Input data returned from PE Laoder Library. Used > to find the > + .PDB file name of the PE Image. > + @return > + ModHandle - ModHandle assoicated with ImageContext is returned > + NULL - No ModHandle associated with ImageContext > +**/ > +VOID * > +RemoveModHandle ( > + IN PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext > + ) > +{ > + UINTN Index; > + PDB_NAME_TO_MOD_HANDLE *Array; > + > + if (ImageContext->PdbPointer == NULL) { > + // > + // If no PDB pointer there is no ModHandle so return NULL > + // > + return NULL; > + } > + > + Array = mPdbNameModHandleArray; > + for (Index = 0; Index < mPdbNameModHandleArraySize; Index++, Array++) { > + if ((Array->PdbPointer != NULL) && (AsciiStrCmp(Array->PdbPointer, > ImageContext->PdbPointer) == 0)) { > + // > + // If you find a match return it and delete the entry > + // > + HeapFree (GetProcessHeap (), 0, Array->PdbPointer); > + Array->PdbPointer = NULL; > + return Array->ModHandle; > + } > + } > + > + return NULL; > +} > > VOID > EFIAPI > @@ -847,6 +980,7 @@ PeCoffLoaderRelocateImageExtraAction ( > IN OUT PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext > ) > { > + EFI_STATUS Status; > VOID *DllEntryPoint; > CHAR16 *DllFileName; > HMODULE Library; > @@ -855,7 +989,7 @@ PeCoffLoaderRelocateImageExtraAction ( > 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 usw it to load > + // 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), > @@ -913,10 +1047,22 @@ PeCoffLoaderRelocateImageExtraAction ( > } > > if ((Library != NULL) && (DllEntryPoint != NULL)) { > - ImageContext->EntryPoint = (EFI_PHYSICAL_ADDRESS) (UINTN) > DllEntryPoint; > - SecPrint ("LoadLibraryEx (%S,\n NULL, > DONT_RESOLVE_DLL_REFERENCES)\n", DllFileName); > + Status = AddModHandle (ImageContext, Library); > + if (Status == EFI_ALREADY_STARTED) { > + // > + // If the DLL has already been loaded before, then this instance of > the DLL > can not be debugged. > + // > + ImageContext->PdbPointer = NULL; > + SecPrint ("WARNING: DLL already loaded. No source level debug > %S.\n\r", > DllFileName); > + } else { > + // > + // 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); > + } > } else { > - SecPrint ("WARNING: No source level debug %S. \n", DllFileName); > + SecPrint ("WARNING: No source level debug %S. \n\r", > + DllFileName); > } > > free (DllFileName); > @@ -926,13 +1072,22 @@ PeCoffLoaderRelocateImageExtraAction ( VOID > EFIAPI PeCoffLoaderUnloadImageExtraAction ( > - IN PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext > + IN PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext > ) > { > + VOID *ModHandle; > + > ASSERT (ImageContext != NULL); > + > + ModHandle = RemoveModHandle (ImageContext); if (ModHandle != NULL) { > + FreeLibrary (ModHandle); > + SecPrint ("FreeLibrary (\n\r %s)\n\r", ImageContext->PdbPointer); > + } else { > + SecPrint ("WARNING: Unload image without source level debug\n\r"); > + } > } > > - > VOID > _ModuleEntryPoint ( > VOID > -- > 2.21.0.windows.1 > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46244): https://edk2.groups.io/g/devel/message/46244 Mute This Topic: https://groups.io/mt/32986083/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
