Summary: Same as MustBeValid: we want to report the first error on the path.
Reviewed By: skcho
Differential Revision: D28674724
fbshipit-source-id: a2ac04b5b
Summary:
Add a new `PathContext.t` component to the abstract state. For now it
tracks only the current "timestamp" of symbolic execution inside the
procedure, i.e. which step of symbolic execution we are in (bumped by 1
each time we've executed one instruction). In the future this will also
hold, eg, which conditionals we've been through on the path (for
reporting traces with that information).
Most of the diff is about propagating the path context through many of
the APIs.
We use timestamps only in `MustBeValid` attributes to report the first
incorrect access in a function call for now.
Reviewed By: skcho
Differential Revision: D28674726
fbshipit-source-id: 2cd825e73
Summary:
It's better to remember the first reason why an address must be valid,
etc.
Reviewed By: skcho
Differential Revision: D28674729
fbshipit-source-id: 3b69de7ef
Summary:
Spoiler alert: we don't. The next diffs fix that.
When there are several invalid accesses to report at a function call
instruction, we want to report the first one to occur within the
function. This is to avoid confusing reports where pulse reports, eg, a
null dereference for a pointer at a point where it's already been
dereferenced before in the same function.
Reviewed By: skcho
Differential Revision: D28674730
fbshipit-source-id: acb029e4b
Summary:
That was just broken before, but apparently nothing cared. It's needed
for the next diffs.
Reviewed By: skcho
Differential Revision: D28674731
fbshipit-source-id: 2f080238b
Summary:
Each Erlang function now has a Procdesc in `results.db`. The
ProcAttributes record if a function is exported or not by using the
access Public or Private, respectively.
This adds also `ErlangTypeName`. We use a fixed set of "type names" for
the different types of values in Erlang (i.e., for Erlang's "dynamic types").
Reviewed By: jvillard
Differential Revision: D28385954
fbshipit-source-id: f8278505a
Summary:
Update the website to include nil related issues for objective-c
Also fixed a typo + syntax highlighting
Reviewed By: jvillard
Differential Revision: D28638621
fbshipit-source-id: 148f2dd3f
Summary:
This is needed for the next diff. It was a bit annoying to report leaks
in two different places, now it's just in one.
Reviewed By: skcho
Differential Revision: D28576768
fbshipit-source-id: 4f23b43cb
Summary:
Add an option for realloc and fiddle with the other options' help for
consistency.
Moved the memory leak test to memory_leak.c and added more.
Moved the place where we take the options into account closer to their
corresponding models to defend a bit against modifying one without
modifying the other.
Reviewed By: da319
Differential Revision: D28543340
fbshipit-source-id: 75894d06d
Summary:
Let's model all the dynamic memory management functions as they all work
together and are important for a lot of C projects.
Reviewed By: ezgicicek
Differential Revision: D28543008
fbshipit-source-id: f130e1ab6
Summary: We even have matchers in PulseModels that can do the same thing.
Reviewed By: skcho
Differential Revision: D28540278
fbshipit-source-id: 4bfd8a13e
Summary:
It's unclear whether this can happen but it doesn't cost much to do a
last check before reporting an error to the user.
Reviewed By: skcho
Differential Revision: D28382670
fbshipit-source-id: e23f07ebd
Summary:
This fixes a memory leak false positive. When collecting unreachable
values we should be careful to take the equality relation into account.
Equal values are normally canonicalised but only with respect to "known"
equalities. This makes sure variables that are live thanks to the
"pruned" equalities are not discarded from the state.
Reviewed By: skcho
Differential Revision: D28382642
fbshipit-source-id: 2b898d754
Summary:
This makes reports more readable: they were all at the end of functions,
currently.
This is actually quite tricky to do as it involves detecting which
locations are unreachable.
Some of this logic can/should probably be shared with
`AbductiveDomain.discard_unreachable` but at the moment that's not the
case.
Reviewed By: skcho
Differential Revision: D28382590
fbshipit-source-id: bd4239a0c
Summary:
Hi all,
This is just a small fix tries to resolve the leaked type lost issue. It was excluded from previous CIL race condition PR due to irrelevance.
Thanks!
Pull Request resolved: https://github.com/facebook/infer/pull/1446
Reviewed By: skcho
Differential Revision: D28566755
Pulled By: ngorogiannis
fbshipit-source-id: 1c9938d9c
Summary: For Remodel-generated class (https://github.com/facebook/remodel), their properties are stored/loaded at internal fields named "_<property name>". This diff prepends "_" to property names when writing field info to the type environment when the field is of Remodel-generated class.
Reviewed By: ezgicicek
Differential Revision: D28541495
fbshipit-source-id: d0a1e5a4f
Summary: An anonymous class name includes an index number, for example `AnonymousClass$2` represents that it is the 2nd anonymous class implemented in the `AnonymousClass` class. Problem is that when we insert a new anonymous class, all index of anonymous class names below increase by one, which introduces incorrect comparison on reportdiff.
Reviewed By: ezgicicek
Differential Revision: D28568753
fbshipit-source-id: 2a6c576eb
Summary: Objective-C dispatch methods are not specialized, but have a special case during symbolic execution in biabduction. Reuse the same approach for Pulse: retrieve the given block name and its arguments and call it.
Reviewed By: skcho
Differential Revision: D28550468
fbshipit-source-id: 5017bb71e
Summary: Move Objective-C dispatch models to IR to be able to reuse the same approach in Pulse.
Reviewed By: skcho
Differential Revision: D28550389
fbshipit-source-id: 163826647
Summary:
This diff adds fields for ObjC properties to the type environment. For example,
```
property type fieldname;
```
when the property name is "fieldname", this diff adds a struct field of the same name.
The missing type information were problematic in inferbo, since its semantics depend on types.
Reviewed By: ezgicicek
Differential Revision: D28421998
fbshipit-source-id: e24059846
Summary:
Most/all of the time we expect the history of the value to faithfully
trace how it got allocated. That history was then added as a prefix of
the trace leading to the same place, leading to duplicate information in
the report trace.
We may need to do the same for other bug types.
Reviewed By: ezgicicek
Differential Revision: D28536891
fbshipit-source-id: a83a2d038
Summary: Showcase the trace duplication, fixed in a further diff.
Reviewed By: ezgicicek
Differential Revision: D28536889
fbshipit-source-id: f23636368
Summary:
Make it more obvious why we don't add an Allocated attribute in these
models.
Reviewed By: ezgicicek
Differential Revision: D28536892
fbshipit-source-id: 643539ae6
Summary:
More straightforward (and better asymptotic complexity, not that it
matters) that way. Also log when a leak is found in the debug html.
Reviewed By: ezgicicek
Differential Revision: D28536443
fbshipit-source-id: 08c329100
Summary:
The returned options were never used or only used in cases when they can
only be `None`, as far as I can tell.
Reviewed By: da319
Differential Revision: D28536428
fbshipit-source-id: c16ed4698
Summary:
As explained in the code comment, these reports are generally
non-actionable at best and false positives at worst:
skip reporting for constant dereference (eg null dereference) if the source of the null value is
not on the path of the access, otherwise the report will probably be too confusing: the actual
source of the null value can be obscured as any value equal to 0 (or the constant) can be
selected as the candidate for the trace, even if it has nothing to do with the error besides
being equal to the value being dereferenced
Reviewed By: da319
Differential Revision: D28350193
fbshipit-source-id: 0cd76d252
Summary:
Turns out the mistake was pretty simple: we just forgot to keep the
history of the return value in the callee and add it to the caller's.
Reviewed By: skcho
Differential Revision: D28385941
fbshipit-source-id: 40fe09c99
Summary:
This PR adds race condition detection support on CIL backed languages, such as .NET platform languages.
We will add unit tests later since we are still fine tunning Infer# translation module.
Pull Request resolved: https://github.com/facebook/infer/pull/1443
Reviewed By: jvillard
Differential Revision: D28505195
Pulled By: ngorogiannis
fbshipit-source-id: f263f6ba6
Summary:
This diff fixes inefficient config impact data checking.
Problem: When writing `config-impact-report.json`, it checks if a procedure (`f`) is included in the config impact data set as follows. `cut_parameter` is a function that removes parameters from ObjC method names.
```
ConfigProcnameSet.exists (fun g -> cut_parameter f = cut_parameter g) config_data
```
However, this was very inefficient because it must have iterated all members in the set always. This diff changes it to call `Set.mem` by preparing revised config impact data set (`config_data'`) in which parameters were cut in advance:
```
ConfigProcnameSet.mem (cut_parameter f) config_data'
```
Reviewed By: ezgicicek
Differential Revision: D28506113
fbshipit-source-id: 434d1f083
Summary: Similar as for NSDictionary, nil issues for array literals are caught because of the additional load instruction in the frontend, and we leave modelling arrayWithObjects:count: for later.
Reviewed By: jvillard
Differential Revision: D28442767
fbshipit-source-id: a2f0d4dbf
Summary:
Follow similar approach as in the translation of dictionary literal to insert load instruction to catch nil insertion into collection issues. The missing load instruction was causing false negatives in biabduction. This will also help Pulse to catch nil insertion into collection issues for array literals.
Facebook
Reviewed By: skcho
Differential Revision: D28442642
fbshipit-source-id: b530ac21b
Summary: The counter that accumulates the number of modified source files was logged before it is computed, leading to always zero results.
Reviewed By: jvillard
Differential Revision: D28505378
fbshipit-source-id: 833fb6072
Summary: Similar as for other collections we leave modelling setWithObjects:count: and initWithObjects:count for later.
Reviewed By: skcho
Differential Revision: D28473361
fbshipit-source-id: 4bf57035a
Summary:
In Buck/Java the global type environments of each buck target captured need to be merged. So do the capture DBs. These two tasks can be done concurrently, as both have a computation and an I/O component, and interleaving them should improve perf.
Indeed, profiling the merge process with `offcputime.py` and `cpudist.py` (BPF tools) showed a significant amount of off-cpu time in tests (>40%) as well as a distribution of timings for off-cpu intervals that agrees with IO on a fast medium (ssd).
This diff forks a process to merge the type environments while doing the DB merge as normal. Initial results show an almost 2x improvement.
Reviewed By: skcho
Differential Revision: D28438808
fbshipit-source-id: 89c96f25b
Jules Villard
Radu Grigore
Daiva Naudziuniene
Xiaoyu Liu
Sungkeun Cho
Loc Le
Martin Trojer
Ezgi Çiçek
Nikos Gorogiannis
Artem Pianykh