BaseTools/Source/C/GenFw/GenFw.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
GNU Binutils produce a PE debug directory with one
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY:
- the Type field of the entry is EFI_IMAGE_DEBUG_TYPE_CODEVIEW,
- the FileOffset field of the entry points right past the entry itself,
- the data structure placed at FileOffset is a CV_INFO_PDB20 structure,
with an "NB10" signature.
This is all correct, except GNU Binutils include the pointed-to
CV_INFO_PDB20 structure in the size of the debug directory (that is,
Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size).
That's a bug.
The malformed debug directory size causes the loop in GenFw's
ZeroDebugData() function to process the CV_INFO_PDB20 structure as a set
of EFI_IMAGE_DEBUG_DIRECTORY_ENTRY elements, which crashes GenFw.
This problem was exposed by commit e4129b0e5897 ("BaseTools: Update GenFw
to clear unused debug entry generated by VS tool chain", 2017-06-19).
Work around the Binutils issue by noticing when an
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY.FileOffset points back into the debug
directory. (This can never happen with a well-formed PE file.) In this
case, truncate DebugDirectoryEntrySize such that the debug directory will
end right before the debug structure pointed-to by
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY.FileOffset.
Tested with OVMF:
- gcc-4.8.5-14.el7.x86_64
- binutils-2.25.1-27.base.el7.x86_64
and with ArmVirtPkg:
- gcc-aarch64-linux-gnu-6.1.1-2.el7.x86_64
- binutils-aarch64-linux-gnu-2.27-3.el7.x86_64
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Yonghong Zhu <yonghong.zhu@intel.com>
Reported-by: Gerd Hoffmann <kraxel@redhat.com>
Reported-by: Leif Lindholm <leif.lindholm@linaro.org>
Ref: http://mid.mail-archive.com/a1de67a8-57c2-908e-dd4d-9726d60fb388@redhat.com
Ref: http://mid.mail-archive.com/20170705134136.GB26676@bivouac.eciton.net
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
Repo: https://github.com/lersek/edk2.git
Branch: binutils_debugdirsize_workaround
BaseTools/Source/C/GenFw/GenFw.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 6569460f34f7..a79f485ee681 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -2771,6 +2771,7 @@ Returns:
UINT32 Index;
UINT32 DebugDirectoryEntryRva;
UINT32 DebugDirectoryEntrySize;
+ UINT32 TruncatedDebugDirectorySize;
UINT32 DebugDirectoryEntryFileOffset;
UINT32 ExportDirectoryEntryRva;
UINT32 ExportDirectoryEntryFileOffset;
@@ -2893,6 +2894,25 @@ Returns:
DebugEntry = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset);
Index = 0;
for (Index=0; Index < DebugDirectoryEntrySize / sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); Index ++, DebugEntry ++) {
+ //
+ // Work around GNU Binutils bug: if the debug information pointed-to by
+ // DebugEntry was incorrectly included in DebugDirectoryEntrySize, then
+ // the debug directory doesn't actually extend past the pointed-to debug
+ // information. Truncate DebugDirectoryEntrySize accordingly.
+ //
+ if (DebugEntry->FileOffset >= DebugDirectoryEntryFileOffset &&
+ DebugEntry->FileOffset < (DebugDirectoryEntryFileOffset +
+ DebugDirectoryEntrySize)) {
+ TruncatedDebugDirectorySize = (DebugEntry->FileOffset -
+ DebugDirectoryEntryFileOffset);
+ VerboseMsg (
+ "truncating debug directory size from %u to %u",
+ DebugDirectoryEntrySize,
+ TruncatedDebugDirectorySize
+ );
+ DebugDirectoryEntrySize = TruncatedDebugDirectorySize;
+ }
+
DebugEntry->TimeDateStamp = 0;
if (ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData);
--
2.13.1.3.g8be5a757fa67
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 5 July 2017 at 17:42, Laszlo Ersek <lersek@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. So I don't
see how this could be a binutils bug.
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY:
> - the Type field of the entry is EFI_IMAGE_DEBUG_TYPE_CODEVIEW,
> - the FileOffset field of the entry points right past the entry itself,
> - the data structure placed at FileOffset is a CV_INFO_PDB20 structure,
> with an "NB10" signature.
>
> This is all correct, except GNU Binutils include the pointed-to
> CV_INFO_PDB20 structure in the size of the debug directory (that is,
> Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size).
> That's a bug.
>
> The malformed debug directory size causes the loop in GenFw's
> ZeroDebugData() function to process the CV_INFO_PDB20 structure as a set
> of EFI_IMAGE_DEBUG_DIRECTORY_ENTRY elements, which crashes GenFw.
>
> This problem was exposed by commit e4129b0e5897 ("BaseTools: Update GenFw
> to clear unused debug entry generated by VS tool chain", 2017-06-19).
>
> Work around the Binutils issue by noticing when an
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY.FileOffset points back into the debug
> directory. (This can never happen with a well-formed PE file.) In this
> case, truncate DebugDirectoryEntrySize such that the debug directory will
> end right before the debug structure pointed-to by
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY.FileOffset.
>
> Tested with OVMF:
> - gcc-4.8.5-14.el7.x86_64
> - binutils-2.25.1-27.base.el7.x86_64
>
> and with ArmVirtPkg:
> - gcc-aarch64-linux-gnu-6.1.1-2.el7.x86_64
> - binutils-aarch64-linux-gnu-2.27-3.el7.x86_64
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Reported-by: Gerd Hoffmann <kraxel@redhat.com>
> Reported-by: Leif Lindholm <leif.lindholm@linaro.org>
> Ref: http://mid.mail-archive.com/a1de67a8-57c2-908e-dd4d-9726d60fb388@redhat.com
> Ref: http://mid.mail-archive.com/20170705134136.GB26676@bivouac.eciton.net
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> Repo: https://github.com/lersek/edk2.git
> Branch: binutils_debugdirsize_workaround
>
> BaseTools/Source/C/GenFw/GenFw.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
> index 6569460f34f7..a79f485ee681 100644
> --- a/BaseTools/Source/C/GenFw/GenFw.c
> +++ b/BaseTools/Source/C/GenFw/GenFw.c
> @@ -2771,6 +2771,7 @@ Returns:
> UINT32 Index;
> UINT32 DebugDirectoryEntryRva;
> UINT32 DebugDirectoryEntrySize;
> + UINT32 TruncatedDebugDirectorySize;
> UINT32 DebugDirectoryEntryFileOffset;
> UINT32 ExportDirectoryEntryRva;
> UINT32 ExportDirectoryEntryFileOffset;
> @@ -2893,6 +2894,25 @@ Returns:
> DebugEntry = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset);
> Index = 0;
> for (Index=0; Index < DebugDirectoryEntrySize / sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); Index ++, DebugEntry ++) {
> + //
> + // Work around GNU Binutils bug: if the debug information pointed-to by
> + // DebugEntry was incorrectly included in DebugDirectoryEntrySize, then
> + // the debug directory doesn't actually extend past the pointed-to debug
> + // information. Truncate DebugDirectoryEntrySize accordingly.
> + //
> + if (DebugEntry->FileOffset >= DebugDirectoryEntryFileOffset &&
> + DebugEntry->FileOffset < (DebugDirectoryEntryFileOffset +
> + DebugDirectoryEntrySize)) {
> + TruncatedDebugDirectorySize = (DebugEntry->FileOffset -
> + DebugDirectoryEntryFileOffset);
> + VerboseMsg (
> + "truncating debug directory size from %u to %u",
> + DebugDirectoryEntrySize,
> + TruncatedDebugDirectorySize
> + );
> + DebugDirectoryEntrySize = TruncatedDebugDirectorySize;
> + }
> +
> DebugEntry->TimeDateStamp = 0;
> if (ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
> memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData);
> --
> 2.13.1.3.g8be5a757fa67
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 07/05/17 18:45, Ard Biesheuvel wrote: > On 5 July 2017 at 17:42, Laszlo Ersek <lersek@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. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 5 July 2017 at 18:28, Laszlo Ersek <lersek@redhat.com> wrote: > On 07/05/17 18:45, Ard Biesheuvel wrote: >> On 5 July 2017 at 17:42, Laszlo Ersek <lersek@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
On 07/05/17 19:33, Ard Biesheuvel wrote: > On 5 July 2017 at 18:28, Laszlo Ersek <lersek@redhat.com> wrote: >> On 07/05/17 18:45, Ard Biesheuvel wrote: >>> On 5 July 2017 at 17:42, Laszlo Ersek <lersek@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 > I swear I found the same (and sent my previous email) before getting yours. :) (I mirror my IMAP stuff every 5 minutes.) So yes, this works. (Checked with OVMF IA32 and IA32X64.) Thanks, Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 07/05/17 18:45, Ard Biesheuvel wrote:
> On 5 July 2017 at 17:42, Laszlo Ersek <lersek@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. So I don't
> see how this could be a binutils bug.
Geez, you are totally right. From
"BaseTools/Source/C/GenFw/Elf64Convert.c":
> STATIC
> VOID
> WriteDebug64 (
> VOID
> )
> {
> UINT32 Len;
> EFI_IMAGE_OPTIONAL_HEADER_UNION *NtHdr;
> EFI_IMAGE_DATA_DIRECTORY *DataDir;
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *Dir;
> EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY *Nb10;
>
> Len = strlen(mInImageName) + 1;
>
> Dir = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY*)(mCoffFile + mDebugOffset);
> Dir->Type = EFI_IMAGE_DEBUG_TYPE_CODEVIEW;
> Dir->SizeOfData = sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) + Len;
> Dir->RVA = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> Dir->FileOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>
> Nb10 = (EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY*)(Dir + 1);
> Nb10->Signature = CODEVIEW_SIGNATURE_NB10;
> strcpy ((char *)(Nb10 + 1), mInImageName);
>
>
> 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);
> }
The last assignment has the bug. It should be
DataDir->Size = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 5 July 2017 at 18:33, Laszlo Ersek <lersek@redhat.com> wrote:
> On 07/05/17 18:45, Ard Biesheuvel wrote:
>> On 5 July 2017 at 17:42, Laszlo Ersek <lersek@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. So I don't
>> see how this could be a binutils bug.
>
> Geez, you are totally right. From
> "BaseTools/Source/C/GenFw/Elf64Convert.c":
>
>
>> STATIC
>> VOID
>> WriteDebug64 (
>> VOID
>> )
>> {
>> UINT32 Len;
>> EFI_IMAGE_OPTIONAL_HEADER_UNION *NtHdr;
>> EFI_IMAGE_DATA_DIRECTORY *DataDir;
>> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *Dir;
>> EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY *Nb10;
>>
>> Len = strlen(mInImageName) + 1;
>>
>> Dir = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY*)(mCoffFile + mDebugOffset);
>> Dir->Type = EFI_IMAGE_DEBUG_TYPE_CODEVIEW;
>> Dir->SizeOfData = sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) + Len;
>> Dir->RVA = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>> Dir->FileOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>>
>> Nb10 = (EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY*)(Dir + 1);
>> Nb10->Signature = CODEVIEW_SIGNATURE_NB10;
>> strcpy ((char *)(Nb10 + 1), mInImageName);
>>
>>
>> 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);
>> }
>
> The last assignment has the bug. It should be
>
> DataDir->Size = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>
OK, I will take that as an affirmative answer to my question. Are you
sending a patch?
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 07/05/17 19:37, Ard Biesheuvel wrote:
> On 5 July 2017 at 18:33, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 07/05/17 18:45, Ard Biesheuvel wrote:
>>> On 5 July 2017 at 17:42, Laszlo Ersek <lersek@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. So I don't
>>> see how this could be a binutils bug.
>>
>> Geez, you are totally right. From
>> "BaseTools/Source/C/GenFw/Elf64Convert.c":
>>
>>
>>> STATIC
>>> VOID
>>> WriteDebug64 (
>>> VOID
>>> )
>>> {
>>> UINT32 Len;
>>> EFI_IMAGE_OPTIONAL_HEADER_UNION *NtHdr;
>>> EFI_IMAGE_DATA_DIRECTORY *DataDir;
>>> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *Dir;
>>> EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY *Nb10;
>>>
>>> Len = strlen(mInImageName) + 1;
>>>
>>> Dir = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY*)(mCoffFile + mDebugOffset);
>>> Dir->Type = EFI_IMAGE_DEBUG_TYPE_CODEVIEW;
>>> Dir->SizeOfData = sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) + Len;
>>> Dir->RVA = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>>> Dir->FileOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>>>
>>> Nb10 = (EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY*)(Dir + 1);
>>> Nb10->Signature = CODEVIEW_SIGNATURE_NB10;
>>> strcpy ((char *)(Nb10 + 1), mInImageName);
>>>
>>>
>>> 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);
>>> }
>>
>> The last assignment has the bug. It should be
>>
>> DataDir->Size = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
>>
>
> OK, I will take that as an affirmative answer to my question. Are you
> sending a patch?
>
You send it please, just give me some "Co-debugged-by:" or whatever. :)
Also, please add a ref to the mailing list thread.
Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.