[cfe-dev] Static Analyzer: LiveVariables and SymbolReaper

Anna Zaks via cfe-dev cfe-dev at lists.llvm.org
Sun Oct 29 21:56:23 PDT 2017

Sent from my iPhone

> On Oct 29, 2017, at 5:19 PM, Alexander Shaposhnikov <alexander.v.shaposhnikov at gmail.com> wrote:
> ohh, typo above: i meant 
>      bool isLive(const Stmt *S, const VarDecl *D)
> (instead of LiveVariables::isLive(const Stmt *Loc, const Stmt *S))
>> On Sun, Oct 29, 2017 at 5:08 PM, Alexander Shaposhnikov <alexander.v.shaposhnikov at gmail.com> wrote:
>> Hi, 
>> (long read, sorry in advance)
>> i'm looking into the internals of SymbolReaper and LiveVariables and have 
>> several questions on my mind. Since I have not built a consistent understanding of what's going on there yet + assume I might be missing smth, I'd like to start with some very basic example to show how the (potential) issue manifests itself there. Any explanations / suggestions / comments would be greatly appreciated.
>> Example s.c:
>> typedef struct _IO_FILE FILE;
>> extern FILE *fopen(const char *path, const char *mode);
>> void g() {};
>> void h() {};
>> void f(int c) {
>>   FILE *p = fopen("foo.c", "r");
>>   g();
>>   h();
>>   return;
>> }
>> clang -cc1 -analyze -analyzer-checker=debug.ViewCFG s.c
>> says that there 3 blocks: B2(entry) -> B1 -> B0(exit), B2 and B0 are essentially empty and all the interesting things are inside B1. 
>> Now let's try to run clang -c --analyze -Xanalyzer -analyzer-checker=alpha.unix.Stream s.c
>> s.c:8:9: warning: Value stored to 'p' during its initialization is never read
>>   FILE *p = fopen("foo.c", "r");
>>         ^   ~~~~~~~~~~~~~~~~~~~
>> s.c:9:3: warning: Opened File never closed. Potential Resource leakv
>>   g();
>>   ^~~
>> 2 warnings generated.
>> What looks suspicious here - the location of the warning
>> "s.c:9:3: warning: Opened File never closed. Potential Resource leak" 
>> - it points to g() instead of the end of function or return statement.

The analyzer is trying to report the leak at the earliest point. It uses live variable analysis for that:

>> Now let's run the analyzer under lldb and set the breakpoint at 
>> LiveVariables::isLive(const Stmt *Loc, const Stmt *S)
>>    178 	bool LiveVariables::isLive(const Stmt *Loc, const Stmt *S) {
>> (lldb) p D->dump()
>> VarDecl 0x11286c178 <s.c:8:3, col:31> col:9 p 'FILE *' cinit
>> `-CallExpr 0x11286c350 <col:13, col:31> 'FILE *'
>>   |-ImplicitCastExpr 0x11286c338 <col:13> 'FILE *(*)(const char *, const char *)' <FunctionToPointerDecay>
>>   | `-DeclRefExpr 0x11286c1d8 <col:13> 'FILE *(const char *, const char *)' Function 0x11286bce0 'fopen' 'FILE *(const char *, const char *)'
>>   |-ImplicitCastExpr 0x11286c3a0 <col:19> 'const char *' <BitCast>
>>   | `-ImplicitCastExpr 0x11286c388 <col:19> 'char *' <ArrayToPointerDecay>
>>   |   `-StringLiteral 0x11286c238 <col:19> 'char [6]' lvalue "foo.c"
>>   `-ImplicitCastExpr 0x11286c3d0 <col:28> 'const char *' <BitCast>
>>     `-ImplicitCastExpr 0x11286c3b8 <col:28> 'char *' <ArrayToPointerDecay>
>>       `-StringLiteral 0x11286c2a8 <col:28> 'char [2]' lvalue "r"
>> (lldb) p getImpl(impl).stmtsToLiveness[S].isLive(D)
>> (bool) $0 = false
>> (lldb) p S->dump()
>> CallExpr 0x11286c470 'void'
>> `-ImplicitCastExpr 0x11286c458 'void (*)()' <FunctionToPointerDecay>
>>   `-DeclRefExpr 0x11286c400 'void ()' Function 0x11286be18 'g' 'void ()'
>> (lldb) fr sel 7
>> frame #7: 0x0000000106ed4397 clang-6.0`clang::ento::ExprEngine::ProcessStmt(this=0x00007fff5fbf97c0, S=const clang::CFGStmt @ 0x00007fff5fbf8f50, Pred=0x0000000114004a30) at ExprEngine.cpp:497
>> Here getImpl(impl).stmtsToLiveness[S].isLive(D) returns false and
>> Static Analyzer thinks that the variable "p" is dead and will remove the binding (from the store). Since this variable is marked as dead, 
>> the corresponding checkers are called (in ExprEngine::removeDead getCheckerManager().runCheckersForDeadSymbols ...) and the stream checker reports a leak. 
>> In fact, if i'm not mistaken, at this point the variable "p" is not dead yet and the leak should be reported much later at the end of the function.
>> The problem does not appear to be specific to StreamChecker, after playing with several other simple examples, it looks like  LiveVariables::isLive returns false in most cases
>> (and so does SymbolReaper::isLive(const VarRegion *VR, bool includeStoreBindings) ).
>> This results in loosing information about variables or emitting diagnostics at the wrong locations, for example, see FIXMEs at 
>> llvm/tools/clang/test/Analysis/unions.cpp +96,  
>> llvm/tools/clang/test/Analysis/self-assign.cpp +41, 
>> llvm/tools/clang/test/Analysis/pr22954.c in "int f19(int i) {...}", llvm/tools/clang/test/Analysis/pr22954.c in "int f31() {...}" etc.
>> So the question here - if CSA works as expected for this example 
>> (in particular, I'm wondering if LiveVariables::computeLiveness 
>> does the right thing or, alternatively, if it's used properly). 
>> P.S. After playing a little bit more with different toy code snippets and dumping the full state
>> of LiveVariables (everything: stmtsToLiveness, blocksEndToLiveness etc) it looks like in most cases it's almost empty. 
>> P.P.S. The following experiment might be of some interest: if one switches the method
>> SymbolReaper::isLive to 
>> (VarContext == CurrentContext)  || VarContext->isParentOf(CurrentContext) || isAlwaysAlive(....) 
>> the behavior for some very simple tests (i was looking at the toy tests with 3-4 blocks in CFG) becomes more predictable (and a bunch of FIXMEs get corrected), 
>> but this, of course, would not be the correct implementation in the general case.  
>> Kind regards,
>> Alexander Shaposhnikov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20171029/c421a2b8/attachment.html>

More information about the cfe-dev mailing list