Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64

["Liming Gao" <liming.gao@intel.com>]

Laszlo:
  I will verify your option (a) and (b). The problem is described in https://bugzilla.tianocore.org/show_bug.cgi?id=2024. 
  I know there are some discussion in Jordan patch for PeiCore change. Before the conclusion for Jordan change is made, 
  I expect to find the alternate option to resolve CLANG9 tool chain issue first. 

Thanks
Liming
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, October 1, 2019 5:10 AM
> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: Re: [edk2-devel] [Patch 12/12] OvmfPkg SecMain: Add build option "-fno-omit-frame-pointer" for CLANG9 X64
> 
> + Jordan
> 
> On 09/27/19 09:46, Liming Gao wrote:
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2024
> >
> > Signed-off-by: Liming Gao <liming.gao@intel.com>
> > ---
> >  OvmfPkg/Sec/SecMain.inf | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
> > index 63ba4cb555..cd765cac25 100644
> > --- a/OvmfPkg/Sec/SecMain.inf
> > +++ b/OvmfPkg/Sec/SecMain.inf
> > @@ -69,3 +69,7 @@
> >
> >  [FeaturePcd]
> >    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> > +
> > +[BuildOptions]
> > +  # CLANG9 X64 requires it.
> > +  GCC:*_CLANG9_X64_CC_FLAGS = -fno-omit-frame-pointer
> >
> 
> I disagree with this patch for two reasons.
> 
> First, the comment "CLANG9 X64 requires it" is quite lacking. It does
> nothing to explain the issue; it doesn't even provide a pointer in the
> code comment (only in the code message).
> 
> Second, Jordan suggested an alternative approach at the end of
> <https://bugzilla.tianocore.org/show_bug.cgi?id=2024#c3>, which I prefer
> to the one seen above. Can we please try that with CLANG9 too?
> 
> 
> ... Oh wait I see there are new comments in BZ#2024.
> 
> Apparently,
> 
>   #pragma GCC optimize ("no-omit-frame-pointer")
> 
> does not work with CLANG9.
> 
> That's not a problem: we still have two options that are superior to the
> present patch, and should be tested.
> 
> (a) Does Jordan's series linked below fix the problem with CLANG9?
> 
>   [edk2-devel] [PATCH v2 0/6] Fix PEI Core issue during TemporaryRamMigration
>   http://mid.mail-archive.com/20190410084000.19660-1-jordan.l.justen@intel.com
>   https://edk2.groups.io/g/devel/message/38785
> 
> (b) Jordan's full #pragma suggestion was:
> 
> #ifdef __GNUC__
> #pragma GCC push_options
> #pragma GCC optimize ("no-omit-frame-pointer")
> #else
> #pragma optimize ("y", off)
> #endif
> 
> If the '#pragme GCC' branch doesn't help, can we customize the *other*
> branch for CLANG9?
> 
> For example, in patch#4, we rely on defined(__clang__). Therefore, can
> we try the following:
> 
> #ifdef __GNUC__
> #pragma GCC push_options
> #pragma GCC optimize ("no-omit-frame-pointer")
> #elif defined(__clang__)
> #pragma clang optimize off  <----- NOTE THIS <http://clang.llvm.org/docs/LanguageExtensions.html>
> #else
> #pragma optimize ("y", off)
> #endif
> 
> In summary, the fact that CLANG9 breaks is just a symptom; it shows that
> the PEI Core issue fixed by Jordan is real. We should go for the real
> fix (a).
> 
> Alternatively, use a clang-specific optimization override, as tightly as
> possible; (b) might work.
> 
> -*-
> 
> In fact I think this is the perfect time to fix the PEI Core issue: we
> now have a feature request that depends on fixing the bug in the PEI
> Core. We should fix technical debt in edk2 at least *sometimes*. If we
> are not willing to fix technical debt in edk2 for the sake of new
> features, we will *never* fix technical debt.
> 
> (Well, to be technically correct, we also fix technical debt when it
> turns into security issues. Yay!)
> 
> Please test this series with the present patch removed, and Jordan's v2
> series (linked above) applied.
> 
> Thanks
> Laszlo