From 6ea72cd31f36ad37176f614b45fc730613dfde08 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 8 Mar 2018 11:38:21 +0100 Subject: [PATCH] Fix GCC 8 compilation, replace obsolete tools as build-time dependencies --- ....makefile-add-Wno-stringop-truncatio.patch | 72 ++++++++++++ ...ols-header.makefile-add-Wno-restrict.patch | 104 ++++++++++++++++++ ...-silence-false-stringop-overflow-war.patch | 65 +++++++++++ build-iso.sh | 26 ++--- edk2.spec | 12 +- 5 files changed, 260 insertions(+), 19 deletions(-) create mode 100644 0025-BaseTools-header.makefile-add-Wno-stringop-truncatio.patch create mode 100644 0026-BaseTools-header.makefile-add-Wno-restrict.patch create mode 100644 0027-BaseTools-GenVtf-silence-false-stringop-overflow-war.patch diff --git a/0025-BaseTools-header.makefile-add-Wno-stringop-truncatio.patch b/0025-BaseTools-header.makefile-add-Wno-stringop-truncatio.patch new file mode 100644 index 0000000..1da58be --- /dev/null +++ b/0025-BaseTools-header.makefile-add-Wno-stringop-truncatio.patch @@ -0,0 +1,72 @@ +From 3416fa678a9b634910faffbf9479a82f4969f7b1 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Fri, 2 Mar 2018 19:09:22 +0100 +Subject: [PATCH 1/3] BaseTools/header.makefile: add "-Wno-stringop-truncation" + +gcc-8 (which is part of Fedora 28) enables the new warning +"-Wstringop-truncation" in "-Wall". This warning is documented in detail +at ; the +introduction says + +> Warn for calls to bounded string manipulation functions such as strncat, +> strncpy, and stpncpy that may either truncate the copied string or leave +> the destination unchanged. + +It breaks the BaseTools build with: + +> EfiUtilityMsgs.c: In function 'PrintMessage': +> EfiUtilityMsgs.c:484:9: error: 'strncat' output may be truncated copying +> between 0 and 511 bytes from a string of length 511 +> [-Werror=stringop-truncation] +> strncat (Line, Line2, MAX_LINE_LEN - strlen (Line) - 1); +> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +> EfiUtilityMsgs.c:469:9: error: 'strncat' output may be truncated copying +> between 0 and 511 bytes from a string of length 511 +> [-Werror=stringop-truncation] +> strncat (Line, Line2, MAX_LINE_LEN - strlen (Line) - 1); +> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +> EfiUtilityMsgs.c:511:5: error: 'strncat' output may be truncated copying +> between 0 and 511 bytes from a string of length 511 +> [-Werror=stringop-truncation] +> strncat (Line, Line2, MAX_LINE_LEN - strlen (Line) - 1); +> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The right way to fix the warning would be to implement string concat with +snprintf(). However, Microsoft does not appear to support snprintf() +before VS2015 +, +so we just have to shut up the warning. The strncat() calls flagged above +are valid BTW. + +Cc: Ard Biesheuvel +Cc: Cole Robinson +Cc: Liming Gao +Cc: Paolo Bonzini +Cc: Yonghong Zhu +Contributed-under: TianoCore Contribution Agreement 1.1 +Signed-off-by: Laszlo Ersek +Message-Id: <20180302180924.4312-2-lersek@redhat.com> +Signed-off-by: Paolo Bonzini +--- + BaseTools/Source/C/Makefiles/header.makefile | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile +index 063982b82f..6c3826aecb 100644 +--- a/BaseTools/Source/C/Makefiles/header.makefile ++++ b/BaseTools/Source/C/Makefiles/header.makefile +@@ -47,9 +47,9 @@ INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I $(MAKEROOT)/Include/Common -I $(MAKE + BUILD_CPPFLAGS = $(INCLUDE) -O2 + ifeq ($(DARWIN),Darwin) + # assume clang or clang compatible flags on OS X +-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-self-assign -Wno-unused-result -nostdlib -c -g ++BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-self-assign -Wno-unused-result -nostdlib -c -g + else +-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-unused-result -nostdlib -c -g ++BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-unused-result -nostdlib -c -g + endif + BUILD_LFLAGS = + BUILD_CXXFLAGS = -Wno-unused-result +-- +2.14.3 + diff --git a/0026-BaseTools-header.makefile-add-Wno-restrict.patch b/0026-BaseTools-header.makefile-add-Wno-restrict.patch new file mode 100644 index 0000000..3d8f951 --- /dev/null +++ b/0026-BaseTools-header.makefile-add-Wno-restrict.patch @@ -0,0 +1,104 @@ +From 07a446e7f869f79104bcc1d6accdc462e7bf7ba5 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Fri, 2 Mar 2018 19:09:23 +0100 +Subject: [PATCH 2/3] BaseTools/header.makefile: add "-Wno-restrict" + +gcc-8 (which is part of Fedora 28) enables the new warning +"-Wrestrict" in "-Wall". This warning is documented in detail +at ; the +introduction says + +> Warn when an object referenced by a restrict-qualified parameter (or, in +> C++, a __restrict-qualified parameter) is aliased by another argument, +> or when copies between such objects overlap. + +It breaks the BaseTools build (in the Brotli compression library) with: + +> In function 'ProcessCommandsInternal', +> inlined from 'ProcessCommands' at dec/decode.c:1828:10: +> dec/decode.c:1781:9: error: 'memcpy' accessing between 17 and 2147483631 +> bytes at offsets 16 and 16 overlaps between 17 and 2147483631 bytes at +> offset 16 [-Werror=restrict] +> memcpy(copy_dst + 16, copy_src + 16, (size_t)(i - 16)); +> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +> In function 'ProcessCommandsInternal', +> inlined from 'SafeProcessCommands' at dec/decode.c:1833:10: +> dec/decode.c:1781:9: error: 'memcpy' accessing between 17 and 2147483631 +> bytes at offsets 16 and 16 overlaps between 17 and 2147483631 bytes at +> offset 16 [-Werror=restrict] +> memcpy(copy_dst + 16, copy_src + 16, (size_t)(i - 16)); +> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Paolo Bonzini analyzed the Brotli source in detail, +and concluded that the warning is a false positive: + +> This seems safe to me, because it's preceded by: +> +> uint8_t* copy_dst = &s->ringbuffer[pos]; +> uint8_t* copy_src = &s->ringbuffer[src_start]; +> int dst_end = pos + i; +> int src_end = src_start + i; +> if (src_end > pos && dst_end > src_start) { +> /* Regions intersect. */ +> goto CommandPostWrapCopy; +> } +> +> If [src_start, src_start + i) and [pos, pos + i) don't intersect, then +> neither do [src_start + 16, src_start + i) and [pos + 16, pos + i). +> +> The if seems okay: +> +> (src_start + i > pos && pos + i > src_start) +> +> which can be rewritten to: +> +> (pos < src_start + i && src_start < pos + i) +> +> Then the numbers are in one of these two orders: +> +> pos <= src_start < pos + i <= src_start + i +> src_start <= pos < src_start + i <= pos + i +> +> These two would be allowed by the "if", but they can only happen if pos +> == src_start so they degenerate to the same two orders above: +> +> pos <= src_start < src_start + i <= pos + i +> src_start <= pos < pos + i <= src_start + i +> +> So it is a false positive in GCC. + +Disable the warning for now. + +Cc: Ard Biesheuvel +Cc: Cole Robinson +Cc: Liming Gao +Cc: Paolo Bonzini +Cc: Yonghong Zhu +Reported-by: Cole Robinson +Contributed-under: TianoCore Contribution Agreement 1.1 +Signed-off-by: Laszlo Ersek +Message-Id: <20180302180924.4312-3-lersek@redhat.com> +Signed-off-by: Paolo Bonzini +--- + BaseTools/Source/C/Makefiles/header.makefile | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile +index 6c3826aecb..3feae10095 100644 +--- a/BaseTools/Source/C/Makefiles/header.makefile ++++ b/BaseTools/Source/C/Makefiles/header.makefile +@@ -47,9 +47,9 @@ INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I $(MAKEROOT)/Include/Common -I $(MAKE + BUILD_CPPFLAGS = $(INCLUDE) -O2 + ifeq ($(DARWIN),Darwin) + # assume clang or clang compatible flags on OS X +-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-self-assign -Wno-unused-result -nostdlib -c -g ++BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict -Wno-self-assign -Wno-unused-result -nostdlib -c -g + else +-BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-unused-result -nostdlib -c -g ++BUILD_CFLAGS = -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-stringop-truncation -Wno-restrict -Wno-unused-result -nostdlib -c -g + endif + BUILD_LFLAGS = + BUILD_CXXFLAGS = -Wno-unused-result +-- +2.14.3 + diff --git a/0027-BaseTools-GenVtf-silence-false-stringop-overflow-war.patch b/0027-BaseTools-GenVtf-silence-false-stringop-overflow-war.patch new file mode 100644 index 0000000..7d8a339 --- /dev/null +++ b/0027-BaseTools-GenVtf-silence-false-stringop-overflow-war.patch @@ -0,0 +1,65 @@ +From 8633e2951d8eba59755c82ef10099ed47eafd474 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Fri, 2 Mar 2018 19:09:24 +0100 +Subject: [PATCH 3/3] BaseTools/GenVtf: silence false "stringop-overflow" + warning with memcpy() + +gcc-8 (which is part of Fedora 28) enables the new warning +"-Wstringop-overflow" in "-Wall". This warning is documented in detail at +; the +introduction says + +> Warn for calls to string manipulation functions such as memcpy and +> strcpy that are determined to overflow the destination buffer. + +It breaks the BaseTools build with: + +> GenVtf.c: In function 'ConvertVersionInfo': +> GenVtf.c:132:7: error: 'strncpy' specified bound depends on the length +> of the source argument [-Werror=stringop-overflow=] +> strncpy (TemStr + 4 - Length, Str, Length); +> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +> GenVtf.c:130:14: note: length computed here +> Length = strlen(Str); +> ^~~~~~~~~~~ + +It is a false positive because, while the bound equals the length of the +source argument, the destination pointer is moved back towards the +beginning of the destination buffer by the same amount (and this amount is +range-checked first, so we can't precede the start of the dest buffer). + +Replace both strncpy() calls with memcpy(). + +Cc: Ard Biesheuvel +Cc: Cole Robinson +Cc: Liming Gao +Cc: Paolo Bonzini +Cc: Yonghong Zhu +Reported-by: Cole Robinson +Contributed-under: TianoCore Contribution Agreement 1.1 +Signed-off-by: Laszlo Ersek +Message-Id: <20180302180924.4312-4-lersek@redhat.com> +Signed-off-by: Paolo Bonzini +--- + BaseTools/Source/C/GenVtf/GenVtf.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/BaseTools/Source/C/GenVtf/GenVtf.c b/BaseTools/Source/C/GenVtf/GenVtf.c +index 2ae9a7be2c..0cd33e71e9 100644 +--- a/BaseTools/Source/C/GenVtf/GenVtf.c ++++ b/BaseTools/Source/C/GenVtf/GenVtf.c +@@ -129,9 +129,9 @@ Returns: + } else { + Length = strlen(Str); + if (Length < 4) { +- strncpy (TemStr + 4 - Length, Str, Length); ++ memcpy (TemStr + 4 - Length, Str, Length); + } else { +- strncpy (TemStr, Str + Length - 4, 4); ++ memcpy (TemStr, Str + Length - 4, 4); + } + + sscanf ( +-- +2.14.3 + diff --git a/build-iso.sh b/build-iso.sh index 4643e16..413c119 100644 --- a/build-iso.sh +++ b/build-iso.sh @@ -6,25 +6,21 @@ dir="$1" # cfg shell="$dir/Shell.efi" enroll="$dir/EnrollDefaultKeys.efi" +root="$dir/image" vfat="$dir/shell.img" iso="$dir/UefiShell.iso" -export MTOOLS_SKIP_CHECK=1 -# calc size -s1=$(stat --format=%s -- $shell) -s2=$(stat --format=%s -- $enroll) -size=$(( ($s1 + $s2) * 11 / 10 )) -set -x - -# create non-partitioned FAT image -/usr/sbin/mkdosfs -C "$vfat" -n UEFI_SHELL -- "$(( $size / 1024 ))" -mmd -i "$vfat" ::efi -mmd -i "$vfat" ::efi/boot -mcopy -i "$vfat" "$shell" ::efi/boot/bootx64.efi -mcopy -i "$vfat" "$enroll" :: -#mdir -i "$vfat" -/ :: +# create non-partitioned (1.44 MB floppy disk) FAT image +mkdir "$root" +mkdir "$root"/efi +mkdir "$root"/efi/boot +cp "$shell" "$root"/efi/boot/bootx64.efi +cp "$enroll" "$root" +qemu-img convert --image-opts \ + driver=vvfat,floppy=on,fat-type=12,label=UEFI_SHELL,dir="$root/" \ + $vfat # build ISO with FAT image file as El Torito EFI boot image genisoimage -input-charset ASCII -J -rational-rock \ -efi-boot "${vfat##*/}" -no-emul-boot -o "$iso" -- "$vfat" -rm -f "$vfat" +rm -rf "$root/" "$vfat" diff --git a/edk2.spec b/edk2.spec index 37febc9..f68b0c1 100644 --- a/edk2.spec +++ b/edk2.spec @@ -66,11 +66,12 @@ Patch0018: 0018-ArmVirtPkg-set-early-hello-message.patch Patch0019: 0019-MdeModulePkg-PciBus-Fix-bug-that-PCI-BUS-claims-too-much-resource.patch Patch0020: 0020-MdeModulePkg-Bds-Remove-assertion-in-BmCharToUint.patch Patch0021: 0021-MdeModulePkg-Bds-Check-variable-name-even-if-OptionNumber-is-NULL.patch - -# submitted upstream Patch0022: 0022-OvmfPkg-make-it-a-proper-BASE-library.patch Patch0023: 0023-OvmfPkg-create-a-separate-PlatformDebugLibIoPort-ins.patch Patch0024: 0024-OvmfPkg-save-on-I-O-port-accesses-when-the-debug-por.patch +Patch0025: 0025-BaseTools-header.makefile-add-Wno-stringop-truncatio.patch +Patch0026: 0026-BaseTools-header.makefile-add-Wno-restrict.patch +Patch0027: 0027-BaseTools-GenVtf-silence-false-stringop-overflow-war.patch %if 0%{?cross:1} # Tweak the tools_def to support cross-compiling. @@ -105,8 +106,7 @@ BuildRequires: gcc-x86_64-linux-gnu %endif BuildRequires: iasl BuildRequires: nasm -BuildRequires: dosfstools -BuildRequires: mtools +BuildRequires: qemu-img BuildRequires: genisoimage @@ -447,6 +447,10 @@ ln -sf ../%{name}/arm/QEMU_EFI-pflash.raw %{buildroot}/usr/share/AAVMF/ %changelog +* Wed Mar 07 2018 Paolo Bonzini - 20171011git92d07e4-5 +- Fix GCC 8 compilation +- Replace dosfstools and mtools with qemu-img vvfat + * Wed Feb 07 2018 Fedora Release Engineering - 20171011git92d07e4-4 - Rebuilt for https://fedoraproject.org/wiki/Fedora_28_Mass_Rebuild