[cfe-commits] r168070 - in /cfe/trunk: lib/StaticAnalyzer/Core/BugReporter.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp lib/StaticAnalyzer/Core/PathDiagnostic.cpp test/Analysis/coverage.c test/Analysis/keychainAPI.m test/Analysis/malloc-annotations.c test/Analysis/malloc-interprocedural.c test/Analysis/malloc-plist.c test/Analysis/malloc.c test/Analysis/malloc.cpp test/Analysis/plist-output-alternate.m test/Analysis/retain-release.m test/Analysis/simple-stream-checks.c test/Analysis/stackaddrleak.c

[Jordan Rose]

On Nov 16, 2012, at 14:47 , Anna Zaks <ganna at apple.com> wrote:

> 
> On Nov 15, 2012, at 11:11 AM, Jordan Rose <jordan_rose at apple.com> wrote:
> 
>> Author: jrose
>> Date: Thu Nov 15 13:11:43 2012
>> New Revision: 168070
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=168070&view=rev
>> Log:
>> [analyzer] Report leaks at the closing brace of a function body.
>> 
>> This fixes a few cases where we'd emit path notes like this:
>> 
>> +---+
>> 1|   v
>> p = malloc(len);
>> ^   |2
>> +---+
>> 
>> In general this should make path notes more consistent and more correct,
>> especially in cases where the leak happens on the false branch of an if
>> that jumps directly to the end of the function. There are a couple places
>> where the leak is reported farther away from the cause; these are usually
>> cases where there are several levels of nested braces before the end of
>> the function. This still matches our current behavior for when there /is/
>> a statement after all the braces, though.
>> 
>> Modified:
>>   cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
>>   cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
>>   cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
>>   cfe/trunk/test/Analysis/coverage.c
>>   cfe/trunk/test/Analysis/keychainAPI.m
>>   cfe/trunk/test/Analysis/malloc-annotations.c
>>   cfe/trunk/test/Analysis/malloc-interprocedural.c
>>   cfe/trunk/test/Analysis/malloc-plist.c
>>   cfe/trunk/test/Analysis/malloc.c
>>   cfe/trunk/test/Analysis/malloc.cpp
>>   cfe/trunk/test/Analysis/plist-output-alternate.m
>>   cfe/trunk/test/Analysis/retain-release.m
>>   cfe/trunk/test/Analysis/simple-stream-checks.c
>>   cfe/trunk/test/Analysis/stackaddrleak.c
>> 
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=168070&r1=168069&r2=168070&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Thu Nov 15 13:11:43 2012
>> @@ -1673,6 +1673,9 @@
>>      if (const BinaryOperator *B = dyn_cast<BinaryOperator>(S))
>>        return PathDiagnosticLocation::createOperatorLoc(B, SM);
>> 
>> +      if (isa<PostStmtPurgeDeadSymbols>(ErrorNode->getLocation()))
>> +        return PathDiagnosticLocation::createEnd(S, SM, LC);
>> +
> 
> It is unfortunate that we have to copy and paste this multiple times. Maybe we could create a subroutine that constructs a PathDiagnosticLocation for a StmtPoint?
> 
> Anna.

We have several of these, but each of them does something slightly different. Sometimes we want to get a location within the statement, sometimes we want the end of the statement, and sometimes we want the beginning of the statement.

There is probably much cleanup that can be made, though. This was point-fixing the possible places I thought PostStmtPurgeDeadSymbols could show up.

Jordan