Skip to content

Commit 7e064f2

Browse files
Detect whether ghost variable is phase ghost (#2001)
* Detect if accesses come from different threads * Add phase ghosts to `goblint_lib.ml` * More documentation for extraGhosts * Start second part of phaseGhost detection * Progress on `phaseGhost` detection * Test `phaseGhost` detection * Uniqueness + Example * Turn into cram test * Fix verifier atomic type * Reuse Locator
1 parent 9748454 commit 7e064f2

12 files changed

Lines changed: 528 additions & 20 deletions

File tree

src/analyses/accessAnalysis.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ struct
2929
let init _ =
3030
collect_local := get_bool "witness.yaml.enabled" && get_bool "witness.invariant.accessed";
3131
let activated = get_string_list "ana.activated" in
32-
emit_single_threaded := List.mem (ModifiedSinceSetjmp.Spec.name ()) activated || List.mem (PoisonVariables.Spec.name ()) activated || List.mem (UseAfterFree.Spec.name ()) activated (* TODO: some of these don't have access as dependency *)
32+
emit_single_threaded := List.mem (ModifiedSinceSetjmp.Spec.name ()) activated || List.mem (PoisonVariables.Spec.name ()) activated || List.mem (UseAfterFree.Spec.name ()) activated || List.mem (PhaseGhost.Spec.name ()) activated (* TODO: some of these don't have access as dependency *)
3333

3434
let do_access (man: (D.t, G.t, C.t, V.t) man) (kind:AccessKind.t) (reach:bool) (e:exp) =
3535
if M.tracing then M.trace "access" "do_access %a %a %B" d_exp e AccessKind.pretty kind reach;

src/analyses/phaseGhost.ml

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
(** Analysis for checking whether ghost globals are only accessed by one unique thread ([phaseGhost]). *)
2+
3+
open Analyses
4+
open GoblintCil
5+
6+
module TID = ThreadIdDomain.Thread
7+
module TIDs = ConcDomain.ThreadSet
8+
9+
module Const =
10+
struct
11+
include Lattice.Flat (IntOps.BigIntOps)
12+
let name () = "ghost-constant"
13+
end
14+
15+
module IncByOne =
16+
struct
17+
include BoolDomain.MustBool
18+
let name () = "increment-by-one"
19+
end
20+
21+
module Spec =
22+
struct
23+
include IdentitySpec
24+
25+
let name () = "phaseGhost"
26+
27+
module D = MapDomain.MapBot (Basetype.Variables) (Const)
28+
include ValueContexts (D)
29+
module P = IdentityP (D)
30+
31+
module V = VarinfoV
32+
module G =
33+
struct
34+
include Lattice.Prod (TIDs) (IncByOne)
35+
let tids = fst
36+
let inc_by_one = snd
37+
let create_tids tids = (tids, IncByOne.bot ())
38+
let create_inc_by_one inc_by_one = (TIDs.bot (), inc_by_one)
39+
end
40+
41+
let initial_ghost_values () =
42+
List.fold_left (fun acc -> function
43+
| GVar (v, initinfo, _) when YamlWitness.VarSet.mem v !(YamlWitness.ghostVars) ->
44+
begin match initinfo.init with
45+
| Some (SingleInit exp) ->
46+
begin match Cil.getInteger (Cil.constFold true exp) with
47+
| Some z -> D.add v (`Lifted z) acc
48+
| None -> acc
49+
end
50+
| None when Cil.isIntegralType v.vtype ->
51+
D.add v (`Lifted Z.zero) acc
52+
| _ ->
53+
acc
54+
end
55+
| _ ->
56+
acc
57+
) (D.bot ()) !Cilfacade.current_file.globals
58+
59+
let startstate _ = initial_ghost_values ()
60+
let exitstate _ = initial_ghost_values ()
61+
62+
let tids_of_current_thread man =
63+
match man.ask Queries.CurrentThreadId with
64+
| `Lifted tid when TID.is_unique tid -> TIDs.singleton tid
65+
| _ -> TIDs.top ()
66+
67+
let increment_constant lval rval =
68+
let is_same_lval e =
69+
match Cil.stripCasts e with
70+
| Lval lval' -> CilType.Lval.equal lval lval'
71+
| _ -> false
72+
in
73+
let is_one_constant e =
74+
match Cil.getInteger (Cil.constFold true e) with
75+
| Some k -> Z.equal k Z.one
76+
| None -> false
77+
in
78+
match Cil.stripCasts rval with
79+
| BinOp (PlusA, e1, e2, _) ->
80+
if is_same_lval e1 then
81+
is_one_constant e2
82+
else if is_same_lval e2 then
83+
is_one_constant e1
84+
else
85+
false
86+
| _ ->
87+
false
88+
89+
(* This local constant folding intentionally disregards writes from other threads.
90+
It is only for the phaseGhost checker itself. *)
91+
(* This information must **not** be used to refine other analyses or returned by any query,
92+
because it is unsound in the presence of other threads interfering. By the same token, it must
93+
be not used to raise Deadcode in branch. *)
94+
let rec eval_const state e =
95+
match Cil.stripCasts e with
96+
| Const _ ->
97+
Cil.getInteger (Cil.constFold true e)
98+
| Lval (Var var, NoOffset) when YamlWitness.VarSet.mem var !(YamlWitness.ghostVars) ->
99+
begin match D.find_opt var state with
100+
| Some (`Lifted z) -> Some z
101+
| _ -> None
102+
end
103+
| UnOp (Neg, e, _) ->
104+
Option.map Z.neg (eval_const state e)
105+
| BinOp (PlusA, e1, e2, _)
106+
| BinOp (IndexPI, e1, e2, _)
107+
| BinOp (PlusPI, e1, e2, _) ->
108+
Option.bind (eval_const state e1) (fun z1 ->
109+
Option.map (Z.add z1) (eval_const state e2)
110+
)
111+
| BinOp (MinusA, e1, e2, _) ->
112+
Option.bind (eval_const state e1) (fun z1 ->
113+
Option.map (Z.sub z1) (eval_const state e2)
114+
)
115+
| BinOp (Mult, e1, e2, _) ->
116+
Option.bind (eval_const state e1) (fun z1 ->
117+
Option.map (Z.mul z1) (eval_const state e2)
118+
)
119+
| _ ->
120+
None
121+
122+
let is_increment_by_one state lval rval =
123+
increment_constant lval rval ||
124+
match eval_const state (Lval lval), eval_const state rval with
125+
| Some old_value, Some new_value ->
126+
Z.equal new_value (Z.succ old_value)
127+
| _ ->
128+
false
129+
130+
let event man e oman =
131+
match e with
132+
| Events.Access {ad; _} ->
133+
let tids = tids_of_current_thread man in
134+
Queries.AD.iter (function
135+
| Queries.AD.Addr.Addr (var, _) when YamlWitness.VarSet.mem var !(YamlWitness.ghostVars) ->
136+
man.sideg var (G.create_tids tids)
137+
| _ ->
138+
()
139+
) ad;
140+
man.local
141+
| _ ->
142+
man.local
143+
144+
let assign man lval rval =
145+
if !AnalysisState.global_initialization then
146+
man.local
147+
else
148+
match lval with
149+
| Var var, NoOffset when YamlWitness.VarSet.mem var !(YamlWitness.ghostVars) ->
150+
let inc_by_one = is_increment_by_one man.local lval rval in
151+
let local =
152+
match inc_by_one, eval_const man.local rval with
153+
| true, Some z -> D.add var (`Lifted z) man.local
154+
| _ -> D.add var (Const.top ()) man.local
155+
in
156+
man.sideg var (G.create_inc_by_one inc_by_one);
157+
local
158+
| _ ->
159+
man.local
160+
161+
let query man (type a) (q: a Queries.t): a Queries.result =
162+
match q with
163+
| Queries.WarnGlobal g ->
164+
let g: V.t = Obj.obj g in
165+
let (tidset, inc_by_one) = man.global g in
166+
if TIDs.is_top tidset then
167+
M.warn_noloc ~category:Witness "phaseGhost: global %a is accessed by a non-unique or unknown thread id" CilType.Varinfo.pretty g
168+
else
169+
(match TIDs.elements tidset with
170+
| [tid] when TID.is_unique tid ->
171+
if inc_by_one then
172+
M.info_noloc ~category:Witness "phaseGhost: global %a is only accessed by unique thread %a and is only ever increased by one" CilType.Varinfo.pretty g TID.pretty tid
173+
else
174+
M.warn_noloc ~category:Witness "phaseGhost: global %a is only accessed by unique thread %a, but is not only ever increased by one" CilType.Varinfo.pretty g TID.pretty tid
175+
| _ ->
176+
M.warn_noloc ~category:Witness "phaseGhost: global %a is accessed by multiple unique threads: %a" CilType.Varinfo.pretty g TIDs.pretty tidset)
177+
| _ ->
178+
Queries.Result.top q
179+
end
180+
181+
let _ =
182+
MCP.register_analysis ~dep:["access"; "threadid"] (module Spec : MCPSpec)

src/config/options.schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2849,7 +2849,7 @@
28492849
},
28502850
"extraGhosts": {
28512851
"title": "witness.yaml.extraGhosts",
2852-
"description": "Existing global variables to additionally treat as ghost variables during YAML witness handling.",
2852+
"description": "Existing global variables to additionally treat as ghost variables during YAML witness handling. N.B. This will assume the global is used in a way that is consistent with the rules for ghosts in witnesses.",
28532853
"type": "array",
28542854
"items": {
28552855
"type": "string"

src/goblint_lib.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ module StackTrace = StackTrace
169169
Analyses which only support other analyses. *)
170170

171171
module AccessAnalysis = AccessAnalysis
172+
module PhaseGhost = PhaseGhost
172173
module WrapperFunctionAnalysis = WrapperFunctionAnalysis
173174
module TaintPartialContexts = TaintPartialContexts
174175
module UnassumeAnalysis = UnassumeAnalysis

src/witness/yamlWitness.ml

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -425,9 +425,31 @@ let ghost_instr_loc = function
425425
| Asm (_, _, _, _, _, loc) -> Some loc
426426
| _ -> None
427427

428+
let show_ghost_update_location (loc: Cil.location) =
429+
Printf.sprintf "%s:%d:%d" loc.file loc.line loc.column
430+
431+
module GhostUpdateLocator = WitnessUtil.Locator (CilType.Location)
432+
module GhostUpdateLocationH = Hashtbl.Make (CilType.Location)
433+
434+
class ghostUpdateLocationVisitor (locations : GhostUpdateLocator.t) = object
435+
inherit nopCilVisitor
436+
437+
method! vstmt s =
438+
(match s.skind with
439+
| Instr il ->
440+
List.iter (fun instr ->
441+
Option.iter (fun loc ->
442+
GhostUpdateLocator.add locations loc loc
443+
) (ghost_instr_loc instr)
444+
) il
445+
| _ -> ());
446+
DoChildren
447+
end
448+
428449
(** CIL visitor that inserts ghost updates around matching statements.
429-
[updates] maps ["basename:line"] to the list of assignment instructions
430-
to insert after the original instruction at that location. The original
450+
[updates] maps exact resolved instruction locations to the list of
451+
assignment instructions to insert after the original instruction at that
452+
location. The original
431453
instruction together with the ghost update assignments are wrapped in
432454
[__VERIFIER_atomic_begin()] / [__VERIFIER_atomic_end()]:
433455
{[
@@ -436,9 +458,9 @@ let ghost_instr_loc = function
436458
ghostupdate;
437459
__VERIFIER_atomic_end();
438460
]}
439-
[placed] is populated with every key from [updates] that was successfully
461+
[placed] is populated with every location from [updates] that was successfully
440462
matched; callers can use this to warn about unmatched keys. *)
441-
class ghostUpdateVisitor (updates : (string, Cil.instr list) Hashtbl.t) (placed : (string, unit) Hashtbl.t) (atomic_begin : Cil.varinfo) (atomic_end : Cil.varinfo) = object
463+
class ghostUpdateVisitor (updates : Cil.instr list GhostUpdateLocationH.t) (placed : unit GhostUpdateLocationH.t) (atomic_begin : Cil.varinfo) (atomic_end : Cil.varinfo) = object
442464
inherit nopCilVisitor
443465

444466
method! vstmt s =
@@ -449,11 +471,10 @@ class ghostUpdateVisitor (updates : (string, Cil.instr list) Hashtbl.t) (placed
449471
match ghost_instr_loc instr with
450472
| None -> [instr]
451473
| Some loc ->
452-
let key = Filename.basename loc.file ^ ":" ^ string_of_int loc.line in
453-
(match Hashtbl.find_opt updates key with
474+
(match GhostUpdateLocationH.find_opt updates loc with
454475
| None | Some [] -> [instr]
455476
| Some update_instrs ->
456-
Hashtbl.replace placed key ();
477+
GhostUpdateLocationH.replace placed loc ();
457478
let abegin = Formatcil.cInstr "%v:__VERIFIER_atomic_begin();" loc
458479
[("__VERIFIER_atomic_begin", Cil.Fv atomic_begin)] in
459480
let aend = Formatcil.cInstr "%v:__VERIFIER_atomic_end();" loc
@@ -476,6 +497,7 @@ let ghostVars = ref VarSet.empty
476497
let unplaced_ghost_updates : string list ref = ref []
477498

478499
let init () =
500+
unplaced_ghost_updates := [];
479501
let file = !Cilfacade.current_file in
480502
let find_global_var name =
481503
List.find_map (function
@@ -578,13 +600,21 @@ let init () =
578600
| _ -> None
579601
)
580602
in
581-
let updates : (string, Cil.instr list) Hashtbl.t = Hashtbl.create 16 in
603+
let instruction_locations = GhostUpdateLocator.create () in
604+
visitCilFile (new ghostUpdateLocationVisitor instruction_locations) file;
605+
let updates : Cil.instr list GhostUpdateLocationH.t = GhostUpdateLocationH.create 16 in
606+
let add_update_instrs resolved_loc instrs =
607+
match GhostUpdateLocationH.find_opt updates resolved_loc with
608+
| Some old_instrs ->
609+
GhostUpdateLocationH.replace updates resolved_loc (old_instrs @ instrs)
610+
| None ->
611+
GhostUpdateLocationH.replace updates resolved_loc instrs
612+
in
582613
let collect_ghost_updates yaml_entry =
583614
match YamlWitnessType.Entry.of_yaml yaml_entry with
584615
| Ok {entry_type = YamlWitnessType.EntryType.GhostInstrumentation {ghost_updates; _}; _} ->
585616
List.iter (fun (lu: YamlWitnessType.GhostInstrumentation.LocationUpdate.t) ->
586617
let loc = loc_of_location lu.location in
587-
let key = Filename.basename loc.file ^ ":" ^ string_of_int loc.line in
588618
let instrs = List.filter_map (fun (upd: YamlWitnessType.GhostInstrumentation.Update.t) ->
589619
match find_global_var upd.variable with
590620
| None ->
@@ -601,26 +631,32 @@ let init () =
601631
| Some value_exp -> Some (Set (Cil.var v, value_exp, loc, locUnknown)))
602632
) lu.updates in
603633
if instrs <> [] then
604-
Hashtbl.replace updates key instrs
634+
match GhostUpdateLocator.find_opt instruction_locations loc with
635+
| Some resolved_locs ->
636+
GhostUpdateLocator.ES.iter (fun resolved_loc ->
637+
add_update_instrs resolved_loc instrs
638+
) resolved_locs
639+
| None ->
640+
unplaced_ghost_updates := show_ghost_update_location loc :: !unplaced_ghost_updates
605641
) ghost_updates
606642
| Ok _ -> ()
607643
| Error (`Msg m) ->
608644
M.error_noloc ~category:Witness "couldn't parse entry while extracting ghost updates: %s" m
609645
in
610646
List.iter collect_ghost_updates yaml_entries;
611-
if Hashtbl.length updates > 0 then begin
647+
if GhostUpdateLocationH.length updates > 0 then begin
612648
let find_or_make name =
613649
match find_global_var name with
614650
| Some v -> v
615-
| None -> makeVarinfo true name (TVoid [])
651+
| None -> makeVarinfo true name (TFun (TVoid [], Some [], false, []))
616652
in
617653
let atomic_begin = find_or_make "__VERIFIER_atomic_begin" in
618654
let atomic_end = find_or_make "__VERIFIER_atomic_end" in
619-
let placed : (string, unit) Hashtbl.t = Hashtbl.create 16 in
655+
let placed : unit GhostUpdateLocationH.t = GhostUpdateLocationH.create 16 in
620656
visitCilFile (new ghostUpdateVisitor updates placed atomic_begin atomic_end) file;
621-
Hashtbl.iter (fun key _ ->
622-
if not (Hashtbl.mem placed key) then
623-
unplaced_ghost_updates := key :: !unplaced_ghost_updates
657+
GhostUpdateLocationH.iter (fun loc _ ->
658+
if not (GhostUpdateLocationH.mem placed loc) then
659+
unplaced_ghost_updates := show_ghost_update_location loc :: !unplaced_ghost_updates
624660
) updates
625661
end
626662

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// CRAM
2+
// Dominik's empire thing example
3+
#include<pthread.h>
4+
#include<goblint.h>
5+
extern void __VERIFIER_atomic_begin();
6+
extern void __VERIFIER_atomic_end();
7+
8+
int x;
9+
10+
11+
void fun() {
12+
int y = x;
13+
while (y> 0) {
14+
y--;
15+
}
16+
17+
x++;
18+
}
19+
20+
int main(void) {
21+
int z;
22+
int top;
23+
24+
if(top) { x = 10000; } else { x = 1; }
25+
26+
pthread_t thread;
27+
pthread_create(&thread, NULL, (void*)fun, NULL);
28+
29+
while(z != 0) {
30+
z--;
31+
}
32+
33+
x++;
34+
35+
pthread_join(thread, NULL);
36+
37+
top = 8;
38+
top = 3;
39+
40+
return 0;
41+
}

0 commit comments

Comments
 (0)