Summary:
A long-standing easter egg from infer error messages is the "object
`null` could be null and is dereferenced at line ...". I tried to fix
this but the part that generates the first "null" in the message and the
part that generates the second one are very far apart and it's hard to
see how to make the second part aware of the first in a clean way.
Instead, hack around it by detecting if the string representing the
value is literally `null` and in that case chop `could be null ` from
the error messages...
Reviewed By: jeremydubreil
Differential Revision: D14972324
fbshipit-source-id: ccc48ce6b
Summary:
We get messages like " object returned by `getArguments()` at line 101."
instead of " object returned by `getArguments()` could be null and is
dereferenced at line 101.". Tracking it down, it happens for
nullable-looking values, but I don't know why.
It seems that something regressed but I couldn't track it down.
So, just generate the error message in the same way as for non-nullable
objects in this case to fix the non-sensical message.
Reviewed By: jeremydubreil
Differential Revision: D14972325
fbshipit-source-id: 2a97501cc
Summary:
Feedback from peterogithub:
- mention which access path is being invalidated and accessed in the message
- mention the line at which it was invalidated (the line at which it's accessed is already the line at which we report)
- traces for stack variable/C++ temporary address escapes
- delete double implementation of the same functionality in
`PulseTrace`: `location_of_action_start` is the same as
`outer_location_of_action`...
Reviewed By: jberdine
Differential Revision: D14800294
fbshipit-source-id: 3d9ab9b3d
Summary:
Similarly to function parameters (and the return value), we need to
apply the pre/post of a function call to the globals mentioned in its
summary.
- tigthen summaries further to remember only abducible variables in the
post (as well as in the pre)
- take globals into account when applying pre/post pairs
Reviewed By: jberdine
Differential Revision: D14780800
fbshipit-source-id: fc0d180bb
Summary:
The heuristic to detect variables going out of scope was to detect any
access expression passed as argument to an injected destructor call.
However destructor calls are also injected in destructor bodies to
destruct each field of an object, so the heuristic would detect fields
going out of scope, which, erm, doesn't make sense. Limit the heuristic
to local program variables.
Reviewed By: jberdine
Differential Revision: D14771454
fbshipit-source-id: ffa3c9fe3
Summary:
Only throw values to the pre if they can be followed from "abducible"
variables: formals of the current method and globals.
Because figuring out if a `Pvar.t` is a formal of the current procedure
is actually a giant pain, hack something not too bad instead:
pre-register all formals at the start of the analysis of the
procedure. Then the only other variables we care about in the
precondition are globals, which we can detect easily.
This is mostly an optimisation (summaries won't include irrelevant
"abduced" facts about the procedure's local variables anymore), but it
also fixes a bug where we would sometimes overwrite things in the pre. I
think that's why the tests improved.
Reviewed By: ngorogiannis
Differential Revision: D14753493
fbshipit-source-id: 08e73637f
Summary:
This is useful for the model of `exit` that returns 0 disjuncts. All
other models return 1 disjunct for now, but in the future things like
`malloc()` will need to return 2 possible states for instance.
Reviewed By: ngorogiannis
Differential Revision: D14753491
fbshipit-source-id: 3e7387d6d
Summary:
This mostly doesn't make sense. The only thing this would have been good
for was to give the most accurate result on access paths such as
`*(&(x.f))`, but these are normalised anyway (into `x.f`) so we actually
never see these. That said there might be some use to some similar logic
in the future, but in the meantime let's delete the current feature as
it wasn't thought through.
Reviewed By: ezgicicek
Differential Revision: D14753492
fbshipit-source-id: 597cec027
Summary:
The previous message formatting had regressed and produced non-sensical messages.
More importantly, remove template parameters from error messages to
trigger the heuristic in `InferPrint` that deduplicates errors that are
on the same line with the same error type and message. Without this we
get hundreds of reports that correspond to as many instantiations of the
same code.
Reviewed By: ngorogiannis
Differential Revision: D14747979
fbshipit-source-id: 3c4aad2b1
Summary:
We see the magic function `__variable_initialization` at the point where
the variable is declared, eg `int x = foo()`. It's safe to reset `&x` at
that point. This circumvents an issue that pops up in some rare cases
where the ternary conditional operator `?:` and variable initialization
conspire to produce weird frontend results.
Some test becomes a FN again, but I think it was being reported for the
wrong reasons; will investigate more later.
Reviewed By: ngorogiannis
Differential Revision: D14747980
fbshipit-source-id: e75d6e30f
Summary:
This is ~2.5x wall clock faster, ~5x user time faster, and finds 98% of
the bugs. Not very scientific yet but seems better than the previous
non-scientific arbitrary default.
Reviewed By: ngorogiannis
Differential Revision: D14753494
fbshipit-source-id: b72cdd613
Summary:
Useful to know which disjunct is being executed. Reprinting them
wholesale is too spammy so compromise by outputting just enough to be
able to reconstruct the info "which disjunct was executed and which new
disjuncts were produced?".
Reviewed By: ngorogiannis
Differential Revision: D14753495
fbshipit-source-id: f5aa68160
Summary:
This isn't needed now that this information is recorded in
`PulseTrace.action` instead.
Reviewed By: mbouaziz
Differential Revision: D14645089
fbshipit-source-id: 9c3f38722
Summary:
This ensures that each attribute type can only be present once per
address. Makes ~80x time improvement on pathological cases such as
Duff's device.
This introduces a new kind of Set in `PrettyPrintable`.
Reviewed By: mbouaziz
Differential Revision: D14645091
fbshipit-source-id: c7f9b760c
Summary:
Detect when a variable goes out of scope. When that's the case, mark its
address *and* its contents as invalid.
Give subsequent uses a USE_AFTER_LIFETIME error type instead of
USE_AFTER_DESTRUCTOR.
Reviewed By: jberdine
Differential Revision: D14387147
fbshipit-source-id: a2c530fda
Summary:
This provides a way for AI checkers to read the formals of a procedure,
or other things related to its `Procdesc.t`.
Reviewed By: mbouaziz
Differential Revision: D14258483
fbshipit-source-id: a28e28d3c
Summary:
For each operation on the domain, try to record what it requires of the
precondition of the function. This is akin to what happens in the
biabduction backend, hence the terminology used.
Reviewed By: jberdine
Differential Revision: D14387148
fbshipit-source-id: a61fe30c8
Summary:
This is in preparation of interprocedural pulse. The abstract addresses
generator keeps a reference to create fresh addresses, but that's a
piece of global state that needs to persist across ondemand analyses.
Reviewed By: jberdine
Differential Revision: D14324760
fbshipit-source-id: 5cdb1d3f5
Summary:
Increases precision a bit. I didn't observe speed problems on what I tested. (But, who knows?)
Closes https://github.com/facebook/infer/pull/799
Reviewed By: jvillard
Differential Revision: D6284206
Pulled By: rgrig
fbshipit-source-id: 6f1e8631f
Summary:
Instead of emitting an ad-hoc builtin on variable declaration emit a new
metadata instruction. This allows us to remove the code matching on that
ad-hoc builtin that had to be inserted in several checkers.
Inferbo & pulse used that information meaningfully and had to undergo
some minor changes to cope with the new metada instruction.
Reviewed By: ezgicicek
Differential Revision: D14833100
fbshipit-source-id: 9b3009d22
Summary:
Bundle all non-semantic-bearing instructions into a `Metadata _`
instruction in SIL.
- On a documentation level this makes clearer the distinction between
instructions that encode the semantics of the program and those that are
just hints for the various backend analysis.
- This makes it easier to add more of these auxiliary instructions in
the future. For example, the next diff introduces a new `Skip` auxiliary
instruction to replace the hacky `ExitScope([], Location.dummy)`.
- It also makes it easier to surface all current and future such
auxiliary instructions to HIL as the datatype for these syntactic hints
can be shared between SIL and HIL. This diff brings `Nullify` and
`Abstract` to HIL for free.
Reviewed By: ngorogiannis
Differential Revision: D14827674
fbshipit-source-id: f68fe2110
Jules Villard
Lee Howes
Jeremy Dubreil
Daniel Mercier
Ezgi Çiçek
Mehdi Bouaziz
Mitya Lyubarskiy
Martin Trojer
David Lively
Radu Grigore
Nikos Gorogiannis