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

Add unknown thread ID #1224

Merged
merged 15 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from 13 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
20 changes: 10 additions & 10 deletions src/analyses/apron/relationPriv.apron.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1011,17 +1011,17 @@ struct
)
)
else (
match ConcDomain.ThreadSet.elements tids with
| [tid] ->
let lmust',l' = G.thread (getg (V.thread tid)) in
{st with priv = (w, LMust.union lmust' lmust, L.join l l')}
| _ ->
(* To match the paper more closely, one would have to join in the non-definite case too *)
(* Given how we handle lmust (for initialization), doing this might actually be beneficial given that it grows lmust *)
st
| exception SetDomain.Unsupported _ ->
(* elements throws if the thread set is top *)
if ConcDomain.ThreadSet.is_top tids then
st
else
match ConcDomain.ThreadSet.elements tids with
| [tid] ->
let lmust',l' = G.thread (getg (V.thread tid)) in
{st with priv = (w, LMust.union lmust' lmust, L.join l l')}
| _ ->
(* To match the paper more closely, one would have to join in the non-definite case too *)
(* Given how we handle lmust (for initialization), doing this might actually be beneficial given that it grows lmust *)
st
)

let thread_return ask getg sideg tid (st: relation_components_t) =
Expand Down
1 change: 1 addition & 0 deletions src/analyses/base.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2372,6 +2372,7 @@ struct
| Int n when GobOption.exists (BI.equal BI.zero) (ID.to_int n) -> st
| Address ret_a ->
begin match eval_rv (Analyses.ask_of_ctx ctx) gs st id with
| Thread a when ValueDomain.Threads.is_top a -> invalidate ~ctx (Analyses.ask_of_ctx ctx) gs st [ret_var]
Copy link
Member

Choose a reason for hiding this comment

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

In light of #1264, this and the other invalidates for ThreadJoin should also be shallow I think. pthread_join writes the return value, but doesn't dereference its previous value for writing (it even can't because it's void*).

| Thread a ->
let v = List.fold VD.join (VD.bot ()) (List.map (fun x -> G.thread (ctx.global (V.thread x))) (ValueDomain.Threads.elements a)) in
(* TODO: is this type right? *)
Expand Down
20 changes: 10 additions & 10 deletions src/analyses/basePriv.ml
Original file line number Diff line number Diff line change
Expand Up @@ -544,17 +544,17 @@ struct
)
)
else (
match ConcDomain.ThreadSet.elements tids with
| [tid] ->
let lmust',l' = G.thread (getg (V.thread tid)) in
{st with priv = (w, LMust.union lmust' lmust, L.join l l')}
| _ ->
(* To match the paper more closely, one would have to join in the non-definite case too *)
(* Given how we handle lmust (for initialization), doing this might actually be beneficial given that it grows lmust *)
st
| exception SetDomain.Unsupported _ ->
(* elements throws if the thread set is top *)
if ConcDomain.ThreadSet.is_top tids then
st
else
match ConcDomain.ThreadSet.elements tids with
| [tid] ->
let lmust',l' = G.thread (getg (V.thread tid)) in
{st with priv = (w, LMust.union lmust' lmust, L.join l l')}
| _ ->
(* To match the paper more closely, one would have to join in the non-definite case too *)
(* Given how we handle lmust (for initialization), doing this might actually be beneficial given that it grows lmust *)
st
)

let thread_return ask getg sideg tid (st: BaseComponents (D).t) =
Expand Down
10 changes: 6 additions & 4 deletions src/analyses/threadAnalysis.ml
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,18 @@ struct
| ThreadJoin { thread = id; ret_var } ->
(* TODO: generalize ThreadJoin like ThreadCreate *)
(let has_clean_exit tid = not (BatTuple.Tuple3.third (ctx.global tid)) in
let tids = ctx.ask (Queries.EvalThread id) in
let join_thread s tid =
if has_clean_exit tid && not (is_not_unique ctx tid) then
D.remove tid s
else
s
in
match TS.elements (ctx.ask (Queries.EvalThread id)) with
| [t] -> join_thread ctx.local t (* single thread *)
| _ -> ctx.local (* if several possible threads are may-joined, none are must-joined *)
| exception SetDomain.Unsupported _ -> ctx.local)
if TS.is_top tids
then ctx.local
else match TS.elements tids with
| [t] -> join_thread ctx.local t (* single thread *)
| _ -> ctx.local (* if several possible threads are may-joined, none are must-joined *))
| _ -> ctx.local

let query ctx (type a) (q: a Queries.t): a Queries.result =
Expand Down
4 changes: 2 additions & 2 deletions src/analyses/threadJoins.ml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ struct
if TIDs.is_top threads then
ctx.local
else (
(* elements throws if the thread set is top *)
(* all elements are known *)
let threads = TIDs.elements threads in
match threads with
| [tid] when TID.is_unique tid->
Expand All @@ -70,7 +70,7 @@ struct
(MustTIDs.bot(), true) (* consider everything joined, MustTIDs is reversed so bot is All threads *)
)
else (
(* elements throws if the thread set is top *)
(* all elements are known *)
let threads = TIDs.elements threads in
if List.compare_length_with threads 1 > 0 then
M.info ~category:Unsound "Ambiguous thread ID assume-joined, assuming all of those threads must-joined.";
Expand Down
2 changes: 1 addition & 1 deletion src/analyses/useAfterFree.ml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ struct
end
else if HeapVars.mem heap_var (snd ctx.local) then begin
if is_double_free then set_mem_safety_flag InvalidFree else set_mem_safety_flag InvalidDeref;
M.warn ~category:(Behavior behavior) ~tags:[CWE cwe_number] "%s might occur in current unique thread %a for heap variable %a" bug_name ThreadIdDomain.FlagConfiguredTID.pretty current CilType.Varinfo.pretty heap_var
M.warn ~category:(Behavior behavior) ~tags:[CWE cwe_number] "%s might occur in current unique thread %a for heap variable %a" bug_name ThreadIdDomain.Thread.pretty current CilType.Varinfo.pretty heap_var
end
end
| `Top ->
Expand Down
21 changes: 20 additions & 1 deletion src/cdomains/concDomain.ml
Original file line number Diff line number Diff line change
@@ -1,6 +1,25 @@
(** Domains for thread sets and their uniqueness. *)

module ThreadSet = SetDomain.ToppedSet (ThreadIdDomain.Thread) (struct let topname = "All Threads" end)
module ThreadSet =
struct
include SetDomain.Make (ThreadIdDomain.Thread)

let is_top = mem UnknownThread

let top () = singleton UnknownThread

let merge uop cop x y =
match is_top x, is_top y with
| true, true -> uop x y
| false, true -> x
| true, false -> y
| false, false -> cop x y

let meet x y = merge join meet x y

let narrow x y = merge (fun x y -> widen x (join x y)) narrow x y

end
module MustThreadSet = SetDomain.Reverse(ThreadSet)

module CreatedThreadSet = ThreadSet
Expand Down
2 changes: 1 addition & 1 deletion src/cdomains/mHP.ml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ include Printable.Std

let name () = "mhp"

module TID = ThreadIdDomain.FlagConfiguredTID
module TID = ThreadIdDomain.Thread
module Pretty = GoblintCil.Pretty

type t = {
Expand Down
75 changes: 74 additions & 1 deletion src/cdomains/threadIdDomain.ml
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,79 @@ struct
let name () = "FlagConfiguredTID: " ^ if history_enabled () then H.name () else P.name ()
end

module Thread = FlagConfiguredTID
type thread =
| Thread of FlagConfiguredTID.t
| UnknownThread
[@@deriving eq, ord, hash]

module Thread : Stateful with type t = thread =
struct
include Printable.Std
type t = thread [@@deriving eq, ord, hash]

let name () = "Thread id"
let pretty () t =
match t with
| Thread tid -> FlagConfiguredTID.pretty () tid
| UnknownThread -> Pretty.text "Unknown thread id"

let show t =
match t with
| Thread tid -> FlagConfiguredTID.show tid
| UnknownThread -> "Unknown thread id"

let printXml f t =
match t with
| Thread tid -> FlagConfiguredTID.printXml f tid
| UnknownThread -> BatPrintf.fprintf f "<value>\n<data>\nUnknown thread id\n</data>\n</value>\n"

let to_yojson t =
match t with
| Thread tid -> FlagConfiguredTID.to_yojson tid
| UnknownThread -> `String "Unknown thread id"

let relift t =
match t with
| Thread tid -> Thread (FlagConfiguredTID.relift tid)
| UnknownThread -> UnknownThread

let lift t = Thread t

let threadinit v ~multiple = Thread (FlagConfiguredTID.threadinit v ~multiple)

let is_main t =
match t with
| Thread tid -> FlagConfiguredTID.is_main tid
| UnknownThread -> false

let is_unique t =
match t with
| Thread tid -> FlagConfiguredTID.is_unique tid
| UnknownThread -> false

let may_create t1 t2 =
match t1, t2 with
| Thread tid1, Thread tid2 -> FlagConfiguredTID.may_create tid1 tid2
| _, _ -> true

let is_must_parent t1 t2 =
match t1, t2 with
| Thread tid1, Thread tid2 -> FlagConfiguredTID.is_must_parent tid1 tid2
| _, _ -> false

module D = FlagConfiguredTID.D

let threadenter ~multiple (t, d) node i v =
match t with
| Thread tid -> List.map lift (FlagConfiguredTID.threadenter ~multiple (tid, d) node i v)
| UnknownThread -> assert false

let threadspawn = FlagConfiguredTID.threadspawn

let created t d =
match t with
| Thread tid -> Option.map (List.map lift) (FlagConfiguredTID.created tid d)
| UnknownThread -> None
end

module ThreadLifted = Lift (Thread)
14 changes: 5 additions & 9 deletions src/cdomains/valueDomain.ml
Original file line number Diff line number Diff line change
Expand Up @@ -552,11 +552,9 @@ struct
| y, Blob (x,s,o) -> Blob (join (x:t) y, s, o)
| (Thread x, Thread y) -> Thread (Threads.join x y)
| (Int x, Thread y)
| (Thread y, Int x) ->
Thread y (* TODO: ignores int! *)
| (Thread y, Int x) -> Thread (Threads.join y (Threads.top ()))
| (Address x, Thread y)
| (Thread y, Address x) ->
Thread y (* TODO: ignores address! *)
| (Thread y, Address x) -> Thread (Threads.join y (Threads.top ()))
| (JmpBuf x, JmpBuf y) -> JmpBuf (JmpBufs.join x y)
| (Mutex, Mutex) -> Mutex
| (MutexAttr x, MutexAttr y) -> MutexAttr (MutexAttr.join x y)
Expand Down Expand Up @@ -585,11 +583,9 @@ struct
| (Blob x, Blob y) -> Blob (Blobs.widen x y) (* TODO: why no blob special cases like in join? *)
| (Thread x, Thread y) -> Thread (Threads.widen x y)
| (Int x, Thread y)
| (Thread y, Int x) ->
Thread y (* TODO: ignores int! *)
| (Thread y, Int x) -> Thread (Threads.widen y (Threads.join y (Threads.top ())))
| (Address x, Thread y)
| (Thread y, Address x) ->
Thread y (* TODO: ignores address! *)
| (Thread y, Address x) -> Thread (Threads.widen y (Threads.join y (Threads.top ())))
| (Mutex, Mutex) -> Mutex
| (JmpBuf x, JmpBuf y) -> JmpBuf (JmpBufs.widen x y)
| (MutexAttr x, MutexAttr y) -> MutexAttr (MutexAttr.widen x y)
Expand Down Expand Up @@ -708,7 +704,7 @@ struct
let v = invalidate_value ask voidType (CArrays.get ask n (array_idx_top)) in
Array (CArrays.set ask n (array_idx_top) v)
| t , Blob n -> Blob (Blobs.invalidate_value ask t n)
| _ , Thread _ -> state (* TODO: no top thread ID set! *)
| _ , Thread tid -> Thread (Threads.join (Threads.top ()) tid)
| _ , JmpBuf _ -> state (* TODO: no top jmpbuf *)
| _, Bot -> Bot (* Leave uninitialized value (from malloc) alone in free to avoid trashing everything. TODO: sound? *)
| t , _ -> top_value t
Expand Down
34 changes: 34 additions & 0 deletions tests/regression/51-threadjoins/07-trivial-unknowntid.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//PARAM: --set ana.activated[+] threadJoins
#include <pthread.h>

int g = 10;
int h = 10;
pthread_mutex_t A = PTHREAD_MUTEX_INITIALIZER;

void *t_fun(void *arg) {
g++; // RACE!
return NULL;
}

void *t_benign(void *arg) {
h++; // NORACE
pthread_t id2;
pthread_create(&id2, NULL, t_fun, NULL);
foo(&id2);
pthread_join(id2, NULL);
return NULL;
}

int main(void) {
int t;

pthread_t id2;
pthread_create(&id2, NULL, t_benign, NULL);
pthread_join(id2, NULL);
// t_benign and t_fun should be in here

g++; // RACE!
h++; // NORACE

return 0;
}