Summary:
Make checks context-aware, to increase flexibility.
As an example application of this change, whenever an atomic property is accessed from within a synchronized block, skip reporting a `DIRECT_ATOMIC_PROPERTY_ACCESS` warning.
Reviewed By: jvillard
Differential Revision: D3648831
fbshipit-source-id: c033f45
Summary: Follow up D3579581. We forget about memory acquired in resources with assumption that developers use raii and free memory in destructors.
Reviewed By: jvillard
Differential Revision: D3614056
fbshipit-source-id: 08fa112
Summary:
Previously, we would translate `throw` with `return`. However, `throw` in
ObjC/C++ is often used to mean "abort". We now translate `throw` the same as
`exit` to prune these paths.
Reviewed By: akotulski
Differential Revision: D3594156
fbshipit-source-id: 81083bb
Summary:
The extra dereference in stmtexpr was wrong. When a dereference is needed, we have a cast.
This was causing one dereference too many, and creating wrong results.
Reviewed By: akotulski
Differential Revision: D3393294
fbshipit-source-id: 7a1ec8e
Summary:Local variable created by conditional operator translation is now declared in scope of whole
procedure. Semantically there is no difference, hopefuly backend will not complain about this
change. Also, nullifying that variable is deferred to preanalysis instead of calling it manually
Reviewed By: jvillard
Differential Revision: D3155733
fb-gh-sync-id: 6cec8fc
fbshipit-source-id: 6cec8fc
Summary:BinaryConditionalOperator should evaluate condition expression once, but we used to evaluate it twice.
Fix translation to account for it.
Reviewed By: dulmarod
Differential Revision: D3179803
fb-gh-sync-id: a801a7e
fbshipit-source-id: a801a7e
Summary:public
When a conditional is the last instruction, there will be a join node leading directly to the exit node.
Some instructions, such as nullification of dead variables, and abstraction, are added to the control flow graph automatically. But, join nodes cannot contain instructions. So when a procedure ends with a conditional, there might be no place to store these instructions.
This diff adds one extra node between the join and the exit node in that situation.
Reviewed By: jvillard
Differential Revision: D3179056
fb-gh-sync-id: 2b9cd7e
fbshipit-source-id: 2b9cd7e
Summary:public
Instead of using location of init_stmt, use location of variable when translating initialization.
Most of the time it change anything with some exceptions:
// example1 - C/C++/objC
int x = // now: assignment happens in this line
3; // past: assignment happens in this line
// example2: valid in C++11 only
struct X {
int x = 0; // now: one assignment here
int y = 2; // now: one assigmnent here
X() = default; // before: 2 assignments in this line
};
Reviewed By: dulmarod
Differential Revision: D3155870
fb-gh-sync-id: f38c78c
fbshipit-source-id: f38c78c
Summary:public
Add modeling of methods that can raise exceptions when parameters are
nil, and that return nil when passed nil.
Reviewed By: dulmarod
Differential Revision: D3101739
fb-gh-sync-id: 76af5a2
fbshipit-source-id: 76af5a2
Summary:public This also required a refactoring of InitListExpr.
The idea is that ImplicitValueInitExpr can stand for initialising a whole struct,
so we translating as a list of zero expressions, according to the struct's fields,
which is then paired with a list of field expressions, such that one get a list of
assignment instructions.
Reviewed By: ddino
Differential Revision: D2999875
fb-gh-sync-id: 7f609a0
shipit-source-id: 7f609a0
Summary:public
An observer object that registered to a notification center needs to be
unregistered before it is deallocated.
If not, the notification center may send a notification to a gost object.
This diff introduce a checker for this problem.
Reviewed By: dulmarod
Differential Revision: D2949692
fb-gh-sync-id: 1653cec
shipit-source-id: 1653cec
Summary:public
Improved/simplified framework for fronend checkers.
Now we have a unique hook from cTrans to run checkers on statements and a unique
hook from cFrontent to run checkers on declarations.
So now when adding a checker we don't have to modify cTrans/cFrontend.
Moreover made more sistematic the way checkers are invoked. This simplify the definition
of checkers and the way we use them.
Code is now simpler.
Reviewed By: jvillard
Differential Revision: D2976589
fb-gh-sync-id: fbe22d4
shipit-source-id: fbe22d4
Summary:public We model it as the builtin __instanceof which models the instanceof construct of Java.
The behaviour is the same.
Reviewed By: jvillard
Differential Revision: D2938969
fb-gh-sync-id: 2258de3
shipit-source-id: 2258de3
Summary:
public
Remove some of the explicit pattern matches in favor of using trans_state.var_exp for
init lists and compound literals
Reviewed By: jvillard
Differential Revision: D2855203
fb-gh-sync-id: ce929f6
Summary: public Did this by adding an option to rearrange that turns of error reporting.
Reviewed By: dulmarod
Differential Revision: D2768396
fb-gh-sync-id: 4898d2d
Summary:
public To deal with ObjC nullability and give meaningful error
messages, we introduced the ObjC_NULL attribute in the symbolic execution to
mean that the object carrying the attribute is null because it was the result
of a method call from a null object. However, one cannot add attributes to null,
so we had to delay nullifying the object in order to have the attribute until we
can assign it to a program variable. However, if the temp variable was used in a condition,
we were not taking into account that its meaning is null. This diff addresses that and fixes
many FPs that we have encounter.
Reviewed By: ddino
Differential Revision: D2765167
fb-gh-sync-id: c0878dd
Summary:
public
Lines other than the first of multi-line comments in non-ocaml files
were flush right instead of aligned.
Reviewed By: jvillard
Differential Revision: D2739752
fb-gh-sync-id: c85f56e
Summary:
public This continues the work on adding builtins for getters and setters
and removing the generated code from the frontend. This is extending it to setters and
removing the preanalysis that is no longer needed.
Reviewed By: akotulski
Differential Revision: D2734499
fb-gh-sync-id: 7cf5749
Summary:
public The ivar corresponding to the property is only available in the ast when the
implementation of the peroperty is available. Otherwise we add an ivar with the correct type
and the default name to the tenv and use it in the getter (and later in the setter).
This was not causing crashes because the generated code was swallowing the Missing_fld exception.
Now it flags it.
Reviewed By: akotulski
Differential Revision: D2734217
fb-gh-sync-id: 21c62af
Summary:
public
Use generic location visitor from facebook-clang-plugins
Therefore, there is no need to write our custom visitor which is hard to maintain
This introduces some level of magic, but makes it easier to maintain the code.
Reviewed By: jvillard
Differential Revision: D2734282
fb-gh-sync-id: ed9711a
Summary:
public
Update fcp, changes:
1. decl_ref from ObjcPropertyDecl to IvarDecl
2. Export location information in macro expansion, not macro definition
Reviewed By: dulmarod, jvillard
Differential Revision: D2733967
fb-gh-sync-id: 7d8ce00
Summary:
public Using the new information from the plugin that links method decls in objc with their
properties if they are getters. Setters to be done in next diff.
Reviewed By: ddino
Differential Revision: D2729511
fb-gh-sync-id: dc58894
Summary:
public
Modify do_branch inside conditionalOperator_trans:
1. Remove TempJoinNode optimization - instead create join node as usually. It produces one more node, but it's much cleaner
2. Claim priority inside do_branch instead of appending instructions to node created by children
3. Use compute_results_to_parent to some extent
4. Random changes to loop_instruction and trans_assertion_failure to make tests pass
Reviewed By: ddino
Differential Revision: D2708076
fb-gh-sync-id: d429167
Summary: public This only supports parameters for now, but should be easy to extend to return values and fields. The work of this diff is all in the translation--the task of finding annotations and doing the actual checking is handled by existing code.
Reviewed By: akotulski
Differential Revision: D2706791
fb-gh-sync-id: 0d706a8
Summary: public These two functions were doing very same things.
After last refactor code of both functions started to look very similar.
Reviewed By: dulmarod
Differential Revision: D2707502
fb-gh-sync-id: b0559a3
Summary: public
We were generating getters and setters in the frontend, and then removing them if they were not needed
in the preanalysis. This diff adds a builtin getter that gets called if we are going to skip the function. That
means, if there was a getter written by the developers we still use that one rather than the builtin.
Code for setter and cleanup of preanalysis will come in a next diff.
Reviewed By: jvillard
Differential Revision: D2702890
fb-gh-sync-id: d65f25e
Summary: public Make it uniform with other function call translations. This is just reordering, no functional change intended
Reviewed By: dulmarod
Differential Revision: D2696370
fb-gh-sync-id: 63656e3
Summary: public
Nodes in these functions can be created later (after translation of subexpressions is done).
Change triggers many differences in .dot files, but they are all about different numbering of
nodes.
There is no functional change in this diff.
Reviewed By: dulmarod
Differential Revision: D2699703
fb-gh-sync-id: a7b8d2a
Summary: public
Using clang's method resolution. This means that, in method calls, clang gives you a pointer to the declaration of the method.
In some cases though, clang doesn't find the right method. For example, when it finds a method in a category, we
need to make it into a method in the corresponding class, because that's how we treat categories in Infer. Moreover,
when it finds a method in a protocol, that is not useful for us, since the implementation will be in some class. Finally,
sometimes the call is on an object of type id, in which case clang doesn't know what is the correct declaration. In
those cases, we fall back to what we were doing before of approximating the method resolution. We also refactor
some of the code.
Reviewed By: akotulski
Differential Revision: D2679766
fb-gh-sync-id: b79bb85
Summary: public
-Wno-error flag doesn't turn off errors which were turned on using -Werror=$WARN_TYPE.
In order to turn these off we'd have to add -Wno-error=$WARN_TYPE for each warning type.
So instead, turn off all warnings so they can't get elevated to error
Reviewed By: jvillard
Differential Revision: D2690810
fb-gh-sync-id: 7d6611f
Summary: public
Update facebook-clang-plugins. Changes:
- [major] new clang
- dump class template instantiations
- change naming for qual names of anonymous structs
Changes are due to:
1. Different setter properties are exported for defaults (more correct). Fix the test that was wrong
2. New unary operator type
3. Naming for qual names changed
Reviewed By: jvillard
Differential Revision: D2685734
fb-gh-sync-id: f2c568a
Summary: public
The static and global variables used in blocks don't appear in the ast as captured.
We need them however to try and find retain cycles involving those variables.
This diff adds a way of collecting the static variables used in blocks and treat them
like we treat other captured variables to find retain cycles.
Reviewed By: ddino
Differential Revision: D2663727
fb-gh-sync-id: d5b44ec
Summary: public
Move dealing with init expressions from VarDecl_trans to standalone function.
This will be useful for constructor initializer.
I didn't improve anything, just moved stuff around so that no test breaks.
Main change is that we pass Sil code responsible for LHS (var dereference) as
result_trans.
Currently we assume that var_result_trans will have no nodes, no instructions,
no ids. This is true for current usages (it's always Sil.Lvar expression), but in the future it
will change.
Reviewed By: dulmarod
Differential Revision: D2647108
fb-gh-sync-id: cae60b6
Summary: public
Dictionary literals are normally implemented using
`+dictionaryWithObjects:forKeys:count:` but were modeled as
`+dictionaryWithObjectsAndKeys:`
In particular, `@{@"aaa": nil}` would trigger a sentinel error instead of an NPE.
This models dictionary literals as a special infer builtin that the backend
interprets so as to give NPEs when passed nil objects or keys.
Reviewed By: dulmarod
Differential Revision: D2550039
fb-gh-sync-id: 1a10656
Summary: @public
This removes the old way of finding variable declarations to create sil variables and replaces it with
a a new way based on the map from pointers to declarations.
Basically, every variable dereference contains a pointer to the variable declaration, with that we can
build the corresponding sil variable.
Reviewed By: @akotulski
Differential Revision: D2536000
fb-gh-sync-id: dd29cf9
Summary:
In the new clang the parameters to these functions have notnull annotations, because of that infer tests fail. More concretely, the tests say there would be a memory leak. In the symbolic execution of those functions though, an inconsistency is created, because the parameter was nil, and the constraint argument should not be nil was also there, which leads to an error in the execution and no object is created, hence, no memory leak.
Summary:
This is the second of 3 stack diffs to deal with replacing the parser of types.
This diff is about changes to translate record types, as well as class types and enum
types. For class types and enum types we store the declaration pointer in the map of
types to find the type easier later.
For record declarations, we change the way we build record names.
Moreover, we don't translate typedefs anymore, because when we have a pointer to a typedef,
we can find the actual type it points to.
Summary:
each procedure has a different scope, so we can restart the fresh name generator and have more stable instructions in the cfg, that don't change when other procedures are changed
Summary:
The methods in objc can have the same name in the same class, but one be instance and the other class,
so that we need to take the instance flag into account when defining unique names for ObjC methods.
Summary:
This adds a sentinel check every time a function carrying a sentinel attribute
is called, regardless of whether we have a definition for that function or not.
Summary:
@public
Sorting the fields in structs and classes. Was needed in the backend and forgotten.
Fixes the github issue https://github.com/facebook/infer/issues/90.
Test Plan: Added a new test that shows that we now get a spec for the example from the github issue.
Summary:
@public
The clang location information is described in an incremental way: each location information is a delta with respect to the previous one in the AST. This is based on a the visit of the AST nodes which corresponds to the order in which the lines are printed with the standard clang AST dump:
clang -cc1 -ast-dump filename.c
This diff adds a preprocessing phase to the front-end so that location information is composed during a visit, and explicit location information is used instead.
In the case of include files, we report the last known location before including the file.
The current file for a function is the file where it is defined. So if a function is entirely defined in a .h file, then the location information will consistently be about the .h file. If instead a function is defined in the source file being analyzed, and some AST nodes come from macro expansion, line information will refer to the original file.
The front-end tests reveal that the location information was incorrect in a few dot files.
Test Plan: arc unit, after having fixed the wrong location in the existing .dot files
Martino Luca
Dulma Churchill
Daiva Naudziuniene
Jules Villard
Sam Blackshear
Andrzej Kotulski
Dino Distefano
Cristiano Calcagno
Josh Berdine
jrm
Open Source Synchronization