Hey Andrew, Thanks for your reply!
In case you haven't read Laszlo's reply, he pointed out the same issue and proposed that both CpuDeadLoop() and Breakpoint() should be flagged as ' analyzer_noreturn', which is ignored by the compiler and thus should not break anyone's code, but only suppress the analyzer warnings. :) Laszlo, If I understood it correctly, the Clang Static Analyzer is separate from main Clang (compiler) and, either way, can be used standalone. For that reason, tools_def should probably be avoided as this requires BaseTools invoking the analyzer. Instead, one could check for the analyzer define (Clang Static Analyzer can be detected via ifdef) and define ANALYZER_NORETURN either as empty, when there is no analyzer detected, or as the analyzer-noreturn attribute, when a supported analyzer is found. I can get a patch ready by tomorrow, I think. Regards, Marvin. > -----Original Message----- > From: [email protected] [mailto:[email protected]] > Sent: Wednesday, June 1, 2016 10:56 PM > To: Marvin H?user <[email protected]> > Cc: [email protected] > Subject: Re: [edk2] ASSERT and static code analysis > > > > On Jun 1, 2016, at 1:12 PM, Marvin H?user > <[email protected]> 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 > > > > 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). If I didn't > understand the documentation incorrectly, this should fix the issue > described in the first paragraph. > > > > Marvin, > > Sometimes people use CpuDeadLoop() to debug with a JTAG debugger so > they will step over the code. So you can't use noreturn as that tells the > optimizer it can remove the code following the no return function. So for > example your entire program could get optimized away if you place a > CpuBreakpoint() at the start of your function. > Simple clang example: > ~/work/Compiler>cat noreturn.c > > int NoReturn(void) __attribute__ ((noreturn)); > > int > main() > { > NoReturn(); > return 0; > } > ~/work/Compiler>clang -Os -S noreturn.c ~/work/Compiler>cat noreturn.S > .section __TEXT,__text,regular,pure_instructions > .macosx_version_min 10, 11 > .globl _main > _main: ## @main > pushq %rbp > movq %rsp, %rbp > callq _NoReturn > > > .subsections_via_symbols > > > Depending on how much Heisenberg uncertainty you can stand.... > > You can -D MDEPKG_NDEBUG for your analyzer run. > https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/D > ebugLib.h#L288 > > In the DSC map the DebugLib library class to > https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseDeb > ugLibNull/BaseDebugLibNull.inf all the functions are empty and you would > avoid your issues. > > PcdDebugPropertyMask, set in DSC, can control what happens after an > ASSERT(), but I'm guessing that is to far into the optimizer to be useful for > you? If you compiled with clang LTO the CpuBreakpoint() and CpuDeadLoop() > would get dead stripped. > https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseDeb > ugLibSerialPort/DebugLib.c#L146 > > Thanks, > > Andrew Fish > > > 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. > > > > Regards, > > Marvin. > > _______________________________________________ > > 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
