[pulse] A new issue type OPTIONAL_EMPTY_ACCESS for trying to access folly::Optional when it is folly::none

Summary:
We need to check if `folly::Optional` is not `folly::none` if we want to retrieve the value, otherwise a runtime exception is thrown:

```
  folly::Optional<int> foo{folly::none};
  return foo.value(); // bad
```

```
  folly::Optional<int> foo{folly::none};
  if (foo) {
    return foo.value(); // ok
  }
```

This diff adds a new issue type that reports if we try to access `folly::Optional` value when it is known to be `folly::none`.

Reviewed By: ezgicicek

Differential Revision: D22053352

fbshipit-source-id: 32cb00a99
master
Daiva Naudziuniene 5 years ago committed by Facebook GitHub Bot
parent d8a3c4c2a3
commit 2c48e61031

@ -461,6 +461,7 @@ OPTIONS
Missing_fld (enabled by default),
NULLPTR_DEREFERENCE (disabled by default),
NULL_DEREFERENCE (enabled by default),
OPTIONAL_EMPTY_ACCESS (disabled by default),
PARAMETER_NOT_NULL_CHECKED (enabled by default),
POINTER_TO_CONST_OBJC_CLASS (enabled by default),
PRECONDITION_NOT_FOUND (enabled by default),

@ -191,6 +191,7 @@ OPTIONS
Missing_fld (enabled by default),
NULLPTR_DEREFERENCE (disabled by default),
NULL_DEREFERENCE (enabled by default),
OPTIONAL_EMPTY_ACCESS (disabled by default),
PARAMETER_NOT_NULL_CHECKED (enabled by default),
POINTER_TO_CONST_OBJC_CLASS (enabled by default),
PRECONDITION_NOT_FOUND (enabled by default),

@ -461,6 +461,7 @@ OPTIONS
Missing_fld (enabled by default),
NULLPTR_DEREFERENCE (disabled by default),
NULL_DEREFERENCE (enabled by default),
OPTIONAL_EMPTY_ACCESS (disabled by default),
PARAMETER_NOT_NULL_CHECKED (enabled by default),
POINTER_TO_CONST_OBJC_CLASS (enabled by default),
PRECONDITION_NOT_FOUND (enabled by default),

@ -745,6 +745,11 @@ let nullptr_dereference =
~user_documentation:"See [NULL_DEREFERENCE](#null_dereference)."
let optional_empty_access =
register ~enabled:false ~id:"OPTIONAL_EMPTY_ACCESS" Error Pulse
~user_documentation:"Reports on accessing folly::Optional when it is none."
let parameter_not_null_checked =
register ~id:"PARAMETER_NOT_NULL_CHECKED" Warning Biabduction
~user_documentation:[%blob "../../documentation/issues/PARAMETER_NOT_NULL_CHECKED.md"]

@ -264,6 +264,8 @@ val null_dereference : t
val nullptr_dereference : t
val optional_empty_access : t
val parameter_not_null_checked : t
val precondition_not_found : t

@ -47,6 +47,7 @@ type t =
| CppDelete
| EndIterator
| GoneOutOfScope of Pvar.t * Typ.t
| OptionalEmpty
| StdVector of std_vector_function
| JavaIterator of java_iterator_function
[@@deriving compare]
@ -64,6 +65,8 @@ let issue_type_of_cause = function
IssueType.vector_invalidation
| GoneOutOfScope _ ->
IssueType.use_after_lifetime
| OptionalEmpty ->
IssueType.optional_empty_access
| JavaIterator _ | StdVector _ ->
IssueType.vector_invalidation
@ -87,6 +90,8 @@ let describe f cause =
else F.fprintf f "is the address of a stack variable `%a`" Pvar.pp_value pvar
in
F.fprintf f "%a whose lifetime has ended" pp_var pvar
| OptionalEmpty ->
F.pp_print_string f "is folly::None"
| StdVector std_vector_f ->
F.fprintf f "was potentially invalidated by `%a()`" pp_std_vector_function std_vector_f
| JavaIterator java_iterator_f ->
@ -101,7 +106,7 @@ let pp f invalidation =
F.fprintf f "ConstantDereference(%a)" describe invalidation
| CppDelete ->
F.fprintf f "CppDelete(%a)" describe invalidation
| EndIterator | GoneOutOfScope _ ->
| EndIterator | GoneOutOfScope _ | OptionalEmpty ->
describe f invalidation
| StdVector _ ->
F.fprintf f "StdVector(%a)" describe invalidation

@ -29,6 +29,7 @@ type t =
| CppDelete
| EndIterator
| GoneOutOfScope of Pvar.t * Typ.t
| OptionalEmpty
| StdVector of std_vector_function
| JavaIterator of java_iterator_function
[@@deriving compare]

@ -139,6 +139,82 @@ module ObjC = struct
|> PulseOperations.ok_continue
end
module FollyOptional = struct
let internal_value =
Fieldname.make
(Typ.CStruct (QualifiedCppName.of_list ["__infer_pulse_model"]))
"__infer_model_backing_value"
let internal_value_access = HilExp.Access.FieldAccess internal_value
let to_internal_value location optional astate =
PulseOperations.eval_access location optional internal_value_access astate
let to_internal_value_deref location optional astate =
let* astate, pointer = to_internal_value location optional astate in
PulseOperations.eval_access location pointer Dereference astate
let write_value location this ~value ~desc astate =
let event = ValueHistory.Call {f= Model desc; location; in_call= []} in
let* astate, value_field = to_internal_value location this astate in
let value_hist = (fst value, event :: snd value) in
let+ astate = PulseOperations.write_deref location ~ref:value_field ~obj:value_hist astate in
(astate, value_hist)
let assign_value_fresh location this ~desc astate =
write_value location this ~value:(AbstractValue.mk_fresh (), []) ~desc astate
let assign_none this ~desc : model =
fun _ ~callee_procname:_ location ~ret:_ astate ->
let* astate, value = assign_value_fresh location this ~desc astate in
let astate = PulseArithmetic.and_eq_int (fst value) IntLit.zero astate in
let+ astate = PulseOperations.invalidate location Invalidation.OptionalEmpty value astate in
[ExecutionDomain.ContinueProgram astate]
let assign_value this init ~desc : model =
fun _ ~callee_procname:_ location ~ret:_ astate ->
let* astate, value = to_internal_value_deref location init astate in
let+ astate, _ = write_value location this ~value ~desc astate in
[ExecutionDomain.ContinueProgram astate]
let emplace optional : model =
fun _ ~callee_procname:_ location ~ret:_ astate ->
let+ astate, _ =
assign_value_fresh location optional ~desc:"folly::Optional::emplace()" astate
in
[ExecutionDomain.ContinueProgram astate]
let value optional : model =
fun _ ~callee_procname:_ location ~ret:(ret_id, _) astate ->
let event = ValueHistory.Call {f= Model "folly::Optional::value()"; location; in_call= []} in
let* astate, (value_addr, value_hist) = to_internal_value_deref location optional astate in
PulseOperations.write_id ret_id (value_addr, event :: value_hist) astate
|> PulseOperations.ok_continue
let has_value optional : model =
fun _ ~callee_procname:_ location ~ret:(ret_id, _) astate ->
let ret_addr = AbstractValue.mk_fresh () in
let ret_value =
(ret_addr, [ValueHistory.Allocation {f= Model "folly::Optional::has_value()"; location}])
in
let+ astate, (value_addr, _) = to_internal_value_deref location optional astate in
let astate = PulseOperations.write_id ret_id ret_value astate in
let astate_non_empty = PulseArithmetic.and_positive value_addr astate in
let astate_true = PulseArithmetic.and_positive ret_addr astate_non_empty in
let astate_empty = PulseArithmetic.and_eq_int value_addr IntLit.zero astate in
let astate_false = PulseArithmetic.and_eq_int ret_addr IntLit.zero astate_empty in
[ExecutionDomain.ContinueProgram astate_false; ExecutionDomain.ContinueProgram astate_true]
end
module Cplusplus = struct
let delete deleted_access : model =
fun _ ~callee_procname:_ location ~ret:_ astate ->
@ -742,8 +818,26 @@ module ProcNameDispatcher = struct
ref-counting *)
-"folly" &:: "fbstring_core" &:: "category" &--> Misc.return_int Int64.zero
; -"folly" &:: "DelayedDestruction" &:: "destroy" &--> Misc.skip
; -"folly" &:: "Optional" &:: "reset" &--> Misc.skip
; -"folly" &:: "SocketAddress" &:: "~SocketAddress" &--> Misc.skip
; -"folly" &:: "Optional" &:: "Optional" <>$ capt_arg_payload
$+ any_arg_of_typ (-"folly" &:: "None")
$--> FollyOptional.assign_none ~desc:"folly::Optional::Optional(=None)"
; -"folly" &:: "Optional" &:: "Optional" <>$ capt_arg_payload
$--> FollyOptional.assign_none ~desc:"folly::Optional::Optional()"
; -"folly" &:: "Optional" &:: "Optional" <>$ capt_arg_payload $+ capt_arg_payload
$+...$--> FollyOptional.assign_value ~desc:"folly::Optional::Optional(arg)"
; -"folly" &:: "Optional" &:: "assign" <>$ capt_arg_payload
$+ any_arg_of_typ (-"folly" &:: "None")
$--> FollyOptional.assign_none ~desc:"folly::Optional::assign(=None)"
; -"folly" &:: "Optional" &:: "assign" <>$ capt_arg_payload $+ capt_arg_payload
$+...$--> FollyOptional.assign_value ~desc:"folly::Optional::assign()"
; -"folly" &:: "Optional" &:: "emplace<>" $ capt_arg_payload $+...$--> FollyOptional.emplace
; -"folly" &:: "Optional" &:: "emplace" $ capt_arg_payload $+...$--> FollyOptional.emplace
; -"folly" &:: "Optional" &:: "has_value" <>$ capt_arg_payload
$+...$--> FollyOptional.has_value
; -"folly" &:: "Optional" &:: "reset" <>$ capt_arg_payload
$+...$--> FollyOptional.assign_none ~desc:"folly::Optional::reset()"
; -"folly" &:: "Optional" &:: "value" <>$ capt_arg_payload $+...$--> FollyOptional.value
; -"std" &:: "basic_string" &:: "data" <>$ capt_arg_payload $--> StdBasicString.data
; -"std" &:: "basic_string" &:: "~basic_string" <>$ capt_arg_payload
$--> StdBasicString.destructor

@ -30,6 +30,11 @@ codetoanalyze/cpp/pulse/nullptr.cpp, deref_nullptr_bad, 2, NULLPTR_DEREFERENCE,
codetoanalyze/cpp/pulse/nullptr.cpp, no_check_return_bad, 2, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,when calling `may_return_nullptr` here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,passed as argument to `may_return_nullptr`,return from call to `may_return_nullptr`,assigned,invalid access occurs here]
codetoanalyze/cpp/pulse/nullptr.cpp, std_false_type_deref_bad, 3, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,assigned,invalid access occurs here]
codetoanalyze/cpp/pulse/nullptr.cpp, std_true_type_deref_bad, 3, NULLPTR_DEREFERENCE, no_bucket, ERROR, [invalidation part of the trace starts here,assigned,is the null pointer,use-after-lifetime part of the trace starts here,assigned,invalid access occurs here]
codetoanalyze/cpp/pulse/optional.cpp, assign2_bad, 4, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,when calling `folly::Optional<int>::operator=` here,passed as argument to `folly::Optional::reset()` (modelled),return from call to `folly::Optional::reset()` (modelled),is folly::None,use-after-lifetime part of the trace starts here,passed as argument to `folly::Optional<int>::operator=`,passed as argument to `folly::Optional::reset()` (modelled),return from call to `folly::Optional::reset()` (modelled),return from call to `folly::Optional<int>::operator=`,passed as argument to `folly::Optional::value()` (modelled),return from call to `folly::Optional::value()` (modelled),invalid access occurs here]
codetoanalyze/cpp/pulse/optional.cpp, assign_bad, 5, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),is folly::None,use-after-lifetime part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),passed as argument to `folly::Optional<int>::operator=`,parameter `other` of folly::Optional<int>::operator=,passed as argument to `folly::Optional::assign()` (modelled),return from call to `folly::Optional::assign()` (modelled),return from call to `folly::Optional<int>::operator=`,passed as argument to `folly::Optional::value()` (modelled),return from call to `folly::Optional::value()` (modelled),invalid access occurs here]
codetoanalyze/cpp/pulse/optional.cpp, none_copy_bad, 3, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),is folly::None,use-after-lifetime part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),passed as argument to `folly::Optional::Optional(arg)` (modelled),return from call to `folly::Optional::Optional(arg)` (modelled),passed as argument to `folly::Optional::value()` (modelled),return from call to `folly::Optional::value()` (modelled),invalid access occurs here]
codetoanalyze/cpp/pulse/optional.cpp, none_no_check_bad, 2, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),is folly::None,use-after-lifetime part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),passed as argument to `folly::Optional::value()` (modelled),return from call to `folly::Optional::value()` (modelled),invalid access occurs here]
codetoanalyze/cpp/pulse/optional.cpp, operator_arrow_bad, 0, OPTIONAL_EMPTY_ACCESS, no_bucket, ERROR, [invalidation part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),is folly::None,use-after-lifetime part of the trace starts here,passed as argument to `folly::Optional::Optional(=None)` (modelled),return from call to `folly::Optional::Optional(=None)` (modelled),when calling `emplace` here,parameter `state` of emplace,passed as argument to `folly::Optional<State>::operator->`,return from call to `folly::Optional<State>::operator->`,invalid access occurs here]
codetoanalyze/cpp/pulse/reference_wrapper.cpp, reference_wrapper_heap_bad, 2, USE_AFTER_DELETE, no_bucket, ERROR, [invalidation part of the trace starts here,variable `rw` declared here,when calling `getwrapperHeap` here,variable `a` declared here,passed as argument to `WrapsB::WrapsB`,assigned,return from call to `WrapsB::WrapsB`,when calling `WrapsB::~WrapsB` here,parameter `this` of WrapsB::~WrapsB,when calling `WrapsB::__infer_inner_destructor_~WrapsB` here,parameter `this` of WrapsB::__infer_inner_destructor_~WrapsB,was invalidated by `delete`,use-after-lifetime part of the trace starts here,variable `rw` declared here,passed as argument to `getwrapperHeap`,variable `a` declared here,passed as argument to `WrapsB::WrapsB`,assigned,return from call to `WrapsB::WrapsB`,passed as argument to `ReferenceWrapperHeap::ReferenceWrapperHeap`,parameter `a` of ReferenceWrapperHeap::ReferenceWrapperHeap,passed as argument to `WrapsB::getb`,return from call to `WrapsB::getb`,assigned,return from call to `ReferenceWrapperHeap::ReferenceWrapperHeap`,return from call to `getwrapperHeap`,invalid access occurs here]
codetoanalyze/cpp/pulse/reference_wrapper.cpp, reference_wrapper_stack_bad, 2, USE_AFTER_LIFETIME, no_bucket, ERROR, [invalidation part of the trace starts here,variable `rw` declared here,when calling `getwrapperStack` here,variable `b` declared here,is the address of a stack variable `b` whose lifetime has ended,use-after-lifetime part of the trace starts here,variable `rw` declared here,passed as argument to `getwrapperStack`,variable `b` declared here,passed as argument to `ReferenceWrapperStack::ReferenceWrapperStack`,parameter `bref` of ReferenceWrapperStack::ReferenceWrapperStack,assigned,return from call to `ReferenceWrapperStack::ReferenceWrapperStack`,return from call to `getwrapperStack`,invalid access occurs here]
codetoanalyze/cpp/pulse/returns.cpp, returns::return_literal_stack_reference_bad, 0, STACK_VARIABLE_ADDRESS_ESCAPE, no_bucket, ERROR, [variable `C++ temporary` declared here,returned here]

@ -0,0 +1,130 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
#include <vector>
namespace folly {
template <class Value>
class Optional;
struct None {
enum class _secret { _token };
explicit constexpr None(_secret) {}
};
constexpr None none{None::_secret::_token};
template <class Value>
class Optional {
public:
typedef Value value_type;
constexpr Optional();
Optional(const Optional& src);
constexpr Optional(const None&) noexcept;
constexpr Optional(const Value& newValue);
void assign(const None&);
void assign(const Optional& src);
Optional& operator=(None) noexcept {
reset();
return *this;
}
Optional& operator=(const Optional& other) {
assign(other);
return *this;
}
template <class... Args>
Value& emplace(Args&&... args);
void reset() noexcept;
constexpr Value& value() &;
constexpr bool has_value() const noexcept;
constexpr explicit operator bool() const noexcept { return has_value(); }
constexpr const Value* operator->() const { return &value(); }
constexpr Value* operator->() { return &value(); }
};
} // namespace folly
int not_none_ok() {
folly::Optional<int> foo{5};
return foo.value();
}
int none_check_ok() {
folly::Optional<int> foo{folly::none};
if (foo) {
return foo.value();
}
return -1;
}
int none_no_check_bad() {
folly::Optional<int> foo{folly::none};
return foo.value();
}
int none_copy_ok() {
folly::Optional<int> foo{5};
folly::Optional<int> bar{foo};
return bar.value();
}
int none_copy_bad() {
folly::Optional<int> foo{folly::none};
folly::Optional<int> bar{foo};
return bar.value();
}
int assign_ok() {
folly::Optional<int> foo{5};
folly::Optional<int> bar{foo};
foo = folly::none;
return bar.value();
}
int assign_bad() {
folly::Optional<int> foo{folly::none};
folly::Optional<int> bar{5};
int sum = bar.value();
bar = foo;
sum += bar.value();
return sum;
}
int assign2_bad() {
folly::Optional<int> foo{5};
int sum = foo.value();
foo = folly::none;
sum += foo.value();
return sum;
}
struct State {
std::vector<int> vec;
};
void emplace(folly::Optional<State> state) {
if (state) {
state.emplace();
}
auto pos = state->vec.begin();
}
void operator_arrow_bad() { emplace(folly::none); }
Loading…
Cancel
Save