[cfe-dev] [RFC] Tutorial for Clang Analyzer Plugins

Fri Jan 11 17:06:53 PST 2013

Sam,

I think that the weakest spot in the analyzer documentation right now is the Checker Development Manual. So it would be great if we could use your content to improve it. We basically want to keep the format as is but extend/fill in the missing sections. The code segments that are be used to present different concepts could come from referenced checker/checkers. 

I've looked at your tutorial in more detail and here are some concerns/comments.

--------------- Bigger issues --

Section on Checker as a Plugin
We've discussed this internally and decided that we should not advertise "the checker as a plugin" capability until we are ready to support it. Currently, we have not tested it on any platforms other than OS X, so we are not sure, for example, if the plugins are going to work on Windows. We also do not know if the compilers shipped on various platforms have their symbols stripped or not. In addition, there is no effort to guarantee a rigid checker API. The compiler and the plugins are expected to use the same headers. Due to this, the plugins will only work with the compiler they are written for. With the second restriction, in most cases, just adding the checker to the source code of the compiler would be a good option. 

Said that, it would be too bad to loose all the content you've added to explain how the plugins could be used. Maybe we could make it an internal document in the docs directory (not exposed on the website) with a disclaimer that it is an experimental feature.

The second issue is that the LockUnlockChecker checker is rather weak. It does not solve a real problem (ex: assumes that all the locking/unlocking happen in one function, which is not realistic); in addition, it does not showcase a lot of topics we would want to explain (ex: symbols). How about using the SimpleStreamChecker checker that we've presented in the talk? It's not too easy and not too complicated. The source code is available in the repository.

I know that these two issues would make less of your content applicable, but I think it would be better for the final product. What do you think?

---------------  Smaller issues (I've only briefly read this, will have more through comments for the future patches.)--

- We should assume that the reader works with TOT, not the latest released clang version. We'll try to do our best to keep the manual up to date.

ProgramStates:
 - Would be good to visualize the ExplodedGraph. 
 - You could mention the checker writer should not depend on the order in which the nodes are being explored. I think the explanation could be easier to follow if you use DFS, but one would have to try to know for sure.
 - "Since both of the states have a value of ``a`` defined" -> "Since both of the states have a value of ``a`` perfectly constrained"

Manually defining classes:
We should not expose it in the Manual. We have a lot to talk about, so the shorter we keep it the better. We could create a separate internal-use doc (in the docs folder) about it and link to it.

"When using the ``ProgramStateRef``, any action that changes the ``ProgramState`` will, instead of changing the original object, generate a modified copy, which is then assigned to the ``ProgramStateRef``."
ProgramStateRef is just a smart pointer to ProgramState. See r149081.

"Detailed Rule Logic"
 Unfortunately, does not belong in a manual.

"When we detect an mistake in the analyzed code (in the Analyzer, this is called a *bug*), we need to report it to the Analyzer so that it can be output." 
Wording is off.

When describing BugReport, you are only describing one of the APIs for its creation. You can mention that this is the case. 

"it is standard to transition the current program state to a *sink node*" This is not standard, but depends on the severity of the bug. See the presentation.

Please, use the LLVM coding standard in the examples (for example "if(blah" -> "if (blah)" ).

To summarize, I think the manual would benefit from the content presented in the following sections:
 - The Structure of a Checker
 - Program States - An Example
 - Extending Program States
 - Reporting Bugs
 - Missing info on testing and additional sources of information

Thanks!
Anna.

On Jan 8, 2013, at 3:57 PM, Anna Zaks <ganna at apple.com> wrote:

> 
> On Jan 7, 2013, at 4:12 PM, Anna Zaks <ganna at apple.com> wrote:
> 
>> 
>> On Jan 7, 2013, at 4:01 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>> 
>>> 
>>> On Jan 7, 2013, at 14:00 , Anna Zaks <ganna at apple.com> wrote:
>>> 
>>>> 
>>>> On Jan 7, 2013, at 12:50 PM, Daniel Dunbar <daniel at zuster.org> wrote:
>>>>> 
>>>>> It makes total sense to me to have the internal documentation browsable on the websites somewhere, and this is what we do for LLVM and Clang and all the other projects, so I see no reason for the analyzer to be different. Note that its only "exposed" as much as there is a URL for it. You can decide if and when you want to link to it from the website.
>>>> 
>>>> Makes sense.
>>> 
>>> This isn't exactly true in Sphinx, because the default mode becomes "include" instead of "exclude".
>>> 
>>> It probably does make sense to make the "Development" part of the analyzer site become equivalent to LLVM and Clang's "docs".
>> 
>> I am not sure I agree that everything/anything under "Development" tab should be in different format than the rest of the site.
> 
> We've discussed this more internally. It makes sense for the analyzer manual to be in the same format as the rest of the site. So unless the rest of the analyzer page gets converted to Sphinx, the manual should stay in HTMl format. 
> 
> Unfortunately, we do not have time for full Sphinx conversion. But would be glad if someone else would contribute the effort to do it.
> 
> Sam, I am going to review the tutorial in more detail soon.
> 
> Thanks,
> Anna.
> 
>> 
>>> This is where the current Checker Developer Manual lives (and thanks for working on that, Sam; I think I agree with Anna that we'd rather have one merged doc for now), and where some of the current internal stuff could go if it were cleaned up a bit more, but I think we'd want to help moderate that transition. (If you think we should move our text-file notes out of docs/ for now, we could do that too.)
>>> 
>>> Jordan
>>> 
>> 
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130111/db753041/attachment.html>