Summary:
Another step in the refactoring of the starvation domain:
- Main purpose is to mediate access to the set of critical pairs in a summary through a fold function (`fold_critical_pairs_of_summary`) and not through direct field access to that set. This will allow eliding storage of critical pairs entirely and dynamically generating those when folding.
- Remove optional arguments as much as possible, as this led to unused arguments not being caught.
- Helper functions distributed more logically among modules.
Reviewed By: skcho
Differential Revision: D24275399
fbshipit-source-id: d23123a48
Summary: As part of a refactor, push `thread` from the enclosing type (`CriticalPairElement`) into `Event.t`.
Reviewed By: jvillard
Differential Revision: D24161709
fbshipit-source-id: bd812f3fd
Summary:
In ObjC, `NSObject.copy` returns the object returned by `copyWithZone:` on the given class. This method must be implemented if the class complies with `NSCopying` protocol. Since we don't have access to `NSObject`'s code, to follow calls into `copyWithZone:`, we replace such `copy` calls with calls to `copyWithZone:` when a) such a method exists in the class and b) the class conforms to `NSCopying` protocol.
This is done in the preanalysis because
- we need to know if there is a `copyWithZone:` method in the class.
- so that other analyses also benefit (as opposed to doing this in cost and inferbo models).
Note that `NSObject` doesn't itself conform to `NSCopying` but all its subclasses must confrom to the protocol and support the same behavior as above.
https://developer.apple.com/documentation/objectivec/nsobject/1418807-copy
Similarly for `mutableCopy` -> `mutableCopyWithZone:` for classes implementing `NSMutableCopying` protocol.
Reviewed By: skcho
Differential Revision: D24218102
fbshipit-source-id: 42900760e
Summary:
`NonBlocking` methods have starvation errors silenced (but not deadlock ones). This is implemented by summarising as usual and then filtering out such events when the summary is finalised, if the method is annotated as such.
It's better to not record the events in the first place.
Reviewed By: ezgicicek
Differential Revision: D24237465
fbshipit-source-id: 1b24a26f0
Summary: This will be needed in the next diff so that we can find all classes that conform to `NSCopying` protocol.
Reviewed By: skcho
Differential Revision: D24216549
fbshipit-source-id: 297b527a6
Summary:
- rename the checker "Uninitialized Variable" to "Uninitialized Value"
as this is the name of the issue type
- delete timestamp XML comment from the man pages to avoid future git
churn when updating the website
- counting is hard
Reviewed By: martintrojer
Differential Revision: D24219165
fbshipit-source-id: cf3057373
Summary:
`make new-website-version` would fail if run a second time, because it
assumes the version is brand new. Now, it will delete previous traces of
the new version, which allows one to easily update commits upgrading the
website instead of reverting the diff then running `make
new-website-version` again.
Reviewed By: martintrojer
Differential Revision: D24218463
fbshipit-source-id: 61f416677
Summary:
I wanted to change the default to "callgraph" but that created issues in
our tests, introducing flaky behaviours and even a failure due to trying
to run the pre-analysis multiple times (not 100% sure it was related).
Instead, document the various options and put the option in the analysis
manual so users can choose.
Reviewed By: martintrojer
Differential Revision: D24193751
fbshipit-source-id: 4b7c33a79
Summary:
To publish a new release, simply push a new tag named "v<version number>" to GitHub and this action will take care of creating a new release template and uploading the Linux (ubuntu-latest) and Mac binaries.
How it works:
- add a new "job" to the action that conditionally creates a new draft
release with pre-filled body. This runs only *once* (no matrix build),
unlike the main build job that runs on Linux + OSX (hence why we need
a separate job to avoid creating two copies of the release)
- the main build jobs depend on that release job so they have access to
the newly-created release if needed
- at the end of the main build job, create the release tarball for the
current architecture and add it to the release
Reviewed By: martintrojer
Differential Revision: D24193447
fbshipit-source-id: fd1bd447a
Summary:
This can be used to push a new version of the website when publishing a
new version of infer.
Reviewed By: martintrojer
Differential Revision: D24193377
fbshipit-source-id: d2f357558
Summary:
This is required for versioning to work correctly when creating a new
version of the website.
Reviewed By: martintrojer
Differential Revision: D24217125
fbshipit-source-id: 8f7a4bc44
Summary:
We forgot to add this when adding the new subcommand. These should
really be auto-generated but let's save that for another diff.
Reviewed By: martintrojer
Differential Revision: D24217111
fbshipit-source-id: 860860533
Summary:
Adding a Changelog file. Collected previous release changelogs and did
just a light bit of formatting on some of them.
Reviewed By: martintrojer
Differential Revision: D24191063
fbshipit-source-id: 692e2101c
Summary:
Use the cache action to cache the clang build instead of the
facebook-clang-plugins releases.
- the facebook-clang-plugins repo is no longer by infer so its clang
will soon be out of date
- much simpler to use the cache action
- build clang before setting opam: this is unfortunate but needed to
ensure there is enough space on GitHub's machines to build clang...
- install ninja on Linux + Mac machines for faster and less OOM-y builds
Reviewed By: martintrojer
Differential Revision: D24160876
fbshipit-source-id: 705e06151
Summary:
Fix the path to ~/.opam, and move *before* the OCaml setup so that OCaml
is cached too.
I'm not convinced it ever worked, as `${HOME}` is not interpreted by the
action.
Reviewed By: martintrojer
Differential Revision: D24160822
fbshipit-source-id: b1e0fcd59
Summary: Model it similar to `NSArray.initWithArray` as copying from the given dictionary elements. Removes a FP as expected.
Reviewed By: ngorogiannis
Differential Revision: D24136868
fbshipit-source-id: ed31c3c8f
Summary:
Make sure that we only download the clang tarballs needed to build clang
if:
1. clang is not installed
2. the user intends to build clang
Reviewed By: martintrojer
Differential Revision: D24137085
fbshipit-source-id: bfea1bf02
Summary:
This diff is a preparation to extend the polynomial domain. What I will do in the following diff is
to extend the polynomial domain to include a symbol representing "cost of function pointer call".
Then, the symbol will be instantiated to another polynomial cost at call sites.
The polynomial domain is defined as a sum of
* non negative insteger
* map from `bound` to polynomail domain
```
type poly = {const: NonNegativeInt.t; terms: poly M.t}
```
The problem is all `bound`s are adderessed as integer values, rather than polynomial, thus it
does not fit to include the symbol for the "cost of function pointer call".
In this diff, it introduces a `Key` module for the map `M`, the type of `Key.t` is the same to the previous key of `M`. The actual extension of the key type will be in the following diff.
Reviewed By: ezgicicek
Differential Revision: D23991401
fbshipit-source-id: cec64347f
Summary: Subsequent diff will push information down into `Event.t` so as preparation, turn all variant values into records.
Reviewed By: jvillard
Differential Revision: D24115201
fbshipit-source-id: d2126dd49
Summary:
`std::lock(x,y,z)` simultaneously acquires locks `x,y,z` in a deadlock free manner (essentially an unspecified fixed order).
Starvation currently deals with it by exploiting properties of the state domain. It's a map from locks to number of times the lock is held, so the count of many locks can be increased at the same time without recording any particular lock order.
In upcoming diffs the domain will be refactored into a tree of nested lock acquisitions (for other reasons) and that domain necessarily records lock order. The obvious way of doing this correctly is to allow `std::lock` as an atomic even (ie, without trying to break it into multiple acquisitions).
This diff does exactly that, by changing the `Event.LockAcquire` variant to take a list of locks.
Reviewed By: ezgicicek
Differential Revision: D24052304
fbshipit-source-id: 410c812d7
Summary:
This diff avoids some usages of functors when they are applied only once in `polynomials.ml`. While
functor is in general helpful to abstract data and make code cleaner, it makes us hard to understand
code, eg `merlin-locate` does not give an actual definition point of terms when it is from a module
parameter.
Reviewed By: ngorogiannis
Differential Revision: D23991164
fbshipit-source-id: c61289e84
Summary:
This diff extends inferbo's domain to include closure values. The goal of this extension is to
follow missing semantics where closures are handled as values, for example, a closure is assigned to
an object field, then it is got later to call.
Due to the bottom-up nature of the analyzer, sometimes we don't know which values are written in a
field, which is the same for the other non-closure values.
Reviewed By: ezgicicek
Differential Revision: D23932186
fbshipit-source-id: 4a575d0de
Summary: Dispatch & function call mechanism was so jumbled up together. Let's refactor it to be cleaner.
Reviewed By: skcho
Differential Revision: D24049889
fbshipit-source-id: 42a218016
Summary:
This makes sure we call `AbductiveDomain.summary_of_post` exactly once
per post-condition. Notice in particular in the diff:
- in Pulse.ml we remove a now-certified-useless "is_unsat_expensive"
call
- in PulseOperations.ml we add a previously-missing call to
`summary_of_post` (it's needed to remove local variables from the
symbolic state + normalize)
The price to pay is ugly type annotations and down-casting peppered in a
few places, in reasonable number.
Reviewed By: da319
Differential Revision: D24078564
fbshipit-source-id: 3102cacf0
Summary:
Take another page from the Incorrectness Logic book and refrain from reporting issues on paths unless we know for sure that this path will be taken.
Previously, we would report on paths that are merely *not impossible*. This goes very far in the other direction, so it's possible we'll want to go back to some sort of middle ground. Or maybe not. See the changes in the tests to get a sense of what we're missing.
Reviewed By: ezgicicek
Differential Revision: D24014719
fbshipit-source-id: d451faf02
Summary: We add a naive model for `forEach` idiom for Java's Iterable and Maps. The model is naive because it doesn't take the cost of the lambda into account. This will be fixed later.
Reviewed By: da319
Differential Revision: D23868203
fbshipit-source-id: 37d169c6f
Summary: Gradle produces a number of compilation units which are currently captured sequentially. This diff parallelizes this step.
Reviewed By: jvillard
Differential Revision: D23930978
fbshipit-source-id: d71c22ba3
Summary:
The problem: current enumerator semantics does not work on symbolic enumerator that is given as a
parameter.
Reviewed By: ezgicicek
Differential Revision: D24017059
fbshipit-source-id: 378e75bb0
Summary:
Change the hash stored to be the same as output of "--clang-hash". This change simplifies future work with building clang from alternative sources.
Also remove outdated src/checksums.txt
Reviewed By: jvillard
Differential Revision: D24016467
fbshipit-source-id: 3c24ff029
Summary:
This can be used by additional tooling for further analysis (e.g.
codemods, autofixes, etc).
Reviewed By: ngorogiannis
Differential Revision: D23987694
fbshipit-source-id: b9fa343ac
Summary: When using the database write server process, the parent has to wait for it to fully start up before continuing. Instead of dying after a fixed amount of time, never terminate (external timeouts can take care of that) and use exponential backoff to avoid spamming.
Reviewed By: jvillard
Differential Revision: D23989959
fbshipit-source-id: ff5232c14
Summary:
Testing procedure for java source parser
- we can run directly the parser without compiling and analysing the source file
- we add a test file
Reviewed By: ngorogiannis
Differential Revision: D23705199
fbshipit-source-id: 2103c1681
Summary: The code that parses `/proc/cpuinfo` expects IDs to be sequential. This is not always the case, so keep track of which IDs appear instead of keeping the maximum, when counting.
Reviewed By: jvillard
Differential Revision: D23987776
fbshipit-source-id: 98d267560
Summary:
This diff keeps closure parameters in closure-specializated procedures.
What the closure-specialization is doing is a propagation of concrete closures. For example, it
translates:
```
foo(block b) {
b();
}
goo() {
foo(^{...});
}
```
to
```
foo_new() {
(^{...})();
}
goo() {
foo_new();
}
```
However, if `foo` addresses `b` as a normal value like
```
foo(block b) {
block c = b;
}
```
this is translated to
```
foo_new() {
block c = b;
}
```
Note that the closure parameter of `foo` is removed, thus `b` becomes a free variable. Not good.
To avoid the situation, this diff keeps the closure parameters intact.
Reviewed By: da319
Differential Revision: D23905580
fbshipit-source-id: 014989fbf
Summary: Subtle false positives and negatives in Hil make Sil preferable. This diff gets rid of the CFG-emulation of Hil, while still using Hil expressions.
Reviewed By: da319
Differential Revision: D23815026
fbshipit-source-id: 731a6d299
Summary: Without curly braces, it is declaration of globals, not fields.
Reviewed By: ezgicicek
Differential Revision: D23866604
fbshipit-source-id: dd685c8d6
Summary:
The previous diffs recorded it for the case when the unvetted value is
dereferenced or otherwise used wrongly. This case finishes the work,
recording the needed signature for the remaining case (when the
offending third party has a non-nullable param with nullable passed
inside)
Reviewed By: ngorogiannis
Differential Revision: D23706679
fbshipit-source-id: e6f641223
Nikos Gorogiannis
Ezgi Çiçek
Mitya Lyubarskiy
Jules Villard
Sungkeun Cho
Martin Trojer
David Pichardie
Daiva Naudziuniene
Josh Berdine