On Thu, 14 Dec 2023 at 15:59, Sami Mujawar <sami.muja...@arm.com> wrote:
>
> 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>
>

Merged as #5148

Thanks all

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