- User Since
- Aug 23 2013, 11:08 AM (213 w, 20 h)
Mon, Sep 18
Please, commit. I think we can take further comments post-commit.
Fri, Sep 15
Looks like I rushed into accepting. This needs a test case of cause!
Thu, Sep 14
Wed, Sep 13
How about committing the refactor of the code without test modifications. And committing changes to the test separately?
Wed, Sep 6
Thanks for addressing the non-determinism!!!
Yes, please, commit.
Mon, Aug 28
But I've never used the taint tracking mode, so I don't know what would be a reasonable default for MaxComp.
that one is very experimental anyway. I'd just keep the functional changes to tain out of this patch and use the current default that taint uses.
Is this blocked on the same reasons as what was raised in https://reviews.llvm.org/D35109?
Sun, Aug 27
Ping... While extending isCalled is not a must, this should not crash on the sample ObjC code.
I think we should have these is .rst format as this is what the rest of the documentation predominantly uses. (Note, the formatting can be very basic, it's the format that I care about.)
Fri, Aug 25
Aug 20 2017
Aug 10 2017
Aug 9 2017
What do you suggest? Should we widen the type of the difference, or abandon this patch and revert back to the local solution I originally used in the iterator checker?
Aug 1 2017
I tried to keep this as a minimal starting example because this currently blocks @yamaguchi 's GSoC project for bash completion. There we want to complete the values for -analyzer-config and we currently don't have a good way to get a complete list of available configs from the driver :).
Jul 31 2017
Jul 20 2017
Jun 28 2017
I agree that we should not print the values of all variables. The users will be overwhelmed with the huge amount of information. It is very valuable to highlight just the right information. (I believe even the current diagnostics often produce too much info and highlighting the more important notes would be very valuable.) However, the examples you presented are very compelling. Are there ways we could highlight the same information without printing values of all variables?
Jun 27 2017
Jun 26 2017
@dcoughlin As a reviewer of both patches - could you tell us what's the difference between them? And how are we going to resolve this issue?
Jun 22 2017
This just supports the statement that this particular check should not go under unix. I understand that it will be inconsistent with the name of the malloc checker, which we probably should not change as people might be relying on the package names. I think it's better to have inconsistency than having checks applicable to windows in a package named portability.unix. If there will be checks that need to be in portability and only used for unix, we could create that sub-package later on.
Jun 20 2017
Hmm, should there be new tests that demonstrate that we cover the new APIs?
Unless we use a new registration mechanism for some of these APIs, I'd be fine without adding a test for every API that has localization constraints.
The problem here is not in ScopeContext - it isn't going to be a huge patch. The problem is CFG-related mostly - there are many corner cases. We'll also need some help with
fixing Objective-C checkers (RetainCount, for example).
In particular, there are patches to generate more symbolic expressions, that could also degrade the performance (but fix some fixmes along the way).
Jun 17 2017
Jun 16 2017
LGTM. Thank you!
eg. checkers for portability across linux/bsd should be off on windows by default, checkers for non-portable C++ APIs should be off in plain C code, etc
Is the checker you are moving to portability off and not useful on Windows?
Thanks! Do you have commit access?
Once Artem gives more details about the codebase he tested on, I think it would be fine to commit this. We can revert/address concerns later if @a.sidorin or anyone else raises concerns based on further testing on their codebases. @a.sidorin, would this work for you?
Maybe we should introduce another UMK_* for deeper analysis instead?
The difference here is not substantial enough to warrant a new level. The general motivation for bumping these numbers is that we've set the timeouts many years ago and the hardware improved quite a bit since then.
Jun 15 2017
-(Anna) Scan-build-py integration of the functionality is nearly finished (see https://github.com/rizsotto/scan-build/issues/83) (--ctu switch performs both analysis phases at once). This I think could go in a different patch, but until we could keep the ctu-build.py and ctu-analyze.py scripts. Do you agree?
Jun 14 2017
Looks good overall, but we should try and extend isCalled if possible.
Should this revision be "abandoned" or is the plan to iterate on it?
Looks good with a nit!
Great cleanup! Some comments below.
This generally sounds good. Definitely do submit these changes in small pieces! See http://llvm.org/docs/DeveloperPolicy.html#incremental-development for rationale.
Jun 12 2017
Jun 5 2017
Jun 1 2017
These are great additions!
May 31 2017
May 23 2017
LGTM. Thank you!
May 22 2017
Is it possible to add a regression test that demonstrates the IR pattern this diff is fixing?
May 17 2017
Thanks! A couple of minor comments below.
May 16 2017
Looks good. Thank you!
Do you have commit access or should we commit?
May 10 2017
Thank you for the patch! Could you please re-submit the patch with context? Instructions on how to do that can be found here:
May 9 2017
That wouldn't work this way because we'd have the completely redundant "calling property accessor" piece before that, and "returning..." after that.
Sorry for the delay!!!
I am fine to land this without a test. @szepet, Do you have commit access or should we commit on your behalf?
These new "extra notes" of mine might be useful (we could put them at property declaration), i could add them if everybody likes this idea.
What are these? Is there a PR?
May 1 2017
Apr 28 2017
Looks good to me as well. Thanks!
Apr 19 2017
I agree that scan-build or scan-build-py integration is an important issue to resolve here. What I envision is that users will just call scan-build and pass -whole-project as an option to it. Everything else will happen automagically:)
@baloghadamsoftware Thanks for working on this!
Apr 13 2017
Apr 7 2017
Apr 5 2017
Mar 16 2017
Are there other cases where makeNull would need to be replaced?
Mar 11 2017
Correct, this will suppress some valid warnings that occur due to user errors and valid warnings coming from the standard library.
Mar 9 2017
I've committed the change, but would very much appreciate community feedback here if if there is any!
We are going to turn this off by default, see https://reviews.llvm.org/D30798.
Mar 8 2017
All this leads to the need of several types of taintednesses (string taintedness, array taintedness, general bound check taintedness) because the cleansing can only take down one type of taintedness at a time. That would be the only way for a checker to be able to access that the taintedness type specific to the checker is still there or was already cleansed by the central logic. For me it shows that cleansing rules belong to specific checkers and cannot be efficiently generalized even in case of a single int type.
Thi has been committed in r290505.
Mar 7 2017
Do you have commit access or should we commit on your behalf?
Mar 6 2017
Following Gabor's suggestion, we should investigate if ArrayBoundCheckerV2 supports this. If not it's possible that we are hitting the Constraint Solver limitations.
This is waiting for a resolution on a CallEvent API patch as described in https://reviews.llvm.org/D27091.
Mar 3 2017
Thanks. looks good.
Mar 2 2017
I’ve added the single file output option but I would like to keep the multi-file option default
This sounds good to me! I agree that this is a very useful addition.
Mar 1 2017
I am not clear why need to calculate the precise allocated size?
The information generated by this checker is used for array bounds checking. For example, see https://reviews.llvm.org/D24307