Summary: This diff suppresses integer overflow issues in functions that includes "hash" in its name.
Reviewed By: jvillard
Differential Revision: D19942654
fbshipit-source-id: d86fa4f00
Summary:
Currently the free variables of the equality relation of a formula are
contained in the free variables of the rest of the formula, so Sh.fv
ignores them. Propagating equality facts across the star-or structure
of a formula, as necessary for quantifier elimination, breaks this
invariant. Some use cases, such as detecting which variables survive
applying a witness substitution, need to ignore the variables that
appear in the equality relation. This diff adds an argument to
conditionally ignore the variables in the equality relations.
Reviewed By: ngorogiannis
Differential Revision: D19580430
fbshipit-source-id: 2d417d89b
Summary: In line with changes to loom query in D19903057, let's adjust Infer's processing of the results.
Reviewed By: martintrojer
Differential Revision: D19902933
fbshipit-source-id: 200b3a03e
Summary: Add the number of cores used and the scheduler type to the environment info printed before running.
Reviewed By: ngorogiannis
Differential Revision: D19941243
fbshipit-source-id: 576e9f610
Summary: Not needed any more as infer's concurrency isn't controlled via make.
Reviewed By: jvillard
Differential Revision: D19905712
fbshipit-source-id: f97ef4421
Summary:
Some annotation processors / transormers might generate artificial fields. These
are not expicitly written by code writer, hence non actionable.
We distinct this fields heuristically: if they start with "$", they are
surely not user-written.
Reviewed By: ezgicicek
Differential Revision: D19947988
fbshipit-source-id: a0a15fc23
Summary:
Add let*/+ syntax to `result` types to simplify all the applications of
`>>=`, `>>|` that are followed by a binding (eg `>>= fun x -> ...`) in
pulse.
Reviewed By: skcho
Differential Revision: D19940728
fbshipit-source-id: 4df159029
Summary:
We can already tell that a summary cannot be applied by raising
`Contradiction`, so use this mechanism to stop applying a summary if the
number of formals doesn't match the number of actuals provided.
Previously we would return an option type and `None` in case of
mismatch, on top of the `raise Contradiction` mechanism (used for
aliasing and arithmetic contradictions).
This changes the behaviour of pulse in this case: before we would skip
over the function call, but now we stop the analysis.
Reviewed By: dulmarod
Differential Revision: D19940729
fbshipit-source-id: 6def40cd6
Summary: Once we identify a weakSelf variable that is being used in a Noescape block, we want to report only the first occurrence.
Reviewed By: skcho
Differential Revision: D19941502
fbshipit-source-id: 2b6d4648b
Summary: For each variable that we identify as a captured strong self, we want to report only the first occurrence.
Reviewed By: skcho
Differential Revision: D19940031
fbshipit-source-id: f38f642c9
Summary: When we discovered that a strongSelf var was not checked for null, we then report in each occurrence which is spammy. Now we report only the first occurrence. To achieve that, we store a `reported` flag in the domain that gets set to true after we report once, and we only report if it's false.
Reviewed By: jvillard
Differential Revision: D19877218
fbshipit-source-id: c44109ae9
Summary:
This adds `let*/+`, `and*/+` operators for Option. See [the manual](https://caml.inria.fr/pub/docs/manual-ocaml/manual046.html) for more information.
Example usage:
```
let foo =
let open IOption.Let_syntax in
let* a = get_optional () in
let* b = get_another_optional () in
return (a + b)
```
Reviewed By: jvillard
Differential Revision: D19880033
fbshipit-source-id: c7998b0c6
Summary:
Previous implementation supported only stringy params (strings and
stringified bools). Current one exposes a proper variant `Annot.t`,
with support for all possible param values in Java except
numbers (more on that below).
This change is required for implementing `Nullsafe(LOCAL)` as the
annotation used to specify nullsafe behaviour has a more complex
structure than what we've dealt with before.
**Why support for number values was not added**: supporting numbers
requires using `int64`. Unfortunately, adding another variant `Vnum
int64` to `Annot.t` causes a runtime failure on assert in
`MaximumSharing.ml:133`. It seems that it might be enough to flip
`fail_on_nonstring` from `true` to `false`, but since this would
require additional testing and is not required for my case, I'll leave
checking this to whoever needs to use numeric annot params in future.
Reviewed By: ezgicicek
Differential Revision: D19855923
fbshipit-source-id: 878e33856
Summary: No reason to use a set when an integer will suffice. This further reduces GC churn.
Reviewed By: fgasperij
Differential Revision: D19888300
fbshipit-source-id: 9fc8c73f5
Summary: Queues are implemented using a circular array, so should be less GC-heavy than continually allocating/freeing list nodes.
Reviewed By: jberdine, fgasperij
Differential Revision: D18504104
fbshipit-source-id: 93d29c253
Summary:
Building the call graph should be done only in the scheduler process after having forked all workers. This was achieved by a lazy init pattern, whereby the first time `next` was called, it would build the call graph, on the assumption that `next` is only ever called in the scheduler after forking.
D19769741 made this compulsory regardless the scheduler by passing a thunk to `ProcessPool` which is called to obtain the actual scheduler, on the right process and after the fork.
This means we don't need the custom lazy init logic any more. In addition, that set up used a DB query to overapproximate the number of procedures to analyse, because this was supposed to be provided *before* forking. Now this is also not needed, and on top of that we can provide the exact number after building the call graph.
Reviewed By: ezgicicek
Differential Revision: D19833974
fbshipit-source-id: 7f6d51d93
Summary:
In Inferbo, the bottom memory is introduced when a node is unreachable by pruning, i.e.
`[[e]] <= [0,0]` on `prune(e)`. This diff distinguishes whether `[[e]]` is `[0,0]` (unreachable)
or bottom (it could not evaluate `e` by some unknown reasons).
Reviewed By: ezgicicek
Differential Revision: D19902046
fbshipit-source-id: 7706017d6
Summary:
More newer = more better.
This flips the Not_found -> Not_found_s switch, and forbids a bunch more
polymorphic comparisons (mostly turned into `int` comparisons for
convenience). Earlier diffs prepare for this so this diff is only about
breaking changes in the API, of which there are only a few.
Reviewed By: jberdine
Differential Revision: D19861583
fbshipit-source-id: fe54ce8f0
Summary:
Core v13 APIs stopped raising `Not_found` and instead raise
`Not_found_s`, which wreaks havoc in our codebase. Carefully inspect
each `Not_found` and add `Not_found_s` where needed (that way it's
compatible with both Core v12 and v13 for now).
Reviewed By: jberdine
Differential Revision: D19861585
fbshipit-source-id: 9a5361ae9
Summary:
The big one:
- stop using polymorphic `<>`, `<`, `>`, ..
- add `<>` to `PolyVariantEqual` escape hatch now that `<>` is as taboo as `=`
- Interestingly, there were a lot of uses of `Z.(x < y)`, which although
they seem to use `Z.lt` actually used polymorphic comparison. The actual
comparison infix operators of `Z` are cleverly hidden in `Z.Compare`
instead, which makes them impractical to use...
Reviewed By: jberdine
Differential Revision: D19861584
fbshipit-source-id: 5dce08ad9
Summary:
Polymorphic compare is bad, 'mkay? I also tried changing this to do what
the comment says the function does (keep the first term instead of the
"largest" term) but that resulted in test changes so I kept the original
behaviour instead.
Reviewed By: jberdine
Differential Revision: D19861587
fbshipit-source-id: d57fd4a02
Summary:
That trick briefly worked when it was introduced but I think a change in
merlin broke it again as I see "sexp_list", "sexp_option" in the types
shown by merlin again. Disgusting!
Remove useless hack. Also referring to types such as `sexp_list` is
deprecated in core v13 for some reason.
Reviewed By: jberdine
Differential Revision: D19861586
fbshipit-source-id: 1c4c3af13
Summary:
This adds a violation of baos.topl found in github/seata/seata. However,
it is not a bug (see comment in commit).
Reviewed By: ngorogiannis
Differential Revision: D19518641
fbshipit-source-id: e219245ee
Summary:
Since Javalib 3.2, a new feature allows to rewrite
methods that contain (some specific form of) closures. Infer
now uses it. When loading each class we rewrite them and
new classes generated by Javalib to implements closures
(i.e. Java interfaces)<
Reviewed By: ngorogiannis
Differential Revision: D19389227
fbshipit-source-id: 245dd4404
Summary:
When finding a proper constructor for `std::make_shared`, the given parameter types are sometimes
slightly different, e.g., const int vs int. This diff loosens the condition of the types on finding
constructors.
Reviewed By: ngorogiannis
Differential Revision: D19743198
fbshipit-source-id: f90213109
Summary:
Javalib 3.2.1. ans Sawja 1.5.8 are out
and infer can use them now. Theses releases contain fixes
about the rewriting of java lambdas (upcoming diff).
Reviewed By: ngorogiannis
Differential Revision: D19811102
fbshipit-source-id: 0b6bc01d1
Summary: Instrument the ProcLocker to get the aggregated systime that is used by `lock` and `unlocks`.
Reviewed By: ngorogiannis
Differential Revision: D19814554
fbshipit-source-id: 5fd928b9c
Summary:
This was introduced in D19770219, and skcho caught it, but for some
reason I seemed to have a blind spot and confused the semantics of `instanceof`.
Reviewed By: skcho
Differential Revision: D19813031
fbshipit-source-id: d939b981b
Summary:
This diff fixes the clang translation for switch statement. It assumed that `default:` comes always
at last, which introduced some unreachable nodes inadvertently, e.g. when `default:` comes at first.
Reviewed By: dulmarod
Differential Revision: D19793138
fbshipit-source-id: 1e8b52c0d
Summary: After looking at some reports with blocks inside blocks, it seemed more obvious that adding which method we are talking about makes more clear which block we are talking about.
Reviewed By: mityal
Differential Revision: D19789285
fbshipit-source-id: 20e0e6804
Summary:
The domain has a notion of lock state (taken/not taken) and any access occurring will remember the current lock state.
Methods in Java can be `synchronized` meaning the lock is taken automatically at method start and release on return.
Currently, instead of starting analysis of a `synchronized` method with an initial lock state of "lock taken", the summary would be computed as if there is no lock, and then a caller would peek at whether the callee is `synchronized` and change the callee summary accordingly before taking it into account. Also, when an access happens (not a call) the analysis always consults the pdesc of the current method to check whether the method is `synchronized`.
Do things the right way: if method is `synchronized`, start with the lock taken. When the method exits, the lock-state postcondition is computed by releasing once the lock.
This is a behaviour-preserving change.
Reviewed By: mityal
Differential Revision: D19742559
fbshipit-source-id: 1d0fce3f6
Summary: This diff makes the taint analysis in Inferbo inter-procedural: adding symbolic taint values and substitute them at function call statements.
Reviewed By: ezgicicek
Differential Revision: D19411022
fbshipit-source-id: 4ff9a590a
Summary:
We already warn about lack of nullable annotations in `equals()`, and even have a specialized error message for that.
But lack of an annotation is not as severe as direct dereference: the
latter is a plain bug which is also a time bomb: it will lead to an NPE not immediately.
This is widespread enough to be reported separately.
Reviewed By: dulmarod
Differential Revision: D19719598
fbshipit-source-id: a535d43ea
Summary:
Since we fixed a bug in implementation of FalseOnNull (see stack below),
we can finally ship this change.
Side note: this change is essential for the follow up diff (which adds extra check
for user-defined implementations of equal()), without it the follow
up change would introduce a lot of false positives.
Reviewed By: ngorogiannis
Differential Revision: D19771057
fbshipit-source-id: 7d7cf1ef7
Summary:
If we managed to whitelist a function as TrueOnNull, we should teach
nullsafe the nullability of its arguments, otherwise it will ask not to
pass null here.
This fixes a silly FP warning, see the test.
Reviewed By: dulmarod
Differential Revision: D19770341
fbshipit-source-id: 0f861fae1
Summary:
Yay, the previous refactoring finally makes it possible to do some actual
changes to the code in `TypeCheck.ml`!
Changes in this diff:
1. Fixes the bug: TrueOnNull and FalseOnNull were working only for
static methods. Surpsingly nobody noticed that. It is because the first
argument for non-static method was `this`.
2. Behavior change: TrueOnNull/FalseOnNull were not working correctly
where there are several argumens. See the task attached for the example
of the legit usecase. Now the behavior is the following: if there are
several Nullable arguments infer nullability for all of them.
Reviewed By: skcho
Differential Revision: D19770219
fbshipit-source-id: 7dffe42cd
Summary:
This diff proceeds clean up the mess in TypeState.ml
This refactoring unblocks the change in the logic for true on null and
allows to fix a bug, see follow up diffs.
The current code is trying to do two things at once: processing boolean
results (which normally need manipulations with the argument of a
function + additional logic for containsKey), and comparisons witn null
(which requires manipulation with return value in typestate plus check
for redundancy).
All this was done in sort of generic fashion, which, among other, lead
to weird situations for edge cases, e.g. in processing containsKey we used to override nullability
already inferred as a non-null twice with losing information about
original nullability; which made logs weird).
Reviewed By: ngorogiannis
Differential Revision: D19768795
fbshipit-source-id: c928e2cff
Summary: The main job the schedulers do is building their work queues. That's being performed before the workers are forked which means they get copied into all of them. These changes push the initialization of the schedulers just after the forking takes place.
Reviewed By: ngorogiannis
Differential Revision: D19769741
fbshipit-source-id: 0b20ddd5c
Sungkeun Cho
Josh Berdine
Ezgi Çiçek
Fernando Gasperi Jabalera
Nikos Gorogiannis
Mitya Lyubarskiy
Jules Villard
Dulma Churchill
Artem Pianykh
Radu Grigore
David Pichardie