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:
`IList.map_changed` relies on `take_append`.
The existing code was a `take_rev_append` instead.
Fixed it.
Tests are still passing, we might want to look at callsites and consider a `map_changed_unordered`.
Reviewed By: jvillard
Differential Revision: D8201187
fbshipit-source-id: 151b95c
Summary: In preparation for allowing unbalanced locking, we need the lock state in the summary.
Reviewed By: mbouaziz
Differential Revision: D8201932
fbshipit-source-id: 05a1b38
Summary:
Preparing for the future change, we won't see instructions as lists but as an abstract type.
This change may be a very minor perf regression: does a few more (but bounded by a constant) instructions traversals only for the nodes involving a Printf-like function call, only for the PrintfArgs checker...
Reviewed By: jvillard
Differential Revision: D8094124
fbshipit-source-id: e2e2c5e
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: In preparation to change the underlying module structure so as to allow three-point traces (call-site, intermediate call-site and endpoint), rename modules to better reflect function plus use records vs pairs of pairs :P
Reviewed By: mbouaziz
Differential Revision: D8187369
fbshipit-source-id: ed3e4ac
Summary:
There can be multiple reports per line, especially when calling in a method which has itself multiple reports.
When reporting at the callsite, report only the issue with the highest severity (picked manually per type of event).
Deadlocks are not de-duplicated, as they are relatively rare and the info in mupltiple reports may be important.
Reviewed By: mbouaziz
Differential Revision: D8160940
fbshipit-source-id: ea6a5c0
Summary:
The main motivation is preparing for a future change.
This also reduces lifetime of potential garbage.
Reviewed By: jeremydubreil
Differential Revision: D8185648
fbshipit-source-id: 6d0a568
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:
Rely on the underlying CFG preds:
- perf: no need to append lists
- correctness: the underlying CFG may be removing duplicates
Reviewed By: ngorogiannis
Differential Revision: D8185638
fbshipit-source-id: 3b6f70a
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: The order of nodes means nothing, and should not matter, let's save the whales!
Reviewed By: ngorogiannis
Differential Revision: D8182137
fbshipit-source-id: bc14a2c
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: Prevent the analysis to default to absolute paths which would invalidate the cache.
Reviewed By: mbouaziz
Differential Revision: D6997490
fbshipit-source-id: 3c17658
Summary: We want both pointer and pointer dereference to be uninitialised at the beginning. Forgot to add the expression of type pointer when updating the analysis from access paths to access expressions.
Reviewed By: ddino
Differential Revision: D8117011
fbshipit-source-id: 534f7ef
Summary: Set arguments of pointer type as initialised for indirect function calls.
Reviewed By: mbouaziz
Differential Revision: D8097895
fbshipit-source-id: 830f568
Summary: Track and surface the reasons why a method is assessed to run on the UI thread.
Reviewed By: mbouaziz
Differential Revision: D8096099
fbshipit-source-id: 2403c6c
Summary:
The reported location was always the start of the enclosing procedure, which is wrong in many ways.
A nice side-effect is that some code can then be eliminated and Ondemand.analyze used, avoiding getting the procdescs in the process.
Reviewed By: jeremydubreil
Differential Revision: D8056306
fbshipit-source-id: 67c2c8d
Summary: Treat array accesses as initialised if they are passed by reference.
Reviewed By: jvillard
Differential Revision: D8071247
fbshipit-source-id: 5480e90
Summary: The type of array element is not preserved correctly in the translation from SIL to HIL. When array element is passed by a reference, i.e. `f(&(array[0]))`, the type of array element gets the type of a pointer of array element.
Reviewed By: jvillard
Differential Revision: D8071188
fbshipit-source-id: 3e6635e
Summary:
The deadcode checker is poorly written and as a result is more useful than
intended!
Reviewed By: mbouaziz
Differential Revision: D8088654
fbshipit-source-id: 19a94b8
Summary: Use AccessExpressions instead of AccessPath in uninit analysis. This will allow us to distinguish between pointers and their dereferences.
Reviewed By: jvillard
Differential Revision: D8042359
fbshipit-source-id: 604bcbc
Summary:
It improves the precision of widening operations of interval:
upper_bound_widen (min(n, s), s) = s
lower_bound_widen (max(n, s), s) = s
Reviewed By: mbouaziz
Differential Revision: D8038941
fbshipit-source-id: 61b10cb
Summary:
Labels inside switch statements were causing havoc (see test), and the translation of switch statements in general could be improved to handle more cases.
It turns out that `case` (and `default`) statements are more or less fancy labels into the code. In other words, if you erase all the `case XXX:` and `default:` strings in the `switch` statement you get the real structure of the program, and `switch` just jumps straight to the first `case` directives (and to the second if the first one is not satisfied, etc. until all `case`/`default` have been considered).
This suggests an alternative implementation: translate the body of the `switch` and simply record the list of switch cases inside that body, along with where they point to. Then post-process this list to construct the control flow of the `switch`, which points into the control-flow of the `body`. In order not to modify every function in `CTrans` to propagate the current list of cases, I created an ugly `ref` inside `SwitchCase` instead (but it cannot be directly accessed and it's guaranteed to be well-parenthesised wrt nested switches by the `SwitchCase` API so it's not too bad).
[unrelated] Also make translation failures output more information about what exactly in the source code is causing the crash, and the ancestors in the AST that lead to the crash site.
Reviewed By: martinoluca
Differential Revision: D8011046
fbshipit-source-id: 8455090
Summary: Moving this function since it's about a single procdesc. Slight rewrite too.
Reviewed By: da319
Differential Revision: D8030494
fbshipit-source-id: f7cc58e
Summary:
- Reorder modules in mli for readability.
- Match mli module order in the implementation.
- Move some functions that operate on domains from RacerD.ml to the domain file.
- Kill some module type invocation.
- Use standard module signatures.
Reviewed By: mbouaziz
Differential Revision: D8026386
fbshipit-source-id: ee2af22
Summary: Method overloading creates the potential for report duplication even though the reports are actually distinct. Make the report message unique by not using shortened method names.
Reviewed By: jeremydubreil
Differential Revision: D8005862
fbshipit-source-id: 53d8ea0
Summary: Follow C++ in having local variables owned plus silence reports on paths rooted on logical vars. We need both because when propagating ownership from right to left, the initial status of a temp var as owned is lost.
Reviewed By: sblackshear
Differential Revision: D7988575
fbshipit-source-id: 2e817d7
Summary: There was a bit of code and comments referring to potential soundness. Kill those.
Reviewed By: da319
Differential Revision: D8004256
fbshipit-source-id: c20b62a
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: Fixing the support for single core analysis in 8ce79a0613 was improving the performance of the genrule-based integration by more that 2X. Running Infer in single core more with the external compiler integration for Buck also improves the performance significantly.
Reviewed By: mbouaziz
Differential Revision: D7989255
fbshipit-source-id: 71fb842
Summary:
This diff:
- translates C++ `catch` blocks
- adds an exceptional control-flow edge from the end of a `try` block to the beginning of a `catch` block
This obviously doesn't reflect the way exceptions actually work, but I think it is better than what we have now. For one thing, we'll see/translate code inside `catch` blocks, which were opaque before. If Clang analyses don't want this behavior, they can simply use `ProcCfg.Normal` (which, up until this diff, behaved identically to `ProcCfg.Exceptional`.
In the future, we can extend `trans_state` to track blocks that might throw an exception, and have each of these blocks transition to `catch` instead.
Reviewed By: jvillard
Differential Revision: D7814521
fbshipit-source-id: 67b86a6
Summary:
- delete getter for `CContext.context.procdesc`
- change API of `CLocation`, in particular to take just a source file instead of a `CContext` since that's all they need (but maybe we'd rather type less?)
- thread `source_range` of source statement to where useful for logging (could do more in the future)
Reviewed By: da319
Differential Revision: D7950573
fbshipit-source-id: 2755f7d
Summary: Makes sense given that they share a lot of the same `Intent`-related sinks.
Reviewed By: mbouaziz
Differential Revision: D7877282
fbshipit-source-id: 38b2040
Summary:
Before we were computing the size of an abstract state (`range`) using the `NonNegativeBound` domain but it wasn't able to express product of symbolic values.
This diff introduces a domain for that.
The range of an interval is still computed in `NonNegativeBound` but then the product is done in `TopLiftedPolynomial` so all costs end up being of that type.
The //symbols// of a polynomial are `NonNegativeBound` (so the polynomial only represent non-negative values, perfect for a cost), which handles substitution correctly, i.e. it gives zero instead of negative values.
Reviewed By: ddino
Differential Revision: D7397229
fbshipit-source-id: 6868bb7
Summary:
The annotation UiThread can, and is, used on classes as opposed to just methods, so extend the modelling to account for this.
Also, consider the annotation hereditary.
Reviewed By: jeremydubreil
Differential Revision: D7910762
fbshipit-source-id: 0df2c81
Summary:
Java arrays have an internal length that can be retrieved with the internal `__get_array_length`.
Here is a model for it.
Reviewed By: jvillard
Differential Revision: D7931572
fbshipit-source-id: fd4c179
Dulma Churchill
Daiva Naudziuniene
Mehdi Bouaziz
Nikos Gorogiannis
Jeremy Dubreil
Brandon Kieft
Martino Luca
Dino Distefano
Jules Villard
Ezgi Çiçek
Sam Blackshear
Sungkeun Cho
Jose Blanco