Skip to content

Commit b4734c3

Browse files
committed
Fix unsound ghost witness invariant in 56-witness/69-ghost-ptr-protection
1 parent 64c11c2 commit b4734c3

2 files changed

Lines changed: 6 additions & 3 deletions

File tree

src/analyses/basePriv.ml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,10 @@ struct
891891
else if VD.equal (getg (V.protected g')) (getg (V.unprotected g')) then
892892
Invariant.none (* don't output protected invariant because it's the same as unprotected *)
893893
else (
894-
let inv = ValueDomain.invariant_global (fun g -> getg (V.protected g)) g' in (* TODO: This takes protected values of every [g], not just [g'], which might be unsound with pointers. See: https://github.com/goblint/analyzer/pull/1394#discussion_r1698136411. *)
894+
(* Only read g' as protected, everything else (e.g. pointed to variables) may be unprotected.
895+
See 56-witness/69-ghost-ptr-protection and https://github.com/goblint/analyzer/pull/1394#discussion_r1698136411. *)
896+
let read_global g = getg (if CilType.Varinfo.equal g g' then V.protected g else V.unprotected g) in
897+
let inv = ValueDomain.invariant_global read_global g' in
895898
(* Very conservative about multiple (write-)protecting mutexes: invariant is not claimed when any of them is held.
896899
It should be possible to be more precise because writes only happen with all of them held,
897900
but conjunction is unsound when one of the mutexes is temporarily unlocked.

tests/regression/56-witness/69-ghost-ptr-protection.t

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
unsafe: 1
1717
total memory locations: 3
1818

19-
TODO: Should not contain unsound flow-insensitive invariant m2_locked || (p == & g && *p == 0):
19+
Should not contain unsound flow-insensitive invariant m2_locked || (p == & g && *p == 0):
2020

2121
$ yamlWitnessStrip < witness.yml
2222
- entry_type: ghost_update
@@ -81,7 +81,7 @@ TODO: Should not contain unsound flow-insensitive invariant m2_locked || (p == &
8181
initial: "0"
8282
- entry_type: flow_insensitive_invariant
8383
flow_insensitive_invariant:
84-
string: '! multithreaded || (m2_locked || (p == & g && *p == 0))'
84+
string: '! multithreaded || (m2_locked || ((0 <= *p && *p <= 1) && p == & g))'
8585
type: assertion
8686
format: C
8787
- entry_type: flow_insensitive_invariant

0 commit comments

Comments
 (0)