+ 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....@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48309): https://edk2.groups.io/g/devel/message/48309 Mute This Topic: https://groups.io/mt/34309065/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
