Re: [PATCH] BaseTools/tools_def: add "-fno-unwind-tables" to GCC_AARCH64_CC_FLAGS

[Laszlo Ersek <lersek@redhat.com>]

On 05/21/18 12:30, Leif Lindholm wrote:
> On Fri, May 18, 2018 at 08:23:01PM +0200, Laszlo Ersek wrote:
>> The ElfConvert routines in GenFw don't handle the ".eh_frame" ELF section
>> emitted by gcc. For this reason, Leif disabled the generation of that
>> section for AARCH64 with "-fno-asynchronous-unwind-tables" in commit
>> 28e80befa4fe [1], and Ard did the same for IA32 and X64 in commit
>> 26ecc55c027d [2]. (The CLANG38 toolchain received the same flag at its
>> inception, in commit 6f756db5ea05 [3].)
>>
>> However, ".eh_frame" is back now; in upstream gcc commit 9cbee213b579 [4]
>> (part of tag "gcc-8_1_0-release"), both "-fasynchronous-unwind-tables" and
>> "-funwind-tables" were made the default for AARCH64. (The patch author
>> described the effects on the gcc mailing list [5].) We have to counter the
>> latter flag with "-fno-unwind-tables", otherwise GenFw chokes on
>> ".eh_frame" again (triggered for example on Fedora 28).
>>
>> "-f[no-]unwind-tables" goes back to at least gcc-4.4 [6], so it's safe to
>> add to GCC_AARCH64_CC_FLAGS.
> 
> I'm not against this patch (which fixes a real problem), but would
> it be possible to take the opposite approach, as Peter Jones did for
> GRUB when the same issue hit there?:
> 
> http://lists.gnu.org/archive/html/grub-devel/2018-02/msg00095.html
> 
> I.e., rather than keep adding flags when new "special" sections are
> added by toolchains, explicitly filter those out as part of the link
> step.

I guess this could be implemented in two places in BaseTools:

(1) In GenFw / ElfConvert. I'm afraid that's beyond my binutils-fu at
the moment.

(2) In "BaseTools/Scripts/GccBase.lds". This is similarly beyond my
binutils-fu, but at least I can make more comments on it:

We already discard a good number of sections, but we keep (and realign)
"eh_frame". From Ard's commit message 26ecc55c027d, I believe the idea
here was to "retain the .eh_frame data for external debugging", and then
strip it with "objcopy -R .eh_frame" finally (see
[Dynamic-Library-File], <Command.GCC, Command.GCCLD> in
"BaseTools/Conf/build_rule.template").

However, in practice we stub out objcopy (see under "GCC Common" in
"BaseTools/Conf/tools_def.template"), and then we must prevent the
generation of .eh_frame in the first place.

Rather than adding "-fno-unwind-tables", I considered moving .eh_frame
to /DISCARD/, or else turning $(OBJCOPY) into the real thing. However,
that would affect more targets than just AARCH64/GCC, and I really
didn't (don't) remember why we have this redundancy with "objcopy -R
.eh_frame" vs. "-no-fasynchronous-unwind-tables". The latest practice
seemed to be "add more -fno-..." (see the commits I referenced above),
so I went with that.


I suggest we push this patch now, plus I could:

- submit a followup for IA32/X64, if that's deemed best,
- file a BZ for GenFw / ElfConvert about the programmatic relocation
  filtering,
- file a BZ for GccBase.lds / tools_def.template, to discard or strip
  .eh_frame under GCC.

Thanks
Laszlo