It seems that during the build of edk2-20171011git92d07e4-3.fc28, no attempt was made to inject the RPM build flags. BootSectImage is installed as /usr/bin/BootSectImage, so I expect it to be build with the usual complement of compiler/ELF linker flags. Yet it is compiled with the RPM build flags: make -C BootSectImage make[2]: Entering directory '/builddir/build/BUILD/tianocore-edk2-92d07e4/BaseTools/Source/C/BootSectImage' gcc -c -MD -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-deprecated-declarations -Wno-unused-result -nostdlib -c -g -I .. -I ../Include/Common -I ../Include/ -I ../Include/IndustryStandard -I ../Common/ -I .. -I . -I ../Include/X64/ -O2 bootsectimage.c -o bootsectimage.o gcc -o ../bin/BootSectImage bootsectimage.o -L../libs -lCommon make[2]: Leaving directory '/builddir/build/BUILD/tianocore-edk2-92d07e4/BaseTools/Source/C/BootSectImage'
This bug appears to have been reported against 'rawhide' during the Fedora 28 development cycle. Changing version to '28'.
ping?
Thanks for the CC -- I don't understand the report. I understand that the compiler options aren't right, but the details are unclear: - "usual complement of compiler/ELF linker flags" vs. - "compiled with the RPM build flags" I would expect the latter to mean exactly the former; i.e. I'd expect RPM to inject the necessary flags (and then the BZ would mean that the spec file fails to use the RPM-exposed build options). Florian, did you mean that the edk2-tools package was built *without* the RPM build flags? (Anyway I'll definitely follow Paolo's lead here, so please excuse me if the report is clear and my understanding is lacking.)
BTW, external compiler options should not be injected for building the firmware binaries themselves (the edk2-ovmf and edk2-aarch64 noarch packages) -- those builds take very quirky sets of compiler options that upstream manages closely. (The argument "the firmware binaries follow a different ABI" applies.) Regarding the userspace ELF applications in edk2-tools, I believe that the bug report may be valid.
(In reply to Laszlo Ersek from comment #5) > Thanks for the CC -- I don't understand the report. I understand that the > compiler options aren't right, but the details are unclear: > > - "usual complement of compiler/ELF linker flags" > > vs. > > - "compiled with the RPM build flags" > > I would expect the latter to mean exactly the former; i.e. I'd expect RPM to > inject the necessary flags (and then the BZ would mean that the spec file > fails to use the RPM-exposed build options). > > Florian, did you mean that the edk2-tools package was built *without* the > RPM build flags? There's two things to deal with When *compiling* native binaries we need to honour the flags in %{optflags}. On F28 that expands to -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection When *linking* native binaries we need to honour the flags in %{__global_ldflags}. On Fedora 28 that expands to -Wl,-z,relro -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld NB, the spec should use the %{...} macros not hardcode the flags i illustrate here. EDKs build system is a custom job so its a matter of figuring out how to pass these options into it. For the actual firmware builds, as you say, there's quite specific needs that it has to avoid breakage. We should none the less look at each of the flags above and determine which, if any, can be practically & usefully enabled.
(quoting slightly out of order) (In reply to Daniel Berrange from comment #7) > For the actual firmware builds, as you say, there's quite specific needs > that it has to avoid breakage. We should none the less look at each of the > flags above and determine which, if any, can be practically & usefully > enabled. We build all firmware binaries for the DEBUG build target. This target preserves assertions and debug messages, and does not, as a rule, forbid compiler optimizations. (Optimizations may be disabled nonetheless, DEBUG just means that the user doesn't specifically request disabling compiler optimizations. That would be the NOOPT build target.) In addition, in Fedora we still use the GCC49 toolchain settings, AFAICS. We could as well select GCC5 (the upstream default / recommendation is GCC5, AIUI). The main difference with GCC5 is link time optimization (LTO). Checking the flags/build logs for DEBUG, and GCC49 / GCC5, yields: > %{optflags}. On F28 that expands to > > -O2 ARM AARCH64 IA32 X64 GCC49 -O0 -O0 -Os -Os GCC5 -Os -Os -Os -Os Optimizing for size in firmware binaries makes sense. In the GCC49/{ARM,AARCH64} cases, -O0 goes back to commit 4afd3d042215 ("Sync BaseTool trunk (version r2599) into EDKII BaseTools.", 2013-08-23). At that time, the upstream edk2 project's source control practices weren't really advanced, so I can't tell why -O0 was picked. Either way, the solution for this should be to go with the GCC5 toolchain settings down-stream. (Tweaking compiler flags for old toolchains won't be much welcomed up-stream.) > -g Present in all six cases. (Note that actual live debugging the firmware, using the GDB server in QEMU, and debug symbols, takes very awkward measures that are not upstream. The procedure also does not cover all parts of the firmware. So extracting debug symbols for RPMs is not useful; the "-g" here is just a "best effort".) > -pipe Does not affect the produced object code. > -Wall Present in all six cases. A very carefully curated set of -Wno-xxx flags is employed, dependent on toolchain and architecture. I wouldn't like to track down each and every one of those, but the general practice has been for years that each such addition is posted and reviewed as a separate patch, with concrete justification. Examples are: -Wno-array-bounds -Wno-address -Wno-unused-but-set-variable -Wno-unused-const-variable Usually these address false positives that are not possible to suppress or work around from the source code. > -Werror=format-security "-Werror" in itself is present in all six cases. "format-security" wouldn't be helpful because (a) the firmware isn't built against a standard C library, (b) while there are printf-like facilities, their format specifications don't closely follow ISO C. (QEMU uses GCC_FMT_ATTR(), as a "printf format attribute" wrapper; in edk2, such isn't currently used, and I wonder if it would be possible at all.) > -Wp,-D_FORTIFY_SOURCE=2 According to feature_test_macros(7), this instruments some glibc functions and some gcc intrinsics to catch some buffer overflows. The firmware isn't built against a C library, and compiler intrinsics are specifically forbidden / disabled. (I recall that some ARM/AARCH64 intrinsics were impossible to disable, so they have been reimplemented / open-coded in the edk2 tree. Those can't be instrumented either way.) > -Wp,-D_GLIBCXX_ASSERTIONS According to the libstdc++ manual, "enables extra error checking in the form of precondition assertions, such as bounds checking in strings and null pointer checks when dereferencing smart pointers". C++ is not used or allowed in edk2. > -fexceptions Ditto. > -fstack-protector-strong We've got an interesting picture here: ARM AARCH64 IA32 X64 GCC49 -fstack-protector [none] -fno-stack-protector -fno-stack-protector GCC5 -fstack-protector [none] -fno-stack-protector -fno-stack-protector The IA32 and X64 switches go back to historical commit a709adfaf0be ("Sync tool code to BuildTools project r1783.", 2009-12-29), which is when the GCC toolchain was enabled in the first place (starting with GCC44). The ARM setting comes from commit cb6032832355 ("BaseTools: Added support for GCC stack protector for ARM architecture", 2014-08-20). This suggests that supporting a stack protector in the firmware requires extra work. The AARCH64 case (lack of any relevant option) likely signals that noone has implemented the stack protector for that architecture yet. > -grecord-gcc-switches Absent in all six cases. According to the gcc manual, this saves GCC switches as part of the DWARF debugging information. edk2 uses PE/COFF though, so even if we wanted to save the compiler switches somehow, this likely wouldn't be appropriate. > -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 I can't really parse this file, but it appears to relate to position independent code / executables. ARM AARCH64 IA32 X64 GCC49 -fno-pic -fno-pie -fno-pic -fno-pie [none] -fpie GCC5 -fno-pic -fno-pie -fno-pic -fno-pie [none] -fpie I recall repeated discussions and reworkings of these flags up-stream (including code model switches); so I won't even try to track down the list of commits that have led to the above situation. We definitely cannot change these flags down-stream. > -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 After consulting <https://fedoraproject.org/wiki/Changes/Annobin>; this is not supported by edk2. If we enabled it, it would likely break the ELF-to-PE/COFF conversion tools. Intel seems to run static analysis tools on the edk2 codebase on-and-off, but I don't know any details. > -m64 > -mtune=generic These will require a larger table. GCC49 and GCC5 specify the same: ARM AARCH64 IA32 X64 -mthumb -mlittle-endian -m32 -m64 -march=armv7-a -mcmodel=small -march=i586 -maccumulate-outgoing-args -mlittle-endian -malign-double -mno-red-zone -mabi=aapcs -mno-mmx -mcmodel=small -mfloat-abi=soft -mno-sse -mno-mmx -mword-relocations -mno-sse I don't think we can change anything about these down-stream. > -fasynchronous-unwind-tables All six cases specify the opposite, that is, "-fno-asynchronous-unwind-tables". The ".eh_frame" ELF section is not handled by the ELF-to-PE/COFF conversion tools in edk2; for more background, please see e.g. commit cbf00651eda6 ("BaseTools/tools_def: add "-fno-unwind-tables" to GCC_AARCH64_CC_FLAGS", 2018-05-23). > -fstack-clash-protection Not used in either of the six cases. This option is new in gcc-8, and as thus it would require introducing a "GCC8" toolchain in edk2. Even in that case, the option seems to require dedicated target / OS support, which edk2 most likely doesn't have. (The edk2 core provides a feature called "guard pages", however at this point it is qualified as a debug feature, not a production one. Indeed it has produced a good number of regressions up-stream, sometimes even for firmware platforms that don't enable the feature. Thus, none of the six platforms under discussion enable it, up- or down-stream.) > -fcf-protection Again, new in gcc-8. Based on the manual and the release notes, I'm pretty sure it would take dedicated work to support this in firmware code (if it wasn't ruled out at once). > %{__global_ldflags}. On Fedora 28 that expands to > > -Wl,-z,relro > -Wl,-z,now To my understanding, these together allow ld-linux.so to map various sections of the executable as read-only into memory. In the firmware, no dynamic ELF loader like ld-linux.so is used. > -specs=/usr/lib/rpm/redhat/redhat-hardened-ld Seems to accompany the compile-time pie/pic flags; indeed this is handled explicitly in the firmware together with those, wherever appropriate. If one searches the edk2 git log for "-pie", there are tons of matches. --*-- Regarding the build tools: > EDKs build system is a custom job so its a matter of figuring out how to > pass these options into it. It requires patching "BaseTools/Source/C/Makefiles/header.makefile".
Wow. I managed to mistype "all *eight* cases" as "all six cases" several times. I'm sorry; I meant "all *eight* cases" every time, corresponding to { GCC49, GCC5 } x { ARM, AARCH64, IA32, X64 }
(In reply to Laszlo Ersek from comment #8) > (The edk2 core provides a feature called "guard pages", however at this > point it is qualified as a debug feature, not a production one. Indeed it > has produced a good number of regressions up-stream, sometimes even for > firmware platforms that don't enable the feature. Thus, none of the six > platforms under discussion enable it, up- or down-stream.) * To elaborate a bit, the above applies to the following features from "MdeModulePkg.dec": - UEFI heap page guard (bit#0 in PcdHeapGuardPropertyMask) - UEFI heap pool guard (bit#1) - SMM heap page guard (bit#2) - SMM heap pool guard (bit#3) - UEFI Stack Guard (bool-valued PcdCpuStackGuard) Beyond the robustness aspects, the SMM heap guard is known to conflict with building static page tables in SMM (see "PcdCpuSmmStaticPageTable" in "UefiCpuPkg.dec", and commit 1015fb3c1beb), which we do enable, both up-stream and down-stream. (For some more background on the SMM static page tables, search RHBZ#1468526 for "PcdCpuSmmStaticPageTable".) * The following feature is enabled however (both upstream and downstream), from "UefiCpuPkg.dec": - SMM Stack Guard (bool-valued PcdCpuSmmStackGuard) See commit 509f8425b75d ("UefiCpuPkg: change PcdCpuSmmStackGuard default to TRUE", 2016-06-06), and RHBZ#1341733.
Posted the upstream series "[edk2] [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller". http://mid.mail-archive.com/20180726004415.13381-1-lersek@redhat.com https://lists.01.org/pipermail/edk2-devel/2018-July/027606.html Can be tested like this (outside of an RPM spec file), for example: source edksetup.sh make -C "$EDK_TOOLS_PATH" \ EXTRA_OPTFLAGS="$(rpm --eval '%{optflags}')" \ EXTRA_LDFLAGS="$(rpm --eval '%{__global_ldflags}')" build -a IA32 -a X64 -p OvmfPkg/OvmfPkgIa32X64.dsc -t GCC5 -b DEBUG -n 4 \ -D SMM_REQUIRE -D SECURE_BOOT_ENABLE -D TLS_ENABLE -D NETWORK_IP6_ENABLE \ -D HTTP_BOOT_ENABLE --cmd-len=65536 --hash --genfds-multi-thread
clearing needinfo from comment 2 first
My patches from comment 11 have received a Reviewed-by from one of the upstream BaseTools maintainers, Liming Gao. However, Liming has a preference/request that I have to bring back here, to this RHBZ, for discussion. For inclusion in the "edk2-tools" subpackage, we build several native (ELF) userspace executables. They should be built -- compiled, and linked -- with the appropriate RPM flags, i.e. %{optflags} and %{__global_ldflags}, respectively. However, one of the utilities is a bit special, namely "VfrCompile". For that, I first need to provide some background. ( The UEFI spec documents the "IFR" format, as a part of the huge HII (Human Interface Infrastructure) definition. "IFR" stands for "Internal Forms Representation". It is a byte-code format for describing "setup dialogs" in a compact and platform-independent manner. The IFR blobs are provided as separate "resource-like" sections in the UEFI executables. The UEFI Setup Browser program interprets them according to the spec (platform-independently), but it displays (renders) them in a platform *dependent* manner. Consider a discrete PCI network interface card, and a discrete PCI video card, both coming with UEFI option ROMs. The NIC will let you configure various knobs in the "BIOS Setup" (the kind of knob that you might be familiar with from "ethtool" in Linux, for example). The video card will let you configure -- also in the "BIOS Setup" -- the preferred boot-time resolution, refresh rate, preferred output connector, possibly multi-head, and so on. For this, the UEFI drivers in the oproms of the cards provide their setup dialogs represented as IFR blobs. *How* exactly those dialogs are displayed is platform-dependent. On a "gaming platform", the platform firmware (integrated in the motherboard) will likely provide a graphical "BIOS Setup", with bells and whistles, USB mouse support, animations, nice colors, basically everything you do *not* need in a "BIOS Setup". Whereas, on a "server platform", you'll probably end up with a bare-bones TUI-like "BIOS Setup", with no mouse support, etc. The point is that the NIC's and the GPU's setup forms will not look and feel "out of place" on either platform, and they will let you configure the exact same knobs. On both platforms, the "logic" comes from the oprom drivers (native executable code + IFR byte code), while the rendering and the human interaction come from the platform firmware. Now, the IFR bytecode of the UEFI driver can be produced at both build time and runtime. Usually a hybrid approach is used: the statically built IFR contains the main form layout, with the labels, questions, buttons, anchors, and so on, while the C code of the UEFI driver dynamically populates some of the fields (such as drop-down lists, radio buttons, ...), by inserting dynamically generated opcodes relative to various static anchors. Obviously this is extremely messy, and the general approach is to build as much of the IFR as possible statically. Enter "VfrCompile". Intel developed a *textual* form descriptor language called "VFR" -- Visual Forms Representation. Several edk2 drivers have VFR files among their source files, for describing their "BIOS Setup" forms. At build time, the "VfrCompile" utility turns those files into IFR bytecode. The "VfrCompile" utility itself comprises three parts: - C++ code that is developed manually, - a VFR *lexer* whose C++ source code is generated from "VfrParser.dlg", with the "dlg" lexer generator from the PCCTS project, - a VFR *parser* whose C++ source code is generated from "VfrSyntax.g", with the "antlr" parser generator from the PCCTS project. The above approach is not exceptional at all (many projects use "flex" and "bison" for similar purposes). The difference is that the "dlg" and "antlr" lexer and parser generators come from the PCCTS project, and that edk2 *bundles* a modified instance of the PCCTS 1.33 source release <http://www.antlr2.org/pccts133.html>. Therefore, for building "VfrCompile", - we first build the "dlg" and "antlr" utilities from the PCCTS source code (which is written in C); - then we generate C++ source code for "VfrCompile", from "VfrParser.dlg" and "VfrSyntax.g", by running the just-built native "dlg" and "antlr" executables; - finally, "VfrCompile" is built from (partly generated) source, with g++. ) Now the question is, whether we should apply %{optflags} and %{__global_ldflags} to the "dlg" and "antlr" utilities themselves. Importantly, those lexer/parser generators are run only once -- they are never installed (or even used for the *firmware* builds); we don't include them in "edk2-tools". My patch set in comment 11 does apply %{optflags} and %{__global_ldflags} to the PCCTS source code too. And, in the past, the upstream edk2 project *has* modified the bundled PCCTS source code (including makefiles and C code), whenever it was *necessary* to clean up warnings emitted by the compilers of various Linux distributions. (I compiled a list at <http://mid.mail-archive.com/c1f5468f-8ac2-79c3-d3fe-fa35a98cdad9@redhat.com>.) That said, Liming prefers if we limit the patching of the bundled PCCTS source to the necessary minimum (including makefiles and C code). Therefore, my question is, if we consider it *necessary* to build the one-shot, never installed or shipped, "dlg" and "antlr" lexer/parser generators too, with the Fedora RPM flags. If we consider that necessary, Liming is OK with me pushing my patches from comment 11. If we think it's nice to have, but not really necessary, to build those one-shot utilities with our RPM flags, then Liming prefers we don't touch the bundled PCCTS source. Thoughts? Thanks.
(In reply to Laszlo Ersek from comment #13) > Therefore, my question is, if we consider it *necessary* to build the > one-shot, never installed or shipped, "dlg" and "antlr" lexer/parser > generators too, with the Fedora RPM flags. Florian, given that you reported the BZ, can you please comment? If I get no feedback on this soon, then I'll pick "least resistance" and rework my upstream patches so that "dlg" and "antlr" are not built with the flags. The upstream maintainer prefers that. Subsequently, further RPM flag-related BZs -- possibly reported against "dlg" and "antlr" build lines, taken from Koji logs -- will be closed as WONTFIX. The time to discuss and decide is now. Thanks.
(In reply to Laszlo Ersek from comment #13) > Now the question is, whether we should apply %{optflags} and > %{__global_ldflags} to the "dlg" and "antlr" utilities themselves. > Importantly, those lexer/parser generators are run only once -- they are > never installed (or even used for the *firmware* builds); we don't include > them in "edk2-tools". Has antlr a library part? > Therefore, my question is, if we consider it *necessary* to build the > one-shot, never installed or shipped, "dlg" and "antlr" lexer/parser > generators too, with the Fedora RPM flags. If it's just the lexer/parser generator, and not support libraries used at run time, it is not necessary to use the Fedora build flags for them.
(In reply to Florian Weimer from comment #15) > (In reply to Laszlo Ersek from comment #13) > > Now the question is, whether we should apply %{optflags} and > > %{__global_ldflags} to the "dlg" and "antlr" utilities themselves. > > Importantly, those lexer/parser generators are run only once -- they are > > never installed (or even used for the *firmware* builds); we don't include > > them in "edk2-tools". > > Has antlr a library part? No, it doesn't. (1) The VfrCompile utility (which we include in the edk2-tools package) only contains source code generated by antlr and dlg: # ldd /usr/bin/VfrCompile linux-vdso.so.1 (0x00007fff913e5000) libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007f530b273000) libm.so.6 => /lib64/libm.so.6 (0x00007f530aedf000) libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f530acc7000) libc.so.6 => /lib64/libc.so.6 (0x00007f530a908000) /lib64/ld-linux-x86-64.so.2 (0x00007f530b605000) (2) The antlr and dlg generators themselves are also not linked against any particular shared library: BaseTools/Source/C/VfrCompile/Pccts/antlr/antlr: linux-vdso.so.1 (0x00007fff6c5eb000) libc.so.6 => /lib64/libc.so.6 (0x00007f25c4331000) /lib64/ld-linux-x86-64.so.2 (0x00007f25c46f0000) BaseTools/Source/C/VfrCompile/Pccts/dlg/dlg: linux-vdso.so.1 (0x00007ffe4bb75000) libc.so.6 => /lib64/libc.so.6 (0x00007f7178e06000) /lib64/ld-linux-x86-64.so.2 (0x00007f71791c5000) (Not sure which one your question concerns.) > > Therefore, my question is, if we consider it *necessary* to build the > > one-shot, never installed or shipped, "dlg" and "antlr" lexer/parser > > generators too, with the Fedora RPM flags. > > If it's just the lexer/parser generator, and not support libraries used at > run time, it is not necessary to use the Fedora build flags for them. Ah, OK, I understand; you meant (1). So, no, antlr and dlg do not inject any particular shared library dependency to VfrCompile, and correspondingly, edk2-tools does not install or require any PCCTS-related .so. # rpm -q --requires edk2-tools /usr/bin/bash libc.so.6()(64bit) libc.so.6(GLIBC_2.14)(64bit) libc.so.6(GLIBC_2.2.5)(64bit) libc.so.6(GLIBC_2.3)(64bit) libc.so.6(GLIBC_2.7)(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libm.so.6()(64bit) libm.so.6(GLIBC_2.2.5)(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) libstdc++.so.6(GLIBCXX_3.4)(64bit) libstdc++.so.6(GLIBCXX_3.4.9)(64bit) libuuid.so.1()(64bit) rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(PayloadIsXz) <= 5.2-1 rtld(GNU_HASH) I'll go ahead and rework my series to leave the PCCTS build flags alone. Thank you!
This is not about shared libraries only. Statically linked objects count as well. BaseTools/Source/C/VfrCompile/Pccts/README says this: 6. You need not create a library. The makefile created by genmk assumes that the files are not part of a library. If you wish to create a library from elements of pccts/h: If the first letter of the filename is lowercase (uppercase) it is related to the code generated using the pccts C mode (C++ mode). Some of the .c and .cpp files in the h directory are not meant to be placed in a library and will not compile because they are meant to be #include in pccts generated files which are grammar specific. Those files are in the BaseTools/Source/C/VfrCompile/Pccts/h directory. If object files compiled from those files end up in installed executables, you should compile the objects with the proper Fedora build flags. /usr/bin/VfrCompile contains this string: ANTLRParser::ANTLRParser - Demand lookahead not supported in C++ mode That's from BaseTools/Source/C/VfrCompile/Pccts/h/AParser.cpp, so the files are indeed compiled and installed. If you use the wrong build flags for them, annocheck failures will be the result.
(all pathnames relative to BaseTools/Source/C/VfrCompile) Sure, the files of the kind that you mention are: - Pccts/h/ATokenBuffer.cpp - Pccts/h/DLexerBase.cpp - Pccts/h/AParser.cpp They are compiled to object files by "GNUmakefile". Conceptually, these files are no different from the antlr / dlg *output*, i.e. from the generated files; the only difference is that they are always "generated" the exact same way, by virtue of them being provided statically by the PCCTS distribution. And, importantly, my patch set covers them, even *without* the flags for the one-shot "antlr" and "dlg" utilities. In other words, the Pccts/h/*.cpp files go into VfrCompile (and not into "antlr" nor "dlg"), therefore Pccts/h/*.cpp will receive the Fedora RPM compile and link flags, regardless of what we do for "antlr" and "dlg". Thanks!
Posted the upstream series "[edk2] [PATCH v2 0/5] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller". https://lists.01.org/pipermail/edk2-devel/2018-August/028297.html http://mid.mail-archive.com/20180809132258.21874-1-lersek@redhat.com
(In reply to Laszlo Ersek from comment #19) > Posted the upstream series "[edk2] [PATCH v2 0/5] BaseTools/Source/C: take > EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller". > > https://lists.01.org/pipermail/edk2-devel/2018-August/028297.html > http://mid.mail-archive.com/20180809132258.21874-1-lersek@redhat.com Upstream commits: 1 67983484a443 BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too 2 03252ae287c4 BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS 3 b8a661702643 BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS 4 b0ca5dae78ff BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller 5 81502cee20ac BaseTools/Source/C: take EXTRA_LDFLAGS from the caller
The f28 branch in dist-git also misses the following upstream commit (independently of this RHBZ, but without it, we can't build edk2 for aarch64 at all): cbf00651eda6 ("BaseTools/tools_def: add "-fno-unwind-tables" to GCC_AARCH64_CC_FLAGS", 2018-05-23).
So this is the spec file patch on top of the f28 dist-git branch (currently at commit 158dacfc991c, "Fixes for AMD SEV on OVMF_CODE.fd; add Provides for bundled OpenSSL", 2018-07-23): [click "Unwrap Comments" near the top first] > commit dc8cb9975261381a403f25639ed640f15f063384 (HEAD -> extra_flags_rhbz1540244) > Author: Laszlo Ersek <lersek> > Date: Fri Aug 17 01:26:18 2018 +0200 > > use RPM build flags for BaseTools (RHBZ#1540244) > > Signed-off-by: Laszlo Ersek <lersek> > > diff --git a/edk2.spec b/edk2.spec > index 094bde94a363..9635043008f6 100644 > --- a/edk2.spec > +++ b/edk2.spec > @@ -35,7 +35,7 @@ > > Name: edk2 > Version: %{edk2_date}git%{edk2_githash} > -Release: 4%{dist} > +Release: 4.rhbz1540244.2%{dist} > Summary: EFI Development Kit II > > Group: Applications/Emulators > @@ -78,6 +78,12 @@ Patch0055: 0055-OvmfPkg-QemuFlashFvbServicesRuntimeDxe-mark-Flash-me.patch > Patch0056: 0056-OvmfPkg-QemuFlashFvbServicesRuntimeDxe-Do-not-expose.patch > Patch0057: 0057-OvmfPkg-QemuFlashFvbServicesRuntimeDxe-Restore-C-bit.patch > Patch0058: 0058-MdeModulePkg-Variable-Check-EFI_MEMORY_RUNTIME-attri.patch > +Patch0059: 0059-BaseTools-tools_def-add-fno-unwind-tables-to-GCC_AARCH64_CC_FLAGS.patch > +Patch0060: 0060-BaseTools-footer.makefile-expand-BUILD_CFLAGS-last-for-C-files-too.patch > +Patch0061: 0061-BaseTools-header.makefile-remove-c-from-BUILD_CFLAGS.patch > +Patch0062: 0062-BaseTools-Source-C-split-O2-to-BUILD_OPTFLAGS.patch > +Patch0063: 0063-BaseTools-Source-C-take-EXTRA_OPTFLAGS-from-the-caller.patch > +Patch0064: 0064-BaseTools-Source-C-take-EXTRA_LDFLAGS-from-the-caller.patch > > %if 0%{?cross:1} > # Tweak the tools_def to support cross-compiling. > @@ -279,7 +285,9 @@ ARM_FLAGS="${CC_FLAGS}" > ARM_FLAGS="${ARM_FLAGS} -D DEBUG_PRINT_ERROR_LEVEL=0x8040004F" > > unset MAKEFLAGS > -make -C BaseTools #%{?_smp_mflags} > +make -C BaseTools %{?_smp_mflags} \ > + EXTRA_OPTFLAGS="%{optflags}" \ > + EXTRA_LDFLAGS="%{__global_ldflags}" > sed -i -e 's/-Werror//' Conf/tools_def.txt > > Patch 59 is a verbatim pick of upstream cbf00651eda6 (comment 21); the rest is listed in comment 20 (also verbatim picks). Patch 59 enables the fw to build for aarch64 at all. The rest is put to use in the last hunk of the spec file patch. Note that I also un-commented %{?_smp_mflags} -- BaseTools now builds with parallel make.
Scratch build with the changes described in comment 23: https://koji.fedoraproject.org/koji/taskinfo?taskID=29125708 ... I was about to ask Florian to check this, but then I noticed that I had missed the linker flags for VfrCompile, in the upstream series. :/
Posted [PATCH] BaseTools/VfrCompile: honor EXTRA_LDFLAGS https://lists.01.org/pipermail/edk2-devel/2018-August/028714.html http://mid.mail-archive.com/20180817023802.7474-1-lersek@redhat.com
(In reply to Laszlo Ersek from comment #25) > Posted > [PATCH] BaseTools/VfrCompile: honor EXTRA_LDFLAGS > https://lists.01.org/pipermail/edk2-devel/2018-August/028714.html > http://mid.mail-archive.com/20180817023802.7474-1-lersek@redhat.com This is now upstream commit aa4e0df1f0c7 ("BaseTools/VfrCompile: honor EXTRA_LDFLAGS", 2018-08-22).
https://koji.fedoraproject.org/koji/taskinfo?taskID=29230753 Upstream edk2 patches picked into this build, on top of dist-git 158dacfc991c: 1 cbf00651eda6 BaseTools/tools_def: add "-fno-unwind-tables" to GCC_AARCH64_CC_FLAGS 2 67983484a443 BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too 3 03252ae287c4 BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS 4 b8a661702643 BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS 5 b0ca5dae78ff BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller 6 81502cee20ac BaseTools/Source/C: take EXTRA_LDFLAGS from the caller 7 aa4e0df1f0c7 BaseTools/VfrCompile: honor EXTRA_LDFLAGS Relevant hunks of the spec file diff: > @@ -78,6 +78,13 @@ Patch0055: 0055-OvmfPkg-QemuFlashFvbServicesRuntimeDxe-mark-Flash-me.patch > Patch0056: 0056-OvmfPkg-QemuFlashFvbServicesRuntimeDxe-Do-not-expose.patch > Patch0057: 0057-OvmfPkg-QemuFlashFvbServicesRuntimeDxe-Restore-C-bit.patch > Patch0058: 0058-MdeModulePkg-Variable-Check-EFI_MEMORY_RUNTIME-attri.patch > +Patch0059: 0059-BaseTools-tools_def-add-fno-unwind-tables-to-GCC_AARCH64_CC_FLAGS.patch > +Patch0060: 0060-BaseTools-footer.makefile-expand-BUILD_CFLAGS-last-for-C-files-too.patch > +Patch0061: 0061-BaseTools-header.makefile-remove-c-from-BUILD_CFLAGS.patch > +Patch0062: 0062-BaseTools-Source-C-split-O2-to-BUILD_OPTFLAGS.patch > +Patch0063: 0063-BaseTools-Source-C-take-EXTRA_OPTFLAGS-from-the-caller.patch > +Patch0064: 0064-BaseTools-Source-C-take-EXTRA_LDFLAGS-from-the-caller.patch > +Patch0065: 0065-BaseTools-VfrCompile-honor-EXTRA_LDFLAGS.patch > > %if 0%{?cross:1} > # Tweak the tools_def to support cross-compiling. > @@ -279,7 +286,9 @@ ARM_FLAGS="${CC_FLAGS}" > ARM_FLAGS="${ARM_FLAGS} -D DEBUG_PRINT_ERROR_LEVEL=0x8040004F" > > unset MAKEFLAGS > -make -C BaseTools #%{?_smp_mflags} > +make -C BaseTools %{?_smp_mflags} \ > + EXTRA_OPTFLAGS="%{optflags}" \ > + EXTRA_LDFLAGS="%{__global_ldflags}" > sed -i -e 's/-Werror//' Conf/tools_def.txt > > Florian, can you please check if this build is OK? Thanks.
Created attachment 1477959 [details] edk2-tools-20180529gitee3198e672e2-4.rhbz1540244.3.fc28.x86_64.log I ran “annocheck -v” on edk2-tools-20180529gitee3198e672e2-4.rhbz1540244.3.fc28.x86_64, after installing the debuginfo package. Hardening gaps are still being reported. This looks a tooling issue (that is, false positives). I will have another look tomorrow.
Okay, I had missed that these RPMs were built on Fedora 28. I took the source RPM and built it on rawhide: $ annocheck --debug-rpm=edk2-tools-debuginfo-20180529gitee3198e672e2-4.rhbz1540244.3.fc30.x86_64.rpm edk2-tools-20180529gitee3198e672e2-4.rhbz1540244.3.fc30.x86_64.rpm Hardened: BootSectImage: PASS. Hardened: Brotli: PASS. Hardened: DevicePath: PASS. Hardened: EfiLdrImage: PASS. Hardened: EfiRom: PASS. Hardened: GenCrc32: PASS. Hardened: GenFfs: PASS. Hardened: GenFv: PASS. Hardened: GenFw: PASS. Hardened: GenPage: PASS. Hardened: GenSec: PASS. Hardened: GenVtf: PASS. Hardened: GnuGenBootSector: PASS. Hardened: LzmaCompress: PASS. Hardened: Split: PASS. Hardened: TianoCompress: PASS. Hardened: VfrCompile: PASS. Hardened: VolInfo: PASS. So it should be fixed. Thanks for working on this!
Thank you Florian for the verification! Cole, Gerd, Paolo: can one of you guys please submit official Fedora builds for Fedora 28+? The BZ was filed for Fedora 28, and the necessary changes for Fedora 28 are described in comment 27. Regarding Fedora 29 and 30 (rawhide), Cole rebased edk2 in those releases to upstream commit cb5f4f45ce1f (the "edk2-stable201808" tag) yesterday, for bug 1609827. That commit contains commit #1 from comment 27, so for Fedora 29 and 30, commits #2 through #7 (plus the spec file changes) from comment 27 are needed. Thanks!
edk2-20180815gitcb5f4f45ce-1.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2018-9403743de2
edk2-20180815gitcb5f4f45ce-1.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-9403743de2
edk2-20180815gitcb5f4f45ce-1.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.