Skip to content

Commit

Permalink
Merge pull request #1198 from goblint/race-ignorable
Browse files Browse the repository at this point in the history
Check all offsets, attributes and `typedef`s for ignored race memory locations
  • Loading branch information
sim642 authored Oct 2, 2023
2 parents 7857a68 + 8166b40 commit 2a84196
Show file tree
Hide file tree
Showing 9 changed files with 264 additions and 56 deletions.
7 changes: 3 additions & 4 deletions goblint.opam
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,11 @@ dev-repo: "git+https://github.com/goblint/analyzer.git"
# on `dune build` goblint.opam will be generated from goblint.opam.template and dune-project
# also remember to generate/adjust goblint.opam.locked!
available: os-distribution != "alpine" & arch != "arm64"
# pin-depends: [
# published goblint-cil 2.0.2 is currently up-to-date, so no pin needed
# [ "goblint-cil.2.0.2" "git+https://github.com/goblint/cil.git#98598d94f796a63751e5a9d39c6b3a9fe1f32330" ]
pin-depends: [
[ "goblint-cil.2.0.2" "git+https://github.com/goblint/cil.git#398dca3d94a06a9026b3737aabf100ee3498229f" ]
# TODO: add back after release, only pinned for optimization (https://github.com/ocaml-ppx/ppx_deriving/pull/252)
# [ "ppx_deriving.5.2.1" "git+https://github.com/ocaml-ppx/ppx_deriving.git#0a89b619f94cbbfc3b0fb3255ab4fe5bc77d32d6" ]
# ]
]
post-messages: [
"Do not benchmark Goblint on OCaml 5 (https://goblint.readthedocs.io/en/latest/user-guide/benchmarking/)." {ocaml:version >= "5.0.0"}
]
6 changes: 6 additions & 0 deletions goblint.opam.locked
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,9 @@ conflicts: [
post-messages: [
"Do not benchmark Goblint on OCaml 5 (https://goblint.readthedocs.io/en/latest/user-guide/benchmarking/)." {ocaml:version >= "5.0.0"}
]
pin-depends: [
[
"goblint-cil.2.0.2"
"git+https://github.com/goblint/cil.git#398dca3d94a06a9026b3737aabf100ee3498229f"
]
]
7 changes: 3 additions & 4 deletions goblint.opam.template
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
# on `dune build` goblint.opam will be generated from goblint.opam.template and dune-project
# also remember to generate/adjust goblint.opam.locked!
available: os-distribution != "alpine" & arch != "arm64"
# pin-depends: [
# published goblint-cil 2.0.2 is currently up-to-date, so no pin needed
# [ "goblint-cil.2.0.2" "git+https://github.com/goblint/cil.git#98598d94f796a63751e5a9d39c6b3a9fe1f32330" ]
pin-depends: [
[ "goblint-cil.2.0.2" "git+https://github.com/goblint/cil.git#398dca3d94a06a9026b3737aabf100ee3498229f" ]
# TODO: add back after release, only pinned for optimization (https://github.com/ocaml-ppx/ppx_deriving/pull/252)
# [ "ppx_deriving.5.2.1" "git+https://github.com/ocaml-ppx/ppx_deriving.git#0a89b619f94cbbfc3b0fb3255ab4fe5bc77d32d6" ]
# ]
]
post-messages: [
"Do not benchmark Goblint on OCaml 5 (https://goblint.readthedocs.io/en/latest/user-guide/benchmarking/)." {ocaml:version >= "5.0.0"}
]
19 changes: 19 additions & 0 deletions src/analyses/libraryFunctions.ml
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ let c_descs_list: (string * LibraryDesc.t) list = LibraryDsl.[
("setjmp", special [__ "env" [w]] @@ fun env -> Setjmp { env });
("longjmp", special [__ "env" [r]; __ "value" []] @@ fun env value -> Longjmp { env; value });
("atexit", unknown [drop "function" [s]]);
("atomic_flag_clear", unknown [drop "obj" [w]]);
("atomic_flag_clear_explicit", unknown [drop "obj" [w]; drop "order" []]);
("atomic_flag_test_and_set", unknown [drop "obj" [r; w]]);
("atomic_flag_test_and_set_explicit", unknown [drop "obj" [r; w]; drop "order" []]);
("atomic_load", unknown [drop "obj" [r]]);
("atomic_store", unknown [drop "obj" [w]; drop "desired" []]);
]

(** C POSIX library functions.
Expand Down Expand Up @@ -434,7 +440,20 @@ let gcc_descs_list: (string * LibraryDesc.t) list = LibraryDsl.[
("__builtin_popcountl", unknown [drop "x" []]);
("__builtin_popcountll", unknown [drop "x" []]);
("__atomic_store_n", unknown [drop "ptr" [w]; drop "val" []; drop "memorder" []]);
("__atomic_store", unknown [drop "ptr" [w]; drop "val" [r]; drop "memorder" []]);
("__atomic_load_n", unknown [drop "ptr" [r]; drop "memorder" []]);
("__atomic_load", unknown [drop "ptr" [r]; drop "ret" [w]; drop "memorder" []]);
("__atomic_clear", unknown [drop "ptr" [w]; drop "memorder" []]);
("__atomic_compare_exchange_n", unknown [drop "ptr" [r; w]; drop "expected" [r; w]; drop "desired" []; drop "weak" []; drop "success_memorder" []; drop "failure_memorder" []]);
("__atomic_compare_exchange", unknown [drop "ptr" [r; w]; drop "expected" [r; w]; drop "desired" [r]; drop "weak" []; drop "success_memorder" []; drop "failure_memorder" []]);
("__atomic_fetch_add", unknown [drop "ptr" [r; w]; drop "val" []; drop "memorder" []]);
("__atomic_fetch_sub", unknown [drop "ptr" [r; w]; drop "val" []; drop "memorder" []]);
("__atomic_fetch_and", unknown [drop "ptr" [r; w]; drop "val" []; drop "memorder" []]);
("__atomic_fetch_xor", unknown [drop "ptr" [r; w]; drop "val" []; drop "memorder" []]);
("__atomic_fetch_or", unknown [drop "ptr" [r; w]; drop "val" []; drop "memorder" []]);
("__atomic_fetch_nand", unknown [drop "ptr" [r; w]; drop "val" []; drop "memorder" []]);
("__atomic_test_and_set", unknown [drop "ptr" [r; w]; drop "memorder" []]);
("__atomic_thread_fence", unknown [drop "memorder" []]);
("__sync_fetch_and_add", unknown (drop "ptr" [r; w] :: drop "value" [] :: VarArgs (drop' [])));
("__sync_fetch_and_sub", unknown (drop "ptr" [r; w] :: drop "value" [] :: VarArgs (drop' [])));
("__builtin_va_copy", unknown [drop "dest" [w]; drop "src" [r]]);
Expand Down
110 changes: 93 additions & 17 deletions src/domains/access.ml
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,105 @@ module M = Messages
(* Some helper functions to avoid flagging race warnings on atomic types, and
* other irrelevant stuff, such as mutexes and functions. *)

let is_ignorable_type (t: typ): bool =
match t with
| TNamed ({ tname = "atomic_t" | "pthread_mutex_t" | "pthread_rwlock_t" | "pthread_spinlock_t" | "spinlock_t" | "pthread_cond_t"; _ }, _) -> true
| TComp ({ cname = "__pthread_mutex_s" | "__pthread_rwlock_arch_t" | "__jmp_buf_tag" | "_pthread_cleanup_buffer" | "__pthread_cleanup_frame" | "__cancel_jmp_buf_tag"; _}, _) -> true
| TComp ({ cname; _}, _) when String.starts_with_stdlib ~prefix:"__anon" cname ->
let is_ignorable_comp_name = function
| "__pthread_mutex_s" | "__pthread_rwlock_arch_t" | "__jmp_buf_tag" | "_pthread_cleanup_buffer" | "__pthread_cleanup_frame" | "__cancel_jmp_buf_tag" | "_IO_FILE" -> true
| cname when String.starts_with_stdlib ~prefix:"__anon" cname ->
begin match Cilfacade.split_anoncomp_name cname with
| (true, Some ("__once_flag" | "__pthread_unwind_buf_t" | "__cancel_jmp_buf"), _) -> true (* anonstruct *)
| (false, Some ("pthread_mutexattr_t" | "pthread_condattr_t" | "pthread_barrierattr_t"), _) -> true (* anonunion *)
| _ -> false
end
| TComp ({ cname = "lock_class_key"; _ }, _) -> true
| TInt (IInt, attr) when hasAttribute "mutex" attr -> true
| t when hasAttribute "atomic" (typeAttrs t) -> true (* C11 _Atomic *)
| "lock_class_key" -> true (* kernel? *)
| _ -> false

let is_ignorable = function
| None -> false
| Some (v,os) when hasAttribute "thread" v.vattr && not (v.vaddrof) -> true (* Thread-Local Storage *)
| Some (v,os) when BaseUtil.is_volatile v && not (get_bool "ana.race.volatile") -> true (* volatile & races on volatiles should not be reported *)
| Some (v,os) ->
try isFunctionType v.vtype || is_ignorable_type v.vtype
with Not_found -> false
let is_ignorable_attrs attrs =
let is_ignorable_attr = function
| Attr ("volatile", _) when not (get_bool "ana.race.volatile") -> true (* volatile & races on volatiles should not be reported *)
| Attr ("atomic", _) -> true (* C11 _Atomic *)
| _ -> false
in
List.exists is_ignorable_attr attrs

let rec is_ignorable_type (t: typ): bool =
(* efficient pattern matching first *)
match t with
| TNamed ({ tname = "atomic_t" | "pthread_mutex_t" | "pthread_rwlock_t" | "pthread_spinlock_t" | "spinlock_t" | "pthread_cond_t" | "atomic_flag" | "FILE" | "__FILE"; _ }, _) -> true
| TComp ({ cname; _}, _) when is_ignorable_comp_name cname -> true
| TInt (IInt, attr) when hasAttribute "mutex" attr -> true (* kernel? *)
| TFun _ -> true
| _ ->
if is_ignorable_attrs (typeAttrsOuter t) then (* only outer because we unroll TNamed ourselves *)
true
else (
(* unroll TNamed once *)
(* can't use unrollType because we want to check TNamed-s at all intermediate typedefs as well *)
match t with
| TNamed ({ttype; _}, attrs) -> is_ignorable_type (typeAddAttributes attrs ttype)
| _ -> false
)

let rec is_ignorable_type_offset (t: typ) (o: _ Offset.t): bool =
(* similar to Cilfacade.typeOffset but we want to check types at all intermediate offsets as well *)
if is_ignorable_type t then
true (* type at offset so far ignorable, no need to recurse *)
else (
match o with
| `NoOffset -> false (* already checked t *)
| `Index (_, o') ->
begin match unrollType t with
| TArray (et, _, attrs) ->
let t' = Cilfacade.typeBlendAttributes attrs et in
is_ignorable_type_offset t' o'
| _ -> false (* index on non-array *)
end
| `Field (f, o') ->
begin match unrollType t with
| TComp (_, attrs) ->
let t' = Cilfacade.typeBlendAttributes attrs f.ftype in
is_ignorable_type_offset t' o'
| _ -> false (* field on non-compound *)
end
)

(** {!is_ignorable_type} for {!typsig}. *)
let is_ignorable_typsig (ts: typsig): bool =
(* efficient pattern matching first *)
match ts with
| TSComp (_, cname, _) when is_ignorable_comp_name cname -> true
| TSFun _ -> true
| TSBase t -> is_ignorable_type t
| _ -> is_ignorable_attrs (typeSigAttrs ts)

(** {!is_ignorable_type_offset} for {!typsig}. *)
let rec is_ignorable_typsig_offset (ts: typsig) (o: _ Offset.t): bool =
if is_ignorable_typsig ts then
true (* type at offset so far ignorable, no need to recurse *)
else (
match o with
| `NoOffset -> false (* already checked t *)
| `Index (_, o') ->
begin match ts with
| TSArray (ets, _, attrs) ->
let ts' = Cilfacade.typeSigBlendAttributes attrs ets in
is_ignorable_typsig_offset ts' o'
| _ -> false (* index on non-array *)
end
| `Field (f, o') ->
begin match ts with
| TSComp (_, _, attrs) ->
let t' = Cilfacade.typeBlendAttributes attrs f.ftype in
is_ignorable_type_offset t' o' (* switch to type because it is more precise with TNamed *)
| _ -> false (* field on non-compound *)
end
)

let is_ignorable_mval = function
| ({vaddrof = false; vattr; _}, _) when hasAttribute "thread" vattr -> true (* Thread-Local Storage *)
| (v, o) -> is_ignorable_type_offset v.vtype o (* can't use Cilfacade.typeOffset because we want to check types at all intermediate offsets as well *)

let is_ignorable_memo = function
| (`Type ts, o) -> is_ignorable_typsig_offset ts o
| (`Var v, o) -> is_ignorable_mval (v, o)

module TSH = Hashtbl.Make (CilType.Typsig)

Expand Down Expand Up @@ -195,8 +272,7 @@ let get_val_type e: acc_typ =

(** Add access to {!Memo} after distributing. *)
let add_one ~side memo: unit =
let mv = Memo.to_mval memo in
let ignorable = is_ignorable mv in
let ignorable = is_ignorable_memo memo in
if M.tracing then M.trace "access" "add_one %a (ignorable = %B)\n" Memo.pretty memo ignorable;
if not ignorable then
side memo
Expand Down
9 changes: 9 additions & 0 deletions src/util/cilfacade.ml
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,15 @@ and typeOffset basetyp =
| t -> raise (TypeOfError (Field_NonCompound (fi, t)))


let typeBlendAttributes baseAttrs = (* copied from Cilfacade.typeOffset *)
let (_, _, contageous) = partitionAttributes ~default:AttrName baseAttrs in
typeAddAttributes contageous

let typeSigBlendAttributes baseAttrs =
let (_, _, contageous) = partitionAttributes ~default:AttrName baseAttrs in
typeSigAddAttrs contageous


(** {!Cil.mkCast} using our {!typeOf}. *)
let mkCast ~(e: exp) ~(newt: typ) =
let oldt =
Expand Down
83 changes: 71 additions & 12 deletions tests/regression/04-mutex/62-simple_atomic_nr.c
Original file line number Diff line number Diff line change
@@ -1,24 +1,83 @@
#include <pthread.h>
#include <stdio.h>
#include <stdatomic.h>

atomic_int myglobal;
pthread_mutex_t mutex1 = PTHREAD_MUTEX_INITIALIZER;
pthread_mutex_t mutex2 = PTHREAD_MUTEX_INITIALIZER;
atomic_int g1;
_Atomic int g2;
_Atomic(int) g3;

atomic_int a1[1];
_Atomic int a2[1];
_Atomic(int) a3[1];

struct s {
int f0;
atomic_int f1;
_Atomic int f2;
_Atomic(int) f3;
};

struct s s1;
_Atomic struct s s2;
_Atomic(struct s) s3;

typedef atomic_int t_int1;
typedef _Atomic int t_int2;
typedef _Atomic(int) t_int3;

t_int1 t1;
t_int2 t2;
t_int3 t3;

typedef int t_int0;

_Atomic t_int0 t0;
_Atomic(t_int0) t00;

atomic_int *p0 = &g1;
int x;
// int * _Atomic p1 = &x; // TODO: https://github.com/goblint/cil/issues/64
// _Atomic(int*) p2 = &x; // TODO: https://github.com/goblint/cil/issues/64
// atomic_int * _Atomic p3 = &g1; // TODO: https://github.com/goblint/cil/issues/64

atomic_flag flag = ATOMIC_FLAG_INIT;

void *t_fun(void *arg) {
pthread_mutex_lock(&mutex1);
myglobal=myglobal+1; // NORACE
pthread_mutex_unlock(&mutex1);
g1++; // NORACE
g2++; // NORACE
g3++; // NORACE
a1[0]++; // NORACE
a2[0]++; // NORACE
a3[0]++; // NORACE
s1.f1++; // NORACE
s1.f2++; // NORACE
s1.f3++; // NORACE
s2.f0++; // NORACE
s3.f0++; // NORACE
t1++; // NORACE
t2++; // NORACE
t3++; // NORACE
t0++; // NORACE
t00++; // NORACE
(*p0)++; // NORACE
// p1++; // TODO NORACE: https://github.com/goblint/cil/issues/64
// p2++; // TODO NORACE: https://github.com/goblint/cil/issues/64
// p3++; // TODO NORACE: https://github.com/goblint/cil/issues/64
// (*p3)++; // TODO NORACE: https://github.com/goblint/cil/issues/64

struct s ss = {0};
s2 = ss; // NORACE
s3 = ss; // NORACE

atomic_flag_clear(&flag); // NORACE
atomic_flag_test_and_set(&flag); // NORACE
return NULL;
}

int main(void) {
pthread_t id;
pthread_t id, id2;
pthread_create(&id, NULL, t_fun, NULL);
pthread_mutex_lock(&mutex2);
myglobal=myglobal+1; // NORACE
pthread_mutex_unlock(&mutex2);
pthread_join (id, NULL);
pthread_create(&id2, NULL, t_fun, NULL);
pthread_join(id, NULL);
pthread_join(id2, NULL);
return 0;
}
51 changes: 43 additions & 8 deletions tests/regression/04-mutex/99-volatile.c
Original file line number Diff line number Diff line change
@@ -1,18 +1,53 @@
// PARAM: --disable ana.race.volatile
#include <pthread.h>
#include <stdio.h>

volatile int myglobal;
volatile int g1;

volatile int a1[1];

struct s {
int f0;
volatile int f1;
};

struct s s1;
volatile struct s s2;

typedef volatile int t_int1;

t_int1 t1;

typedef int t_int0;

volatile t_int0 t0;

volatile int *p0 = &g1;
int x;
int * volatile p1 = &x;
volatile int * volatile p2 = &g1;

void *t_fun(void *arg) {
myglobal= 8; //NORACE
g1++; // NORACE
a1[0]++; // NORACE
s1.f1++; // NORACE
s2.f0++; // NORACE
t1++; // NORACE
t0++; // NORACE
(*p0)++; // NORACE
p1++; // NORACE
p2++; // NORACE
(*p2)++; // NORACE

struct s ss = {0};
s2 = ss; // NORACE
return NULL;
}

int main(void) {
pthread_t id;
pthread_create(&id, NULL, t_fun, (void*) &myglobal);
myglobal = 42; //NORACE
pthread_join (id, NULL);
pthread_t id, id2;
pthread_create(&id, NULL, t_fun, NULL);
pthread_create(&id2, NULL, t_fun, NULL);
pthread_join(id, NULL);
pthread_join(id2, NULL);
return 0;
}
}
Loading

0 comments on commit 2a84196

Please sign in to comment.