> Could you please send patches inline? Attachments are a pain to review.
Sure, I will do that. Because I use Outlook I prefer the attachment since I can pull it into a nice patch viewer. > Applying your patch and doing 'git show --find-copies-harder' gives me > (tools_def.tempate omitted): Unfortunately git found a copy that is not appropriate for comparison. It found the X86/X64 linker script that, while similar, is different than what we were already doing for AArch64. (I'm not saying the X86 approach doesn't work but my goal here was to minimize the impact to what was already being done for AArch64.) On AArch64 the .text section was being placed with this linker switch: -Ttext=0x0 But this was not sufficient to pack the .data section in a manner that is consistent with the PE-COFF conversion that happens later in the build. So when I converted this to a linker control file, I maintained the zero starting address with: . = 0; > So are you saying that the resulting PE/COFF is identical (for all > intents and purposes), and only the ELF intermediate file deviates? Yes, from my testing the PE/COFF looks the same but the ELF is updated to move .data such that its offset relative to .text is consistent with PE/COFF. This fixes debugger correlation for stuff in the .data section like global variables. > How does this affect > ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf, > which outputs lines like >From my testing it has no effect since this output is only outputting the >PE-COFF image address offset by the size of the PE-COFF headers (0x260 in this >case): (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)) The reason the SizeOfHeaders is added is because the debugger is unaware of the PE-COFF conversion and added executable headers. Eugene -----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] Sent: Tuesday, June 16, 2015 5:56 AM To: edk2-devel@lists.sourceforge.net Subject: Re: [edk2] [PATCH] BaseTools for AArch64 GCC: Ensure that the correlation .text and .data is consistent between ELF and PE-COFF so the debugger sees global variables correctly On 4 June 2015 at 21:48, Cohen, Eugene <eug...@hp.com> wrote: > Oops, left off the contribution agreement line, trying again > > Dear ArmPkg maintainers (and later BaseTools maintainer), > > This is a fix for debugger correlation of global variables for AArch64 built > on GCC. > > Before this change looking at global variables with a debugger showed bogus > memory locations. This is because the offset of the .data section in the ELF > file did not reflect where it was placed in the PE-COFF (.efi) output. > > This change passes a linker control script so that the data section is packed > next to .text so the ELF accurately reflects the relationship between the > sections when converted to PE-COFF by GenFw. > > I have tested this with the Lauterbach debugger. I don't know how well it > will work with other debuggers and debug scripts. > > If you would rather view the change as a github pull request: > https://github.com/tianocore/edk2/pull/12 > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Eugene Cohen <eug...@hp.com> > Could you please send patches inline? Attachments are a pain to review. Also, since the patch introduces a completely new linker script, it is hard to review how yours deviates from the default. Git is actually quite helpful since it can figure out if your newly introduced file resembles an existing file, and only shows the diff with respect to the original. Applying your patch and doing 'git show --find-copies-harder' gives me (tools_def.tempate omitted): """ diff --git a/BaseTools/Scripts/gcc4.4-ld-script b/BaseTools/Scripts/gcc-arm-ld-script similarity index 59% copy from BaseTools/Scripts/gcc4.4-ld-script copy to BaseTools/Scripts/gcc-arm-ld-script index 68b2767590ac..e1589a4d03bf 100644 --- a/BaseTools/Scripts/gcc4.4-ld-script +++ b/BaseTools/Scripts/gcc-arm-ld-script @@ -1,8 +1,10 @@ -/* OUTPUT_FORMAT(efi-bsdrv-x86_64) */ SECTIONS { - /* . = 0 + SIZEOF_HEADERS; */ - . = 0x280; + /* Start at 0 so we can meet more aggressive alignment requires after the PE-COFF conversion + like those for ARM exception vectors. This requires debugger scripts to offset past + the PE-COFF header (typically 0x260). When the PE-COFF conversion occurs we will + get proper alignment since the ELF section alignment is applied in the conversion process. */ + . = 0; .text ALIGN(0x20) : { *(.text .stub .text.* .gnu.linkonce.t.*) """ So are you saying that the resulting PE/COFF is identical (for all intents and purposes), and only the ELF intermediate file deviates? How does this affect ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf, which outputs lines like add-symbol-file /home/ard/build/uefi-next/Build/ArmVirtQemu-AARCH64/DEBUG_GCC48/AARCH64/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll 0x5F2C7260 where the .dll is and ELF file, and the line seems to incorporate the header offset that you are removing. Regards, Ard. ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
