On 5 July 2017 at 18:28, Laszlo Ersek <ler...@redhat.com> wrote: > On 07/05/17 18:45, Ard Biesheuvel wrote: >> On 5 July 2017 at 17:42, Laszlo Ersek <ler...@redhat.com> wrote: >>> GNU Binutils produce a PE debug directory with one >> >> This sentence already confuses me. This crash is reproducible on ARM, >> but the ARM toolchains are strictly ELF based, and all PE/COFF data >> structures are created by GenFw itself, never by binutils. > > According to binutils commit 61e2488cd849: > > Add support for generating and inserting build IDs into COFF binaries. > > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=61e2488cd849 > > the write_build_id() function from that commit does produce PE/COFF > artifacts. > > /* Construct a debug directory entry which points to an immediately > following CodeView record. */ > > /* Record the location of the debug directory in the data directory. */ > > I can't exactly say where the bug is (it may have been added later -- > I'm not a binutils developer), and the code I quoted above might not > even be related to the symptoms we're seeing at all, but binutils can > definitely generate PE stuff. > > Plus, the mal-sized debug directory in the GenFw-crasher DLL files seems > to fall onto a section called ".build-id". > > OTOH, after reviewing the commands from Gerd's Jenkins log that lead to > the GenFw crash on "SecMain.dll", I think you are right... All of these > commands use ELF formats, apparently: > >> "gcc" \ >> -g \ >> -fshort-wchar \ >> -fno-builtin \ >> -fno-strict-aliasing \ >> -Wall \ >> -Werror \ >> -Wno-array-bounds \ >> -ffunction-sections \ >> -fdata-sections \ >> -include AutoGen.h \ >> -fno-common \ >> -DSTRING_ARRAY_NAME=SecMainStrings \ >> -m32 \ >> -march=i586 \ >> -malign-double \ >> -fno-stack-protector \ >> -D EFI32 \ >> -fno-asynchronous-unwind-tables \ >> -Wno-address \ >> -Os \ >> -mno-mmx \ >> -mno-sse \ >> -D DISABLE_NEW_DEPRECATED_INTERFACES \ >> -c \ >> -o >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/OUTPUT/./SecMain.obj \ >> -IOvmfPkg/Sec/Ia32 \ >> -IOvmfPkg/Sec \ >> -IBuild/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG \ >> -IMdePkg \ >> -IMdePkg/Include \ >> -IMdePkg/Include/Ia32 \ >> -IMdeModulePkg \ >> -IMdeModulePkg/Include \ >> -IUefiCpuPkg \ >> -IUefiCpuPkg/Include \ >> -IOvmfPkg \ >> -IOvmfPkg/Include \ >> OvmfPkg/Sec/SecMain.c >> >> "gcc" \ >> -E \ >> -x assembler-with-cpp \ >> -include >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.h \ >> -IOvmfPkg/Sec/Ia32 \ >> -IOvmfPkg/Sec \ >> -IBuild/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG \ >> -IMdePkg \ >> -IMdePkg/Include \ >> -IMdePkg/Include/Ia32 \ >> -IMdeModulePkg \ >> -IMdeModulePkg/Include \ >> -IUefiCpuPkg \ >> -IUefiCpuPkg/Include \ >> -IOvmfPkg \ >> -IOvmfPkg/Include \ >> OvmfPkg/Sec/Ia32/SecEntry.nasm \ >> > >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/OUTPUT/Ia32/SecEntry.i >> >> Trim \ >> --trim-long \ >> --source-code \ >> -o >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/OUTPUT/Ia32/SecEntry.iii >> \ >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/OUTPUT/Ia32/SecEntry.i >> >> "nasm" \ >> -IOvmfPkg/Sec/Ia32/ \ >> -f elf32 \ >> -o >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/OUTPUT/Ia32/SecEntry.obj >> \ >> >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/OUTPUT/Ia32/SecEntry.iii >> >> "gcc" \ >> -g \ >> -fshort-wchar \ >> -fno-builtin \ >> -fno-strict-aliasing \ >> -Wall \ >> -Werror \ >> -Wno-array-bounds \ >> -ffunction-sections \ >> -fdata-sections \ >> -include AutoGen.h \ >> -fno-common \ >> -DSTRING_ARRAY_NAME=SecMainStrings \ >> -m32 \ >> -march=i586 \ >> -malign-double \ >> -fno-stack-protector \ >> -D EFI32 \ >> -fno-asynchronous-unwind-tables \ >> -Wno-address \ >> -Os \ >> -mno-mmx \ >> -mno-sse \ >> -D DISABLE_NEW_DEPRECATED_INTERFACES \ >> -c \ >> -o >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/OUTPUT/./AutoGen.obj \ >> -IOvmfPkg/Sec/Ia32 \ >> -IOvmfPkg/Sec \ >> -IBuild/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG \ >> -IMdePkg \ >> -IMdePkg/Include \ >> -IMdePkg/Include/Ia32 \ >> -IMdeModulePkg \ >> -IMdeModulePkg/Include \ >> -IUefiCpuPkg \ >> -IUefiCpuPkg/Include \ >> -IOvmfPkg \ >> -IOvmfPkg/Include \ >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c >> >> "ar" \ >> cr \ >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/OUTPUT/SecMain.lib \ >> >> @Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/OUTPUT/object_files.lst >> >> "gcc" \ >> -o Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.dll \ >> -nostdlib \ >> -Wl,-n,-q,--gc-sections \ >> -z common-page-size=0x40 \ >> -Wl,--entry,_ModuleEntryPoint \ >> -u _ModuleEntryPoint \ >> >> -Wl,-Map,Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.map >> \ >> -Wl,-m,elf_i386,--oformat=elf32-i386 \ >> >> -Wl,--start-group,@Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/OUTPUT/static_library_files.lst,--end-group >> \ >> -g \ >> -fshort-wchar \ >> -fno-builtin \ >> -fno-strict-aliasing \ >> -Wall \ >> -Werror \ >> -Wno-array-bounds \ >> -ffunction-sections \ >> -fdata-sections \ >> -include AutoGen.h \ >> -fno-common \ >> -DSTRING_ARRAY_NAME=SecMainStrings \ >> -m32 \ >> -march=i586 \ >> -malign-double \ >> -fno-stack-protector \ >> -D EFI32 \ >> -fno-asynchronous-unwind-tables \ >> -Wno-address \ >> -Os \ >> -mno-mmx \ >> -mno-sse \ >> -D DISABLE_NEW_DEPRECATED_INTERFACES \ >> -Wl,--defsym=PECOFF_HEADER_SIZE=0x220 \ >> -Wl,--script=BaseTools/Scripts/GccBase.lds >> >> "objcopy" >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.dll >> >> cp \ >> -f \ >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.dll \ >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.debug >> >> objcopy \ >> --strip-unneeded \ >> -R .eh_frame \ >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.dll >> >> objcopy \ >> >> --add-gnu-debuglink=Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.debug >> \ >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.dll >> >> cp \ >> -f \ >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.debug \ >> Build/OvmfIa32/DEBUG_GCC49/IA32/SecMain.debug >> >> "GenFw" \ >> -e SEC \ >> -o Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.efi \ >> Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.dll >> >> GNUmakefile:379: recipe for target >> 'Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.efi' >> failed >> make: *** >> [Build/OvmfIa32/DEBUG_GCC49/IA32/OvmfPkg/Sec/SecMain/DEBUG/SecMain.efi] >> Segmentation fault (core dumped) > > What I don't understand though is, if GenFw creates the debug directory > contents in the first place, then why clear it separately later (which > currently crashes); why not just skip pulling stuff into the debug > directory? > > Anyway, this was just an experiment on my part, I don't mind if the > regressive patch is reverted first. >
GenFw can take both PE/COFF and ELF files as input, and in the latter case, it performs the PE/COFF conversion itself. I tried the patch below, and it seems to get rid of the segfault. Could you please confirm? diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c b/BaseTools/Source/C/GenFw/Elf32Convert.c index f7b084dc9b84..14fe4a285857 100644 --- a/BaseTools/Source/C/GenFw/Elf32Convert.c +++ b/BaseTools/Source/C/GenFw/Elf32Convert.c @@ -1142,7 +1142,7 @@ WriteDebug32 ( NtHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + mNtHdrOffset); DataDir = &NtHdr->Pe32.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG]; DataDir->VirtualAddress = mDebugOffset; - DataDir->Size = Dir->SizeOfData + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); + DataDir->Size = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); } STATIC diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c index 7eed7b92d30f..c39bdff063ab 100644 --- a/BaseTools/Source/C/GenFw/Elf64Convert.c +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c @@ -1095,7 +1095,7 @@ WriteDebug64 ( NtHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + mNtHdrOffset); DataDir = &NtHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG]; DataDir->VirtualAddress = mDebugOffset; - DataDir->Size = Dir->SizeOfData + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); + DataDir->Size = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); } STATIC _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
