Andrew: Thanks for your explanation. So, they are the different topics. I suggest to separate this patch. 1. NORETURN attribute and UNREACHABLE() macro will be used for the really NORETURN case, like phase change. 2. ANALYZER_UNREACHABLE and ANALYZER_NORETURN are for static code analyzer, like clang compiler analyzer.
Thanks Liming > -----Original Message----- > From: [email protected] [mailto:[email protected]] > Sent: Sunday, June 12, 2016 11:11 AM > To: Gao, Liming <[email protected]> > Cc: Marvin Häuser <[email protected]>; edk2- > [email protected]; Kinney, Michael D <[email protected]> > Subject: Re: [edk2] [PATCH v1 1/2] MdePkg: Add NORETURN attribute and > UNREACHABLE() macro. > > > > On Jun 11, 2016, at 7:58 PM, Gao, Liming <[email protected]> wrote: > > > > Hi, > > I don't see the usage on NORETURN attribute and UNREACHABLE() macro. > Why we need to add them also? > > > > When you factor in debugging I think the only thing we have that are really > NORETURN would be phase changes. SEC calling PEI Core. DXE IPL PEIM > loading DXE Core, DXE Core entry point. I debug across and ASSERT(), > CpuBreakpoint(), or CpuDeadloop() all the time so turn that compiler feature > on would break stuff. For the mode transitions we could save a few > unreachable stack cleanup instructions? Thus I guess things like the PEI Core > entry point function are kind of NORETURN, and I guess the UNREACHABLE > would be like __builtin_unreachable() and end up after the call to the PEI > Core entry point. Given things like the PEI Core entry point are defined in > the > PI spec, it would probably be better to define UNREACHABLE(). > > But I'm still not sure what problem we are trying to solve. > > Thanks, > > Andrew Fish > > > Thanks > > Liming > >> -----Original Message----- > >> From: Marvin Häuser [mailto:[email protected]] > >> Sent: Saturday, June 11, 2016 5:02 AM > >> To: [email protected] > >> Cc: Gao, Liming <[email protected]>; Kinney, Michael D > >> <[email protected]> > >> Subject: [PATCH v1 1/2] MdePkg: Add NORETURN attribute and > >> UNREACHABLE() macro. > >> > >> The NORETURN attribute informs compilers and analyzers that the flagged > >> function cannot return. This may improve the quality of the optimizations. > >> > >> The UNREACHABLE() macro informs compilers and analyzers that its > position > >> cannot be reached, for example eliminating implicit returns. > >> It is recommended to be used together with the NORETURN attribute to > >> prevent > >> warnings regarding the function flagged as 'noreturn' returning. > >> > >> The ANALYZER-prefixed versions have the same effects, but exclude > >> compilers. > >> They may be used to surpress warnings of static analyzers, such as > possible > >> dereferencing of a NULL pointer when dereferencing it after having > checked > >> it > >> via ASSERT(). > >> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Marvin Haeuser <[email protected]> > >> --- > >> MdePkg/Include/Base.h | 103 ++++++++++++++++++++ > >> 1 file changed, 103 insertions(+) > >> > >> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h > >> index 89b2aed07229..9dfafb282177 100644 > >> --- a/MdePkg/Include/Base.h > >> +++ b/MdePkg/Include/Base.h > >> @@ -85,6 +85,109 @@ VERIFY_SIZE_OF (CHAR16, 2); > >> #endif > >> > >> // > >> +// Should be used in combination with NORETURN to avoid 'noreturn' > >> returns > >> +// warnings. > >> +// > >> +#ifndef UNREACHABLE > >> + #ifdef __GNUC__ > >> + /// > >> + /// Signal compilers and analyzers that this call is not reachable. > >> It is > >> + /// up to the compiler to remove any code past that point. > >> + /// > >> + #define UNREACHABLE() __builtin_unreachable() > >> + #elif defined (__has_feature) > >> + #if __has_builtin (__builtin_unreachable) > >> + /// > >> + /// Signal compilers and analyzers that this call is not reachable. > >> It is > >> + /// up to the compiler to remove any code past that point. > >> + /// > >> + #define UNREACHABLE() __builtin_unreachable() > >> + #endif > >> + #endif > >> + > >> + #ifndef UNREACHABLE > >> + /// > >> + /// Signal compilers and analyzers that this call is not reachable. > >> It is > >> + /// up to the compiler to remove any code past that point. > >> + /// > >> + #define UNREACHABLE() > >> + #endif > >> +#endif > >> + > >> +// > >> +// Signaling compilers and analyzers that a certain function cannot return > >> may > >> +// remove all following code and thus lead to better optimization and > less > >> +// false positives. > >> +// > >> +#ifndef NORETURN > >> + #ifdef __GNUC__ > >> + /// > >> + /// Signal compilers and analyzers that the function cannot return. > >> + /// It is up to the compiler to remove any code past a call to > >> functions > >> + /// flagged with this attribute. > >> + /// > >> + #define NORETURN __attribute__ ((noreturn)) > >> + #else > >> + /// > >> + /// Signal compilers and analyzers that the function cannot return. > >> + /// It is up to the compiler to remove any code past a call to > >> functions > >> + /// flagged with this attribute. > >> + /// > >> + #define NORETURN > >> + #endif > >> +#endif > >> + > >> +// > >> +// Should be used in combination with ANALYZER_NORETURN to avoid > >> 'noreturn' > >> +// returns warnings. > >> +// > >> +#ifndef ANALYZER_UNREACHABLE > >> + #ifdef __clang_analyzer__ > >> + #if __has_builtin (__builtin_unreachable) > >> + /// > >> + /// Signal the analyzer that this call is not reachable. > >> + /// This excludes compilers. > >> + /// > >> + #define ANALYZER_UNREACHABLE() __builtin_unreachable() > >> + #endif > >> + #endif > >> + > >> + #ifndef ANALYZER_UNREACHABLE > >> + /// > >> + /// Signal the analyzer that this call is not reachable. > >> + /// This excludes compilers. > >> + /// > >> + #define ANALYZER_UNREACHABLE() > >> + #endif > >> +#endif > >> + > >> +// > >> +// Static Analyzers may issue errors about potential NULL-dereferences > >> when > >> +// dereferencing a pointer, that has been checked before, outside of a > >> +// NULL-check. This may lead to false positives, such as when using > ASSERT() > >> +// for verification. > >> +// > >> +#ifndef ANALYZER_NORETURN > >> + #ifdef __has_feature > >> + #if __has_feature (attribute_analyzer_noreturn) > >> + /// > >> + /// Signal analyzers that the function cannot return. > >> + /// This excludes compilers. > >> + /// > >> + #define ANALYZER_NORETURN __attribute__ ((analyzer_noreturn)) > >> + #endif > >> + #endif > >> + > >> + #ifndef ANALYZER_NORETURN > >> + /// > >> + /// Signal the analyzer that the function cannot return. > >> + /// This excludes compilers. > >> + /// > >> + #define ANALYZER_NORETURN > >> + #endif > >> +#endif > >> + > >> +// > >> // For symbol name in assembly code, an extra "_" is sometimes > necessary > >> // > >> > >> -- > >> 2.7.4.windows.1 > > > > _______________________________________________ > > edk2-devel mailing list > > [email protected] > > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
