Summary: I should have listened to skcho on D28002725 (7207e05682). Needed for the next diff.
Reviewed By: skcho
Differential Revision: D28121203
fbshipit-source-id: 96c01b141
Summary:
On big analyses, we routinely observe analyses nesting levels of the
order of ~5k (i.e. 5342> on the interactive taskar). This call depth
is meaningless given the size of the codebase we analyze.
Instead, I believe this is due to the fact that Ondemand does not
properly snapshot/restore the nesting level when starting an analysis,
and more importantly when abandoning parts of it. This commit
fixes that oversight.
Pull Request resolved: https://github.com/facebook/infer/pull/1431
Reviewed By: ngorogiannis
Differential Revision: D28118297
Pulled By: jvillard
fbshipit-source-id: 0faf28f84
Summary:
the mapping for computing 'leq' relation in isomorphic graphs was sometimes mixed with opposite mappings due to typos in the code.
Please see [CONTRIBUTING.md](./CONTRIBUTING.md) for how to set up your development environment and run tests.
Pull Request resolved: https://github.com/facebook/infer/pull/1424
Reviewed By: ngorogiannis
Differential Revision: D28118324
Pulled By: jvillard
fbshipit-source-id: 56e813bd1
Summary: Some code generation mechanisms produce code under `buck-out/gen/<hash>`. This diff allows handling such sources by producing deterministic capture DBs via omitting the hash.
Reviewed By: skcho
Differential Revision: D27501193
fbshipit-source-id: 3c2ac92a9
Summary:
As explained in the previous diff: when the access trace goes through
the invalidation step there is no need to print the invalidation trace
at all.
Note: only a few sources of invalidation are handled at the moment. The
following diffs gradually fix the other sources of invalidation.
Reviewed By: skcho
Differential Revision: D28098335
fbshipit-source-id: 5a5e6481e
Summary:
The eventual goal is to stop having separate sections of the trace
("invalidation part" + "access part") when the "access part" already
goes through the invalidation step. For this, it needs to record when a
value is made invalid along the path.
This is also important for assignements to NULL/0/nullptr/nil: right now
the way we record that 0 is not a valid address is via an attribute
attached to the abstract value that corresponds to 0. This makes traces
inconsistent sometimes: 0 can appear in many places in the same function
and we won't necessarily pick the correct one. In other words, attaching
traces to *values* is fragile, as the same value can be produced in many
ways. On the other hand, histories are stored at the point of access, eg
x->f, so have a much better chance of being correct. See added test:
right now its traces is completely wrong and makes the 0 in `if
(utf16StringLen == 0)` the source of the NULL value instead of the
return of `malloc()`!
This diff makes the traces slightly more verbose for now but this is
fixed in a following diff as the traces that got longer are those that
don't actually need an "invalidation" trace.
Reviewed By: skcho
Differential Revision: D28098337
fbshipit-source-id: e17929259
Summary:
See added test: pulse sometimes insisted that an issue was latent even
though the condition that made it latent could not be influenced (hence
could the issue could never become manifest) by callers because it was
unrelated to the pre, i.e. it came from a mutation inside the function.
In these cases, we want to report the issue straight away instead of
keeping it latent.
Reviewed By: skcho
Differential Revision: D28002725
fbshipit-source-id: ce9e6f190
Summary:
This diff adds semantics for temporary boolean variables to keep config values.
* It extended value domain to have `TempBool` that is basically a pair of `ConfigChecks.t`; one is a
set of config values checked when the temporary variable is true, and the other is that when the
temporary variable is false.
* It assigns the `TempBool` value when `temp=1` or `temp=0`.
* It uses the `TempBool` value when pruning condition expression.
For example, when there is an `if` statement of
```
return (config && b);
```
it is translated in SIL,
```
if (config) {
if (b) {
temp = 1; // (1)
} else {
temp = 0; // (2)
}
} else {
temp = 0; // (3)
}
return temp;
```
then we can say
* When `temp` is true, i.e. at (1), it is gated by `config`
* When `temp` is false, i.e. at (2) and (3), we are not sure about the the gatedness; at (2) it is gated by `config` but at (3) it is gated by `!config`.
So, we record such information as a `TempBool.t` value.
Next, when we use the return value at its caller,
```
if (ret) {
// then branch
} else {
// else branch
}
```
We can say "then branch" part is gated by `config`, but we are not sure if "else branch" part is gated, by using the `TempBool.t` value of `ret`.
Reviewed By: ezgicicek
Differential Revision: D28056490
fbshipit-source-id: e90d8afd3
Summary: Small refactor as this function belongs more in Pvar than Var.
Reviewed By: skcho
Differential Revision: D28091618
fbshipit-source-id: 259bd82d5
Summary: To be fair that doesn't seem to matter at all, with no test affected.
Reviewed By: ngorogiannis
Differential Revision: D28091608
fbshipit-source-id: 172bd2ff1
Summary:
Just moving stuff around.
This is possibly useful for making Pvar depend on ProcAttributes for
other things, eg checking if a pvar is captured by a procedure (which
would be awkward to have in the API of ProcAttributes and not Pvar).
Overall it forced me to move a few other things around in a way that I
feel makes more sense anyway.
Reviewed By: skcho
Differential Revision: D28091497
fbshipit-source-id: 367a1f17c
Summary:
Before returning a summary, restore formals to their initial values.
This gets rid of a false latent because the value in the path condition
is now garbage-collected.
Added a test for the tricky case of structs passed as values.
Reviewed By: skcho
Differential Revision: D28001229
fbshipit-source-id: 23dda5b43
Summary:
This diff adds semantics for long-typed config values.
* It extended branch types to keep condition expressions passed,
* then used it to in the prune semantics.
Reviewed By: ezgicicek
Differential Revision: D28055936
fbshipit-source-id: 0d12930cf
Summary:
This diff introduces [ISys.file_exists] that is similar to [Sys.file_exists_exn], but returns
[false] when the result is known, instead of raising an exception.
Reviewed By: jvillard
Differential Revision: D28059863
fbshipit-source-id: d54851cfb
Summary: This diff adds an abstract semantics for returning config values at function calls.
Reviewed By: ezgicicek
Differential Revision: D28055544
fbshipit-source-id: 5fe51c538
Summary:
This looks a bit better as it makes it easier to ignore parts of the
arguments in models, which happens all the time. Also easier to add more
to the record in the future, which is the real reason.
Reviewed By: skcho
Differential Revision: D27997695
fbshipit-source-id: a7c680025
Summary:
This diff avoids dereference of C struct, in its frontend and its semantics of Pulse. In SIL, C
struct is not first-class value, thus dereferencing on it does not make sense.
Reviewed By: ezgicicek
Differential Revision: D27953258
fbshipit-source-id: 348d56338
Summary: This diff copies each field values inside setter/getter of ObjC.
Reviewed By: ezgicicek
Differential Revision: D27940521
fbshipit-source-id: 9977cae75
Summary:
This diff does refactoring for the following diff.
* Define Mangled.return_param and Mangled.is_return_param and use it instead of
Ident.name_return_param.
* Share common code from objc_setter and objc_getter
* Move struct_copy to CStructUtils.ml
Reviewed By: da319
Differential Revision: D27940125
fbshipit-source-id: 84eb3109b
Summary:
Looking at the recent silent analysis results, it seems that we report many direct unknown library calls (often cheap)... However, if these were called inside some other callee we wouldn't report them because their costs would be assumed to be constant by the cost analysis.
This is a bit awkward. We should either report all unknown calls or suppress them altogether.
Since we have too many reports per day and not a good way to determine whether an unknown library call is cheap/expensive, let's take option 2.
Then, we would be only relying on two things to determine whether to report/not:
- instantiated cost's degree > 1
- explicitly known to be expensive (i.e. modeled in ConfigImpact, like string append)
Reviewed By: skcho
Differential Revision: D27909003
fbshipit-source-id: 0391d226d
Summary:
The configuration options for the analysis are used only/principally
in Control, they do not belong in the interface of domains. Also, the
definition of the used_globals type for the results of the used
globals pre-analysis belongs to the Domain_used_globals module.
Reviewed By: jvillard
Differential Revision: D27828752
fbshipit-source-id: e42de74e0
Summary:
Just reorder definitions to clarify that as_inlined_location is not
used in the rest of Stack.
Reviewed By: jvillard
Differential Revision: D27828755
fbshipit-source-id: 1436f1e6d
Summary: Preparation for allowing a choice among several scheduler strategies.
Reviewed By: jvillard
Differential Revision: D27828759
fbshipit-source-id: 63d6ec203
Summary:
The priority queue does not crucially depend on the type of
elements. This diff makes it parametric.
Reviewed By: jvillard
Differential Revision: D27828756
fbshipit-source-id: a7bfc4ee5
Summary:
When domain join operations are total, the control scheduler does not
need to handle the case where joining states is undefined. This leads
to some simplification, and in particular removed the need to expose a
remove operation for the scheduling queue.
Reviewed By: jvillard
Differential Revision: D27828761
fbshipit-source-id: b8cdd2eb6
Summary:
The only domain with a partial join is the lifting of a predicate
domain to a relation one, where the entry states are required to be
equal. This situation now indicates a programming error in the
analysis, rather than something that the domain should be responsible
for. Therefore this diff changes that check to an assertion and
simplifies the remaining join operations which are all total.
Reviewed By: jvillard
Differential Revision: D27828763
fbshipit-source-id: ec52ff741
Summary:
llvm-config can return the lib dir without manual manipulation using
an assumed relative path from llvm-config to the lib dir. Also fix
missing semicolon to appease shell variable semantics.
Reviewed By: jvillard
Differential Revision: D27828757
fbshipit-source-id: 5080c8671
Summary:
Mainly code cleanup. The only potential functional change is to
eliminate the test that a function returning with an empty stack is an
entry point, which should always hold anyhow.
Reviewed By: jvillard
Differential Revision: D27828749
fbshipit-source-id: 25124a568
Summary:
Mainly code simplification. The only functional change is to move the
test that a candidate entry-point function has no formals earlier,
with the result of avoiding failure if a function with an entry-point
name but with formals is encountered before an entry-point function
with no formals.
Reviewed By: jvillard
Differential Revision: D27828751
fbshipit-source-id: d5a832952
Summary:
There used to be a bug where recursive calls were not correctly
bounded. This is no longer needed, and this diff removes the
workaround that was added in D15577134 (881a4d10af) / 4cd3b62.
Reviewed By: jvillard
Differential Revision: D27828764
fbshipit-source-id: 80265a588
Summary:
Specifying that sledge should stop execution when the first report is
made via e.g. `sledge l a -t Stop` currently only works when debug
tracing is enabled. This diff fixes this so that `-t Stop` also works
for the release build.
Reviewed By: jvillard
Differential Revision: D27828753
fbshipit-source-id: dd7511ff1
Summary:
Currently all alarms are reported as "Invalid memory access", which is
not accurate for `abort` and hence assertion violations. This diff
adds an explicit type for alarms which distinguishes these two
cases. Further refinement is left for later.
Reviewed By: jvillard
Differential Revision: D27828754
fbshipit-source-id: 9c33f3c86
Summary:
This is mostly useful to avoid duplicating error states, which are
propagated unchanged through both branches of, say, conditionals, and
can end up duplicated if the join is not careful:
```
{[Abort(Error 1), Abort(Error 2), Continue σ']}
if (..) { .. } else { .. }
{JOIN([Abort(Error 1), Abort(Error 2), Continue σ_then],
[Abort(Error 1), Abort(Error 2), Continue σ_else])}
{[Abort(Error 1), Abort(Error 2), Continue σ_then, Continue σ_else]}
```
Whereas before this diff we got
```
{[Abort(Error 1), Abort(Error 2), Continue σ_then, Abort(Error 1), Abort(Error 2), Continue σ_else]}
```
Detect states that do not change simply using `phys_equal` as they
should literally not change. Refactor the code to be able to re-use the
same logic in the stronger join used in widening, that compares states
using the domain's `leq` relation to establish implication.
Reviewed By: ezgicicek
Differential Revision: D27908529
fbshipit-source-id: b461165da
Jules Villard
Boris Yakobowski
Yoonseok Ko
Nikos Gorogiannis
Graham Sutherland
Radu Grigore
Sungkeun Cho
Ezgi Çiçek
Josh Berdine
Matteo Marescotti
Martin Trojer