Hi Leif, Ard,

On 14/12/2023, 14:46, "Leif Lindholm" <quic_llind...@quicinc.com 
<mailto:quic_llind...@quicinc.com>> wrote:


+Sami (who I know once, a very long time ago, used cygwin)
[SAMI] Now that we have WSL, I have stopped using Cygwin.
Also, this patch looks good to me.

Reviewed-by: Sami Mujawar <sami.muja...@arm.com>

Regards,

Sami Mujawar

On Thu, Dec 14, 2023 at 10:20:46 +0100, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <a...@kernel.org <mailto:a...@kernel.org>>
> 
> The DebugPeCoffExtraActionLib implemention in ArmPkg contains some cruft
> that dates back to the original RVCT based ARM port, and support for
> RVCT was dropped a while ago.
> 
> Also drop the handling of Cygwin specific paths, which is highly
> unlikely to be still depended upon by anyone.
> 
> Tweak the logic so that only two versions of the DEBUG() invocations
> remain: one for __GNUC__ when PdbPointer is set, and the fallback that
> just prints the image address and the address of the entrypoint.
> 
> Cc: Mike Beaton <mjsbea...@gmail.com <mailto:mjsbea...@gmail.com>>
> Signed-off-by: Ard Biesheuvel <a...@kernel.org <mailto:a...@kernel.org>>


But as far as I'm concerned, cygwin support feels extremely dated
these days and not worth the maintainance overhead to keep around.
So
Reviewed-by: Leif Lindholm <quic_llind...@quicinc.com 
<mailto:quic_llind...@quicinc.com>>


/
Leif


> ---
> ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c | 100 
> ++++++--------------
> 1 file changed, 31 insertions(+), 69 deletions(-)
> 
> diff --git 
> a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c 
> b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> index 432112354fda..992c14d7ef9b 100644
> --- a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> +++ b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
> @@ -17,45 +17,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/PeCoffExtraActionLib.h>
> #include <Library/PrintLib.h>
> 
> -/**
> - If the build is done on cygwin the paths are cygpaths.
> - /cygdrive/c/tmp.txt vs c:\tmp.txt so we need to convert
> - them to work with RVD commands
> -
> - @param Name Path to convert if needed
> -
> -**/
> -CHAR8 *
> -DeCygwinPathIfNeeded (
> - IN CHAR8 *Name,
> - IN CHAR8 *Temp,
> - IN UINTN Size
> - )
> -{
> - CHAR8 *Ptr;
> - UINTN Index;
> - UINTN Index2;
> -
> - Ptr = AsciiStrStr (Name, "/cygdrive/");
> - if (Ptr == NULL) {
> - return Name;
> - }
> -
> - for (Index = 9, Index2 = 0; (Index < (Size + 9)) && (Ptr[Index] != '\0'); 
> Index++, Index2++) {
> - Temp[Index2] = Ptr[Index];
> - if (Temp[Index2] == '/') {
> - Temp[Index2] = '\\';
> - }
> -
> - if (Index2 == 1) {
> - Temp[Index2 - 1] = Ptr[Index];
> - Temp[Index2] = ':';
> - }
> - }
> -
> - return Temp;
> -}
> -
> /**
> Performs additional actions after a PE/COFF image has been loaded and 
> relocated.
> 
> @@ -71,23 +32,24 @@ PeCoffLoaderRelocateImageExtraAction (
> IN OUT PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext
> )
> {
> - #if !defined (MDEPKG_NDEBUG)
> - CHAR8 Temp[512];
> - #endif
> -
> +#ifdef __GNUC__
> if (ImageContext->PdbPointer) {
> - #ifdef __CC_ARM
> - // Print out the command for the DS-5 to load symbols for this image
> - DEBUG ((DEBUG_LOAD | DEBUG_INFO, "add-symbol-file %a 0x%p\n", 
> DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), 
> (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
> - #elif __GNUC__
> - // This may not work correctly if you generate PE/COFF directly as then the 
> Offset would not be required
> - DEBUG ((DEBUG_LOAD | DEBUG_INFO, "add-symbol-file %a 0x%p\n", 
> DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), 
> (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
> - #else
> - DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Loading driver at 0x%11p 
> EntryPoint=0x%11p\n", (VOID *)(UINTN)ImageContext->ImageAddress, 
> FUNCTION_ENTRY_POINT (ImageContext->EntryPoint)));
> - #endif
> - } else {
> - DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Loading driver at 0x%11p 
> EntryPoint=0x%11p\n", (VOID *)(UINTN)ImageContext->ImageAddress, 
> FUNCTION_ENTRY_POINT (ImageContext->EntryPoint)));
> + DEBUG ((
> + DEBUG_LOAD | DEBUG_INFO,
> + "add-symbol-file %a 0x%p\n",
> + ImageContext->PdbPointer,
> + (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)
> + ));
> + return;
> }
> +#endif
> +
> + DEBUG ((
> + DEBUG_LOAD | DEBUG_INFO,
> + "Loading driver at 0x%11p EntryPoint=0x%11p\n",
> + (VOID *)(UINTN)ImageContext->ImageAddress,
> + FUNCTION_ENTRY_POINT (ImageContext->EntryPoint)
> + ));
> }
> 
> /**
> @@ -106,21 +68,21 @@ PeCoffLoaderUnloadImageExtraAction (
> IN OUT PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext
> )
> {
> - #if !defined (MDEPKG_NDEBUG)
> - CHAR8 Temp[512];
> - #endif
> -
> +#ifdef __GNUC__
> if (ImageContext->PdbPointer) {
> - #ifdef __CC_ARM
> - // Print out the command for the RVD debugger to load symbols for this image
> - DEBUG ((DEBUG_LOAD | DEBUG_INFO, "unload symbols_only %a\n", 
> DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp))));
> - #elif __GNUC__
> - // This may not work correctly if you generate PE/COFF directly as then the 
> Offset would not be required
> - DEBUG ((DEBUG_LOAD | DEBUG_INFO, "remove-symbol-file %a 0x%08x\n", 
> DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), 
> (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
> - #else
> - DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Unloading %a\n", 
> ImageContext->PdbPointer));
> - #endif
> - } else {
> - DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Unloading driver at 0x%11p\n", (VOID 
> *)(UINTN)ImageContext->ImageAddress));
> + DEBUG ((
> + DEBUG_LOAD | DEBUG_INFO,
> + "remove-symbol-file %a 0x%08x\n",
> + ImageContext->PdbPointer,
> + (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)
> + ));
> + return;
> }
> +#endif
> +
> + DEBUG ((
> + DEBUG_LOAD | DEBUG_INFO,
> + "Unloading driver at 0x%11p\n",
> + (VOID *)(UINTN)ImageContext->ImageAddress
> + ));
> }
> --
> 2.43.0.472.g3155946c3a-goog
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112538): https://edk2.groups.io/g/devel/message/112538
Mute This Topic: https://groups.io/mt/103167036/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to