Summary: Impurity traces are quite big due to recording values histories. Let's simplify the traces by removing pulse's value histories.
Reviewed By: skcho
Differential Revision: D20362149
fbshipit-source-id: 8a2a6115e
Summary: Type is not enough to say a function call of `Provider.get` is expensive or not.
Reviewed By: jvillard
Differential Revision: D20366206
fbshipit-source-id: 83d3e8741
Summary:
This diff uses a type parameter of `Provider.get` to decide whether assigning expensive cost to the
function call or not. For example, if the type is small one like `Provider<Integer>`, it be
evaluated to have a unit cost, otherwise a linear cost.
To get the return type of `Provider.get`, I added a simple analyzer that collects "casted" types
backwards. In Sil, while the function call statement loses the return type, e.g,
```
n$5=_fun_Object Provider.get()(n$3:javax.inject.Provider*);
```
the `n$5`'s value is usually casted to a specific type at some point later.
```
*&$irvar0:java.lang.Object*=n$5
n$8=*&$irvar0:java.lang.Object*
n$9=_fun___cast(n$8:java.lang.Object*,sizeof(t=java.lang.Integer;sub_t=( sub )(cast)):void)
```
So, the analyzer starts from the cast statements backward, collecting the types to cast for each
variables.
Reviewed By: ezgicicek
Differential Revision: D20345268
fbshipit-source-id: 704b42ec1
Summary:
The previous diff finally unblocks this important milestone.
Now we have only one place when `convert_complex_exp_to_pvar` needs to
modify the typestate (in Sil.Load typechecking).
Because the only case left is when `~is_assignment: false`, we also can
simplify logic of updating the typestate (remove dependency on
`is_assignment`, and rename the methods so they more clearly say what
they do (instead of vague "update_typestate").
This diff, in turn, unblocks further refactoring. Namely, remove this
remaining usage of
`convert_complex_exp_to_pvar_and_register_field_in_typestate` from
Sil.Load instruction so that there is no tricky field magic left!
We will try doing it in follow up diffs.
Reviewed By: artempyanykh
Differential Revision: D20334924
fbshipit-source-id: 20ce185ed
Summary:
When assigning to field `field = expr()` we need to compare actual
nullability of `expr()` (rhs) with formal nullability of field (lhs).
Formal nullability is based on field declaration.
Before this diff lhs nullability was calculated based on actual
nullability of lhs. This is an absolutely wrong thing to do!
E.g. `field = null` would update nullability of field in typestate, and
from this field will be considered as nullable, so next time field =
null is allowed...
Due to series of hacks pile on top of each other, this wrong behavior
actually works correctly; but this depends on completely wrong things
happening in other places of the program (e.g. typestate modifies in
wrong places to make this work!).
This diff unblocks refactoring that will clean up hacks mentioned above,
and simplify typechecking logic.
Reviewed By: artempyanykh
Differential Revision: D20334925
fbshipit-source-id: 8bbcbd33a
Summary: The macro is dead. It had been used when Inferbo had include-based C++ models.
Reviewed By: jvillard
Differential Revision: D20309031
fbshipit-source-id: bcfd8f923
Summary:
Warning: This might be a bit brutal.
PerfStats and EventLogger are pretty much subsumed by `ScubaLogging`.
It seems no one has been looking at the data they generate recently.
Let's delete them! If we need to re-implement some parts later on, let's
do that using `ScubaLogging`, which is better (eg, still produces data
when infer crashes).
Things we lose:
- errors in the clang frontend due to missing decl translation, etc.
- errors in biabduction due to timeouts, functions not found, etc.
We could also re-implement these using BackendStats and ScubaLogging
instead of brutally deleting everything.
Reviewed By: ngorogiannis
Differential Revision: D20343087
fbshipit-source-id: 90a3121ca
Summary:
At some point we thought disconnected CFGs (where some nodes are not
reachable from the initial node) were signs of bugs in our frontend, but
it turned out not to be the case. Thus, we compute the boolean "is
connected" for each procdesc for the only purpose of logging that
uninteresting piece of information.
Delete it.
Reviewed By: ngorogiannis
Differential Revision: D20342834
fbshipit-source-id: 3f9317003
Summary:
The goals are to have all the checker definitions and documentation in one
place (except how to actually run them, since that's not quite the same
concept; for example inferbo is one checker but several analyses depend on its
symbolic execution), and later on to be able to link issues reported by infer
back to the checker that generated them.
This makes apparent that the documentation of our checkers is lacking,
not touching that in this diff.
Not sure if "analysis" would be a better name than "checker" at this
point? For instance "Linters" is one of the checkers, which historically
at least we have not considered to be the case.
Reviewed By: mityal
Differential Revision: D20252386
fbshipit-source-id: fc611bfb7
Summary:
Some options are deprecated and this is indicated with `~long:""`. But for
boolean options this creates a spurious `"--no-"` flag. Detect when the option
has an empty flag and don't create the "no" version in that case.
Reviewed By: ngorogiannis
Differential Revision: D20286830
fbshipit-source-id: 9a2095ea8
Summary:
Define a `ppx_sledge` ppx rewriter that composes all the ppx rewriters
used, so that the list needs to be specified only once.
Reviewed By: jvillard
Differential Revision: D20322874
fbshipit-source-id: f15540b7f
Summary:
It seems to currently not be possible to make dune `preprocess`
stanzas vary across contexts, without essentially generating the dune
files. A consequence is that it is not possible to pass
context-dependent command line arguments to ppx rewriters. So add
support for an environment variable instead.
Reviewed By: jvillard
Differential Revision: D20322871
fbshipit-source-id: f3f3ea413
Summary:
Move files, adjust build system, etc.
This also separates out the ppx_trace conditional compilation debug
tracing machinery into an independent package and library.
Reviewed By: jvillard
Differential Revision: D20322876
fbshipit-source-id: a50522462
Summary:
`Reg.demangle` is implemented by calling the `_cxa_demangle` C++
runtime system function. This will be linked into the sledge binary,
due to being linked with llvm, but will not necessarily be available
in the sledge library. So make it a dynamically-set function to avoid
calling an undefined function from the library.
Reviewed By: jvillard
Differential Revision: D20323791
fbshipit-source-id: bda9afd37
Summary:
The convenience symlinks from sledge/bin to the built binaries of
various build modes under sledge/_build/_install conflict with
upcoming code rearrangements, so remove them.
Reviewed By: jvillard
Differential Revision: D20322872
fbshipit-source-id: 1a7c99ae0
Summary:
Formulate the canonizer for Extract from Concat terms uniformly as a
concatenation of extracts.
Reviewed By: jvillard
Differential Revision: D20303064
fbshipit-source-id: a45bc45dd
Summary: This diff adds a model for Java's `Object.clone()` method (similar to existing shallow_copy).
Reviewed By: jvillard
Differential Revision: D20341073
fbshipit-source-id: 30ae40fe7
Summary:
Fixes#1234
Turns out I didn't catch this in local testing because docusaurus falls back on
exploring files in website/ locally for some reason.
Reviewed By: skcho
Differential Revision: D20342513
fbshipit-source-id: 87587ccc2
Summary:
Some (all?) of this is already tested in other tests, but this feature
is important enough (and the implementation is scattered accross the
whole code), so I found it useful to have a small test that ensures the
very basic things are working as expected.
See `NestedFieldAccess.java` that tests far more advances things, but
here we focus only very basic things: conditions, local variable
assignments, and explicit assignments.
Reviewed By: jvillard
Differential Revision: D20339056
fbshipit-source-id: a6cfd0043
Summary:
A java byte is not a short and a java character is not a C character. Fix those types by mapping them to what the spec ordains.
An extra advantage is that this establishes a bijection between java types and their project in the `sil` type system, so pretty printing in Java mode can actually be made to work.
Reviewed By: mityal
Differential Revision: D20330525
fbshipit-source-id: 00ac2294b
Summary:
- ignore `ConstantDereference` invalidations since they just represent assignments to constants.
- add `impurity.mli`
- split `extract_impurity` into several sub functions.
Reviewed By: jvillard
Differential Revision: D20335674
fbshipit-source-id: f0fd6b5f4
Summary:
1. Log cases when we fall back to default while typechecking (we aim to
gradually reduce their usage)
2. Log high level operations when checking field assignment
Reviewed By: ngorogiannis
Differential Revision: D20334923
fbshipit-source-id: 80c45b29e
Summary:
Experiment show that we did not rely on fact that this function could modify
typestate.
Reviewed By: artempyanykh
Differential Revision: D20285198
fbshipit-source-id: dc20f4b4b
Summary:
Previous diff introduced convert_complex_exp_to_pvar that does not
modify typestate.
Luckily enough, experiement shows that we don't need it in this place!
Reviewed By: artempyanykh
Differential Revision: D20284942
fbshipit-source-id: ea1471681
Summary:
This function is converting exp to pvar AND sometimes modifies a
typestate (even when ~is_assignment is false, which is something one
would not expect!).
We aim to reduce cases when it is not needed.
This diff splits the function into two: good and evil, and uses good
when it is a no-op.
Reviewed By: ngorogiannis
Differential Revision: D20284301
fbshipit-source-id: 0e5d65a65
Summary:
Extract `Typ.Name.Java.Split` into a standalone module.
- This module is really only used in `Procname` so no need to be in `Typ`.
- Remove some pointless conversions to strings and back.
- Reduce the interface of `Split` to the minimum ahead of possible removal in favour of normal types.
Reviewed By: mityal
Differential Revision: D20322887
fbshipit-source-id: 7963757cb
Summary:
- Use `Typ` constants instead of constructors for basic types.
- Nest single-use unexported functions at the point of use.
- Improve exception safety wrt `Not_found` exceptions by preferring `_opt` functions.
- Simplify deeply nested `match` statements by pushing conditionals to `when` clauses when possible.
Reviewed By: skcho
Differential Revision: D20304392
fbshipit-source-id: edbc08219
Summary:
- Remove dead argument in function in JMain.
- Document order of constructed classpath.
- Stop asking repeatedly for the cwd in `add_source_file`.
- Improve exception safety and readability in `load_from_verbose_output`.
Reviewed By: ezgicicek
Differential Revision: D20290519
fbshipit-source-id: 86c32dcdf
Summary:
Problem: `infer report <specs file name>` is called manually sometimes to see analysis results in CLI. However, giving the specs file name is sometimes annoying, because the specs file name may be quite long and include special characters sometimes.
This diff introduces `--procedures-summary` to lookup the summaries interactively in `infer explore`.
example1: There are 8 procedures that include "max" in their names, then I selected one of them by entering a number.
```
$ infer explore --procedures --procedures-filter '.*max.*' --procedures-summary
0: minmax_div_const2_Bad_FN
1: minmax_div_const_Good
2: use_int64_max_Bad
3: use_uint64_max_Good
4: use_int64_max_Good
5: minmax_div_const_Bad
6: minmax_div_const2_Good
7: use_uint64_max_Bad
Select one number (type 'a' for selecting all, 'q' for quit): 2
void use_int64_max_Bad()
Analyzed
ERRORS: BUFFER_OVERRUN_L1
WARNINGS:
FAILURE:NONE SYMOPS:0
BufferOverrunAnalysis: StackLocs: { } MemPure: { } Alias: { ret= }
BufferOverrunChecker: Safety conditions:
{ }
```
example2: If there is only one specs file that satisfies the given filter, it reports the summary of that procedure without an interaction.
```
$ infer explore --procedures --procedures-filter '.*add_in_loop_ok.*' --procedures-summary
Selected proc name: void ArrayListTest.add_in_loop_ok()
void void ArrayListTest.add_in_loop_ok()(ArrayListTest* this)
Analyzed
ERRORS:
WARNINGS:
FAILURE:NONE SYMOPS:0
BufferOverrunAnalysis: StackLocs: { } MemPure: { } Alias:
{ i=size(__new-390022197-0-1.elements), ret= }
LatestPrune: latest { i -> (5, { }, { }) by ((5, { }, { }) >= (5, { }, { })),
__new-390022197-0-1.elements -> (⊥, { }, { __new-390022197-1-1 -> length : 5 }) by ((5, { }, { }) >= (5, { }, { })) }
BufferOverrunChecker: Safety conditions:
{ }
```
Reviewed By: jvillard
Differential Revision: D20284052
fbshipit-source-id: 2131339f1
Summary:
Found a high number of `uname` syscalls while tracing infer. Figured it
must be the `Unix.gethostname` call. Stop continuously asking for the
hostname.
Reviewed By: jberdine
Differential Revision: D20309048
fbshipit-source-id: b8be1f2dc
Summary:
These are important enough operations to be tracked in logs.
This diff also tweaks logs for the main loop processing instuctions, so
that we log instr before processing it, not the reverse
Reviewed By: artempyanykh
Differential Revision: D20282443
fbshipit-source-id: 40b8b6627
Summary: The class map of biabduction models does not need to be part of the program type. This diff centralises all that in JModels module.
Reviewed By: jvillard
Differential Revision: D20283623
fbshipit-source-id: 5f95a7477
Summary:
- Factor out function that walks zip filename entries in jar file.
- Kill dead type (`classpath`) and record field (`classpath`).
- Make some string arguments named.
- Introduce function that folds over classnames in jar file and reduces occurrences of pattern "produce list, fold over it".
Reviewed By: artempyanykh
Differential Revision: D20245756
fbshipit-source-id: ccca47cdd
Summary: We don't need skipped calls for pre and post. Let's pull them out to `PulseAbductiveDomain`, next to pre and post.
Reviewed By: jvillard
Differential Revision: D20283589
fbshipit-source-id: 5cf970292
Summary: We forgot to take skipped calls into account for state comparison. This diff fixes that.
Reviewed By: skcho
Differential Revision: D20282739
fbshipit-source-id: 7b4d84bb0
Summary:
Change constructor for solver goals to enforce variable context
conditions, and simplify other context manipulations that are now
unneeded.
Reviewed By: jvillard
Differential Revision: D20248543
fbshipit-source-id: a255c792b
Summary:
In a few places, implicitly witnessed existential variables could
remain in the existential context. This led to weakness in the solver,
where occurrences bound by the existential context would be not known
to be constrained by their witnesses.
Reviewed By: jvillard
Differential Revision: D20248542
fbshipit-source-id: 44f62839c
Ezgi Çiçek
Sungkeun Cho
Martin Trojer
Josh Berdine
Mitya Lyubarskiy
Jules Villard
Nikos Gorogiannis
Alexander Mols