Summary:
Before, the liveness pre-analysis would place extra instructions in the
CFG for either:
1. marking an `Ident.t` as dead, or
2. marking a `Pvar.t` as `= 0`
But we have no way of marking pvars dead without setting them to 0. This
is bad because setting pvars to 0 is not possible everywhere they are
dead. Indeed, we only do it when we haven't seen their address being
taken anyway. This prevents the following situation, recorded in our tests:
```
int address_taken() {
int** x;
int* y;
int i = 7;
y = &i;
x = &y;
// if we don't reason about taken addresses while adding nullify instructions,
// we'll add
// `nullify(y)` here and report a false NPE on the next line
return **x;
}
```
So we want to mark pvars as dead without nullifying them. This diff
extends the `Remove_temps` SIL instruction to accept pvars as well, and
so renames it to `ExitScope`.
Reviewed By: da319
Differential Revision: D13102953
fbshipit-source-id: aa7f03a52
Summary:
Useful to understand the changes in the pre-analysis, or to inspect the
CFG that checkers actually get.
This means that the pre-analysis always runs when we output the dotty,
but I don't really see a reason why not. In fact, we could probably
*always* store the CFGs as pre-analysed.
Reviewed By: mbouaziz
Differential Revision: D13102952
fbshipit-source-id: 89f3102ec
Summary:
When initialising a variable via semi-exotic means, the frontend loses
the information that the variable was initialised. For instance, it
translates:
```
struct Foo { int i; };
...
Foo s = {42};
```
as:
```
s.i := 42
```
This can be confusing for backends that need to know that `s` actually
got initialised, eg pulse.
The solution implemented here is to insert of dummy call to
`__variable_initiazition`:
```
__variable_initialization(&s);
s.i := 42;
```
Then checkers can recognise that this builtin function does what its
name says.
Reviewed By: mbouaziz
Differential Revision: D12887122
fbshipit-source-id: 6e7214438
Summary:
Change the license of the source code from BSD + PATENTS to MIT.
Change `checkCopyright` to reflect the new license and learn some new file
types.
Generated with:
```
git grep BSD | xargs -n 1 ./scripts/checkCopyright -i
```
Reviewed By: jeremydubreil, mbouaziz, jberdine
Differential Revision: D8071249
fbshipit-source-id: 97ca23a
Summary:
Previously, the type of `trans_result` contained a list of SIL expressions.
However, most of the time we expect to get exactly one, and getting a different
number is a soft(!) error, usually returning `-1`.
This splits `trans_result` into `control`, which contains the information
needed for temporary computation (hence when we don't necessarily know the
return value yet), and a new version of `trans_result` that includes `control`,
the previous `exps` list but replaced by a single `return` expression instead,
and a couple other values that made sense to move out of `control`. This allows
some flexibility in the frontend compared to enforcing exactly one return
expression always: if they are not known yet we stick to `control` instead (see
eg `compute_controls_to_parent`).
This creates more garbage temporary identifiers, however they do not show up in
the final cfg. Instead, we see that temporary IDs are now often not
consecutive...
The most painful complication is in the treatment of `DeclRefExpr`, which was
actually returning *two* expressions: the method name and the `this` object.
Now the method name is a separate (optional) field in `trans_result`.
Reviewed By: mbouaziz
Differential Revision: D7881088
fbshipit-source-id: 41ad3b5
Summary:
This is an attempt to make things more consistent, and maybe save some work
from the `Format` module in case flambda doesn't have our backs.
Reviewed By: jberdine
Differential Revision: D7775496
fbshipit-source-id: 59a6314
Summary:
Not sure what an "iCFG" is but the dotty is only about CFGs anyway.
Diff obtained by mass-`sed`.
Reviewed By: sblackshear
Differential Revision: D6324280
fbshipit-source-id: b7603bb
Summary:
The previous domain for SIOF was duplicating some work with the generic Trace
domain, and basically was a bit confused and confusing. A sink was a set of
global accesses, and a state contains a set of sinks. Then the checker has to
needlessly jump through hoops to normalize this set of sets of accesses into a
set of accesses.
The new domain has one sink = one access, as suggested by sblackshear. This simplifies
a few things, and makes the dedup logic much easier: just grab the first report
of the list of reports for a function.
We only report on the fake procedures generated to initialise a global, and the
filtering means that we keep only one report per global.
Reviewed By: sblackshear
Differential Revision: D5932138
fbshipit-source-id: acb7285
Summary: The prune nodes where translated as `prune (expr = false)` and `prune ( expr != false)`. This case is a bit tricky to deconstruct in HIL. This diff translates the prune instructions as just `prune !expr` for the true branch and `prune expr` for the false branch.
Reviewed By: dulmarod
Differential Revision: D5832147
fbshipit-source-id: 2c3502d
Summary: CFG nodes were not connected and some instructions ended up in wrong place. Fix those issues
Reviewed By: dulmarod
Differential Revision: D5406720
fbshipit-source-id: 2a70e1a
Summary:
Currently cfg nodes are written into dot files in whatever order they
appear in a hash table. This seems unnecessarily sensitive, so this
diff sorts the nodes.
Reviewed By: dulmarod
Differential Revision: D4232377
fbshipit-source-id: a907cc6
Summary:
Dealing with symbolic links in project root is tricky. To avoid it, always normalize all paths to sources with `realpath`.
Changes to tests are expected - infer started to resolve symbolic links which screws up with our testing mechanism.
Reviewed By: jberdine
Differential Revision: D4237587
fbshipit-source-id: fe1cb01
Summary:
Run all clang tests with project-root at `infer/tests`. I need it because we'll start resolving symbolic links
soon and some tests would lead outside of project root which means we'd start seeing absolute paths in recorded tests.
Diff that does same thing for java tests: D4233236
Reviewed By: jberdine
Differential Revision: D4233194
fbshipit-source-id: c261a2b
Summary: These are dangerous if you are trying to compare a type to a string, and they're also unsightly.
Reviewed By: jvillard
Differential Revision: D4189956
fbshipit-source-id: 14ce127
Summary:
Create dummy functions representing the initializers of global variables. This
is so we can implement checks in the backend that can look at the initializer
expressions of global variables. We try not to create these dummy functions
when the initializer is not present, although for some reason we sometimes end
up with empty initializers.
Also add source file info to global variables in the backend (Pvar.re).
Reviewed By: sblackshear
Differential Revision: D3780238
fbshipit-source-id: 2dca87e
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:public
Running clang-format on symbolic link replaced it with another file.
This is second diff out of two to fix that problem.
Reviewed By: jvillard
Differential Revision: D2960206
fb-gh-sync-id: 0f21a8f
shipit-source-id: 0f21a8f
Summary:public
Running clang-format on symbolic link replaced it with another file.
This is first diff out of two to fix that problem.
Reviewed By: jvillard
Differential Revision: D2960204
fb-gh-sync-id: aaa3231
shipit-source-id: aaa3231
Summary:
public
It turns out that C-like structs in C++ may have methods generated by clang (constructors for example).
If struct has a method, it needs to have Sil.Class type - make all CXXRecordDecls Sil.Class types by default.
Reviewed By: cristianoc
Differential Revision: D2895567
fb-gh-sync-id: 8eb18c3
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 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
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
New qual_name exporting logic in facebook-clang-plugins handles anonymous
classes in much better way. This allows us to simplify name generation for
classes that don't rely on a type of the object (which could be wrong for
methods from superclasses).
Reviewed By: dulmarod
Differential Revision: D2663810
fb-gh-sync-id: 08146b8
Summary: public
In C pre-increment/decrement returns rvalue, but in C++ it returns lvalue.
Make translation aware of the difference and treat these cases differently.
Reviewed By: ddino
Differential Revision: D2575136
fb-gh-sync-id: 952c095
Summary: public
Adds incomplete translation of constructor bodies. Treat constructors as
methods with something 'extra'.
We still don't translate initializer lists, just pass the information to cTrans
where it's ignored
Reviewed By: dulmarod
Differential Revision: D2550214
fb-gh-sync-id: 102c13a
Summary: public anonymous types have file:line in its name.
Since file is relative path, type name can have '/' in its name.
This is very fragile since we might create file wiht typename in its name (for example for methods).
Replacing '/' with '_' should make frontend more resilient to failure.
Translation of anonymous structs is still pretty fragile (due to relative path in its name),
but at least it doesn't crash frontend
Reviewed By: dulmarod
Differential Revision: D2559936
fb-gh-sync-id: 647fd7f
Summary: public
C++ assignment operation result is lvalue, while in C it was rvalue.
This leads to different AST produced by clang for then same code!
Use language information from clang (`-x` flag) to distinguish these cases.
More specifically, let's look at following code:
int r;
int f = (r = 3);
// type of (r = 3) expression:
// C/objC -> int rvalue
// C++/objC++ -> int lvalue
Existing code did extra dereference because it was rvalue in C and there was no cast afterwards
in C++ there will be extra LValueToRvalue cast when neccesary so we don't have to do extra dereference manually
Reference:
http://en.cppreference.com/w/c/language/value_category (search for 'assignment and compound assignment operators')
NOTE: AST output doesn't change when something is hidden behind `extern "C"`, so we should use global language information
Reviewed By: ddino
Differential Revision: D2549866
fb-gh-sync-id: b193b11
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
Jules Villard
Mehdi Bouaziz
Ezgi Çiçek
Jeremy Dubreil
Andrzej Kotulski
Josh Berdine
Sam Blackshear
Cristiano Calcagno
Dulma Churchill
Open Source Synchronization