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:
Minor stuff:
- GCCAst -> GCCAsm
- separate constants and mutable global state in cFrontend_config
- alphabetical ordering in cFrontend_config
Reviewed By: akotulski
Differential Revision: D3593858
fbshipit-source-id: 6f4d9c3
Summary:
No longer allow pointer types to be passed inside var_exp_typ. We used to accept both forms,
but it won't be possible any longer once shared_ptr becomes pointer type.
Reviewed By: dulmarod
Differential Revision: D3593003
fbshipit-source-id: a830914
Summary:
Python needs to know about these because it does different things depending on
whether these flags are passed or not.
Reviewed By: cristianoc
Differential Revision: D3593381
fbshipit-source-id: fe3194d
Summary:
When analyzing C model in C++, we were seeing some SKIP function triggered by generated constructors/operators= for C structs.
In C they weren't present, but in C++ compiler generates them for us. To avoid this (and future) problems
with models, translate all functions that are needed when computing the model
Reviewed By: dulmarod
Differential Revision: D3561873
fbshipit-source-id: f8ad2a0
Summary:
Backend has the same treatment for all Typ.Int types and so UnaryOperator
should be translated the same for all of them
Reviewed By: jvillard
Differential Revision: D3534277
fbshipit-source-id: 8569b65
Summary:
Call infer with `--unsafe-malloc` or set `unsafe-malloc: true,` in .inferconfig to
have infer assume that `malloc()` never returns null.
closes#389
Reviewed By: jberdine
Differential Revision: D3522169
fbshipit-source-id: 6b88a16
Summary:
To specify a new flag to run some test using our framework, one currently has
to add a new argument to the method that runs Infer and modify existing methods
to give a default value to that argument everywhere. With this diff, it's
possible to just specify extra arguments as parameters to
`createCInferCommand()` et al.
Reviewed By: martinoluca
Differential Revision: D3522161
fbshipit-source-id: 1499fc2
Summary:
This call was producing confusing false positives when deleted object was possible to be null.
Changing frontend to add that check is not trivial so I turned it off for now (we don't handle
destructors in other cases anyway)
Reviewed By: dulmarod
Differential Revision: D3509354
fbshipit-source-id: c23dc81
Summary:
When clang instantiates template function with argument pack, it will
give the same name to all parameters coming from the pack. To avoid
name collisions, always add index of argument's position to mangled part
of the variable.
Seemingly unrelated changes are to make existing tests pass (don't use
simple variable name where it matters)
Reviewed By: dulmarod
Differential Revision: D3503608
fbshipit-source-id: 794093a
Summary: Those functions have simple enough implementations for infer to understand them
Reviewed By: jvillard
Differential Revision: D3463084
fbshipit-source-id: f84160f
Summary:
Assume that std::vector::resize will always create nonempty vector. While this is clearly
wrong for resize(0), it removes many FPs for `resize(n)` calls, where value of `n` is unknown.
Without it, infer was thinking that `n` could be 0 and reported empty vector access.
Reviewed By: jvillard
Differential Revision: D3424355
fbshipit-source-id: cb476de
Summary:
Array types where the length is not statically known were represented
using fresh variables. This diff:
- Makes array type length optional, reducing the amount of work needed
for renaming, substitution, and normalization.
- Revises uses of array length so that the length component of a
Tarray type represents only the statically determined constant
length of an array type, and the length component of a Sizeof
expression represents the dynamically determined length of an array
value.
- Restricts the type of static lengths from a general expression
(Sil.exp) to an integer (Sil.Int.t), enforcing that static types are
constant. This in particular ensures that types contain no
variables, and so are invariant under operations such as renaming
and substitution.
- Removes the type substitution and renaming functions typ_sub,
typ_normalize, and typ_captured_ren. Now that array type lengths
are constant integers, all of these functions are the identity.
Reviewed By: cristianoc
Differential Revision: D3387343
fbshipit-source-id: b5db768
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:
Use the output of `clang -###` to drive which commands to run. Attach the plugin to all commands starting with `-cc1`.
Benefits:
- support for compiling multiple files in one clang command, eg `infer -- clang -c file1.c file2.c`
- support for compile commands that do not target a `.o` file, eg `infer -- clang -S hello.c`
- support for `-cc1` compile commands
- more generally, run all commands that clang would run, and attach plugin in all compilation cases
Reviewed By: martinoluca
Differential Revision: D3366912
fbshipit-source-id: 98d5e3b
Summary:
This removes some boilerplate and duplicated code and makes it easier to add
more tests.
Reviewed By: martinoluca, jeremydubreil
Differential Revision: D3365807
fbshipit-source-id: 9a2e0e5
Summary:
Now all code in tests is reachable by the analyzer which increases
test quality.
Reviewed By: dulmarod
Differential Revision: D3358591
fbshipit-source-id: d54877e
Summary:
When syntax highlighting the source excerpts that Infer prints on stdout, we
would crash if `pygments.lexers` did not find a suitable class given the name
of the source file. Instead, do not colorize when that's the case.
Reviewed By: martinoluca
Differential Revision: D3358115
fbshipit-source-id: ccb9b41
Summary:
Pass object by reference every time struct object is passed by value
in C++. Do it only for C++/objC++ where we have guarantee that the
object which is passed will be temporary one (created by copy constructor).
Reviewed By: jberdine
Differential Revision: D3346271
fbshipit-source-id: d3e5daa
Summary:
Make analyzer find out when null dereference comes from std::vector method.
If it does, it means that it's really empty vector access (due to the
way infer models std::vector)
Reviewed By: sblackshear
Differential Revision: D3327933
fbshipit-source-id: b9e11d6
Summary:
Turns out, analyzer was getting confused with complicated
model and it was reporting empty access in places it
shouldn't. Fixing backend is not trivial (tracing mode is the answer),
but the model can be simplified.
It introduces the problem that get() method doesn't return fresh value
every time, but we should be able to change backend later to deal with it.
Reviewed By: sblackshear
Differential Revision: D3328228
fbshipit-source-id: dddbaf8
Summary:
Part of the migration of .inferconfig-specific options into options accepted
both by .inferconfig and the CLI.
Reviewed By: jberdine
Differential Revision: D3304783
fbshipit-source-id: 4a7ee6f
Summary:
Create model of C++ std::vector to find occurrences when vector which might be empty is accessed. Do it by triggering null dereference every time empty vector access is performed.
Note: model will be used only when c++11 (or c++14) are used.
Reviewed By: sblackshear
Differential Revision: D3276203
fbshipit-source-id: 420a95a
Summary:
- [python] decode strings coming from `os.*` commands
- [python] decode strings coming from the command-line
- [python] encode a few remaining unicodes into strings
- [java] replace lex/yacc parser for javac verbose output by regex-based matching to handle unicode in paths
- [make] random fix of `make test` to have `make clean test` work
- [integration tests] add e2e build integration tests for utf8 in the PWD
Closes#76
Reviewed By: martinoluca
Differential Revision: D3240809
fb-gh-sync-id: 8c2e1ed
fbshipit-source-id: 8c2e1ed
Summary:
The philosophy of the tracing mode reporting is to not report the errors in a method if reaching this error does depend on information that can be false at call site. Typically with:
void foo(Object obj, int x) {
if (x == 3) {
obj.toString();
}
}
it may be that we always call `foo` with a non-null parameter or `x != 3`.
Thechnically, the reporting code matches the pairs of the form (precondition, error) and filtering out the cases where the precondtions was not imposing constraints on the calling context, and report the other cases. So the NPE could be reported in the following case:
void bar() {
foo(null, 3);
}
However, we were missing the case where there was anyway no way to call a method in a safe way, i.e. all the preconditions were of the form: (precondition, error), for example:
void baz(boolean b) {
if (b) {
foo(null, 3);
} else {
foo(null, 3);
}
}
In that case, the summary is of the form
PRE (1): b = false
POST: NullPointerException
PRE (2): b = true
POST: NullPointerException
In which case it is legit to report `NullPointerException` in `baz`.
Reviewed By: sblackshear, jberdine
Differential Revision: D3220501
fb-gh-sync-id: 7fb7d70
fbshipit-source-id: 7fb7d70
Summary: Example of dynamic dispatch with interfaces were already working. Adding some tests now so that we don't break this.
Reviewed By: sblackshear
Differential Revision: D3220360
fb-gh-sync-id: 11395dd
fbshipit-source-id: 11395dd
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: For performance critical sections of the code, this checker detects memory allocations or calls to methods annotated as expensive. However, such cases of memory allocations or expensive calls are acceptable is occuring in rare cases. This diff adds supports for the "unlikely" branch prediction method and does not track expensive calls in unlikely branches.
Reviewed By: sblackshear
Differential Revision: D3193473
fb-gh-sync-id: ea87e49
fbshipit-source-id: ea87e49
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:This diff translate cpp lambdas. For the moment it does not take care of
captured variables. Captured variables will come in the next diff.
Reviewed By: dulmarod
Differential Revision: D3114790
fb-gh-sync-id: bf36450
fbshipit-source-id: bf36450
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
The code:
DataInputStream in = new DataInputStream(new BufferedInputStream(new FileInputStream(file)));
creates a resource with `FileInputStream()` and wraps it twice as a field of `BufferedInputStream` and then as a field of `DataInputStream`. Then calling:
in.close();
needs to go down the wrappers hierachy: `DataInputStream.close()` -> `FilterInputStream.close()` which then calls `BufferedInputStream.close()` -> `FilterInputStream.close()` -> `FileInputStream.close()`.
Going down the wrapper was not working before because `FilterInputStream.close()` was only going further when the type of field `in` was `FileInputStream` wheras it should also continue when the type of the field is any subtype of `FilterInputStream`, e.g. `DataInputStream` and `BufferedInputStream` like in the test example. This diff fixes this last aspect.
Reviewed By: sblackshear
Differential Revision: D3174822
fb-gh-sync-id: 3adbb7e
fbshipit-source-id: 3adbb7e
Summary:public
Instead of translating code from headers blindly, translate only gets transitively referenced from source code.
It won't translate functions from system headers, but in the future we could do that as well
since most of them aren't used and it shouldn't add much overhead.
For now this functionality is hidden behind --cxx-experimental flag
Reviewed By: dulmarod
Differential Revision: D3163519
fb-gh-sync-id: 0c53b10
fbshipit-source-id: 0c53b10
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
Simplifies the way to configure cmake to run infer's fake compiler commands.
Instead of `CC=/path/to/infer/infer/lib/capture/clang cmake .`, which is what
is advised on #25 and is now outdated because the paths to infer's clang has
changed since, simply run `infer -- cmake .`. The only caveat is that infer
tries to analyze the end result, and prints "No issues found". This could be
fixed later.
Reviewed By: jeremydubreil
Differential Revision: D3093162
fb-gh-sync-id: 99df50a
fbshipit-source-id: 99df50a
Summary:public
In Python 2, `shutil.rmtree()`, `os.walk()`, `os.path.join()`, etc. are not
happy when the locale cannot decode the filenames they have to deal with.
Decrease the likelihood of this happening by making the file names generated by
infer ascii-only.
Also ignore character decoding errors optimistically when reading the json
report file.
Add tests that we are able to run the analysis and report the bug on a function
with a utf8 name, and that we are able to remove the previous results
directory.
closes#287
Reviewed By: cristianoc
Differential Revision: D3058858
fb-gh-sync-id: b88cd35
shipit-source-id: b88cd35
Summary:public
see the file for some example usage
Reviewed By: sblackshear, jeremydubreil
Differential Revision: D3058440
fb-gh-sync-id: c891dfe
shipit-source-id: c891dfe
Martino Luca
Ryan Rhee
Lázaro Clapp Jiménez Labora
Sam Blackshear
Jules Villard
Dulma Churchill
Daiva Naudziuniene
Andrzej Kotulski
Peter O'Hearn
Josh Berdine
Jeremy Dubreil
Dino Distefano
Cristiano Calcagno