Summary: The `NonBlocking` annotation should zero out all domain elements that represent blocking calls. The current implementation only really removes such elements when they are generated by the current method under analysis, leaving such elements from callees unaffected. This diff fixes that.
Reviewed By: jvillard
Differential Revision: D26874704
fbshipit-source-id: 2d4859b30
Summary:
This changes the results. I think it's because we cut short paths to
ISL errors sooner now, before they are duplicated and moved. I could not
really assess what was going on though so could be wrong.
On OpenSSL 1.0.2d:
Before: 106 issues
After: 90 issues
Reviewed By: ezgicicek
Differential Revision: D26822331
fbshipit-source-id: e861e7fc2
Summary:
This will enable further improvements: basically we want to be able to
abort the symbolic execution of a single disjunct whenever an error is
detected. Right now there is only one kind of error, which is now
explicitly called `ReportableError`.
The next diff refactors Pulse.ISL to add its own error type so that we
are able to get rid of the isl_status field (ISLOk/ISLError) inside
abductive states. ISLError states are really `Error _` states but
previously it would have been too much of an API change to expose that.
Now it's all going to be part of `AccessResult.t`.
A further change will add another error type for when a value is found
to be 0 after the fact by the arithmetic.
Reviewed By: ezgicicek
Differential Revision: D26821178
fbshipit-source-id: 2923db8e7
Summary:
It makes more sense to return a list of results than a result of lists:
the latter stops the execution on *all* the disjuncts that would have
been in the list as soon as *one* of them fails. This is the same issue
we solved for non-ISL pulse models earlier.
Reviewed By: skcho
Differential Revision: D26818409
fbshipit-source-id: 7cc1d8b39
Summary: In ObjC, when a method is called on nil, there is no NPE, the method is actually not called and the return value is 0/false/nil. There is an exception in the case where the return type is non-POD. In that case it's UB and we want to report an error.
Reviewed By: jvillard
Differential Revision: D26815687
fbshipit-source-id: 8126414ab
Summary: We were missing a part of the trace if it was going through a nil summary as the invalidation was set in the nil summary. Instead of creating a fresh value for return in the nil summary {self=0}{return=0}, we return self {self=0}{return=self}. This way we keep all the information about invalidation in the trace.
Reviewed By: jvillard
Differential Revision: D26871098
fbshipit-source-id: 6eb175e68
Summary:
Providing model for the android function TextUtils.isEmpty(). For now,
this always returns false assuming that the given value is not null.
Reviewed By: jvillard
Differential Revision: D26779619
fbshipit-source-id: 3d8e26813
Summary: Adding support for the Java instanceof operator in Pulse.
Reviewed By: jvillard
Differential Revision: D26275046
fbshipit-source-id: 8ba608cca
Summary:
This diff runs `infer reportdiff` on config impact results, ie previous and current
`config-impact-report.json`s. Ungated and added/removed callees are reported at `introduced.json`.
Reviewed By: ezgicicek
Differential Revision: D26723054
fbshipit-source-id: efabd0d5f
Summary: Adding temporary model for Collections/Map isEmpty() as an attempt to reduce false positives before we provide a full model for Collections.
Reviewed By: ezgicicek
Differential Revision: D26724085
fbshipit-source-id: d3418c173
Summary: `STARVATION` is currently used as a catch-all for several blocking events. This diff splits out `IPC_ON_UI_THREAD`.
Reviewed By: skcho
Differential Revision: D26691868
fbshipit-source-id: 618423793
Summary:
Only register biabduction-style timeouts the first time the function
exe_timeout is called. This avoid getting timeouts in other
long-running analyses. (Especially on windows, where the wall clock is
used.)
Pull Request resolved: https://github.com/facebook/infer/pull/1391
Reviewed By: skcho
Differential Revision: D26780445
Pulled By: jvillard
fbshipit-source-id: 19631b702
Summary:
gcc warnings are more strict starting from gcc10. Not having the const
qualifier triggers an error.
Pull Request resolved: https://github.com/facebook/infer/pull/1393
Reviewed By: skcho
Differential Revision: D26780417
Pulled By: jvillard
fbshipit-source-id: 4507c55eb
Summary:
It removed the result directory without check due to a bug. This diff fixes it to do the check and more careful removement of files.
* Before removing a result directory, it checks
* if the directory is empty: This is ok.
* if the directory is non-empty, but has `results.db`: This is ok, because the directory seems to be an infer's result directory.
* if the directory is non-empty and no `results.db`: This is not ok, so it does not remove the directory.
Reviewed By: jvillard
Differential Revision: D26635059
fbshipit-source-id: fa808265f
Summary:
This diff uses config-impact-issues.exp instead of issues.exp, like in
the cost checker.
Reviewed By: ezgicicek
Differential Revision: D26723761
fbshipit-source-id: 9c6779479
Summary:
Adapting existing model for `new` used in ObjC to Java.
This allows to compute dynamic type information and will facilitate
handling `instanceof`, for instance.
Changing attribute value type from Typ.Name.t to Typ.t to handle arrays.
Reviewed By: da319
Differential Revision: D26687839
fbshipit-source-id: 2cfcd0625
Summary:
Difficult to repro as most of the time other simplifactions catch this
before we actually get to dividing by zero. Nonetheless...
shamecube
Reviewed By: da319
Differential Revision: D26758187
fbshipit-source-id: b8718c515
Summary: Instead of accumulating all reports for a location in a list and then partitioning that list by issue type, just use a map from issue types to report lists.
Reviewed By: ezgicicek
Differential Revision: D26748929
fbshipit-source-id: 81c35cd4e
Summary: Now that all reports are deduplicated using the same criterion (trace length), use that to simplify deduplication functions.
Reviewed By: skcho
Differential Revision: D26726239
fbshipit-source-id: 77e3b319a
Summary:
A "severity" level was used for keeping the highest severity issue when deduplicating across many issues on the same location. This was used for only one of the issue types reported by the analysis.
It turned out this isn't very useful and it complicates significantly the reporting code.
This diff removes the type and uses trace depth to sort all the issue types in the same way all other issue types than `starvation` used.
Next diff will remove now unnecessary stuff.
Reviewed By: skcho
Differential Revision: D26725569
fbshipit-source-id: f9287dcd1
Summary:
The config impact checker prints ungated callees in a separate file config-impact-report.json,
because its results should be compared before actual reporting as the cost checker does.
Reviewed By: ezgicicek
Differential Revision: D26665097
fbshipit-source-id: 0c6e13403
Summary:
The javax.crypto.Mac classes behaves like a container
and can lead to race conditions when used in a concurrent context.
This adds Mac operations as container writes/reads in RacerD's models.
Pull Request resolved: https://github.com/facebook/infer/pull/1395
Test Plan: CI
Reviewed By: skcho
Differential Revision: D26722737
Pulled By: ngorogiannis
fbshipit-source-id: 74f03e9a5
Summary: This diff changes the compare function of `UncheckedCallee` not to distinguish direct/indirect call.
Reviewed By: ngorogiannis
Differential Revision: D26722968
fbshipit-source-id: f83f4de10
Summary: When a method is called in ObjC on nil, there is no NPE, the method is actually not called and the return value is 0/false/nil. (There is an exception in the case where the return type is non-POD. In that case it's UB. This will be addressed later). To implement this behaviour we add additional summary to ObjC instance methods {self = 0} {return = 0}. We also want to make sure that inferred summary will not be used in we call a method on nil, hence, we add a path condition {self > 0} to get a contradiction when needed.
Reviewed By: jvillard
Differential Revision: D26664187
fbshipit-source-id: cdac2a5bb
Summary:
In the following diff, we will add `JsonConfigImpactPrinter` that will share some common code base
with `JsonCostsPrinter`. This diff prepares the sharing.
Reviewed By: jvillard
Differential Revision: D26665070
fbshipit-source-id: 5032e0611
Summary:
In some corner cases where there are a lot of sequential statements, it raised stack overflow. To
avoid the issue, this diff changes the function as tail recursive by passing a callback function.
Reviewed By: ngorogiannis
Differential Revision: D26668338
fbshipit-source-id: 822d9a5f8
Summary:
Currently, we report on all functions that are not config checked. However, the aim of the analysis is to only report on these for specific functions. Moreover, this has performance implications in practice.
This diff instead reports on functions that occur on a json file that is passed by the command line option `config-data-file`.
Reviewed By: skcho
Differential Revision: D26666336
fbshipit-source-id: 290cd3ada
Summary:
As a first step to support the Java `instanceof`
operator, this change allows the path condition to be appended with
`IsInstanceOf(var, typ)`.
Reviewed By: jvillard
Differential Revision: D26664009
fbshipit-source-id: cd19dce83
Summary:
See added tests. Passing a variable by reference to a function `foo` can
cause the variable to be added to the global state so any stores after
that might be live as long as there is another function call after the
store (since the global state shouldn't outlive the scope of the
function). Currently we don't check that the latter is true; to report
these we would need to extend the abstract domain to remember which
stores have been made without a subsequent call.
Also change Blacklisted -> Dangerous everywhere since the corresponding
option is called "liveness_dangerous_classes".
Reviewed By: skcho
Differential Revision: D26606151
fbshipit-source-id: e869e5df1
Summary:
Providing model for Java `instanceof` operator that
avoids to return true when given object is null. This is a temporary
solution that will reduce FPs while we do not provide the correct
semantics for `instanceof`.
Reviewed By: jvillard
Differential Revision: D26608043
fbshipit-source-id: 87c82b906
Summary: We forgot to add `objc_fb-gk-interaction` to Makefile which could cause its test failures to go undetected since we didn't run them on CI.
Reviewed By: skcho
Differential Revision: D26612620
fbshipit-source-id: 0b9c7edd1
Summary: If we record all callees with empty summary, we end up with FPs. This diff instead only records leaf calls. for non-leaf calls, we just load the summary.
Reviewed By: skcho
Differential Revision: D26606228
fbshipit-source-id: 77e76ee9e
Summary: This diff finds a declared variable name or declared field names from trace, then constructs an error message including access paths.
Reviewed By: jvillard
Differential Revision: D26544275
fbshipit-source-id: 135c90a1b
Summary:
This diff makes the analysis inter-procedural.
* If a callee is called in a config check branch, it does nothing.
* if a callee is called outside config check branches,
* If callee's summary is empty, add the callee's name to the set of unchecked callees.
* If callee's summary is not empty, join the summary to the set of unchecked callees. (We intentionally don't add the callee's name here.)
Reviewed By: ezgicicek
Differential Revision: D26465235
fbshipit-source-id: ac3ad3543
Nikos Gorogiannis
Jules Villard
Ezgi Çiçek
Daiva Naudziuniene
Gabriela Cunha Sampaio
Sungkeun Cho
Boris Yakobowski
Andrey Makeev
Daniel Schoepe
Josh Berdine