[Analyzer] [PATCH] Adding file name to SimpleStreamChecker

[Adam Schnitzer]

I had another shot at it. I made a new checker, StreamV2, which is based
off of SimpleStream, so that can stay as an example.

This patch moved the getFirstAllocation function from the malloc checker to
CheckerContext so it can be reused. If there's a better spot for it, let me
know.

I also added the printing for the MemRegion where the stream was opened.
This did change the location where some of the bugs were reported. There is
one example that I don't think is quite right:

int checkLeak(int *Data) {
  FILE *F = fopen("myfile.txt", "w");
  if (F != 0) {
    fputs ("fopen example", F); // expected-warning {{Opened file is never
closed; potential resource leak of 'F'}}
  }

  if (Data)
    return *Data;
  else
    return 0;
}

It seems like the bug should be reported on the line: if (Data). I think
this might be an error with the order in which I'm removing nodes from the
graph and reporting bugs.

Adam

On Mon, Apr 22, 2013 at 3:10 PM, Adam Schnitzer <adamschn at umich.edu> wrote:

> Anna,
>
> Thanks for the feedback. I think I have a better idea of how to go about
> it now. I'll have another shot at it.
>
> Adam
>
>
> On Mon, Apr 22, 2013 at 1:17 PM, Anna Zaks <ganna at apple.com> wrote:
>
>> Adam,
>>
>> The warning looks good for the listed test cases (though the column seems
>> redundant). However, it might not be a good fit when the file name is not a
>> string literal. In other checkers, we often pretty print the region
>> instead; for example, see reportLeak in the MallocChecker.
>>
>> The second issue is storing the string in the state. It should be
>> possible to get the file info only at the point of a leak report, not when
>> processing 'fopen'. Specifically, you would go up the path when reporting
>> the leak and find the statement that opened the file. That logic would be
>> very similar to "getAllocationSite" from mallocChecker. Let's see if we
>> can factor it out so that we do not continue with copying and pasting of
>> that code.
>>
>> Thanks!
>> Anna.
>> On Apr 21, 2013, at 10:56 PM, Adam Schnitzer <adamschn at umich.edu> wrote:
>>
>> Anna,
>>
>> Got it, sorry about the mixup. I will go ahead and work in
>> a separate file. But did it look like I was on the right track for the
>> diagnostics?
>>
>> Adam
>>
>> On Mon, Apr 22, 2013 at 1:20 AM, Anna Zaks <ganna at apple.com> wrote:
>>
>>> Adam,
>>>
>>> Sorry if I was not 100% clear. We'd like to leave the
>>> SimpleStreamChecker.cpp file as is for reference purposes. You can either
>>> create a new file or replace StreamChecker.cpp with your checker.
>>>
>>> Thanks,
>>> Anna.
>>> On Apr 20, 2013, at 11:34 PM, Adam Schnitzer <adamschn at umich.edu> wrote:
>>>
>>> > This is my first patch for the SimpleStreamChecker. It improves
>>> diagnostics by adding the file name in the case of a resource leak. I did
>>> so by adding a std::string to the StreamState to hold the file name.
>>> >
>>> > Any feedback would be great.
>>> >
>>> > Adam
>>> > <SimpleStreamChecker.patch>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130429/4e372d59/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: StreamCheckerV2.patch
Type: application/octet-stream
Size: 19837 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130429/4e372d59/attachment.obj>