Summary:
`Errlog` will merge similar issues (same severity, name, description) reported at the same location, so let's make sure the locaiton is mandatory.
Issues:
- errors happening in `Ondemand` still use the `State` which makes sense only for biabduction and eradicate
- a case of `NullabilitySuggest` didn't have a location, I did my best to patch it but I'm sure the location could be more precise
Reviewed By: jvillard
Differential Revision: D9332840
fbshipit-source-id: ee7898146
Summary: Keeping pushing arguments higher in the stack, `node_id_key` is not used in calls to `log_warning/error`
Reviewed By: jvillard
Differential Revision: D9332826
fbshipit-source-id: e5c48c686
Summary:
Before we would convert it to string in `Reporting` and pass it to `Errlog` which would use it only to 'log events'.
I guess the reason is that there was a cyclic dependency between `Errlog` and `clang_method_kind` defined in `ProcAttributes`.
This diff:
- moves it to its own module
- defers the conversion to string
Reviewed By: jvillard
Differential Revision: D9332819
fbshipit-source-id: 43a028b61
Summary:
- abstracted the type for a node key
- moved it to its own module with an ugly `compute` to avoid cyclic dependencies...
- renamed `node_id` to `node_id_key` where needed
- moved key computation from `State` to `Procdesc.Node`
Reviewed By: jvillard
Differential Revision: D9332803
fbshipit-source-id: fe1ae8c1c
Summary:
- made arguments of `Errlog.log_issue` mandatory
- pushed some arguments of `log_issue_from_errlog` higher in the stack, the goal is to make sure `State` is only used in analyses that update it (biabduction and eradicate, if I'm correct)
Reviewed By: jvillard
Differential Revision: D9332773
fbshipit-source-id: ce79df21c
Summary: The `procedure` field in the final report should use the non-ambiguous fully qualified name containing the Java package declaration and the list of parameter types.
Reviewed By: mbouaziz
Differential Revision: D9237522
fbshipit-source-id: e9b0ff664
Summary:
To keep up with the times. Changes consist of new features and moving modules
around so shouldn't change anything on our side.
Depends on D9239803
The controller you requested could not be found.: facebook-clang-plugins
Reviewed By: da319
Differential Revision: D9239817
fbshipit-source-id: d02a2076a
Summary:
Use `ignore` instead, as this will warn if the argument is an arrow type,
unlike `let _ = ...`. This makes the code more future-proof: if an argument is
added to a function called in `let _ = f x` then the compiler will complain
instead of silently turning a value into a partial evaluation.
Also got rid of particularly irksome `let _ = <stuff returning unit> in` where I could.
Reviewed By: mbouaziz
Differential Revision: D9217176
fbshipit-source-id: 3be463405
Summary:
The internal concept of "kind" should in fact be named "severity" to match the convention used by many other tools, whereas the internal concept of "severity", i.e "HIGH", "MEDIUM" and "LOW" was never used and in any case redundant with the concept of "info", "warning", "error".
This diff maps both the "kind" and "severity" fields to value of the form "advice", "info", "warning", and "error" to be able to progressively migrate the code using the "kind" field.
Reviewed By: mbouaziz, jvillard
Differential Revision: D9187978
fbshipit-source-id: 447d89f51
Summary:
- changes the `Ondemand` callbacks to take the execution environment instead of a `get_proc_desc` function.
- removes all the cases passing `get_proc_desc` as parameter to use `Ondemand.get_proc_desc` instead.
Reviewed By: jvillard
Differential Revision: D9200583
fbshipit-source-id: d16c218b5
Summary:
Exposing the in-memory cache seems dangerous. There were only 2 uses anyway:
eradicate and biabduction. I think the biabduction one is safe to remove. The
eradicate one I do not really understand as we return a different summary than
the one we cache... the tests pass though.
Reviewed By: jeremydubreil
Differential Revision: D9150167
fbshipit-source-id: cf30af232
Summary: This code is no longer necessary because the bug hash does not depend on the name of the anonymous classes
Reviewed By: mbouaziz
Differential Revision: D9176205
fbshipit-source-id: 9a8e9c9f8
Summary:
Print the following for each source file to analyse in non-interactive mode:
```
path/to/source_file.c starting
[...]
path/to/source_file.c DONE in <time>
```
This should help diagnose when infer is stuck. It also logs this information to
the log file regardless of the form of the progress bar.
Also add a `--progress-bar-style` option to allow the user to force a
particular rendering: plain (as above), multiline (The Glorious One), or auto
(selection depends on whether infer is connected to a TTY on stdin *and*
stderr).
Reviewed By: mbouaziz
Differential Revision: D9120509
fbshipit-source-id: 4b43b7464
Summary: Added variant type for statement node to make it cleaner to match a particular statement node.
Reviewed By: mbouaziz
Differential Revision: D8997124
fbshipit-source-id: e19f6eacd
Summary:
When `--reanalyze` is passed, mark the summaries of procedures matching
`--procedures-filter` as needing to be analysed before running the analysis.
This allows one to, for instance, re-run the analysis in debug mode on only
some files or procedures. However, this won't work for the Java Buck
integration since the summaries are hidden away in buck-out.
Reviewed By: mbouaziz
Differential Revision: D8783668
fbshipit-source-id: 9032d83
Summary:
This allows to deduplicate some code related to walking the rows of the results
of a SQLite query. Give more meaningful names to the API while I'm at it.
Reviewed By: mbouaziz
Differential Revision: D8783332
fbshipit-source-id: 4aa6613
Summary: Some values in Config.mli were in the wrong section. Also, use ocamldoc formatting for sections.
Reviewed By: mbouaziz
Differential Revision: D8783244
fbshipit-source-id: f9a0729
Summary:
Filtering on the SQLite side was done to be more efficient, but these are debug
options so it should be fine for them to be not very optimised.
Filtering on the OCaml side will allow us to re-use these filtering options for
other purposes, such as re-analysing certain procedures only.
Reviewed By: mbouaziz
Differential Revision: D8767691
fbshipit-source-id: e232660
Summary:
Specs do not get invalidated by reactive mode, so we need to delete them all.
Reactive just means that the capture remains valid and the analysis will only
use the freshly captured source files to start the analysis, but all the
dependencies will be re-analysed.
Reviewed By: da319
Differential Revision: D8735171
fbshipit-source-id: dc1eab3
Summary: Do not start with an invalid source file when we can avoid it. Follow up from D8418447.
Reviewed By: jeremydubreil
Differential Revision: D8732168
fbshipit-source-id: 28a183b
Summary:
The captured/ directory is not guaranteed to exist when the capture was run without debug mode, but the analysis assumes it will be. So, this crashes infer:
```
infer -- clang -c examples/hello.c
infer -g
```
There's no need for `DB.Results_dir.init` to create infer-out and infer-out/specs/ since this is now done in `ResultsDir`.
Reviewed By: jeremydubreil
Differential Revision: D8732739
fbshipit-source-id: aaac902
Summary: This is make the callbacks are always set instead of silently returning `None`
Reviewed By: ngorogiannis
Differential Revision: D8651971
fbshipit-source-id: 7cd1c02
Summary:
This changes the behaviour back to before the "fork once then pipe new tasks
for each source files" era: before that we would fork once per source file,
which had the effect of emptying the caches for every source file. Without this
the caches can grow unchecked.
This is probably suboptimal but should at least be the same behaviour as before.
Reviewed By: jeremydubreil
Differential Revision: D8615124
fbshipit-source-id: 69fc101
Summary:
Replace the previous outputting of "." and "F" with an actual progress bar and
a multiline display of what procedure each process is currently busy analysing.
Observe:
```lang=text
Found 19 source files to analyze in /home/jul/code/openssl-1.1.0d/infer-out
7/19 [######################......................................] 36%
⊢ [ 1.14s] crypto/mem.c: CRYPTO_malloc
⊢ [ 1.68s] crypto/o_time.c: julian_adj
⊢ [ 0.50s] crypto/mem.c: CRYPTO_zalloc
⊢ [ 1.80s] crypto/o_str.c: OPENSSL_strlcpy
```
This works by setting up a worker pool (as before) that waits to receive jobs
(not as before: we used to fork for each new job). Unix pipes are used for
communication.
The new worker pool can be used to experiment with other concurrency models,
such as reviving per-procedure-parallelism, or making sure each procedure is
analysed only once.
Perf tests indicate that this version is no slower than the previous one,
either on laptops or devserver: about 3% worse user time but ~40% better system time.
This new version forks <jobs> processes whereas the previous version would
fork `O(number of source files)` times.
`infer -j 1` shows a progress bar that doesn't update timing info (because it
would need a second process to do that).
Reviewed By: mbouaziz
Differential Revision: D8517507
fbshipit-source-id: c8ca104
Summary:
The execution environment is really just a cache. It happens to point to a
particular source file which is where the analysis was started from, but that
is not relevant, and in fact is confusing because it suggests that it is
somewhat tied to that file. In reality, exe_env caches information about any
procedure and source file encountered by the analysis.
This will make it easier to make further changes but I think it also brings a
bit more clarity to the code.
Reviewed By: jeremydubreil
Differential Revision: D8513735
fbshipit-source-id: f4b38ce
Summary: Introduce an annotation that forces the summary of a method to be free of blocking events, without suppressing other reports.
Reviewed By: jeremydubreil
Differential Revision: D8276787
fbshipit-source-id: be9eed8
Summary: I don't understand what this function is for. Let's remove it.
Reviewed By: mbouaziz
Differential Revision: D8320839
fbshipit-source-id: eeb14f7
Summary:
`make doc` will use `jbuilder` (which in turn uses `odoc`) to generate the
documentation for infer's modules. This is useful to browse the APIs of infer
and gives a more discoverable place to host more general documentation about
infer's internals.
Besides the actual plumbing necessary to generate the docs, this diff also
- Moves the various infer/src/*/README.md to index.mld files that make it to the generated docs
- Fixes some doc comments that would anger `ocamldoc`
Closes#435
Reviewed By: mbouaziz
Differential Revision: D8314572
fbshipit-source-id: 4a5c70e
Summary: Create mechanism for suppressing starvation reports. To do that, refactor and expose a Checkers function.
Reviewed By: mbouaziz
Differential Revision: D8259583
fbshipit-source-id: f5b5a63
Summary:
Having the `Node` module including in the `CFG` one is confusing.
Let's keep it separate.
Reviewed By: ngorogiannis
Differential Revision: D8185754
fbshipit-source-id: 62077e6
Summary:
It's useful to test that the bucket a given error is classified as doesn't
change over time without notice.
This records the bucket for *all* the tests, even though some never produce a
bucket. This is to be on the safe size instead of risking to forget adding the
bucket information when the test changes, or when copy/pasting from a test that
doesn't have buckets to one that does.
The implementation is pretty crude: it greps the beginning of the qualifier
string for a `[bucket]`.
Reviewed By: mbouaziz
Differential Revision: D8236393
fbshipit-source-id: b3b1eb9
Summary:
Change the license of the source code from BSD + PATENTS to MIT.
Change `checkCopyright` to reflect the new license and learn some new file
types.
Generated with:
```
git grep BSD | xargs -n 1 ./scripts/checkCopyright -i
```
Reviewed By: jeremydubreil, mbouaziz, jberdine
Differential Revision: D8071249
fbshipit-source-id: 97ca23a
Summary:
For now: just moving this list behind an abstract type.
Next: changing the internal representation.
Reviewed By: ngorogiannis
Differential Revision: D8140926
fbshipit-source-id: 5b959b0
Summary:
We never really need the list of nodes/succs/preds, we only need to fold over them.
This will reduce garbage for computed lists like in the Exceptional CFG or the OneInstrPerNode CFG.
Reviewed By: ngorogiannis
Differential Revision: D8185665
fbshipit-source-id: d042beb
Summary:
Append can be costly, let's do it once only.
Depends on D8185619
Reviewed By: jeremydubreil
Differential Revision: D8185634
fbshipit-source-id: 67f84a9
Summary:
- do not `List.rev` for `List.last`
- `List.rev_filter_map` rather than `filter |> map |> rev`
Reviewed By: da319
Differential Revision: D8185619
fbshipit-source-id: aeb41a4
Summary:
Moving away from C++ include-based models means that we cannot reliably detect
anymore whether a file includes <iostream> or not. In order not to be too
spammy, let's always assume standard streams are initialized for now when the
include models are off.
Recent versions of libstdc++ make these models redundant so there is hope that in a
bright future the analysis of std streams initialisation will work correctly without infer
having to have its own models anyway.
Reviewed By: mbouaziz
Differential Revision: D8043467
fbshipit-source-id: d118043
Summary:
Preparing for bigger changes...
- Rename `payload` field to `payloads`
- Move `payload` type to `Payloads.t`
- `SummaryPayload`s only have to implement a change on `Payloads.t` rather than `Summary.t`
Reviewed By: sblackshear
Differential Revision: D7987211
fbshipit-source-id: c9d7a74
Summary:
Attempt at a better naming scheme:
- `Specs.summary` are now `Summary.t`. The `Summary` module (replacing `Specs`) contains the summary of a procedure: the results of all the analyses, etc.
- `Summary.ml` is now `SummaryPayload.ml`. This concerns how each (AI) analysis extracts its payload from the master summary.
- Accordingly, checkers now define a `Payload` module where previously they defined a `Summary` module. The type is also cleaned up to use `t` instead of `payload`, etc.
- Cleaned up some names as a result, for instance `Specs.get_summary` -> `Summary.get`, etc.
Reviewed By: ngorogiannis
Differential Revision: D7935883
fbshipit-source-id: 1766545
Summary:
Move the biabduction-specific payloads (the "`'a spec`" stuff) from specs.ml
into a new `BiabductinoSummary` module, similar to other checkers.
Reviewed By: ngorogiannis
Differential Revision: D7935815
fbshipit-source-id: bdff3b9
Summary: It seems that we don't need to rely on `make` anymore as the internal process manager is working fine on Buck Java projects.
Reviewed By: jvillard
Differential Revision: D7903639
fbshipit-source-id: 9b32f05
Summary:
This is an attempt to make things more consistent, and maybe save some work
from the `Format` module in case flambda doesn't have our backs.
Reviewed By: jberdine
Differential Revision: D7775496
fbshipit-source-id: 59a6314
Summary: Historically, this option was only used to parallelize the biabduction analysis, which is now run using the checkers framework.
Reviewed By: jvillard
Differential Revision: D7895118
fbshipit-source-id: 2a54bca
Summary:
One source of non-determinism is racing on procedure summaries when reporting. In particular, the summary of a method may be computed and stored by one thread, but another may be trying to report on it (eg, in cluster checkers).
One solution (at least until everything is in sqlite) is to have separate files just for the reports, a la linters. This diff improves the interface of LintIssues and generalises it ahead of using it in other analysers.
Reviewed By: jeremydubreil
Differential Revision: D7859973
fbshipit-source-id: 8672d3b
Summary:
This simplifies the frontends and backends in most cases. Before this diff,
returning `void` could be modelled either with a `None` return, or a dummy
return variable with type `Tvoid`. Now it's always the latter.
Reviewed By: sblackshear, dulmarod
Differential Revision: D7832938
fbshipit-source-id: 0a403d1
Summary:
Add warning 60 (unused module) to the list of fatal warnings. Whitelisting
modules at toplevel is tricky (see inline comments) but doable.
Reviewed By: mbouaziz
Differential Revision: D7790073
fbshipit-source-id: 6f591c4
Summary:
When looking at large CFGs, at least in `xdot`, it's often difficult to find
the procedure you're looking for. Sorting the proc names puts them in
alphabetical order, which makes searching one procedure easier.
Reviewed By: mbouaziz
Differential Revision: D7758521
fbshipit-source-id: 8e9997f
Summary:
Upgrade ocamlformat, and base which needs to be done in sync in order to build
ocamlformat, and the other deps can come for the ride.
Reviewed By: jvillard
Differential Revision: D7663537
fbshipit-source-id: 3e90970
Summary:
We were using the "filename" as the key because it's (kinda) unique *and* human
readable, but with the `infer explore --procedures` interface we don't really
need the human readable part anymore, so we can just use the OCaml marshalling
of the pname as the key. The human-readable version (sans unique-fying hash) is
now another column in the table, used to match procedure names in
`--procedures-filter`.
Reviewed By: sblackshear
Differential Revision: D7639158
fbshipit-source-id: e714605
Summary:
Add a `--procedures` option to `infer explore` to print information about the
procedures captured by infer. More precisely, `infer explore --procedures` will
print each row of the "procedures" table in the results database. A new
`--procedures-filter` controls which procedures to print information about, and
there is one flag per column in the db too to print more or less options about
each procedure (in particular, we can now print attributes), with some defaults.
Reviewed By: sblackshear
Differential Revision: D7639062
fbshipit-source-id: 034a2b8
Summary:
Now that everything can run at the same time and we have preanalyses, it can be quite hard to read debug sessions.
Here come session names!
Depends on D7607336
Reviewed By: sblackshear
Differential Revision: D7607481
fbshipit-source-id: 676af86
Summary:
- Less `^`
- `pp_print_string` instead of `F.fprintf fmt "%s"`
- and stuff like that
Reviewed By: jvillard
Differential Revision: D7607336
fbshipit-source-id: 5d985ef
Summary: Only RacerD uses the Cluster callbacks but we used to compute a lot of things anyway.
Reviewed By: sblackshear
Differential Revision: D7516279
fbshipit-source-id: 22f2b86
Josh Berdine
Mehdi Bouaziz
Jeremy Dubreil
Martino Luca
Jules Villard
Daiva Naudziuniene
Dulma Churchill
Ezgi Çiçek
Nikos Gorogiannis
Jose Blanco