* [edk2-devel] [PATCH] ArmPkg/DebugPeCoffExtraActionLib: Drop RVCT and Cygwin support @ 2023-12-14 9:20 Ard Biesheuvel 2023-12-14 14:46 ` Leif Lindholm 0 siblings, 1 reply; 4+ messages in thread From: Ard Biesheuvel @ 2023-12-14 9:20 UTC (permalink / raw) To: devel; +Cc: quic_llindhol, Ard Biesheuvel, Mike Beaton From: Ard Biesheuvel <ardb@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 <mjsbeaton@gmail.com> Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- 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 (#112513): https://edk2.groups.io/g/devel/message/112513 Mute This Topic: https://groups.io/mt/103167036/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] 4+ messages in thread
* Re: [edk2-devel] [PATCH] ArmPkg/DebugPeCoffExtraActionLib: Drop RVCT and Cygwin support 2023-12-14 9:20 [edk2-devel] [PATCH] ArmPkg/DebugPeCoffExtraActionLib: Drop RVCT and Cygwin support Ard Biesheuvel @ 2023-12-14 14:46 ` Leif Lindholm 2023-12-14 14:59 ` Sami Mujawar 0 siblings, 1 reply; 4+ messages in thread From: Leif Lindholm @ 2023-12-14 14:46 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: devel, Ard Biesheuvel, Mike Beaton, Sami Mujawar +Sami (who I know once, a very long time ago, used cygwin) On Thu, Dec 14, 2023 at 10:20:46 +0100, Ard Biesheuvel wrote: > From: Ard Biesheuvel <ardb@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 <mjsbeaton@gmail.com> > Signed-off-by: Ard Biesheuvel <ardb@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_llindhol@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 (#112537): https://edk2.groups.io/g/devel/message/112537 Mute This Topic: https://groups.io/mt/103167036/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] 4+ messages in thread
* Re: [edk2-devel] [PATCH] ArmPkg/DebugPeCoffExtraActionLib: Drop RVCT and Cygwin support 2023-12-14 14:46 ` Leif Lindholm @ 2023-12-14 14:59 ` Sami Mujawar 2023-12-14 16:34 ` Ard Biesheuvel 0 siblings, 1 reply; 4+ messages in thread From: Sami Mujawar @ 2023-12-14 14:59 UTC (permalink / raw) To: Leif Lindholm, Ard Biesheuvel Cc: devel@edk2.groups.io, Ard Biesheuvel, Mike Beaton, nd Hi Leif, Ard, On 14/12/2023, 14:46, "Leif Lindholm" <quic_llindhol@quicinc.com <mailto:quic_llindhol@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.mujawar@arm.com> Regards, Sami Mujawar On Thu, Dec 14, 2023 at 10:20:46 +0100, Ard Biesheuvel wrote: > From: Ard Biesheuvel <ardb@kernel.org <mailto:ardb@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 <mjsbeaton@gmail.com <mailto:mjsbeaton@gmail.com>> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org <mailto:ardb@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_llindhol@quicinc.com <mailto:quic_llindhol@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/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH] ArmPkg/DebugPeCoffExtraActionLib: Drop RVCT and Cygwin support 2023-12-14 14:59 ` Sami Mujawar @ 2023-12-14 16:34 ` Ard Biesheuvel 0 siblings, 0 replies; 4+ messages in thread From: Ard Biesheuvel @ 2023-12-14 16:34 UTC (permalink / raw) To: devel, sami.mujawar; +Cc: Leif Lindholm, Ard Biesheuvel, Mike Beaton, nd On Thu, 14 Dec 2023 at 15:59, Sami Mujawar <sami.mujawar@arm.com> wrote: > > Hi Leif, Ard, > > On 14/12/2023, 14:46, "Leif Lindholm" <quic_llindhol@quicinc.com <mailto:quic_llindhol@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.mujawar@arm.com> > Merged as #5148 Thanks all > > On Thu, Dec 14, 2023 at 10:20:46 +0100, Ard Biesheuvel wrote: > > From: Ard Biesheuvel <ardb@kernel.org <mailto:ardb@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 <mjsbeaton@gmail.com <mailto:mjsbeaton@gmail.com>> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org <mailto:ardb@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_llindhol@quicinc.com <mailto:quic_llindhol@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/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-12-14 16:35 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-14 9:20 [edk2-devel] [PATCH] ArmPkg/DebugPeCoffExtraActionLib: Drop RVCT and Cygwin support Ard Biesheuvel 2023-12-14 14:46 ` Leif Lindholm 2023-12-14 14:59 ` Sami Mujawar 2023-12-14 16:34 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox