Summary:
Looking at the recent silent analysis results, it seems that we report many direct unknown library calls (often cheap)... However, if these were called inside some other callee we wouldn't report them because their costs would be assumed to be constant by the cost analysis.
This is a bit awkward. We should either report all unknown calls or suppress them altogether.
Since we have too many reports per day and not a good way to determine whether an unknown library call is cheap/expensive, let's take option 2.
Then, we would be only relying on two things to determine whether to report/not:
- instantiated cost's degree > 1
- explicitly known to be expensive (i.e. modeled in ConfigImpact, like string append)
Reviewed By: skcho
Differential Revision: D27909003
fbshipit-source-id: 0391d226d
Summary:
The configuration options for the analysis are used only/principally
in Control, they do not belong in the interface of domains. Also, the
definition of the used_globals type for the results of the used
globals pre-analysis belongs to the Domain_used_globals module.
Reviewed By: jvillard
Differential Revision: D27828752
fbshipit-source-id: e42de74e0
Summary:
Just reorder definitions to clarify that as_inlined_location is not
used in the rest of Stack.
Reviewed By: jvillard
Differential Revision: D27828755
fbshipit-source-id: 1436f1e6d
Summary: Preparation for allowing a choice among several scheduler strategies.
Reviewed By: jvillard
Differential Revision: D27828759
fbshipit-source-id: 63d6ec203
Summary:
The priority queue does not crucially depend on the type of
elements. This diff makes it parametric.
Reviewed By: jvillard
Differential Revision: D27828756
fbshipit-source-id: a7bfc4ee5
Summary:
When domain join operations are total, the control scheduler does not
need to handle the case where joining states is undefined. This leads
to some simplification, and in particular removed the need to expose a
remove operation for the scheduling queue.
Reviewed By: jvillard
Differential Revision: D27828761
fbshipit-source-id: b8cdd2eb6
Summary:
The only domain with a partial join is the lifting of a predicate
domain to a relation one, where the entry states are required to be
equal. This situation now indicates a programming error in the
analysis, rather than something that the domain should be responsible
for. Therefore this diff changes that check to an assertion and
simplifies the remaining join operations which are all total.
Reviewed By: jvillard
Differential Revision: D27828763
fbshipit-source-id: ec52ff741
Summary:
llvm-config can return the lib dir without manual manipulation using
an assumed relative path from llvm-config to the lib dir. Also fix
missing semicolon to appease shell variable semantics.
Reviewed By: jvillard
Differential Revision: D27828757
fbshipit-source-id: 5080c8671
Summary:
Mainly code cleanup. The only potential functional change is to
eliminate the test that a function returning with an empty stack is an
entry point, which should always hold anyhow.
Reviewed By: jvillard
Differential Revision: D27828749
fbshipit-source-id: 25124a568
Summary:
Mainly code simplification. The only functional change is to move the
test that a candidate entry-point function has no formals earlier,
with the result of avoiding failure if a function with an entry-point
name but with formals is encountered before an entry-point function
with no formals.
Reviewed By: jvillard
Differential Revision: D27828751
fbshipit-source-id: d5a832952
Summary:
There used to be a bug where recursive calls were not correctly
bounded. This is no longer needed, and this diff removes the
workaround that was added in D15577134 (881a4d10af) / 4cd3b62.
Reviewed By: jvillard
Differential Revision: D27828764
fbshipit-source-id: 80265a588
Summary:
Specifying that sledge should stop execution when the first report is
made via e.g. `sledge l a -t Stop` currently only works when debug
tracing is enabled. This diff fixes this so that `-t Stop` also works
for the release build.
Reviewed By: jvillard
Differential Revision: D27828753
fbshipit-source-id: dd7511ff1
Summary:
Currently all alarms are reported as "Invalid memory access", which is
not accurate for `abort` and hence assertion violations. This diff
adds an explicit type for alarms which distinguishes these two
cases. Further refinement is left for later.
Reviewed By: jvillard
Differential Revision: D27828754
fbshipit-source-id: 9c33f3c86
Summary:
This is mostly useful to avoid duplicating error states, which are
propagated unchanged through both branches of, say, conditionals, and
can end up duplicated if the join is not careful:
```
{[Abort(Error 1), Abort(Error 2), Continue σ']}
if (..) { .. } else { .. }
{JOIN([Abort(Error 1), Abort(Error 2), Continue σ_then],
[Abort(Error 1), Abort(Error 2), Continue σ_else])}
{[Abort(Error 1), Abort(Error 2), Continue σ_then, Continue σ_else]}
```
Whereas before this diff we got
```
{[Abort(Error 1), Abort(Error 2), Continue σ_then, Abort(Error 1), Abort(Error 2), Continue σ_else]}
```
Detect states that do not change simply using `phys_equal` as they
should literally not change. Refactor the code to be able to re-use the
same logic in the stronger join used in widening, that compares states
using the domain's `leq` relation to establish implication.
Reviewed By: ezgicicek
Differential Revision: D27908529
fbshipit-source-id: b461165da
Summary:
When a block value is passed via more than one-depth of function calls, it is not analyzed correctly
because current inlining mechanism (specializing objc block parameters) of the frontend works for
only one-depth of block passing. This diff gives up analyzing initialized-ness of captured
variables in ObjC to avoid FPs.
Reviewed By: da319
Differential Revision: D27885395
fbshipit-source-id: fc6b4663c
Summary: We have some FPs due to unknown init methods that are added dynamically.
Reviewed By: ezgicicek
Differential Revision: D27856371
fbshipit-source-id: b6fb46df3
Summary:
This diff filters known expensive callees when cost is constant.
previous:
```
foo() {
known_expensive_call();
}
```
current:
```
foo() {
known_expensive_call();
goo();
}
// cost is constant
goo() {
known_expensive_call();
unknown_call();
}
```
When callee's cost is constant and its summary includes known expensive callees, the checker addressed it as a non-constant-cost callee, i.e., it copies all ungated callees from the callee's summary. However, sometimes this full-copying introduces unexpected issues. For example, suppose a callee `goo` is added and `goo`'s cost is constant as above. Since it includes `known_expensive_call`, all ungated callees of its summary is copied to the caller `foo`'s summary:
* `foo`'s ungated callees (before): {`known_expensive_call`}
* `foo`'s ungated callees (after): {`known_expensive_call`, `unknown_call`}
As a result, it would report about `unknown_call` is added. However, this is not what we intended: In the example, `unknown_call` is reported because it is called in the same function with `known_expensive_call`, not because it is expensive.
To fix that issue, this diff filters known expensive callees from `goo`'s summary in that case.
Reviewed By: ezgicicek
Differential Revision: D27852552
fbshipit-source-id: d207eef1c
Summary:
It introduced a FP due to reporting addition of `__cast`.
* This diff added known cheap model for `__cast`.
* In addition, moved `match_builtin` to `BuiltinDecl`.
Reviewed By: ezgicicek
Differential Revision: D27791495
fbshipit-source-id: 55aec1728
Summary:
Previously we were only taking constexpr into account on constructors.
Add this info to ProcAttributes.t instead by exporting it from the
plugin for all functions.
This allows SIOF to take constexpr into account in more cases as it's not
always good at capturing which functions *can* be constexpr-evaluated,
which caused false positives.
Delete now-useless is_constexpr in constructor types. This generated the
changes in frontend tests.
Some minor renamings of variants of is_const_expr -> is_constexpr.
Reviewed By: da319
Differential Revision: D27503433
fbshipit-source-id: 3d1972900
Summary:
When a field is assigned by a value,
```
_.field = exp;
```
it should collect the field when the abstract config value of `exp` is non-bottom, rather than
non-top.
Reviewed By: ezgicicek
Differential Revision: D27766188
fbshipit-source-id: a0b1f2c28
Summary: Added a new issue type for sending a message to nil when its return type is non-POD. To distinguish these issues from other nullptr dereference issues, we extend the `MustBeValid` attribute to contain the reason of why an address must be valid. For now a reason can only have `SelfOfNonPODReturnMethod` as it's value, but in the future we will use it for other nullability issue types, such as nil insertion into collections.
Reviewed By: jvillard
Differential Revision: D27762333
fbshipit-source-id: 689e5a431
Summary:
In order to use Inferbo's analysis result, a checker should know current instruction index.
However, for the checkers using `ProcCfg.Normal` CFG, it was impossible to get the instruction
index. To solve the issue, this diff changes the AbsInt framework to give the index together to
`exec_instr`.
Reviewed By: ezgicicek
Differential Revision: D27680894
fbshipit-source-id: 1dc8ff0fb
Summary:
The problem is that `Models.is_field_nonnullable` didn't differentiate
between
- having a nullable model (in which case we want the field to be NULLABLE),
- not having a model at all (in which case we want the field to be THIRDPARTY_NONNULL).
The problem was noticed only now because previously we didn't have any
NULLABLE field models.
Reviewed By: ngorogiannis
Differential Revision: D27709508
fbshipit-source-id: b98c8f86f
Summary:
This diff evaluates a cpp vector given as a parameter symbolically. Especially, it addresses it as an array, so the cost checker can use its symbolic length correctly.
**About handling `cpp.vector_elem` field:**
The field is a virtual field of vector object that points to the array of vector elements. It was introduced in Inferbo to model semantics of vector operations.
Since many semantics of Inferbo depends on type information, it had collected type information of vector elements, whenever `cpp.vector_elem` field was introduced, as a *side-effect*. A problem is that it has *side-effect*, which means it may introduce non-deterministic analysis results depending on the type information of the virtual field.
This diff changes it not to collect the type information on `cpp.vector_elem` as a side-effect. Instead, it tries to write the information to the abstract states (abstract location) when possible.
Reviewed By: ezgicicek
Differential Revision: D27674935
fbshipit-source-id: f3d52cae7
Summary:
In D27430485 (a6ab4d38cf), we used the static cost of the callee to determine whether it was cheap/expensive. This diff improves on that by taking the whole instantiated cost of the function call (not just the callee's cost).
Also, if the callee is an unmodeled call, we consider it to be expensive as before.
Note: cost instantiation was used by hoisting. I refactored bunch of code there to reuse as much as code possible.
Reviewed By: skcho
Differential Revision: D27649302
fbshipit-source-id: 07d11f3dd
Summary: When instantiating the callee's cost, we have picked up the InferBo memory at the node corresponding to the last instruction. Instead, we should pick up right at the call instruction. Picking it up later might cause arguments to go out of scope.
Reviewed By: skcho
Differential Revision: D27652474
fbshipit-source-id: 5ab35cabb
Summary:
The output differs on Java 11 compared to Java 8: one prints an
interface, the other resolves to a class name.
Reviewed By: ezgicicek
Differential Revision: D27678552
fbshipit-source-id: c5a5d0c39
Summary:
The computation of common prefixes and suffixes was wrong. In
particular, the computation of the common suffix did not correctly
consider the common prefix. This manifested in case one entire
sequence is a suffix of the other.
Reviewed By: ngorogiannis
Differential Revision: D27564874
fbshipit-source-id: 267a75102
Summary:
If either all or no tracing is enabled, it is not necessary to compute
the module and function names for each Trace operation to determine
whether or not it is enabled. This diff optimizes this case.
Reviewed By: ngorogiannis
Differential Revision: D27564869
fbshipit-source-id: bf766eb2f
Summary:
Switch to a stronger but more expensive operation to check if a
symbolic heap is unsatisfiable.
Reviewed By: ngorogiannis
Differential Revision: D27564872
fbshipit-source-id: c8e306408
Summary:
By default compiling with `-O0` adds `optnone` annotations, which
prevents the transformations sledge explicitly performs. This diff
adds a flag to disable this uncooperative behavior.
Reviewed By: ngorogiannis
Differential Revision: D27564879
fbshipit-source-id: 061397e3f
Summary:
`Typ.equivalent` relates types that denote the same sets of values in
the semantic model, such as pointers and integers of the appropriate
size. This diff strengthens the treatment of casts between such types
in the first-order solver by translating `(s)(t)e` to `e` for
equivalent types `s` and `t`.
These casts are usually simplified out of the bitcode produced by
clang. However, code using `_Atomic(...)` leads to `load atomic` llvm
instructions that, for some reason, cast pointers to i64 and back.
Reviewed By: ngorogiannis
Differential Revision: D27564881
fbshipit-source-id: 6138eb4f1
Summary:
When solving equations such as `α[o,l) = β`, the memory theory solver
computes a term to represent the length of α. This fails if α is a
variable (which might itself be equal to a sized term elsewhere in the
formula). This diff fixes such failures by generating a fresh variable
for the length in such situations.
Reviewed By: ngorogiannis
Differential Revision: D27564871
fbshipit-source-id: e5e066b77
Summary:
It is desirable to programmatically enable backtrace printing in debug
mode, but in other modes it should not forcably disabled. For example,
after this diff, executing
```
$ OCAMLRUNPARAM=b sledge.trc ...
```
now prints a backtrace if an unhandled exception is raised.
Reviewed By: ngorogiannis
Differential Revision: D27564883
fbshipit-source-id: 66ea5efe2
Summary:
This diff `#define`s the functions to which `assert` compiles to
`abort`. This enables the analyzer to report assertion violations, and
is independent of platform.
Also add a simple test to check that it is detected correctly.
Reviewed By: ngorogiannis
Differential Revision: D27564870
fbshipit-source-id: 5517ca910
Summary:
The removal of CAMLprim left the code in need of an application of
clang-format. There are various other changes made by clang-format
which it seems ought to be rolled together into this diff.
Upstream Differential Revision: https://reviews.llvm.org/D99477
Reviewed By: ngorogiannis
Differential Revision: D27564876
fbshipit-source-id: 8e22eb099
Ezgi Çiçek
Josh Berdine
Jules Villard
Sungkeun Cho
Artem Pianykh
Loc Le
Daiva Naudziuniene
Roman Manevich
Radu Grigore