Summary: CFG nodes were not connected and some instructions ended up in wrong place. Fix those issues
Reviewed By: dulmarod
Differential Revision: D5406720
fbshipit-source-id: 2a70e1a
Summary:
Conversion and reformat of infer source using ocamlformat
auto-formatting tool.
Current status:
- Because Reason does not handle docstrings, the output of the
conversion is not 'Warning 50'-clean, meaning that there are
docstrings with ambiguous placement. I'll need to manually fix
them just before landing.
Reviewed By: jvillard
Differential Revision: D5225546
fbshipit-source-id: 3bd2786
Summary:
This diff tries to achieve the followings: if we have the following C++ codes:
```
bool foo(int x, int y) {
return &x == &y;
}
```
We want the C++ frontend to emit Sil as if the input is written as
```
bool foo(int x, int y) {
if (&x == &y) return 1; else return 0;
}
```
This matches the behavior of our Java frontend.
The reason why we prefer an explicit branch is that it will force the backend to eagerly produce two different specs for `foo`. Without the explicit branch, for the above example the backend would produce one spec with `return = (&x == &y)` as the post condition, which is not ideal because (1) we don't want local variables to escape to the function summary, and (2) with the knowledge that no two local variables may alias each other, the backend could actually determines that `&x == &y` is always false, emitting a more precise postcondition `return = 0`. This is not possible if we do not eagerly resolve the comparison expression.
Reviewed By: akotulski
Differential Revision: D5260745
fbshipit-source-id: 6bbbf99
Summary:
Change the API of `Logging` wrt to writing to files and to the console (see
changes in logging.mli).
Write only to one log file: infer-out/log. Prefix each line with the kind of
warning and the PID of the process emitting it. Writing with `O_APPEND` is
atomic so the file should not get garbled by concurrent writes. To get the
output of a single process, find out which one interests you by looking at
infer-out/log, then `grep ^[<PID>] infer-out/log`.
Introduce 3 log levels for debug output and command-line options to set them
for various categories individually.
Change tons of `"\n"` to `"@\n"` so the `Format` module is aware of newlines
without us having to look through every character of every logged string for
`\n` characters.
Reviewed By: mbouaziz
Differential Revision: D5165317
fbshipit-source-id: 93c922f
Summary:
Spacetime profiling showed that this was allocating a lot more than it needs
to. Switching to a `Hashtbl` makes the memory overhead go away, and halves the
memory consumption of the whole frontend on some pathological files.
This also brings the file "clang_ast_main.ml" into the infer repo, renamed to
"ClangPointers.ml" (and given an mli). It's useful to have something like
"clang_ast_main.ml" checked into the facebook-clang-plugins repo to illustrate
how to use the OCaml AST visitor generated by atdgen, but it can be simplified
to be more pedagogical instead of being the visitor used in infer.
Reviewed By: jberdine
Differential Revision: D4884383
fbshipit-source-id: 88f324a
Summary:
A recent diff tried to replace `L.out "error message"; assert false` with
`failwith "error message"` but infer relies on the type of raised exceptions to
sometimes keep going. A more careful change will be needed but in the meantime
restore the old behaviour.
Reviewed By: jberdine
Differential Revision: D5112969
fbshipit-source-id: 713fe20
Summary:
Try and enforce the following rules:
- stderr is for updating the user about progress or errors
- Introduce Logging.progress that outputs to stderr, but honours --quiet
- Logging.stderr is as before
- Logging.out now prints to stderr (or to log files as before if set up) and
not stdout. If some information should go on stdout then the user should be
able to rely on it (ie, it's not just some progress message). For now only
the summary of the errors is printed on stdout by default.
- Logging.err* functions are gone. If the error is user-visible, it should be
Logging.stderr, or `failwith`. If not, go to the same log file as other
output, which personally I find much more convenient than having to dig through
2 log files every time I'm looking for some output.
Reviewed By: jberdine
Differential Revision: D5095720
fbshipit-source-id: 68999c9
Summary:
Previously all knowledge of the dynamic length of such arrays was lost to infer:
```
void foo(int len) {
int a[len];
}
```
The translation of this program would make no reference to `len` (except as a
param of `foo`).
Translate this "initialization" using the existing `__set_array_length` infer
builtin, as:
```
# Declare local a[_]
n$0 = len;
__set_array_length(a, len);
```
update-submodule: facebook-clang-plugins
Reviewed By: mbouaziz
Differential Revision: D4969446
fbshipit-source-id: dff860f
Summary:
An array has a static or dynamic length (number of elements), but it also has a
stride, determined by the type of the element: `sizeof(element_type)`. We don't
have a good `sizeof()` function available on SIL types, so record that stride
in the array type.
update-submodule: facebook-clang-plugins
Reviewed By: mbouaziz
Differential Revision: D4969697
fbshipit-source-id: 98e0670
Summary:
Add `volatile` and `restrict` type qualifiers. Change `Ast_expressions.create_*_type` functions
to always get optional type quals argument.
update-submodule: facebook-clang-plugins
Reviewed By: jberdine
Differential Revision: D4969634
fbshipit-source-id: 9a63bf7
Summary:
Modify the type of `Exp.Sizeof ...` to include the value that the expression
evaluates to according to the compiler, or None if it cannot be known
statically.
Use this information in inferbo.
Mostly unused in the BiAbduction checker for now, although it could be useful
there too.
update-submodule: facebook-clang-plugins
Reviewed By: mbouaziz
Differential Revision: D4953634
fbshipit-source-id: be0999d
Summary:
Backend needs to know whether type is const or not. In order to achieve it, frontend needs to know it first.
This diff changes bunch of things:
- update clang plugin to have AST exporter actually export that info most of the time
- change types of functions in clang frontend until it compiles
- replace `type_ptr` with `qual_type` and `tp` with `qt` in names where applicable
- cleanup some things in the process
update-submodule: facebook-clang-plugins
Reviewed By: jvillard
Differential Revision: D4938567
fbshipit-source-id: 716b3ef
Summary: `__attribute__((annotate("")))` is better way of passing information to the frontend
Reviewed By: jberdine
Differential Revision: D4818805
fbshipit-source-id: 6e8add2
Summary:
Don't pass names as strings in clang frontend. Instead use QualifiedCppName which preserves
each identifier of qualified name.
Done by
1. change return type of `Cast_utils.get_qualified_name` to return `QualifiedCppName.t`
2. change types in `Typ.Name.t` and `Typ.Procname.t` to use qualified names where applicable
3. Keep changing the code until it compiles
Reviewed By: jberdine
Differential Revision: D4754242
fbshipit-source-id: 9d723cb
Summary:
This issue was spotted in the wild. There may be more of those, unfortunately it's hard to predict
More general problem is that types in infer frontend diverge from clang's types for DerivedToBase cast.
Then, infer uses types from clang anyway and that confuses backend. Getting it always right is very hard
Reviewed By: jvillard
Differential Revision: D4754081
fbshipit-source-id: 5fb7069
Summary:
Provide proper constructor functions for all Typenames following `Typename.Java` module.
Always use those constructor functions.
Reviewed By: jeremydubreil
Differential Revision: D4673943
fbshipit-source-id: 81625c2
Summary:
It used to be string which:
1. Doesn't have enough information for parametric models
2. Doesn't have good type
Changing this blows up in clang frontend, but I think it's for the better
Reviewed By: jberdine
Differential Revision: D4667633
fbshipit-source-id: 9f61bf1
Summary: procnames will depend on Typ.t soon and we need to go from `type_ptr` to `Typ.t` when creating procnames. `CType_decl` does this translation it can't depend on `mk_procname_` functions.
Reviewed By: jvillard
Differential Revision: D4620560
fbshipit-source-id: 9524178
Summary:
This modifies `CTrans_utils.dereference_value_from_result` function. Reasons:
1. Type in Sil.Load tuple is supposed to be the type of Ident.t field (result), not Exp.t (source)
2. Inside `dereference_value_from_result` we used wrong type when `~strip_pointer:true` - it used original type instead of the resulting one
3. Fixing (2), uncovered similar issue in `CTrans_utils.cast_operation` - it should have used resulting type, not original one
Changed tests are expected:
1. `deprecated_hack.cpp` test was how I discovered this issue (it was very wrong)
2. the other test also had issues, now it's correct
Sadly, there was no logic I followed when writing this change - I stuffed changes in a way that fits, but it may be breaking something somewhere else.
Reviewed By: jberdine
Differential Revision: D4481895
fbshipit-source-id: b139c59
Summary:
Stmt shouldn't have to return anything in res_trans.exps - those results shouldn't be used by parent nodes.
I need this change for another diff but given the risk I decided to put it as separate diff
Reviewed By: jvillard
Differential Revision: D4474936
fbshipit-source-id: 0537749
Summary:
Sometimes some instructions were never part of any CFG node and were not written to CFG at all. Add a mechanism that will create node and add them to CFG when they reach compoundStmt translation.
This is step forward to make `*x;` instruction actualy dereference x (it works in C already in C++ AST looks different though)
Reviewed By: dulmarod
Differential Revision: D4469139
fbshipit-source-id: b03b11c
Summary:
Remove the remaining uses of polymorphic equality `=`.
In case of basic types, this is replaced by String.equal or Int.equal.
In case of `= []`, this is replaced by `List.is_empty`.
In case of `= None`, this is replaced by `is_none`.
In case of a datatype definition such as `type a = A | B`,
a `compare_a` function is defined by adding `type a = A | B [@deriving compare]`
and a `equal_a` function is defined as `let equal_a = [%compare.equal : a]`.
In case of comparison with a polymorphic variant `= `Yes`, the equality
defined in `PVariant.(=)` is used. Typically, `open! Pvariant` is added
at the beginning of the file to cover all the uses.
Reviewed By: jberdine
Differential Revision: D4456129
fbshipit-source-id: f31c433
Summary:
Module CFrontend_utils is a container for two modules: Ast_utils and General_utils.
Instead of opening CFrontend_utils in several places, it is now split into two separate modules CAst_utils and CGeneral_utils, which are now accessed directly.
Reviewed By: jberdine
Differential Revision: D4392710
fbshipit-source-id: ea756a2
Summary:
Use In_channel and Out_channel operations instead of those in Pervasives. Don't
use physical equality on values that aren't heap-allocated since it doesn't help
the compiler generate faster code and the semantics is unspecified. Also use
phys_equal for physical equality.
Reviewed By: sblackshear
Differential Revision: D4232459
fbshipit-source-id: 36fcfa8
Summary:
Utils contains definitions intended to be in the global namespace for
all of the infer code-base, as well as pretty-printing functions, and
assorted utility functions mostly for dealing with files and processes.
This diff changes the module opened into the global namespace to
IStd (Std conflict with extlib), and moves the pretty-printing
definitions from Utils to Pp.
Reviewed By: jvillard
Differential Revision: D4232457
fbshipit-source-id: 1e070e0
Summary:
curr_file is remnant of the past when we didn't have good location information
coming from Clang_ast_t location. But it was fixed ~1 year ago.
Reviewed By: jvillard
Differential Revision: D4139750
fbshipit-source-id: 4ce7235
Summary:
this makes frontends no longer depend on SymExec.ml. `ModelBuiltins` was split into two modules:
- `BuiltinDecl` with procnames for builtins (used to determine whether some function is a builtin)
- `BuiltinDefn` with implementations used by `SymExec`
- they both have similar type defined in `BUILTINS.S` which makes sure that new builtin gets added into both modules.
During the refactor I ran some scripts:
`BuiltinDecl.ml`:
let X = create_procname "X"
cat BuiltinDecl.ml | grep "create_procname" | tail -70 | awk ' { print $1,$2,$3,$4,"\42"$2"\42"} '
then manually confirm string match. Exceptions:
"__exit" -> "_exit"
"objc_cpp_throw" -> "__infer_objc_cpp_throw"
__objc_dictionary_literal
nsArray_arrayWithObjects
nsArray_arrayWithObjectsCount
`BuiltinDefn.ml`:
let X = Builtin.register BuiltinDecl.X execute_X
cat BuiltinDecl.ml | grep "create_procname" | tail -70 | awk ' { print $1,$2,$3,"Builtin.register BuiltinDecl."$2,"execute_"$2} '
then, fix all compilation problems
Reviewed By: jberdine
Differential Revision: D3951035
fbshipit-source-id: f059602
Summary: The frontend replaces global variables that are constant with their values as a quick hack to improve the precision of the analysis. This should apply to `constexpr` too.
Reviewed By: dulmarod
Differential Revision: D4058097
fbshipit-source-id: be4fea6
Summary:
This changes executions of the former InferClang into a function call. In
particular, it can be called several times per execution.
The new InferClang must be called as if it was clang, and knows how to run
clang with our plugin to get the AST of the source file.
Reviewed By: akotulski
Differential Revision: D3981017
fbshipit-source-id: 7af6490
Summary: it seems to have no effect on analysis. As such it should be ok to add cg nodes for builtin model calls
Reviewed By: jberdine
Differential Revision: D3967399
fbshipit-source-id: 06c32e5
Summary:
This is needed for later: InferClang will no longer be started once for each
source file to be analysed. Instead, it will be called to analyse several files
at once, and will analyse them one by one. Thus, `clang_lang` and `source_file`
are moved to `cFrontend_config` as references.
The biggest change this entailed was the new logging infrastructure, which was
depending on `Config.source_file`. This diff moves the logic entirely to
`Logging`, and changes the API so that executables wishing to log into files
have to set it up using `Logging.set_log_file_identifier`. This can be called
several times during the execution, allowing to dynamically change the log file
(eg, when analysing several source files one by one!).
Reviewed By: jberdine
Differential Revision: D3944148
fbshipit-source-id: 6129090
Summary: That data was never used and removing it can simplify frontends quite a bit.
Reviewed By: jberdine
Differential Revision: D3967389
fbshipit-source-id: d65c3da
Summary:
- Use the module types in cModule_types.ml instead of redefining them.
- A few occurrences of \n in formatted output replaced by @\n to let the
formatter know of line breaks (by no means complete, these were just a few I
came across while doing something else)
Reviewed By: jberdine
Differential Revision: D3944081
fbshipit-source-id: 4460427
Summary:
Change Sil.Call instruction to have only a single optional return
identifier, insted of a list. Essentially none of the code handled
multiple return identifiers. Also, add the type of the return
identitifier to Call instructions.
Reviewed By: sblackshear
Differential Revision: D3919358
fbshipit-source-id: d2d4f72
Summary:
It is no longer necessary to keep the name of a struct within the
struct, as the name will just have been used to look it up.
Reviewed By: cristianoc
Differential Revision: D3919355
fbshipit-source-id: ab65168
Summary:
This diff removes the redundancy in the representation of types where
struct types could be represented either directly using Tstruct or
indirectly using Tvar to refer to the type environment. A consequence
is that it is much harder to construct large type values.
Reviewed By: sblackshear, cristianoc
Differential Revision: D3839753
fbshipit-source-id: cf04ea5
Summary:
Rename Typ.mk_struct to internal_mk_struct, and add Tenv.mk_struct that
ensures types are added to the environment under the right name.
Reviewed By: cristianoc
Differential Revision: D3791865
fbshipit-source-id: fd4b667
Summary:
The Typ.struct_typ.csu field is now redundant with the Csu.t in the
name: Typename.t field.
Reviewed By: cristianoc
Differential Revision: D3791861
fbshipit-source-id: 5370885
Summary: Replace the struct_name: Mangled.t option field of Typ.struct_typ with name: Typename.t
Reviewed By: sblackshear
Differential Revision: D3791860
fbshipit-source-id: 3ee1d00
Summary:
Given that mangling now respects `extern "C" {}` declarations, pnames of C function will have no mangling and we don't need to discard mangled part from procname.
Move `malloc` detection to `get_builtin_pname_opt` function (together with all others)
Reviewed By: dulmarod
Differential Revision: D3804402
fbshipit-source-id: 9ae9991
Summary:
There were overly complex and different ways of identifying C++/objC languages. Simplify and unify them.
How I came up with naming - there are two languages C and C++. You can apply "Objective-" extention to both of them.
We usually need to differentiate C and C++ (different AST) and whether it's extended (checks specific to ObjC)
Reviewed By: dulmarod
Differential Revision: D3804457
fbshipit-source-id: 685e40a
Summary:
ti_raw field tends to grow exponensially for template instantiations.
ASTexporter no longer gives this bit of information and infer needs to stop using it.
Reviewed By: jvillard
Differential Revision: D3798192
fbshipit-source-id: ba7cbb9
Summary:
We already have logic that deals with adding custom procnames in `function_deref_trans`. This diff simplifies `callExpr_trans` and removes
complex logic responsible for finding type of function.
There is no functional change intended (it will come in future diffs)
Reviewed By: dulmarod
Differential Revision: D3797839
fbshipit-source-id: 21fc13f
Summary: Fix pattern matching of the `parameters` field of `DeprecatedAttr`
Reviewed By: akotulski
Differential Revision: D3782788
fbshipit-source-id: df8dd40
Summary:
1. Add capability to clang frontend to replace some function calls with another SIL code based on `__deprecated__` attribute.
2. Given this capability, use those attributes for shared_ptr getters to generate `Sil.Load` instruction instead of method call
3. Add test that mimics shared_ptr model, but it doesn't have that much scary C++ templated code
Reviewed By: jvillard, jberdine
Differential Revision: D3729176
fbshipit-source-id: 2a330d5
Summary:
Make std::shared_ptr<T> translated as T* inside infer. This will make reporting better
since smart pointers are really pointers not structs - this form is much easier for the analyzer to understand.
This requires changes to the model of shared_ptr as well.
Reviewed By: jvillard
Differential Revision: D3587255
fbshipit-source-id: b86fb36
Summary: With this approach, all the global consts will be inlined in the places where they are used.
Reviewed By: dulmarod
Differential Revision: D3703133
fbshipit-source-id: 3c19479
Summary:
Make necessary changes so that the clang frontend compiles with the
corresponding changes to the facebook clang plugins. Does not actually use the
extra qualifier information yet.
depends on D3684150
Reviewed By: akotulski
Differential Revision: D3684173
fbshipit-source-id: f715f75
Summary:
Cosmetic changes to comments to improve the results of the Reason
comment attachment logic.
These were found using `git grep -nH -e 'in[ ]*(\*'` although the
attachment logic seems ok if the associated `let` is on the same line.
Some others were found with `git grep -nH -e ')[ ]*(\*'` although the
attachment logic seems ok if the associated `(` is on the same line.
Reviewed By: jvillard
Differential Revision: D3654027
fbshipit-source-id: 122aa3b
Summary:
Previously, we would translate `throw` with `return`. However, `throw` in
ObjC/C++ is often used to mean "abort". We now translate `throw` the same as
`exit` to prune these paths.
Reviewed By: akotulski
Differential Revision: D3594156
fbshipit-source-id: 81083bb
Summary:
Minor stuff:
- GCCAst -> GCCAsm
- separate constants and mutable global state in cFrontend_config
- alphabetical ordering in cFrontend_config
Reviewed By: akotulski
Differential Revision: D3593858
fbshipit-source-id: 6f4d9c3
Summary:
No longer allow pointer types to be passed inside var_exp_typ. We used to accept both forms,
but it won't be possible any longer once shared_ptr becomes pointer type.
Reviewed By: dulmarod
Differential Revision: D3593003
fbshipit-source-id: a830914
Summary:
1. Attempt to simplify callExpr_trans by merging multiple if-else branches into one in `CTrans_utils.builtin_trans`. Not every special case works that way, so some of them are still there.
2. On top of that, make type of parameters in `CTrans_models` functions `Procname.t` instead `Procname.t option`. This triggered some more changes to `callExpr_trans`.
3. Semi-randomly reorder instructions in `callExpr_trans`
This is mainly an attempt to clean up the code so any suggestions how to make it better are welcome.
Reviewed By: jvillard
Differential Revision: D3586419
fbshipit-source-id: aef8580
Summary: ClassTemplateSpecializationDecl now contains structures information about the specialization
Reviewed By: jvillard
Differential Revision: D3586446
fbshipit-source-id: d567a0b
Summary:
Use Itanium mangling for C++ functions/methods instead of raw type name. This is a step towards removing expensive `ti_raw` field from `type_info`.
For virtual methods, use mangled name of the method from base class in order for dynamic dispatch to work.
Reviewed By: dulmarod
Differential Revision: D3556118
fbshipit-source-id: e45edb5
Summary:
Class names fragile, easy to break and they introduce another level of indirection.
Since C++ translation doesn't need names, pass decl pointers everywhere.
Doing otherwise will most likely result in assert false.
Handling of class names in objc translation is more involving and it's left
for now. Later, objc can use same mechanism as C++.
Reviewed By: dulmarod
Differential Revision: D3570176
fbshipit-source-id: b957aba
Summary:
Move Sil.call_flags type and operations into separate CallFlags
module.
Reviewed By: dulmarod
Differential Revision: D3548086
fbshipit-source-id: 6d264e9
Summary: Move Sil.binop type and operations into separate Binop module.
Reviewed By: dulmarod
Differential Revision: D3548082
fbshipit-source-id: 356bee3
Summary: Move Sil.const type and operations into separate Const module.
Reviewed By: dulmarod
Differential Revision: D3548073
fbshipit-source-id: 388d03e
Andrzej Kotulski
Josh Berdine
Jia Chen
Jules Villard
Cristiano Calcagno
Sam Blackshear
Dulma Churchill
Martino Luca