Gerd, Ard, Marvin: I think we all agree that this patch set is valuable to reduce the duplication header files.
The current concern is that BASETOOLS macro name. This name matches Edk2 BaseTools directory. If no better name, I would suggest to keep it. And, the second request is to remove #pragma GCC visibility push (hidden) in MdePkg\Include\X64\ProcessorBind.h. It may be not resolved in short time. I don't want this request blocks this patch set. I suggest to separate them, then we can first merge this patch set if no other functional issue. Thanks Liming > -----邮件原件----- > 发件人: Ard Biesheuvel <[email protected]> > 发送时间: 2023年4月18日 23:50 > 收件人: Gerd Hoffmann <[email protected]> > 抄送: [email protected]; Marvin Häuser <[email protected]>; Pawel > Polawski <[email protected]>; Dongyan Qian > <[email protected]>; Sunil V L <[email protected]>; Baoqi > Zhang <[email protected]>; Chao Li <[email protected]>; Rebecca > Cran <[email protected]>; Ard Biesheuvel <[email protected]>; > Zhiguang Liu <[email protected]>; Liming Gao > <[email protected]>; Yuwei Chen <[email protected]>; Leif > Lindholm <[email protected]>; Michael D Kinney > <[email protected]>; Daniel Schaefer <[email protected]>; > Bob Feng <[email protected]>; Oliver Steffen <[email protected]> > 主题: Re: [edk2-devel] [PATCH v4 01/10] BaseTools: add BASETOOLS define > > On Tue, 18 Apr 2023 at 15:20, Gerd Hoffmann <[email protected]> wrote: > > > > On Tue, Apr 18, 2023 at 01:59:43PM +0200, Ard Biesheuvel wrote: > > > On Tue, 18 Apr 2023 at 13:52, Gerd Hoffmann <[email protected]> > wrote: > > > > > > > > Seems to work fine on fedora 37, even without adding --relax, maybe > this > > > > is enabled by default (there is a --no-relax switch after all). I'll go > > > > try older distros / compilers / binutils too. > > > > > > > > What would be the failure mode? Errors on ELF -> PE conversion > because > > > > a GOT is present? Or will things break at runtime? > > > > > > > > > > The problem here is that we rely on --emit-relocs to get at the > > > relocations in the binary, in order to convert the absolute ones into > > > PE/COFF relocations. > > > > > > However, --emit-relocs did not use to cover the GOT, as those are > > > added at the end by the linker and not by the compiler. So if the GOT > > > is non-empty, the resulting PE executable will be corrupt. > > > > So no build error. And at runtime probably random effects, depending on > > whenever the execution path happens to hit a bad relocation or not. So > > compile + boot testing doesn't cut it. Lovely. > > > > So, what can I do instead? Check **/DEBUG/*.debug and see whenever a > > .got section is present? > > > > We should probably add an ASSERT() to the linker script, just like I > did for Linux: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id > =e544ea57ac0734bc > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id > =262b5cae67a67240 > > > > Of course, the answer here is to dump GenFw altogether for ELF to PE > > > conversion, and implement something that consumes the dynamic > > > relocations generated when linking in PIE mode. > > > > Marvin's ImageTool is exactly that I think ... > > > > take care, > > Gerd > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#103338): https://edk2.groups.io/g/devel/message/103338 Mute This Topic: https://groups.io/mt/98403692/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
