Summary:
We can fill the gaps in the trace now: they correspond to processes waiting on
pipes. This suggests a more efficient protocol would help perf, at least on the
small example I tried. Anyhow, it shows it's useful to trace pipe operations.
Some small gaps remain but they look like they could be explained by rounding errors.
Reviewed By: mbouaziz
Differential Revision: D9934437
fbshipit-source-id: 1d5f53a6d
Summary: They actually don't take very much time at all but it's good to know that they don't.
Reviewed By: mbouaziz
Differential Revision: D9832277
fbshipit-source-id: 7486fb40c
Summary: Use `PerfEvent` to record the execution time of individual checkers.
Reviewed By: jeremydubreil, mbouaziz
Differential Revision: D9832102
fbshipit-source-id: 678fca155
Summary:
This adds an option `--trace-events` that generates a Chrome trace event[1] to
quickly visualise the performance of infer.
Reviewed By: mbouaziz
Differential Revision: D9831599
fbshipit-source-id: 96a33c627
Summary:
The model for `getcwd` assumes the first argument should be non-null when in fact a NULL pointer is legitimate and results in allocation:
> As an extension to the POSIX.1-2001 standard, glibc's getcwd() allocates the buffer dynamically using mal‐
> loc(3) if buf is NULL. In this case, the allocated buffer has the length size unless size is zero, when buf
> is allocated as big as necessary. The caller should free(3) the returned buffer.
I suggest this glibc extension be used for the getcwd model to reduce false positives.
Pull Request resolved: https://github.com/facebook/infer/pull/925
Reviewed By: mbouaziz
Differential Revision: D9830450
Pulled By: jvillard
fbshipit-source-id: 95c4862b1
Summary:
Now we see which file/procedure/instruction is responsible for a crash in the
backend. Biabduction and eradicate not supported yet for the instruction-level
debug.
Reviewed By: mbouaziz, da319
Differential Revision: D9915666
fbshipit-source-id: 279472305
Summary:
Previously we wouldn't flush the formatter hence the error message would
generally not make it to the log file. Add the backtrace too, although only the
first few lines appear for some reason...
Reviewed By: ngorogiannis
Differential Revision: D9915499
fbshipit-source-id: 43cd9e36e
Summary: This fixes a flaky test where some issues would disappear and re-appear.
Reviewed By: da319
Differential Revision: D9027686
fbshipit-source-id: 5ac314096
Summary: Always read the attributes from the attributes DB instead of trying to read the attributes from the analysis summaries
Reviewed By: mbouaziz
Differential Revision: D9845085
fbshipit-source-id: aef48e6bf
Summary: No longer report inconsistencies with the annotations with subtyping when the super class is in an external packages since those warnings are not necessarily accurate or actionable.
Reviewed By: ezgicicek
Differential Revision: D9845098
fbshipit-source-id: 1f2bcd739
Summary: This allows Eradicate to detect more issues related to inconsistent annotations with sub-typing.
Reviewed By: ngorogiannis
Differential Revision: D9807306
fbshipit-source-id: 159d5d4e8
Summary: Display the errors report as red in the termninal, warnings as yellow, advice as blue and the like as green.
Reviewed By: mbouaziz
Differential Revision: D9828302
fbshipit-source-id: 30315eac2
Summary:
First version of differential for costs, based on polynomial's degree's variation. The rule is very simple:
For a given polynomial that is available before and after a diff, `if degree_before > degree_after`, then the issue becomes `fixed`. Instead, `if degree_before < degree_after`, then the issue becomes `introduced`.
Reviewed By: ezgicicek
Differential Revision: D9810150
fbshipit-source-id: d08285926
Summary: Buck is allowing compiler commands with no source files and skipping them when using the in-memory complier mode. However, those commands are not skipped when using an external compiler. Simulating this behavior at the level of Infer.
Reviewed By: mbouaziz, ngorogiannis
Differential Revision: D9795043
fbshipit-source-id: e80cfa453
Summary: There may be several reasons why we think a method is on the UI thread. Choose to keep the shortest via join.
Reviewed By: jeremydubreil
Differential Revision: D9806944
fbshipit-source-id: 89d27456d
Summary:
Callsites of `Reporting.log_error/warning` always use `Exceptions.Checkers`, let's simplify the API.
Under the hood it still creates an exception, but this can be cleaned up later.
Reviewed By: jeremydubreil
Differential Revision: D9799860
fbshipit-source-id: 6492a60b4
Summary: This feature is not currently used and crashes when enabled.
Reviewed By: ngorogiannis
Differential Revision: D9805110
fbshipit-source-id: db405c79e
Summary:
For some unexplained reason, some of the functions registered in the Epilogues would sometimes be executed several times. I could not figure out why.
This diff fixes that, but also has more explainable benefits:
- Do not run epilogues registered in the parent in the children. Previously it
would do so, but probably only if the children registered some epilogue given
that `at_exit` must be called again once on the child (but the value of the ref
in `Pervasives` would not have been reset).
- Unified behaviour for early and late epilogues given that we now handle both of these directly
We already have all the control needed to run epilogues when needed: we know
when infer exits, and we know when children processes exit.
Reviewed By: mbouaziz
Differential Revision: D9752046
fbshipit-source-id: 13af40081
Summary:
The constructor `` `Typ`` is never used to build values. Removing type
substitutions from Sil.ml had knock-on effect on Typ.ml etc., resulting in more
deleted code around type substitutions \o/
Reviewed By: mbouaziz
Differential Revision: D9769340
fbshipit-source-id: 509cbd284
Summary: Sometimes it's very confusing to see why infer believes a method is running on the UI thread. Make a trace out of all the relevant info.
Reviewed By: mbouaziz
Differential Revision: D9781212
fbshipit-source-id: 6d018e400
Summary:
Turn off by default until mature enough.
Also rename the dev-strict-mode test dir to highlight the dev part.
Reviewed By: mbouaziz
Differential Revision: D9775571
fbshipit-source-id: c3a41bbdf
Summary:
First step in writing an analyzer that is meant to run only on Android core library implementation.
This will, when finished, compute the library entrypoints that may lead to a strict mode violation.
The normal analyzer will use those to statically flag strict mode violations in app code.
Strict Mode is an Android debug mode, where doing certain things (like disk read/write or network activity) on the UI thread will raise an exception. We want to statically catch these, as well as indirect versions (the UI thread takes a lock and another thread holding that lock calls a method that would be a strict mode violation).
Reviewed By: mbouaziz
Differential Revision: D9634407
fbshipit-source-id: c30bcedb3
Summary:
It detaches the Summary module from BufferOverrunDomain.
Depends on D9194130
Reviewed By: jvillard
Differential Revision: D9194375
fbshipit-source-id: 30392b5ce
Summary: It simplifies instantiataion of `ret_alias`. While it got `ret_alias` values by iterating caller's and callee's memory, now it gets `ret_alias` by evaluating symbol paths included in location values.
Reviewed By: mbouaziz
Differential Revision: D9569606
fbshipit-source-id: a3326bb81
Summary: Now that the def file is stored in the issue type (hence in the issue desc), no need for it here any more.
Reviewed By: martinoluca
Differential Revision: D9654109
fbshipit-source-id: 0b3c413bf
Summary:
- Let's call `IssueType.from_string` once only
- Use properly defined issue types for builtin linters
Reviewed By: martinoluca
Differential Revision: D9654105
fbshipit-source-id: 947b50a51
Summary: This list is built once only, let's avoid exposing it.
Reviewed By: jeremydubreil
Differential Revision: D9654091
fbshipit-source-id: d92f91329
Summary:
Now that we got rid of dummy nodes used non-dummily (biabduction state, reporting), `pname` don't need to be an option anymore.
Let's save a boxing on all nodes.
Reviewed By: jeremydubreil
Differential Revision: D9654152
fbshipit-source-id: 83b00f239
Summary:
Using a dummy node here made the whole reporting wrong because it didn't fail getting a `node_key` when reporting issues from checkers not using the biabduction state.
Now that it's fixed, let's fail hard if someone ever tries again.
Reviewed By: jeremydubreil
Differential Revision: D9654137
fbshipit-source-id: c00273e53
Summary: No dummy node key, as a consequence the option `--skip-duplicated-types` will have no effect on issues with no node key, i.e. issues reported by non-biabduction non-eradicate checkers.
Reviewed By: martinoluca
Differential Revision: D9633564
fbshipit-source-id: 9ff8abf21
Summary: We had a special case for fixing false positives on constexpr implicitly captured by lambdas. However, we do not report dead stores on constexpr anymore, hence, do not need the special case anymore. Moreover, the special case was not only capturing constexpr in lambdas, but also any variables which type had `const` (see new test `capture_const_bad` which was not being reported before this diff)
Reviewed By: mbouaziz
Differential Revision: D9654848
fbshipit-source-id: 882fd2804
Summary:
It simplifies abstract memory instantiations of function calls. Now it instantiates callee memories by directly evaluating symbol paths, rather than constructing `subst_map`.
main changes are:
- no construction of `subst_map` and `trace_map`
- no symbol table in Inferbo's summary
- no `Symbol_not_found` exception (for when a required symbol was unavailable in `subst_map`)
Reviewed By: mbouaziz
Differential Revision: D9495597
fbshipit-source-id: 18cdcd6f7
Summary:
Separate and rename error reporting functions that use the biabduction state.
No checkers should call these functions.
Reviewed By: da319
Differential Revision: D9633579
fbshipit-source-id: 884fcee66
Summary: We report dead store false positives in template arguments when constexpr is used. To remove the false positives, with the expense of some false negatives, we do not report dead stores on constexpr anymore.
Reviewed By: mbouaziz
Differential Revision: D9608095
fbshipit-source-id: 91b0c71c4
Summary: When a typedef-ed structure is defined in another source file, `tenv` returns a structure with empty fields.
Reviewed By: mbouaziz
Differential Revision: D9629200
fbshipit-source-id: 8859803f9
Summary:
Lambdas can capture references to locals of the enclosing method as long as
they are not propagated outside the method. However to keep things simple
always allow them to capture locals of the enclosing method at the price of
some false negatives.
Reviewed By: da319
Differential Revision: D8974434
fbshipit-source-id: 957ae44bd
Summary:
- Was not used by the caller
- Gives smaller summaries
- Will allow adding a intra-proc info, e.g. `node` for reporting (not sure yet)
Reviewed By: skcho
Differential Revision: D9373763
fbshipit-source-id: 322001b53
Summary: The pattern matching could previously be missing some valid cases (in theory).
Reviewed By: mbouaziz, jberdine
Differential Revision: D9491441
fbshipit-source-id: 2bc1fc1aa
Summary:
In SIL, (1) some program variables (e.g., array parameter) are used as pointers to heap addresses and (2) the other program variables (e.g., local array) are used as addresses themselves. So, the values of (1) are retrieved by the `Load` command, while that of (2) are by `Exp.Lvar` expressions directly.
To address them differently, we had managed two maps (`Mem.Stack` and `Mem.Heap`), but which introduced function duplications on abstract memory and increased complexity. This diff merges the two maps, and instead a location set is used for distinguishing two types of abstract locations during analysis.
Reviewed By: mbouaziz
Differential Revision: D9420388
fbshipit-source-id: 13f824850
Summary: It is easier to filter out those reports in `.inferconfig` if we want them that modifying a boolean value ddirectly in the code
Reviewed By: mbouaziz
Differential Revision: D9494082
fbshipit-source-id: 9fb042313
Summary: There no clear alternative to using models of the standard library at this point so we can simplify the code a little bit
Reviewed By: mbouaziz
Differential Revision: D9491062
fbshipit-source-id: 9e5a6eeea
Summary:
It returns unknown values on non-const function calls like on unknown
function calls.
Reviewed By: mbouaziz
Differential Revision: D9478862
fbshipit-source-id: 4b795ec55
Summary:
Not all clang commands are happy with all arguments, but the driver is usually
the place we want to add arguments to.
Reviewed By: martinoluca
Differential Revision: D9421403
fbshipit-source-id: fa6d39a9b
Summary: `CONDITION_ALWAYS_**` can be introduced by global constants.
Reviewed By: ezgicicek, mbouaziz
Differential Revision: D9478528
fbshipit-source-id: 7b1a46e7a
Summary:
After some testing, it looks like getting the pdesc via
`Ondemand.get_proc_desc` will also load models' proc descs from their
summaries, so this code should not be needed.
Reviewed By: jeremydubreil, mbouaziz, martintrojer
Differential Revision: D9197176
fbshipit-source-id: 1b8603bfa
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 id is likely to be trucated with random CRC values so we should probably not use it to match Java names
Reviewed By: mbouaziz
Differential Revision: D9183518
fbshipit-source-id: b450c7869
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: C++17 introduce guaranteed copy elision which omits constructor calls. In ownership analysis, we depended on these constructor calls to acquire ownership. In particular, when a method returns struct, previously, a constructor was used to acquire ownership. In this diff, we acquire ownership of the returned structs directly.
Reviewed By: mbouaziz
Differential Revision: D9244302
fbshipit-source-id: ae8261b99
Summary:
Useful tips and tricks to debug infer's OCaml code. Also emit a developer
warning when the database is not initialised (since it's only expected to
happen when running infer from the toplevel).
Reviewed By: mbouaziz
Differential Revision: D9295782
fbshipit-source-id: 09b7b9a02
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: This test was not re-run when the Java dependencies were changing
Reviewed By: mbouaziz
Differential Revision: D9238288
fbshipit-source-id: 65cc9c03c
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: It moves the functions that constructs array values from BufferOverrunSemantics to ArrayBlk and Val modules.
Reviewed By: mbouaziz
Differential Revision: D9194130
fbshipit-source-id: bf040a01a
Summary:
It removes the sizeof function because most of the cases on static types are addressed in the clang frontend.
Depends on D9193802
Reviewed By: mbouaziz
Differential Revision: D9213876
fbshipit-source-id: 0ce2f3749
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:
Some paths are hardcoded in infer as being relative to the current executable,
for instance the directory where to find the models. By copying infertop.bc to
infer/bin like we do for `infer` these relative paths lead to the expected
place, which means models can be loaded in the toplevel like they would be in a
normal infer execution. This is more useful for debugging than previously.
Reviewed By: jeremydubreil, mbouaziz
Differential Revision: D9197142
fbshipit-source-id: 48c4f82fb
Summary:
Using the proc name directly should be equivalent and does not rely on
`Summary` caching summaries for us.
Reviewed By: mbouaziz
Differential Revision: D9196503
fbshipit-source-id: dea67999a
Summary:
We want to kill the cache in `Summary`, and calling `Summary.get` relies on
that cache existing for efficiency. However in this case it's not needed
because we can pass the summary from above instead.
Reviewed By: mbouaziz
Differential Revision: D9195234
fbshipit-source-id: 5b7023242
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 makes sure that the javac_jar is disabled when setting the external compiler option to point at the Infer wrapper
Closes#976
Reviewed By: jvillard
Differential Revision: D9193336
fbshipit-source-id: abafb51fc
Julian Sutherland
Dino Distefano
Jules Villard
Jeremy Dubreil
Mehdi Bouaziz
Martin Trojer
Thomas M. DuBuisson
dWiGhT
Nikos Gorogiannis
Sungkeun Cho
Daiva Naudziuniene
Martino Luca
Andrey Epin
Ezgi Çiçek
Josh Berdine