[cfe-dev] Proposal: Integrate static analysis test suites
<Alexander G. Riccio> via cfe-dev
cfe-dev at lists.llvm.org
Sun Mar 6 14:49:38 PST 2016
>
> After the Windows-specific improvements are committed, you might be able
> to rewrite the function matching code by using the newly added helper
> function:
> http://reviews.llvm.org/D15921
>
I'm looking into this, and I have a few questions about this:
- Why do we lazily initialize (instead of initializing at
construction) the IdentifierInfo*s in the first place? Is the token info
only valid at some point after construction?
- Is there a better way to check if the IdentifierInfo*s are initialized
than a nullptr comparison on every isCalled? i.e.:
if (!CD.II)
CD.II = &getState()->getStateManager().getContext().Idents.get(CD.FuncName);
- How would I:
- Get the original CallEvent from the FunctionDecl passed to
MallocChecker::isCMemFunction?
OR
- Implement CallEvent::isCalled for the FunctionDecl, as passed to
MallocChecker::isCMemFunction?
Perhaps we could create another helper class, that just wraps the
comparisons? We could try to use some template magic to wrap groups of
allocating functions, so we can replace a bunch of conditions with a single
call, e.g.:
if (Family == AF_Malloc && CheckAlloc) {
if (FunI == II_malloc || FunI == II_realloc || FunI == II_reallocf ||
FunI == II_calloc || FunI == II_valloc || FunI == II_strdup ||
FunI == II_win_strdup || FunI == II_strndup || FunI == II_wcsdup ||
FunI == II_win_wcsdup || FunI == II_kmalloc)
return true;
}
...could become something like this:
if (Family == AF_Malloc && CheckAlloc) {
if (MallocFamily.isMallocStyle(FunI))
return true;
}
Where MallocFamily is something like:
mutable AllocatorTypeGroup<"malloc", "realloc", "reallocf",
"calloc", "valloc", "strdup",
"_strdup", "strndup", "wcsdup",
"_wcsdup", "kmalloc"> MallocFamily;
...and AllocatorTypeGroup would be some kind of variadic template, that
compares each parameter in the parameter pack with FunI, in pseudocode:
template<typename Last>
bool AllocatorTypeGroup::isMallocStyle( const IdentifierInfo* FunI,
Last last) {
if (FunI == last)
return true;
return false;
}
template<typename First, typename... Rest>
bool AllocatorTypeGroup::isMallocStyle( const IdentifierInfo* FunI,
const First& first,
const Rest&... rest) {
if ((FunI == first) || (isMallocStyle(FunI, rest...))
return true;
return false;
}
Sincerely,
Alexander Riccio
--
"Change the world or go home."
about.me/ariccio
<http://about.me/ariccio>
If left to my own devices, I will build more.
⁂
On Sun, Feb 28, 2016 at 12:59 AM, Anna Zaks <ganna at apple.com> wrote:
>
> On Feb 27, 2016, at 8:18 PM, <Alexander G. Riccio> <test35965 at gmail.com>
> wrote:
>
> Hmm. It seems that message got rejected because it was a tiny bit over the
> size limit. It turns out those scripts were wrong anyways. These are
> slightly better - Windows uses _strdup instead of strdup, and I don't know
> how to conditionally #define strdup _strdup in the makefile - but you can
> modify them to work locally.
>
> Because of that strdup/_strdup issue, I've actually found a leak that
> Clang misses when compiling on Windows. The test is SARD #149071,
> mem1-bad.c <https://samate.nist.gov/SARD/view_testcase.php?tID=149071>.
> MallocChecker::isCMemFunction appears to miss it because it doesn't exactly
> match strdup:
> <strdup_leak_missed_small.PNG>
>
> If I understand correctly, to fix this, I just need to:
>
> 1. add a new IdentifierInfo* to MallocChecker
> 2. initialize it in the default constructor
> 3. add a Ctx.Idents.get("_strdup") call
> in MallocChecker::initIdentifierInfo
> 4. add the IdentifierInfo to the check in MallocChecker::isCMemFunction
> 5. add the Identifier info the else if (FunI == II_strdup)
> in MallocChecker::checkPostStmt
>
> ...and that should be a 5 line patch.
>
>
> Maybe we should only do that if we are targeting Windows. Also, I am sure
> there are other functions with the “_” Windows versions we are skipping
> elsewhere in the analyzer.
>
> After the Windows-specific improvements are committed, you might be able
> to rewrite the function matching code by using the newly added helper
> function:
> http://reviews.llvm.org/D15921
>
> Thanks,
> Anna.
>
>
> Sincerely,
> Alexander Riccio
> --
> "Change the world or go home."
> about.me/ariccio
>
> <http://about.me/ariccio>
> If left to my own devices, I will build more.
> ⁂
>
> On Tue, Feb 23, 2016 at 4:35 PM, <Alexander G. Riccio> <
> test35965 at gmail.com> wrote:
>
>> I've attached the scripts for the non-buggy SARD tests. Interestingly,
>> I've actually had to append ".attachment" to the ".sh" and ".cmd"
>> filenames, for gmail to let me send them. Gmail was blocking them as
>> executables.
>>
>> I've dropped nine tests (of 100). I don't think they'll offer any value
>> (for clang), so we won't be missing anything by dropping them.
>>
>> Four of those tests are about SQL injection - which we're quite far from
>> being able to check - and also require third party (MySQL)
>> headers/libraries to run. Anna expressed concern (private email) in
>> non-self-sufficient tests, so those four are not feasible.
>>
>> The other five tests are about cross site scripting - which we're also
>> quite far from being able to check - and require separate compilation. I'm
>> terrible with makefiles, which is why separate compilation is a problem :)
>> ...if anybody with makefile expertise can do this cleanly, please let me
>> know!
>>
>> For the remaining tests, I've had to suppress a few false positives
>> (notably "format string is not a string literal") so that everything builds
>> cleanly.
>>
>> I don't have easy access to a proper Linux system, so I can't fully test
>> these scripts (I can only really run SATestBuild.py manually: py -2
>> "C:\LLVM\llvm\tools\clang\utils\analyzer\SATestBuild.py"), can someone
>> try them for me?
>>
>> Sincerely,
>> Alexander Riccio
>> --
>> "Change the world or go home."
>> about.me/ariccio
>>
>> <http://about.me/ariccio>
>> If left to my own devices, I will build more.
>> ⁂
>>
>> On Wed, Feb 17, 2016 at 2:13 AM, <Alexander G. Riccio> <
>> test35965 at gmail.com> wrote:
>>
>>> An update: I've pretty much gotten the non-bug version of the testsuite
>>> ready, and now that I know what I'm doing, the buggy version will be soon
>>> to follow.
>>>
>>> Sincerely,
>>> Alexander Riccio
>>> --
>>> "Change the world or go home."
>>> about.me/ariccio
>>>
>>> <http://about.me/ariccio>
>>> If left to my own devices, I will build more.
>>> ⁂
>>>
>>> On Thu, Feb 4, 2016 at 6:52 PM, <Alexander G. Riccio> <
>>> test35965 at gmail.com> wrote:
>>>
>>>> Here's an update, I'd been waiting to hear back from NIST/SAMATE about
>>>> licensing of the SARD testsuite, and it *looks* like we can use it:
>>>>
>>>> Hi Alexander,
>>>>>
>>>>> You are free to use, copy, modify, and distribute (in particular,
>>>>> integrate into Clang's static analysis testing infrastructure) all test
>>>>> cases included in the Software Assurance Reference Dataset (SARD) test
>>>>> suites 100 and 101.
>>>>>
>>>>> Most of these test cases were developed by NIST, and so they are in
>>>>> the public domain. The rest of the test cases were contributed by others
>>>>> with permission to use, copy, modify, and distribute them.
>>>>>
>>>>> I hope this is helpful.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Vadim Okun
>>>>> SAMATE Project Leader
>>>>> vadim.okun at nist.gov
>>>>
>>>>
>>>> The only bit that's not crystal clear is "contributed by others with
>>>> permission to use, copy, modify, and distribute them", which I think fits
>>>> the UIUC license requirements.
>>>>
>>>> Sincerely,
>>>> Alexander Riccio
>>>> --
>>>> "Change the world or go home."
>>>> about.me/ariccio
>>>>
>>>> <http://about.me/ariccio>
>>>> If left to my own devices, I will build more.
>>>> ⁂
>>>>
>>>> On Wed, Jan 20, 2016 at 1:11 AM, <Alexander G. Riccio> <
>>>> test35965 at gmail.com> wrote:
>>>>
>>>>> A quick update on this project:
>>>>>
>>>>> I've been slowed by a technical issue, and I lost ~2 weeks as two
>>>>> family members were in the hospital (sorry!).
>>>>>
>>>>> Since I develop on Windows, I quickly hit a testcase that clang didn't
>>>>> detect, as I discussed in "Clang on Windows fails to detect trivial
>>>>> double free in static analysis".
>>>>>
>>>>> That resulted in D16245 <http://reviews.llvm.org/D16245>, which (when
>>>>> accepted) fixes that issue. I want to ensure that novice can simply pass
>>>>> "--analyze", and clang to "just work", so I've intentionally put off
>>>>> further testing work. Otherwise, I could hack around it, and subsequently
>>>>> forget about the workaround. Once that's dealt with, then I can resume work
>>>>> at a faster pace.
>>>>>
>>>>>
>>>>>
>>>>> Sincerely,
>>>>> Alexander Riccio
>>>>> --
>>>>> "Change the world or go home."
>>>>> about.me/ariccio
>>>>>
>>>>> <http://about.me/ariccio>
>>>>> If left to my own devices, I will build more.
>>>>> ⁂
>>>>>
>>>>> On Mon, Jan 4, 2016 at 3:05 PM, Anna Zaks <ganna at apple.com> wrote:
>>>>>
>>>>>>
>>>>>> On Jan 2, 2016, at 12:45 PM, <Alexander G. Riccio> <
>>>>>> test35965 at gmail.com> <Alexander G. Riccio> wrote:
>>>>>>
>>>>>> Devin has started writing scripts for running additional analyzer
>>>>>>> tests as described in this thread:
>>>>>>>
>>>>>>
>>>>>> A buildbot sounds like the perfect idea!
>>>>>>
>>>>>> The idea was to check out the tests/projects from the existing repos
>>>>>>> instead of copying them. Would it be possible to do the same with these
>>>>>>> tests?
>>>>>>>
>>>>>>
>>>>>> Eh? What do you mean? Would that stop someone from running them in
>>>>>> the clang unit test infrastructure?
>>>>>>
>>>>>> I believe that these tests WILL need to be modified to run in the
>>>>>> Clang testing infrastructure.
>>>>>>
>>>>>>
>>>>>> Currently, the analyzer is only tested with the regression tests.
>>>>>> However, those need to be fast (since they effect all clang developers) and
>>>>>> they have limited coverage. Internally, we’ve been testing the analyzer
>>>>>> with the test scripts Devin described in the email I referenced. We use
>>>>>> that testing method to analyze whole projects and long running tests. Those
>>>>>> tests can and should be executed separately as they take more than an hour
>>>>>> to complete. The plan is to set up an external builedbot running those
>>>>>> tests.
>>>>>>
>>>>>> How long would it take to analyze the tests you are planning to add?
>>>>>> Depending on the answer to that question, adding your tests to the new
>>>>>> builedbot might be a better fit than adding them to the regression tests.
>>>>>>
>>>>>> I also prefer not to modify the externally written tests since it
>>>>>> would allow us to update them more easily, for example, when a new version
>>>>>> of the tests comes out.
>>>>>>
>>>>>> Is there any way to treat static analyzer warnings as plain old
>>>>>> warnings/errors? Dumping them to a plist file from a command line
>>>>>> compilation is a bit annoying, and I think is incompatible with the clang
>>>>>> unit testing infrastructure?
>>>>>>
>>>>>>
>>>>>> Plist output is one if the outputs that the clang static analyzer
>>>>>> supports. It is a much richer format than the textual warning since it
>>>>>> contains information about the path on which the error occurred. We did
>>>>>> have some lit tests checking plist output as well.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Sincerely,
>>>>>> Alexander Riccio
>>>>>> --
>>>>>> "Change the world or go home."
>>>>>> about.me/ariccio
>>>>>>
>>>>>> <http://about.me/ariccio>
>>>>>> If left to my own devices, I will build more.
>>>>>> ⁂
>>>>>>
>>>>>> On Mon, Dec 28, 2015 at 12:23 AM, Anna Zaks <ganna at apple.com> wrote:
>>>>>>
>>>>>>>
>>>>>>> On Dec 17, 2015, at 11:01 AM, <Alexander G. Riccio> <
>>>>>>> alexander at riccio.com> <Alexander G. Riccio> wrote:
>>>>>>>
>>>>>>> However, if the goal is to have the tests
>>>>>>>> because you would like to make efforts to have the compiler diagnose
>>>>>>>> their cases properly, that's far more interesting and a good reason
>>>>>>>> to
>>>>>>>> bring in the tests.
>>>>>>>
>>>>>>>
>>>>>>> That's exactly my intention. Improving the static analyzer to detect
>>>>>>> these cases, that will be interesting.
>>>>>>> placeholder text
>>>>>>>
>>>>>>> If the other tests are not clearly licensed, we
>>>>>>>> should try to get NIST to clarify the license of them before
>>>>>>>> inclusion.
>>>>>>>
>>>>>>>
>>>>>>> That sounds like the best idea, as a government agency, they almost
>>>>>>> certainly have lawyers.
>>>>>>>
>>>>>>> I think the next step is to integrate the working (error correctly
>>>>>>> diagnosed) tests, only those that are obviously in the public domain, and
>>>>>>> propose them as a big batched patch. This shouldn't itself be
>>>>>>> controversial.
>>>>>>>
>>>>>>> How exactly do I submit a patch? I see that the LLVM developer
>>>>>>> policy says to send it to the mailing list (cfe-commits), but I also see
>>>>>>> that Phabricator comes into this somewhere
>>>>>>> <http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20151214/145026.html>
>>>>>>> ?
>>>>>>>
>>>>>>>
>>>>>>> Devin has started writing scripts for running additional analyzer
>>>>>>> tests as described in this thread:
>>>>>>>
>>>>>>> http://clang-developers.42468.n3.nabble.com/analyzer-Adding-build-bot-for-static-analyzer-reference-results-td4047770.html
>>>>>>>
>>>>>>> The idea was to check out the tests/projects from the existing repos
>>>>>>> instead of copying them. Would it be possible to do the same with these
>>>>>>> tests?
>>>>>>>
>>>>>>> Sorry for not replying sooner!
>>>>>>> Anna.
>>>>>>>
>>>>>>> Sincerely,
>>>>>>> Alexander Riccio
>>>>>>> --
>>>>>>> "Change the world or go home."
>>>>>>> about.me/ariccio
>>>>>>>
>>>>>>> <http://about.me/ariccio>
>>>>>>> If left to my own devices, I will build more.
>>>>>>> ⁂
>>>>>>>
>>>>>>> On Thu, Dec 10, 2015 at 4:04 PM, Aaron Ballman <
>>>>>>> aaron at aaronballman.com> wrote:
>>>>>>>
>>>>>>>> On Mon, Dec 7, 2015 at 9:50 PM, <Alexander G. Riccio> via cfe-dev
>>>>>>>> <cfe-dev at lists.llvm.org> wrote:
>>>>>>>> > First time Clang contributor here,
>>>>>>>> >
>>>>>>>> > I'd like to add the "C Test Suite for Source Code Analyzer v2", a
>>>>>>>> > relatively small test suite (102 cases/flaws), some of which Clang
>>>>>>>> > doesn't yet detect*. See link at bottom.
>>>>>>>> >
>>>>>>>> > Immediate questions:
>>>>>>>> > 0. Does the Clang community/project like the idea?
>>>>>>>>
>>>>>>>> I've included a few other devs (CCed) to get further opinions.
>>>>>>>>
>>>>>>>> I like the idea of being able to diagnose the issues covered by the
>>>>>>>> test suite, but I don't think including the test suite by itself is
>>>>>>>> particularly useful without that goal in mind. Also, one question I
>>>>>>>> would have has to do with the licensing of the tests themselves and
>>>>>>>> whether we would need to do anything special there.
>>>>>>>>
>>>>>>>> > 1. What's the procedure for including new tests? (not the
>>>>>>>> technical,
>>>>>>>> > but the community/project).
>>>>>>>>
>>>>>>>> Getting the discussion going about the desired goal (as you are
>>>>>>>> doing)
>>>>>>>> is the right first step.
>>>>>>>>
>>>>>>>> > 2. How do I include failing tests without breaking things? Some of
>>>>>>>> > these tests will fail - that's why I'm proposing their inclusion
>>>>>>>> - but
>>>>>>>> > they shouldn't yet cause the regression testing system to
>>>>>>>> complain.
>>>>>>>>
>>>>>>>> Agreed, any test cases that are failing would have to fail
>>>>>>>> gracefully.
>>>>>>>> I assume that by failure, you mean "should diagnose in some way, but
>>>>>>>> currently does not". I would probably split the tests into two
>>>>>>>> types:
>>>>>>>> one set of tests that properly diagnose the issue (can be checked
>>>>>>>> with
>>>>>>>> FileCheck or -verify, depending on the kind of tests we're talking
>>>>>>>> about), and one set of tests where we do not diagnose, but want to
>>>>>>>> see
>>>>>>>> them someday (which can be tested with expect-no-diagnostics, for
>>>>>>>> example). This way, we can ensure test cases continue to diagnose
>>>>>>>> when
>>>>>>>> we want them to, and we can be alerted when new diagnostics start to
>>>>>>>> catch previously uncaught tests. This is assuming that it makes
>>>>>>>> sense
>>>>>>>> to include all of the tests at once, which may not make sense in
>>>>>>>> practice.
>>>>>>>>
>>>>>>>> > 3. How does Clang handle licensing of third party code? Some of
>>>>>>>> these
>>>>>>>> > tests are clearly in the public domain (developed at NIST, says
>>>>>>>> "in
>>>>>>>> > the public domain"), but others are less clearly licensed.
>>>>>>>>
>>>>>>>> Oh look, you asked the same question I asked. ;-) If the tests are
>>>>>>>> in
>>>>>>>> the public domain and clearly state as such, I think we can go ahead
>>>>>>>> and include them. If the other tests are not clearly licensed, we
>>>>>>>> should try to get NIST to clarify the license of them before
>>>>>>>> inclusion. Depending on the license, we may be able to include them
>>>>>>>> under their original license. If we cannot clarify the license, I
>>>>>>>> would guess that we simply should not include those tests as part of
>>>>>>>> our test suite. Note: I could be totally wrong, IANAL. :-)
>>>>>>>>
>>>>>>>> > Should the community accept that testsuite, and I successfully add
>>>>>>>> > that test suite, then I'd like to step it up a bit, and include
>>>>>>>> the
>>>>>>>> > "Juliet Test Suite for C/C++". "Juliet" is a huge test suite by
>>>>>>>> the
>>>>>>>> > NSA Center for Assured Software & NIST's Software Assurance
>>>>>>>> Metrics
>>>>>>>> > And Tool Evaluation project, which has 25,477 test cases (!!) for
>>>>>>>> 118
>>>>>>>> > CWEs. I don't think any other open source compiler could compete
>>>>>>>> with
>>>>>>>> > Clang after this. There's a ton of literature on the "Juliet"
>>>>>>>> suite,
>>>>>>>> > and listing it here is not necessary.
>>>>>>>> >
>>>>>>>> > This project would be my first Clang contribution :)
>>>>>>>> >
>>>>>>>> > Personally, I'm interested in static analysis, and this is the
>>>>>>>> first
>>>>>>>> > step in understanding & improving Clang's static analysis
>>>>>>>> > capabilities.
>>>>>>>> >
>>>>>>>> > I have some ideas on how to detect the currently undetected bugs,
>>>>>>>> and
>>>>>>>> > I'm curious to see where things lead.
>>>>>>>>
>>>>>>>> Adding the tests by themselves is not necessarily interesting to the
>>>>>>>> project unless they exercise the compiler in ways it's not currently
>>>>>>>> being exercised. So just having tests for the sake of having the
>>>>>>>> tests
>>>>>>>> is not too useful (IMO). However, if the goal is to have the tests
>>>>>>>> because you would like to make efforts to have the compiler diagnose
>>>>>>>> their cases properly, that's far more interesting and a good reason
>>>>>>>> to
>>>>>>>> bring in the tests.
>>>>>>>>
>>>>>>>> One possible approach if you are interested in having the compiler
>>>>>>>> diagnose the cases is to bring the tests in one at a time. Start
>>>>>>>> with
>>>>>>>> the initial batch of "these are diagnosed properly", then move on to
>>>>>>>> "this test is diagnosed properly because of this patch." Eventually
>>>>>>>> we'll get to the stage where all of the tests are diagnosed
>>>>>>>> properly.
>>>>>>>>
>>>>>>>> > Secondary questions:
>>>>>>>> > 1. How should I break the new tests up into patches? Should I just
>>>>>>>> > whack the whole 102 case suite into a single patch, or a bunch of
>>>>>>>> > smaller ones?
>>>>>>>>
>>>>>>>> See comments above.
>>>>>>>>
>>>>>>>> > 2. How does the Clang/LLVM static analysis testing infrastructure
>>>>>>>> > work? I'm going to have to figure this out myself anyways, but
>>>>>>>> where
>>>>>>>> > should I start? Any tips on adding new tests?
>>>>>>>>
>>>>>>>> http://clang-analyzer.llvm.org/checker_dev_manual.html
>>>>>>>>
>>>>>>>> Another good place for some of these checkers may be clang-tidy, or
>>>>>>>> the compiler frontend itself. It's likely to depend on case-by-case
>>>>>>>> code patterns.
>>>>>>>>
>>>>>>>> http://clang.llvm.org/extra/clang-tidy/
>>>>>>>>
>>>>>>>> Thank you for looking into this!
>>>>>>>>
>>>>>>>> ~Aaron
>>>>>>>>
>>>>>>>> >
>>>>>>>> > *If I remember correctly,
>>>>>>>> > https://samate.nist.gov/SRD/view_testcase.php?tID=149055 passes
>>>>>>>> > analysis without complaint. I manually spot checked a very small
>>>>>>>> > number of tests.
>>>>>>>> >
>>>>>>>> > "C Test Suite for Source Code Analyzer v2" (valid code):
>>>>>>>> > https://samate.nist.gov/SRD/view.php?tsID=101
>>>>>>>> > "C Test Suite for Source Code Analyzer v2" (invalid code):
>>>>>>>> > https://samate.nist.gov/SRD/view.php?tsID=100
>>>>>>>> >
>>>>>>>> > "Juliet Test Suite for C/C++" (files):
>>>>>>>> >
>>>>>>>> https://samate.nist.gov/SRD/testsuites/juliet/Juliet_Test_Suite_v1.2_for_C_Cpp.zip
>>>>>>>> > "Juliet Test Suite for C/C++" (docs):
>>>>>>>> >
>>>>>>>> https://samate.nist.gov/SRD/resources/Juliet_Test_Suite_v1.2_for_C_Cpp_-_User_Guide.pdf
>>>>>>>> >
>>>>>>>> >
>>>>>>>> > Sincerely,
>>>>>>>> > Alexander Riccio
>>>>>>>> > _______________________________________________
>>>>>>>> > cfe-dev mailing list
>>>>>>>> > cfe-dev at lists.llvm.org
>>>>>>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
> <SARD_SCRIPTS_attachment.7z>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160306/8c0ead3c/attachment.html>
More information about the cfe-dev
mailing list