Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make meet in AddressDomain more precise #1468

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/analyses/base.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1579,7 +1579,7 @@ struct
let set ~(ctx: _ ctx) ?(invariant=false) ?(blob_destructive=false) ?lval_raw ?rval_raw ?t_override (st: store) (lval: AD.t) (lval_type: Cil.typ) (value: value) : store =
let update_variable x t y z =
if M.tracing then M.tracel "set" ~var:x.vname "update_variable: start '%s' '%a'\nto\n%a" x.vname VD.pretty y CPA.pretty z;
let r = update_variable x t y z in (* refers to defintion that is outside of set *)
let r = update_variable x t y z in (* refers to definition that is outside of set *)
if M.tracing then M.tracel "set" ~var:x.vname "update_variable: start '%s' '%a'\nto\n%a\nresults in\n%a" x.vname VD.pretty y CPA.pretty z CPA.pretty r;
r
in
Expand Down
6 changes: 6 additions & 0 deletions src/cdomain/value/cdomains/addressDomain.ml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ struct
| StrPtr _, UnknownPtr -> None
| _, _ -> Some false

let amenable_to_meet x y = match x,y with
| StrPtr _, StrPtr _ -> true
| Addr x, Addr y when Mval.equal (Mval.top_indices x) (Mval.top_indices y) -> true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't the address set buckets already guarantee this to only be called in such cases because Mval.top_indices are the representatives?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if the buckets guarantee this already, this function can be called with two arbitrary addresses, so we can not just return true.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, although it could just be an implementation detail (like the buckets themselves are), which isn't exposed to the outside by the interface. Then such misuse would be impossible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed this assumption now to just assume that things placed into the same bucket are amenable to meet, which seems to be the case in our implementation and I think also makes sense semantically.

This largely simplifies the implementation and makes all sorts of extra considerations obsolete.

| _ -> false

let leq x y = match x, y with
| StrPtr s1, StrPtr s2 -> SD.leq s1 s2
| Addr x, Addr y -> Mval.leq x y
Expand Down Expand Up @@ -178,6 +183,7 @@ struct
struct
include SetDomain.Joined (Addr)
let may_be_equal a b = Option.value (Addr.semantic_equal a b) ~default:true
let amenable_to_meet = Addr.amenable_to_meet
end
module OffsetSplit = DisjointDomain.ProjectiveSetPairwiseMeet (Addr) (J) (Addr.UnitOffsetRepr)

Expand Down
5 changes: 4 additions & 1 deletion src/cdomain/value/cdomains/addressDomain_intf.ml
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,11 @@ sig

val semantic_equal: t -> t -> bool option
(** Check semantic equality of two addresses.

michael-schwarz marked this conversation as resolved.
Show resolved Hide resolved
@return [Some true] if definitely equal, [Some false] if definitely not equal, [None] if unknown. *)

val amenable_to_meet: t -> t -> bool
(** Whether two addresses are amenable to meet operation, i.e., their lattice meet overapproximates the intersection
of concretizations. If true, meet is used instead of semantic_equal *)
end

(** Address lattice with sublattice representatives for {!DisjointDomain}. *)
Expand Down
14 changes: 12 additions & 2 deletions src/domain/disjointDomain.ml
Original file line number Diff line number Diff line change
Expand Up @@ -182,16 +182,26 @@ module type MayEqualSetDomain =
sig
include SetDomain.S
val may_be_equal: elt -> elt -> bool
val amenable_to_meet: elt -> elt -> bool
michael-schwarz marked this conversation as resolved.
Show resolved Hide resolved
end

module ProjectiveSetPairwiseMeet (E: Printable.S) (B: MayEqualSetDomain with type elt = E.t) (R: Representative with type elt = E.t): SetDomain.S with type elt = E.t = struct
module ProjectiveSetPairwiseMeet (E: Lattice.S) (B: MayEqualSetDomain with type elt = E.t) (R: Representative with type elt = E.t): SetDomain.S with type elt = E.t = struct
include ProjectiveSet (E) (B) (R)

let meet m1 m2 =
let meet_buckets b1 b2 acc =
B.fold (fun e1 acc ->
B.fold (fun e2 acc ->
if B.may_be_equal e1 e2 then
if B.amenable_to_meet e1 e2 then
try
let m = E.meet e1 e2 in
if not (E.is_bot m) then
add m acc
else
acc
with Lattice.Uncomparable ->
failwith (GobPretty.sprintf "amenable_to_meet %a %a returned true, but meet throws!" E.pretty e1 E.pretty e2)
else if B.may_be_equal e1 e2 then
add e1 (add e2 acc)
else
acc
Expand Down
30 changes: 30 additions & 0 deletions tests/regression/13-privatized/89-write-lacking-precision.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// PARAM: --set ana.base.privatization write
#include<pthread.h>
struct a {
char* b;
};

struct a *c;
struct a h = {""};
struct a i = {"string"};

void* d(void* args) {
struct a r;
if (c->b) {
__goblint_check(strlen(h.b) == 0); // Should also work for write!
}
}

int main() {
int top;

if(top) {
c = &h;
} else {
c = &i;
}

pthread_t t;
pthread_create(&t, 0, d, 0);
return 0;
}
24 changes: 24 additions & 0 deletions tests/regression/27-inv_invariants/22-meet-ptrs.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//PARAM: --enable ana.int.interval
#include <stdio.h>
#include <stdlib.h>
#include <time.h>


int main() {
int arr[20];

int top;

int i = 2;
if(top) {
i = 8;
}

int* imprecise = &arr[i];

if(imprecise == &arr[2]) {
__goblint_check(imprecise == &arr[2]);
sim642 marked this conversation as resolved.
Show resolved Hide resolved
}

return 0;
}
Loading