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

Address domain bucket conflict with first field offset #998

Closed
sim642 opened this issue Feb 22, 2023 · 0 comments · Fixed by #1031
Closed

Address domain bucket conflict with first field offset #998

sim642 opened this issue Feb 22, 2023 · 0 comments · Fixed by #1031
Assignees
Labels
Milestone

Comments

@sim642
Copy link
Member

sim642 commented Feb 22, 2023

This is an issue which wouldn't be fixed by #967 and occurs in brubeck of goblint/bench#53.

An Lattice.Uncomparable exception is raised from the joining of offsets (last case):

let rec merge cop x y =
let op = match cop with `Join -> Idx.join | `Meet -> Idx.meet | `Widen -> Idx.widen | `Narrow -> Idx.narrow in
match x, y with
| `NoOffset, `NoOffset -> `NoOffset
| `NoOffset, x
| x, `NoOffset -> (match cop, cmp_zero_offset x with (* special cases not used for AddressDomain any more due to splitting *)
| (`Join | `Widen), (`MustZero | `MayZero) -> x
| (`Meet | `Narrow), (`MustZero | `MayZero) -> `NoOffset
| _ -> raise Lattice.Uncomparable)
| `Field (x1,y1), `Field (x2,y2) when CilType.Fieldinfo.equal x1 x2 -> `Field (x1, merge cop y1 y2)
| `Index (x1,y1), `Index (x2,y2) -> `Index (op x1 x2, merge cop y1 y2)
| _ -> raise Lattice.Uncomparable (* special case not used for AddressDomain any more due to splitting *)

The comment claims that the case is no longer used with buckets, but turns out it may be.

In particular, consider the join of bucketed address sets

((alloc@sid:628@tid:[main](#top)), concrat/brubeck/main.c:1841:3-1841:12).sampler ->
	((alloc@sid:628@tid:[main](#top)), concrat/brubeck/main.c:1841:3-1841:12).sampler

and

((alloc@sid:628@tid:[main](#top)), concrat/brubeck/main.c:1841:3-1841:12) ->
	((alloc@sid:628@tid:[main](#top)), concrat/brubeck/main.c:1841:3-1841:12).backend

The offset join is called on .sampler and .backend when the buckets are being merged.

This is surprising because the keys (representatives) of the buckets appear different: one has no offset and the other has .sampler.
Although the address sets buckets intend for them to be separate, the equality of offsets still uses the old special-cased logic:

let rec compare o1 o2 = match o1, o2 with
| `NoOffset, `NoOffset -> 0
| `NoOffset, x
| x, `NoOffset when cmp_zero_offset x = `MustZero -> 0 (* cannot derive due to this special case, special cases not used for AddressDomain any more due to splitting *)
| `Field (f1,o1), `Field (f2,o2) ->
let c = CilType.Fieldinfo.compare f1 f2 in
if c=0 then compare o1 o2 else c
| `Index (i1,o1), `Index (i2,o2) ->
let c = Idx.compare i1 i2 in
if c=0 then compare o1 o2 else c
| `NoOffset, _ -> -1
| _, `NoOffset -> 1
| `Field _, `Index _ -> -1
| `Index _, `Field _ -> 1

Thus, when the maps are being joined, the keys are found to be equal and hence their values are attempted to be joined.

Therefore, to fix the issue, the special cased logic should be stripped out completely (which #967 does not yet do!).

@sim642 sim642 added the bug label Feb 22, 2023
@sim642 sim642 self-assigned this Apr 3, 2023
@sim642 sim642 added this to the v2.2.0 milestone Apr 5, 2023
nathanschmidt pushed a commit to nathanschmidt/goblint-analyzer that referenced this issue Apr 30, 2023
sim642 added a commit to sim642/opam-repository that referenced this issue Sep 13, 2023
CHANGES:

* Add `setjmp`/`longjmp` analysis (goblint/analyzer#887, goblint/analyzer#970, goblint/analyzer#1015, goblint/analyzer#1019).
* Refactor race analysis to lazy distribution (goblint/analyzer#1084, goblint/analyzer#1089, goblint/analyzer#1136, goblint/analyzer#1016).
* Add thread-unsafe library function call analysis (goblint/analyzer#723, goblint/analyzer#1082).
* Add mutex type analysis and mutex API analysis (goblint/analyzer#800, goblint/analyzer#839, goblint/analyzer#1073).
* Add interval set domain and string literals domain (goblint/analyzer#901, goblint/analyzer#966, goblint/analyzer#994, goblint/analyzer#1048).
* Add affine equalities analysis (goblint/analyzer#592).
* Add use-after-free analysis (goblint/analyzer#1050, goblint/analyzer#1114).
* Add dead code elimination transformation (goblint/analyzer#850, goblint/analyzer#979).
* Add taint analysis for partial contexts (goblint/analyzer#553, goblint/analyzer#952).
* Add YAML witness validation via unassume (goblint/analyzer#796, goblint/analyzer#977, goblint/analyzer#1044, goblint/analyzer#1045, goblint/analyzer#1124).
* Add incremental analysis rename detection (goblint/analyzer#774, goblint/analyzer#777).
* Fix address sets unsoundness (goblint/analyzer#822, goblint/analyzer#967, goblint/analyzer#564, goblint/analyzer#1032, goblint/analyzer#998, goblint/analyzer#1031).
* Fix thread escape analysis unsoundness (goblint/analyzer#939, goblint/analyzer#984, goblint/analyzer#1074, goblint/analyzer#1078).
* Fix many incremental analysis issues (goblint/analyzer#627, goblint/analyzer#836, goblint/analyzer#835, goblint/analyzer#841, goblint/analyzer#932, goblint/analyzer#678, goblint/analyzer#942, goblint/analyzer#949, goblint/analyzer#950, goblint/analyzer#957, goblint/analyzer#955, goblint/analyzer#954, goblint/analyzer#960, goblint/analyzer#959, goblint/analyzer#1004, goblint/analyzer#558, goblint/analyzer#1010, goblint/analyzer#1091).
* Fix server mode for abstract debugging (goblint/analyzer#983, goblint/analyzer#990, goblint/analyzer#997, goblint/analyzer#1000, goblint/analyzer#1001, goblint/analyzer#1013, goblint/analyzer#1018, goblint/analyzer#1017, goblint/analyzer#1026, goblint/analyzer#1027).
* Add documentation for configuration JSON schema and OCaml API (goblint/analyzer#999, goblint/analyzer#1054, goblint/analyzer#1055, goblint/analyzer#1053).
* Add many library function specifications (goblint/analyzer#962, goblint/analyzer#996, goblint/analyzer#1028, goblint/analyzer#1079, goblint/analyzer#1121, goblint/analyzer#1135, goblint/analyzer#1138).
* Add OCaml 5.0 support (goblint/analyzer#1003, goblint/analyzer#945, goblint/analyzer#1162).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant