[pulse] don't always mistake equality for aliasing
Summary:
When applying function summaries, we are careful not to violate the
summary's assumptions about non-aliasing. For example, the summary we
generate for `foo(x,y) { *x = *y; }` will have `x` and `y` be allocated
to two different `AbstractValue.t` in the heap, representing
disjointness.
However, the current logic is too coarse and also rejects passing the
same pure value to functions that made no assumption about them being
equal or different, eg `goo(int x,int y) { int z = x + y; }`. This is
because the corresponding `AbstractValue.t` are different in the
callee's summary, but are represented by only one same value in callers
such as `goo(i,i)`.
This diff restricts the "don't violate aliasing" condition to only
consider heap-allocated values. This is consistent with separation logic
by the way: we use the implication `x|->- * y|->- |- x≠y`, which is
valid only when both `x` and `y` are both allocated in the heap as in
the left-hand-side of `|-`.
Reviewed By: skcho
Differential Revision: D22574297
fbshipit-source-id: 206a18499
4 years ago
|
|
|
/*
|
|
|
|
* 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.
|
|
|
|
*/
|
|
|
|
|
|
|
|
void ifthenderef(bool b, int* x) {
|
|
|
|
if (b) {
|
|
|
|
*x = 42;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
void ifnotthenderef(bool b, int* x) {
|
|
|
|
if (!b) {
|
|
|
|
*x = 42;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
void call_ifthenderef_false_null_ok() { ifthenderef(false, nullptr); }
|
|
|
|
|
|
|
|
void call_ifthenderef_true_null_bad() { ifthenderef(true, nullptr); }
|
|
|
|
|
|
|
|
void call_ifnotthenderef_true_null_ok() { ifnotthenderef(true, nullptr); }
|
|
|
|
|
|
|
|
void call_ifnotthenderef_false_null_bad() { ifnotthenderef(false, nullptr); }
|
|
|
|
|
|
|
|
// should be FN given the current "all allocated addresses are assumed
|
|
|
|
// disjoint unless specified otherwise" but we detect the bug because
|
|
|
|
// we don't propagate pure equalities that we discover to the heap part
|
|
|
|
void FN_alias_null_deref_latent(int* x, int* y) {
|
[pulse] don't always mistake equality for aliasing
Summary:
When applying function summaries, we are careful not to violate the
summary's assumptions about non-aliasing. For example, the summary we
generate for `foo(x,y) { *x = *y; }` will have `x` and `y` be allocated
to two different `AbstractValue.t` in the heap, representing
disjointness.
However, the current logic is too coarse and also rejects passing the
same pure value to functions that made no assumption about them being
equal or different, eg `goo(int x,int y) { int z = x + y; }`. This is
because the corresponding `AbstractValue.t` are different in the
callee's summary, but are represented by only one same value in callers
such as `goo(i,i)`.
This diff restricts the "don't violate aliasing" condition to only
consider heap-allocated values. This is consistent with separation logic
by the way: we use the implication `x|->- * y|->- |- x≠y`, which is
valid only when both `x` and `y` are both allocated in the heap as in
the left-hand-side of `|-`.
Reviewed By: skcho
Differential Revision: D22574297
fbshipit-source-id: 206a18499
4 years ago
|
|
|
*x = 32;
|
|
|
|
*y = 52;
|
|
|
|
if (x == y) {
|
|
|
|
// here we have x|-> * x |-> which should be a contradiction
|
|
|
|
int* p = nullptr;
|
|
|
|
*p = 42;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
void diverge_if_alias_ok(int* x, int* y) {
|
|
|
|
if (x == y) {
|
|
|
|
for (;;)
|
|
|
|
;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
void diverge_before_null_deref_ok(int* x) {
|
[pulse] don't always mistake equality for aliasing
Summary:
When applying function summaries, we are careful not to violate the
summary's assumptions about non-aliasing. For example, the summary we
generate for `foo(x,y) { *x = *y; }` will have `x` and `y` be allocated
to two different `AbstractValue.t` in the heap, representing
disjointness.
However, the current logic is too coarse and also rejects passing the
same pure value to functions that made no assumption about them being
equal or different, eg `goo(int x,int y) { int z = x + y; }`. This is
because the corresponding `AbstractValue.t` are different in the
callee's summary, but are represented by only one same value in callers
such as `goo(i,i)`.
This diff restricts the "don't violate aliasing" condition to only
consider heap-allocated values. This is consistent with separation logic
by the way: we use the implication `x|->- * y|->- |- x≠y`, which is
valid only when both `x` and `y` are both allocated in the heap as in
the left-hand-side of `|-`.
Reviewed By: skcho
Differential Revision: D22574297
fbshipit-source-id: 206a18499
4 years ago
|
|
|
diverge_if_alias_ok(x, x);
|
|
|
|
int* p = nullptr;
|
|
|
|
*p = 42;
|
|
|
|
}
|
|
|
|
|
|
|
|
// this test makes more sense in an inter-procedural setting
|
|
|
|
void stack_addresses_are_not_null_ok() {
|
|
|
|
int x;
|
|
|
|
if (&x == nullptr) {
|
|
|
|
int* p = nullptr;
|
|
|
|
*p = 42;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
void stack_addresses_are_distinct_ok() {
|
|
|
|
int x;
|
|
|
|
int y;
|
|
|
|
if (&x == &y) {
|
|
|
|
int* p = nullptr;
|
|
|
|
*p = 42;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
// latent because of the condition "x==0" in the pre-condition
|
|
|
|
void null_test_after_deref_latent(int* x) {
|
|
|
|
*x = 42; // latent error given that x is tested for null below
|
|
|
|
if (x == nullptr) {
|
|
|
|
int* p = nullptr;
|
|
|
|
*p = 42; // should be ignored as we can never get there
|
|
|
|
}
|
|
|
|
}
|