Summary: It is common on Android code to recycle the `View` object by nullifying them in the `onDestroy()` or `onDestroyView()` methods. In this case, the outer `Fragment` object structure is preserve while the inner `View` object are set to null for the garbage collect to release the memory. However, if the fields are only set to `null` in the `onDestroy*()` methods, those fields cannot be `null` during the active lifecycle of the `Fragment`, so it is not necessary to annotate those fields with `Nullable`.
Reviewed By: mbouaziz
Differential Revision: D10024458
fbshipit-source-id: b05e538d9
Summary:
The method matcher is now used sufficiently it warrants refactoring out into its own module.
Also, kill dev-android-strict-mode and leave starvation-strict-mode as the stronger option.
Reviewed By: jeremydubreil
Differential Revision: D9990753
fbshipit-source-id: 626a70a19
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: 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:
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: 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: 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:
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: 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: 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:
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:
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
Summary:
When we see `pthread_create(..., ..., foo, ...)`, we want to call the function
`foo` to check that its precondition is met. The initial goal was to get rid
of the uncouth call to `Summary.get` when what we really want is to analyse
`foo` instead of just betting on the fact that it has been analysed already.
Besides switching to `Ondemand.analyze_proc_name`, this also changes the
matching of the function pointer in the arguments of `pthread_create()` to
detect the common case of a constant function name. I also added tests.
Reviewed By: jeremydubreil
Differential Revision: D9195159
fbshipit-source-id: dfec79f14
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: Errors that include temporary variables are difficult to understand. Do not report stack variable address escape on temporary variables.
Reviewed By: jvillard
Differential Revision: D9117517
fbshipit-source-id: 9ebd75ecc
Summary: Treat calls to Thread.sleep as blocking, even when the timeouts are less than the ANR limit.
Reviewed By: da319
Differential Revision: D9027950
fbshipit-source-id: 001409896
Summary:
It adds relational domains to Inferbo: octagon of Apron and polyhedra of Elina.
- Each Mem domain value includes one relational value containing relations among symbols. The relational values are modified by the `Prune` and `Store` commands.
- Each abstract value includes three symbols, which represent integer value, array offset, and array size of an abstract value.
The relational domain is deactivated by default. Use the `--bo-relational-domain {oct, poly}` option for the activation, though Inferbo with the relational domains does not work at this point because some modifications of Apron and Elina we made has not been applied to their opam repositories yet.
Reviewed By: jvillard
Differential Revision: D8874102
fbshipit-source-id: 08e5883cb
Summary:
Guava subclasses Future in ways that make .get() calls safe from the UI thread.
Treat such methods as skip.
Reviewed By: jeremydubreil
Differential Revision: D9013475
fbshipit-source-id: 38373aa5f
Summary:
Infer does the right thing now, make sure it doesn't regress.
https://github.com/facebook/infer/issues/86
Reviewed By: mbouaziz, dulmarod
Differential Revision: D8442855
fbshipit-source-id: 3df29b88c
Summary:
This previous FP does not occur anymore, make sure it doesn't regress.
https://github.com/facebook/infer/issues/49
Reviewed By: dulmarod
Differential Revision: D8442842
fbshipit-source-id: b3fde1ad1
Summary:
Litho uses an idiom as follows. `Component.Builder` has an abstract method `T getThis()` which is supposed to be implemented as `{return this;}`.
I believe the reason it's there is to have covariant return types in subclasses, though this doesn't really matter.
We don't do dynamic dispatch, so instead of summarising `getThis` as having a return ownership of `OwnedIf({ 0 })` we just return `Unowned`.
This is generating many false positives, which are really hard to debug.
Here I'm special-casing any abstract method whose return type is equal to that of it's only argument, to return conditional ownership.
Reviewed By: da319
Differential Revision: D8947992
fbshipit-source-id: 33c7e952d
Summary: Exceptional successors were not meant to be created for return nodes, but they were created if try block had a single return statement.
Reviewed By: jvillard
Differential Revision: D8913371
fbshipit-source-id: 6ac85b21d
Summary:
Currently, some blocking calls are turning up too many false positives. Adjust severities by fix rate/preexisting numbers.
Also, restrict some calls to exact class, as opposed to superclasses.
Reviewed By: mbouaziz
Differential Revision: D8850599
fbshipit-source-id: 47543d04a
Summary:
It adds relational domains to Inferbo: octagon of Apron and polyhedra of Elina.
- Each `Mem` domain value includes one relational value containing relations among *symbols*. The relational values are modified by the `Prune` and `Store` commands.
- Each abstract value includes three *symbols*, which represent integer value, array offset, and array size of an abstract value.
The relational domain is deactivated by default, so this diff should not make any differences in CI.
Use `--bo-relational-domain {oct, poly}` for the activation, though Inferbo with the relational domains does not work at this point because some modifications of Apron and Elina we made has not been applied to their opam repositories yet.
Reviewed By: mbouaziz, jvillard
Differential Revision: D8478542
fbshipit-source-id: 510ff53
Summary:
Do no computation of stability abstract state if not explicitly requested via the command line flag.
Also, simplify the reporting.
Reviewed By: jeremydubreil
Differential Revision: D8614885
fbshipit-source-id: 25dd9de
Summary:
The addresses of global variables do not need initialisation to exist and be valid as they are part of the code or data segment of the program. This means that taking the address of a global is not in itself a danger for SIOF. However, dereferencing such an address would be. In order to avoid false positives but avoid being too unsound, only ignore them when the address is taken only to set another global. The general case would require a more complicated abstract domain.
Fixes#866
Reviewed By: ngorogiannis
Differential Revision: D8055627
fbshipit-source-id: 92307b2
Summary:
These just point to expressions that we know how to translate.
Fixes#950
Reviewed By: mbouaziz
Differential Revision: D8713784
fbshipit-source-id: 9eafa39
Summary: The frontend got better and now `assert` is correctly translated.
Reviewed By: dulmarod
Differential Revision: D8731877
fbshipit-source-id: ba11b0c
Summary:
- Do not add actuals of a call as unstable.
- Replace access trie with simple set of paths, which is easier to debug/argue correct.
- Fix bug where a prefix path was searched, as opposed to a *proper* prefix.
- Restrict interface to the minimum so that alternative implementations are easier.
Reviewed By: ilyasergey
Differential Revision: D8573792
fbshipit-source-id: 4c4e174
Summary: C/C++ code can, in some cases, generate a large number of temporary (Sil) variables. Since we are already not reporting races on these, not recording them gives some perf back.
Reviewed By: mbouaziz, jvillard
Differential Revision: D8566999
fbshipit-source-id: 148ac46
Summary: Trying to convert a large int literal to an OCaml int raises an exception. The use case here actually needed a float anyway, so add an API for that.
Reviewed By: jeremydubreil
Differential Revision: D8550410
fbshipit-source-id: 382495b
Summary: Record that it is not supported: https://github.com/facebook/infer/issues/8
Reviewed By: mbouaziz, dulmarod
Differential Revision: D8442762
fbshipit-source-id: fa271cb
Summary:
I realized that control variable analysis was broken when we had multiple back-edges for the same loop. This is often the case when we have a switch statement combined with continue in a loop (see `test_switch` in `switch_continue.c`) or when we have disjunctive guards in do-while loops.
This diff fixes that by
- defining a loop by its loophead (the target of its backedges) rather than its back-edges. Then it converts back-edge list to a map from loop_head to sources of the loop's back-edges.
- collecting multiple guard nodes that come from potentially multiple exit nodes per loop head
In addition, it also removes the wrong assumption that an exit node belongs to a single loop head.
Reviewed By: mbouaziz
Differential Revision: D8398061
fbshipit-source-id: abaf288
Jeremy Dubreil
Nikos Gorogiannis
Martin Trojer
Mehdi Bouaziz
Julian Sutherland
Dino Distefano
Thomas M. DuBuisson
Daiva Naudziuniene
Martino Luca
Ezgi Çiçek
Sungkeun Cho
Jules Villard
Katie Ots
Dulma Churchill
Kai Yang