Laszlo: I think this case is same to EFIAPI. For GCC tool chain, "-DEFIAPI=__attribute__((ms_abi))" is defined in CC_FLAGS in tools_def.txt. For this case, "-DCLANG_ANALYZER_NORETURN=__attribute__((analyzer_noreturn))" can only be added into CC_FLAGS for Clang Scan analysis tool chain, like CLANGSCAN38 in https://github.com/shijunjing/edk2/tree/llvm.
Thanks Liming > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of > Laszlo Ersek > Sent: Thursday, June 02, 2016 4:54 AM > To: Marvin H?user <[email protected]>; edk2- > [email protected] <[email protected]> > Subject: Re: [edk2] ASSERT and static code analysis > > On 06/01/16 22:41, Marvin H?user wrote: > > Hey Laszlo, > > > > Thank you very much for your comment! > > > > About the comment in CpuDeadLoop(), once more, I wasn't aware, thanks > for the hint! > > Do you think a define should be introduced > ('NORETURN'/'ANALYZE_NORETURN') to keep the code style consistent and > also allow support for more static analyzers in the future? > > > > If it indeed makes sense, I can get a patch ready, maybe today or tomorrow. > > Honestly, I got no clue how to make this work (in some MdePkg header > files I guess) across *all* toolchains that edk2 supports. I guess by > default the macros should expand to an empty replacement text, and the > "exceptions" should come from toolchain-specific settings in tools_def.txt? > > I'm sure Andrew and the BaseTools maintainers can help. > > Thanks > Laszlo > > >> -----Original Message----- > >> From: Laszlo Ersek [mailto:[email protected]] > >> Sent: Wednesday, June 1, 2016 10:38 PM > >> To: Marvin H?user <[email protected]> > >> Cc: [email protected] <[email protected]> > >> Subject: Re: [edk2] ASSERT and static code analysis > >> > >> On 06/01/16 22:12, Marvin H?user wrote: > >>> Recently I was told that ASSERT() calls to check whether a variable is > >>> NULL breaks the Clang Static Analyzer in terms of generating wrong > >>> warnings. The reason is that, when a variable/parameter is checked for > >>> NULL, this analyzer assumes that it can be. As it doesn't support > >>> EDK2 ASSERTs, but only compiler-provided asserts, to it, the ASSERT() > >>> call is a simple if-check (-> triggers NULL warnings) which does > >>> return to normal code flow (-> any further usages may be dereferencing > >>> NULL). This behavior is documented here: > >>> http://clang-analyzer.llvm.org/faq.html#null_pointer > >> > >> Makes sense to me. > >> > >>> To make clear that EDK2 ASSERT() calls are indeed asserts, in my > >>> opinion, CpuDeadLoop() should be flagged as 'noreturn' (it indeed > >>> should never return) and Breakpoint() as 'analyzer_noreturn' (it may > >>> return, but the analyzer doesn't have to care as the debugger is > >>> invoked). > >> > >> Side point: CpuDeadLoop() too should be flagged as "analyzer_noreturn" > >> then; please see the comments in > >> "MdePkg/Library/BaseLib/CpuDeadLoop.c". > >> > >> Thanks > >> Laszlo > >> > >>> If I didn't understand the documentation incorrectly, this should fix > >>> the issue described in the first paragraph. > >>> > >>> If you have experience with the Clang Static Analyzer or even this > >>> specific issue, I would be happy if you would share your opinion of > >>> the concern. > > > > _______________________________________________ > 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
