Skip to content

Add missing type unrollings #1677

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

Merged
merged 22 commits into from
Mar 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
28dc22b
Use Cil.isIntegralType in constants analysis
sim642 Oct 28, 2024
77457e4
Use Cil.isIntegralType in extractPthread analysis
sim642 Oct 28, 2024
95e136b
Add TODOs about unrollType (issue #1600)
sim642 Oct 28, 2024
169e2ce
No need to match and unroll comparison operator type in LoopUnrolling
sim642 Feb 20, 2025
8aeaff2
Merge branch 'master' into unrolltype
sim642 Feb 21, 2025
4dad1e7
Replace some typ matching with predicates
sim642 Feb 21, 2025
0c20c66
No need to match and unroll comparison operator type in WideningThres…
sim642 Feb 21, 2025
47c9084
Add some type unrolling
sim642 Feb 21, 2025
970abfe
Don't catch exception which is not supposed to get raised
sim642 Feb 21, 2025
1acabef
Fix unrollType TODOs in BaseInvariant
sim642 Feb 24, 2025
cb49fc2
Fix more unrollType TODOs
sim642 Feb 24, 2025
6c4ff69
Unroll types in ValueDomain
sim642 Feb 24, 2025
13c3620
Unroll types in SharedFunctions
sim642 Feb 24, 2025
6b0dbaa
Remove impossible case in ValueDomain.cast
sim642 Feb 24, 2025
c477445
Remove unrollType TODOs related to CIL visitors
sim642 Feb 24, 2025
6101935
Fix ValueDomain indentation
sim642 Feb 24, 2025
cd31c1a
Handle TEnum similarly to TInt in many places
sim642 Mar 3, 2025
031001e
Remove unnecessary result type match for comparison operators
sim642 Mar 3, 2025
a857f75
Merge branch 'master' into unrolltype
sim642 Mar 3, 2025
af5bcfa
Handle TEnum similarly to TInt in BitfieldDomain
sim642 Mar 3, 2025
9c31b81
Use Cil.lenOfArray for AutoTune0.is_large_array
sim642 Mar 3, 2025
917d6bd
Simplify void return assert in base
sim642 Mar 4, 2025
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
17 changes: 8 additions & 9 deletions src/analyses/base.ml
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ struct
(* | Lval (Mem e, ofs) -> get ~man st (eval_lv ~man (Mem e, ofs)) *)
| (Mem e, ofs) ->
(*if M.tracing then M.tracel "cast" "Deref: lval: %a" d_plainlval lv;*)
let rec contains_vla (t:typ) = match t with
let rec contains_vla (t:typ) = match Cil.unrollType t with
| TPtr (t, _) -> contains_vla t
| TArray(t, None, args) -> true
| TArray(t, Some exp, args) when isConstant exp -> contains_vla t
Expand Down Expand Up @@ -1452,7 +1452,8 @@ struct
match eval_rv_address ~man man.local e with
| Address a ->
let slen = Seq.map String.length (List.to_seq (AD.to_string a)) in
let lenOf = function
let lenOf t =
match Cil.unrollType t with
| TArray (_, l, _) -> (try Some (lenOfArray l) with LenOfArray -> None)
| _ -> None
in
Expand Down Expand Up @@ -1563,7 +1564,7 @@ struct
let lval = Addr.Mval.to_cil mval in
(try `Lifted (Bytes.to_string (Hashtbl.find char_array lval))
with Not_found -> Queries.Result.top q)
| _ -> (* what about ISChar and IUChar? *)
| _ -> (* TODO: what about ISChar and IUChar? what about TEnum? *)
(* ignore @@ printf "Type %a\n" d_plaintype t; *)
Queries.Result.top q
end
Expand Down Expand Up @@ -2030,10 +2031,8 @@ struct
match exp with
| None -> nst
| Some exp ->
let t_override = match Cilfacade.fundec_return_type fundec with
| TVoid _ -> M.warn ~category:M.Category.Program "Returning a value from a void function"; assert false
| ret -> ret
in
let t_override = Cilfacade.fundec_return_type fundec in
assert (not (Cil.isVoidType t_override)); (* Returning a value from a void function, CIL removes the Return expression for us anyway. *)
let rv = eval_rv ~man man.local exp in
let st' = set ~man ~t_override nst (return_var ()) t_override rv in
match ThreadId.get_current ask with
Expand Down Expand Up @@ -2284,7 +2283,7 @@ struct
) a
| _ -> false

let get_size_of_ptr_target man ptr =
let get_size_of_ptr_target man ptr = (* TODO: deduplicate with memOutOfBounds *)
let intdom_of_int x =
ID.of_int (Cilfacade.ptrdiff_ikind ()) (Z.of_int x)
in
Expand All @@ -2301,7 +2300,7 @@ struct
let pts_elems_to_sizes (addr: Queries.AD.elt) =
begin match addr with
| Addr (v, _) ->
begin match v.vtype with
begin match Cil.unrollType v.vtype with
| TArray (item_typ, _, _) ->
let item_typ_size_in_bytes = size_of_type_in_bytes item_typ in
begin match man.ask (Queries.EvalLength ptr) with
Expand Down
8 changes: 4 additions & 4 deletions src/analyses/baseInvariant.ml
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ struct
| BinOp(op, rval, Lval x, typ) -> derived_invariant (BinOp(switchedOp op, Lval x, rval, typ)) tv
| BinOp(op, CastE (t1, c1), CastE (t2, c2), t) when (op = Eq || op = Ne) && typeSig t1 = typeSig t2 && VD.is_statically_safe_cast t1 (Cilfacade.typeOf c1) && VD.is_statically_safe_cast t2 (Cilfacade.typeOf c2)
-> derived_invariant (BinOp (op, c1, c2, t)) tv
| BinOp(op, CastE (TInt (ik, _) as t1, Lval x), rval, typ) ->
| BinOp(op, CastE (t1, Lval x), rval, typ) when Cil.isIntegralType t1 ->
begin match eval_rv ~man st (Lval x) with
| Int v ->
if VD.is_dynamically_safe_cast t1 (Cilfacade.typeOfLval x) (Int v) then
Expand All @@ -250,7 +250,7 @@ struct
`NotUnderstood
| _ -> `NotUnderstood
end
| BinOp(op, rval, CastE (TInt (_, _) as ti, Lval x), typ) ->
| BinOp(op, rval, CastE (ti, Lval x), typ) when Cil.isIntegralType ti ->
derived_invariant (BinOp (switchedOp op, CastE(ti, Lval x), rval, typ)) tv
| BinOp(op, (Const _ | AddrOf _), rval, typ) ->
(* This is last such that we never reach here with rval being Lval (it is swapped around). *)
Expand Down Expand Up @@ -615,8 +615,8 @@ struct
else
match exp, c_typed with
| UnOp (LNot, e, _), Int c ->
(match Cilfacade.typeOf e with
| TInt _ | TPtr _ ->
(match Cil.unrollType (Cilfacade.typeOf e) with
| TInt _ | TEnum _ | TPtr _ ->
let ikind = Cilfacade.get_ikind_exp e in
let c' =
match ID.to_bool (unop_ID LNot c) with
Expand Down
2 changes: 1 addition & 1 deletion src/analyses/extractPthread.ml
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ end
module Variable = struct
type t = varinfo

let is_integral v = match v.vtype with TInt _ -> true | _ -> false
let is_integral v = isIntegralType v.vtype

let is_global v = v.vglob

Expand Down
4 changes: 2 additions & 2 deletions src/analyses/memOutOfBounds.ml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ struct
) a
| _ -> false

let get_size_of_ptr_target man ptr =
let get_size_of_ptr_target man ptr = (* TODO: deduplicate with base *)
if points_to_alloc_only man ptr then
(* Ask for BlobSize from the base address (the second component being set to true) in order to avoid BlobSize giving us bot *)
man.ask (Queries.BlobSize {exp = ptr; base_address = true})
Expand All @@ -92,7 +92,7 @@ struct
set_mem_safety_flag InvalidDeref;
M.warn "Var %a is potentially accessed out-of-scope. Invalid memory access may occur" CilType.Varinfo.pretty v
);
begin match v.vtype with
begin match Cil.unrollType v.vtype with
| TArray (item_typ, _, _) ->
let item_typ_size_in_bytes = size_of_type_in_bytes item_typ in
begin match man.ask (Queries.EvalLength ptr) with
Expand Down
6 changes: 1 addition & 5 deletions src/analyses/mutexAnalysis.ml
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,7 @@ struct
| ts when Queries.TS.is_top ts ->
()
| ts ->
let f = function
| TComp (_, _) -> true
| _ -> false
in
if Queries.TS.exists f ts then
if Queries.TS.exists Cilfacade.isStructOrUnionType ts then
old_access None
end;
on_ad ad
Expand Down
3 changes: 2 additions & 1 deletion src/analyses/raceAnalysis.ml
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,8 @@ struct
| ts ->
if not (Queries.TS.is_empty ts) then
includes_uk := true;
let f = function
let f t =
match Cil.unrollType t with
| TComp (ci, _) ->
add_access_struct (conf - 50) ci
| _ -> ()
Expand Down
7 changes: 1 addition & 6 deletions src/analyses/tutorials/constants.ml
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,8 @@ struct
(* No contexts *)
include Analyses.IdentityUnitContextsSpec

let is_integer_var (v: varinfo) =
match v.vtype with
| TInt _ -> true
| _ -> false

let get_local = function
| Var v, NoOffset when is_integer_var v && not (v.vglob || v.vaddrof) -> Some v (* local integer variable whose address is never taken *)
| Var v, NoOffset when isIntegralType v.vtype && not (v.vglob || v.vaddrof) -> Some v (* local integer variable whose address is never taken *)
| _, _ -> None

(** Evaluates expressions *)
Expand Down
3 changes: 2 additions & 1 deletion src/autoTune.ml
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ class addTypeAttributeVisitor = object

(*Set arrays with important types for thread analysis to unroll*)
method! vtype typ =
(* TODO: reuse predicates in Access module (also handles TNamed correctly) *)
Copy link
Member

Choose a reason for hiding this comment

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

Should we not do this rather than leave new TODOs in the code base?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now the ones in Access aren't directly suitable for reuse here: what Access considers ignorable isn't exactly what unrolling considers important (e.g. FILE, jmp_buf, etc).

I think this needs a bit bigger refactoring. In particular, what I'd like to have is something like LibraryTypes (but initially at least less DSLy than LibraryFunctions). These are currently unconditional everywhere, but they should also be controlled by lib.activated.
I think recently @karoliineh found something in sv-benchmarks where our Linux kernel-specific overrides trigger on LDV tasks because they use the same names, but actually have stub implementations different from the kernel's. In the specific case we were perhaps imprecise because of it (?) but this is actually accidental task fingerprinting.

let is_important_type (t: typ): bool = match t with
| TNamed (info, attr) -> List.mem info.tname ["pthread_mutex_t"; "spinlock_t"; "pthread_t"]
| TInt (IInt, attr) -> hasAttribute "mutex" attr
Expand Down Expand Up @@ -375,7 +376,7 @@ class octagonVariableVisitor(varMap, globals) = object

method! vexpr = function
(*an expression of type +/- a +/- b where a,b are either variables or constants*)
| BinOp (op, e1,e2, (TInt _)) when isComparison op -> (
| BinOp (op, e1,e2, _) when isComparison op -> (
List.iter (fun var -> addOrCreateVarMapping varMap var 5 globals) (extractOctagonVars e1);
List.iter (fun var -> addOrCreateVarMapping varMap var 5 globals) (extractOctagonVars e2);
DoChildren
Expand Down
9 changes: 7 additions & 2 deletions src/autoTune0.ml
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ let collectFactors visitAction visitedObject =
ignore (visitAction visitor visitedObject);
factors

let is_large_array = function
| TArray (_,Some (Const (CInt (i,_,_))),_) -> i > Z.of_int @@ 10 * get_int "ana.base.arrays.unrolling-factor"
let is_large_array t =
match Cil.unrollType t with
| TArray (_, e, _) ->
begin match Cil.lenOfArray e with (* TODO: Cil.lenOfArray but with Z.t? *)
| i -> i > 10 * get_int "ana.base.arrays.unrolling-factor"
| exception Cil.LenOfArray -> false
end
| _ -> false
2 changes: 1 addition & 1 deletion src/cdomain/value/cdomains/int/bitfieldDomain.ml
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ module BitfieldFunctor (Ints_t : IntOps.IntOps): Bitfield_SOverflow with type in
let (min_ik, max_ik) = Size.range ik in
let (underflow, overflow) = match torg with
| None -> (false, false) (* ik does not change *)
| Some (GoblintCil.Cil.TInt (old_ik, _)) ->
| Some (GoblintCil.Cil.TInt (old_ik, _) | TEnum ({ekind = old_ik; _}, _)) ->
let underflow = Z.compare (BArith.min old_ik (z,o)) min_ik < 0 in
let overflow = Z.compare max_ik (BArith.max old_ik (z,o)) < 0 in
(underflow, overflow)
Expand Down
4 changes: 2 additions & 2 deletions src/cdomain/value/cdomains/int/congruenceDomain.ml
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ struct
let (min_ikorg, max_ikorg) = range ikorg in
ikorg = t || (max_t >=: max_ikorg && min_t <=: min_ikorg)
in
match torg with
| Some (Cil.TInt (ikorg, _)) when p ikorg ->
match Option.map Cil.unrollType torg with
| Some (Cil.TInt (ikorg, _) | TEnum ({ekind = ikorg; _}, _)) when p ikorg ->
if M.tracing then M.trace "cong-cast" "some case";
x
| _ -> top ()
Expand Down
2 changes: 1 addition & 1 deletion src/cdomain/value/cdomains/intDomain.ml
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ include DefExcDomain
include EnumsDomain
include CongruenceDomain
include BitfieldDomain
include IntDomTuple
include IntDomTuple
18 changes: 8 additions & 10 deletions src/cdomain/value/cdomains/structDomain.ml
Original file line number Diff line number Diff line change
Expand Up @@ -269,18 +269,16 @@ struct
match rem_fields with
| [] -> second_choice
| h::t -> begin
match (h.ftype, get_bool "ana.base.structs.key.prefer-ptrs", get_bool "ana.base.structs.key.avoid-ints") with
match Cil.unrollType h.ftype, get_bool "ana.base.structs.key.prefer-ptrs", get_bool "ana.base.structs.key.avoid-ints" with
| (TPtr (_, _), _, _) -> h
| (TInt (_, _), true, _)
| (TInt (_, _), _, true) -> first_appropriate_key t second_choice
| (TInt (_, _), _, _) -> h
| ((TInt _ | TEnum _), true, _)
| ((TInt _ | TEnum _), _, true) -> first_appropriate_key t second_choice
| ((TInt _ | TEnum _), _, _) -> h
| (_, false, _) -> h
| (_, _, false) -> first_appropriate_key t second_choice
| (_, _, _) ->
let second = match second_choice.ftype with
| TInt (_,_) -> h
| _ -> second_choice
in first_appropriate_key t second
let second = if Cil.isIntegralType second_choice.ftype then h else second_choice in
first_appropriate_key t second
end
in Some (first_appropriate_key fields (List.hd fields))

Expand Down Expand Up @@ -503,8 +501,8 @@ struct
let chosen_domain () = get_string "ana.base.structs.domain"

let pick_combined setting (comp: compinfo) =
let all_bool () = List.for_all (fun f -> match f.ftype with TInt(IBool, _) -> true | _ -> false) comp.cfields in
let has_ptr () = List.exists (fun f -> match f.ftype with TPtr(_, _) -> true | _ -> false) comp.cfields in
let all_bool () = List.for_all (fun f -> Cilfacade.isBoolType f.ftype) comp.cfields in
let has_ptr () = List.exists (fun f -> Cil.isPointerType f.ftype) comp.cfields in
match setting with
| "combined-sk" -> if has_ptr () then "keyed" else "simple"
| "combined-all" ->
Expand Down
Loading
Loading