Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions .semgrep/cil.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ rules:
- pattern: Cil.typeOfLval
- pattern: Cil.typeOfInit
- pattern: Cil.typeOffset
- pattern: Cil.mkCast
- pattern: Cil.get_stmtLoc
paths:
exclude:
Expand Down
8 changes: 4 additions & 4 deletions src/analyses/apron/apronAnalysis.apron.ml
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,15 @@ struct

let assert_type_bounds apr x =
assert (AD.varinfo_tracked x);
let ik = Cilfacade.get_ikind x.vtype in
if not (IntDomain.should_ignore_overflow ik) then ( (* don't add type bounds for signed when assume_none *)
match Cilfacade.get_ikind x.vtype with
| ik when not (IntDomain.should_ignore_overflow ik) -> (* don't add type bounds for signed when assume_none *)
let (type_min, type_max) = IntDomain.Size.range ik in
(* TODO: don't go through CIL exp? *)
let apr = AD.assert_inv apr (BinOp (Le, Lval (Cil.var x), (Cil.kintegerCilint ik (Cilint.cilint_of_big_int type_max)), intType)) false in
let apr = AD.assert_inv apr (BinOp (Ge, Lval (Cil.var x), (Cil.kintegerCilint ik (Cilint.cilint_of_big_int type_min)), intType)) false in
apr
)
else
| _
| exception Invalid_argument _ ->
apr


Expand Down
2 changes: 1 addition & 1 deletion src/analyses/base.ml
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ struct
let toInt i =
match IdxDom.to_int @@ ID.cast_to ik i with
| Some x -> Const (CInt (x,ik, None))
| _ -> mkCast ~e:(Const (CStr ("unknown",No_encoding))) ~newt:intType
| _ -> Cilfacade.mkCast ~e:(Const (CStr ("unknown",No_encoding))) ~newt:intType

in
match o with
Expand Down
17 changes: 10 additions & 7 deletions src/cdomains/apron/apronDomain.apron.ml
Original file line number Diff line number Diff line change
Expand Up @@ -269,11 +269,12 @@ struct
| BinOp (Mod, e1, e2, _) ->
Binop (Mod, texpr1_expr_of_cil_exp e1, texpr1_expr_of_cil_exp e2, Int, Near)
| CastE (TInt _ as t, e) ->
begin match Cilfacade.typeOf e with
| e_typ when IntDomain.Size.is_cast_injective ~from_type:e_typ ~to_type:t -> (* TODO: unnecessary cast check due to overflow check below? or maybe useful in general to also assume type bounds based on argument types? *)
begin match IntDomain.Size.is_cast_injective ~from_type:(Cilfacade.typeOf e) ~to_type:t with (* TODO: unnecessary cast check due to overflow check below? or maybe useful in general to also assume type bounds based on argument types? *)
| true ->
Unop (Cast, texpr1_expr_of_cil_exp e, Int, Zero) (* TODO: what does Apron Cast actually do? just for floating point and rounding? *)
| _
| exception Cilfacade.TypeOfError _ -> (* typeOf inner e, not outer exp *)
| false
| exception Cilfacade.TypeOfError _ (* typeOf inner e, not outer exp *)
| exception Invalid_argument _ -> (* get_ikind in is_cast_injective *)
raise Unsupported_CilExp
end
| _ ->
Expand All @@ -289,7 +290,8 @@ struct
raise Unsupported_CilExp
);
expr
| exception Cilfacade.TypeOfError _ ->
| exception Cilfacade.TypeOfError _
| exception Invalid_argument _ ->
raise Unsupported_CilExp
in
texpr1_expr_of_cil_exp
Expand Down Expand Up @@ -366,7 +368,7 @@ struct
match V.to_cil_varinfo fundec v with
| Some vinfo ->
(* TODO: What to do with variables that have a type that cannot be stored into ILongLong to avoid overflows? *)
let var = Cil.mkCast ~e:(Lval(Var vinfo,NoOffset)) ~newt:longlong in
let var = Cilfacade.mkCast ~e:(Lval(Var vinfo,NoOffset)) ~newt:longlong in
let coeff, flip = coeff_to_const true c in
let prod = BinOp(Mult, coeff, var, longlong) in
if flip then
Expand Down Expand Up @@ -884,7 +886,8 @@ struct
let eval_int d e =
let module ID = Queries.ID in
match Cilfacade.get_ikind_exp e with
| exception Cilfacade.TypeOfError _ ->
| exception Cilfacade.TypeOfError _
| exception Invalid_argument _ ->
ID.top () (* real top, not a top of any ikind because we don't even know the ikind *)
| ik ->
if M.tracing then M.trace "apron" "eval_int: exp_is_cons %a = %B\n" d_plainexp e (exp_is_cons e);
Expand Down
2 changes: 1 addition & 1 deletion src/cdomains/intDomain.ml
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ struct
let name () = "IntDomLifter(" ^ (I.name ()) ^ ")"
let to_yojson x = I.to_yojson x.v
let invariant e x =
let e' = Cil.(mkCast ~e ~newt:(TInt (x.ikind, []))) in
let e' = Cilfacade.mkCast ~e ~newt:(TInt (x.ikind, [])) in
I.invariant_ikind e' x.ikind x.v
let tag x = I.tag x.v
let arbitrary ik = failwith @@ "Arbitrary not implement for " ^ (name ()) ^ "."
Expand Down
16 changes: 12 additions & 4 deletions src/util/cilfacade.ml
Original file line number Diff line number Diff line change
Expand Up @@ -315,16 +315,14 @@ let getFuns fileAST : startfuns =
let getFirstStmt fd = List.hd fd.sbody.bstmts


(* Returns the ikind of a TInt(_) and TEnum(_). Unrolls typedefs. Warns if a a different type is put in and return IInt *)
(* Returns the ikind of a TInt(_) and TEnum(_). Unrolls typedefs. *)
let rec get_ikind t =
(* important to unroll the type here, otherwise problems with typedefs *)
match Cil.unrollType t with
| TInt (ik,_)
| TEnum ({ekind = ik; _},_) -> ik
| TPtr _ -> get_ikind !Cil.upointType
| _ ->
Messages.warn "Something that we expected to be an integer type has a different type, assuming it is an IInt";
Cil.IInt
Comment on lines -325 to -327
Copy link
Member Author

Choose a reason for hiding this comment

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

This default IInt ikind caused some mismatches in query result meets. Here I've replaced it with an exception and all our tests still pass. Although I don't know why the IInt default existed, as opposed to an exception. get_ikind (and get_ikind_exp) are somewhat extensively used, which made me worry about changing this.

Copy link
Member

Choose a reason for hiding this comment

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

It seems reasonable to throw an exception when get_ikind is applied to a type that is not a int, enum, or pointer. In case we will encounter some uncaught exceptions caused by this in the future, there probably needs to be some handling specific to the call-site of get_ikind, anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly! Having the exception immediately here makes it much easier to track down from the backtrace as opposed to the ikind flowing through bunch of other code before crashing at an unrelated place.

| _ -> invalid_arg ("Cilfacade.get_ikind: non-integer type " ^ CilType.Typ.show t)

let ptrdiff_ikind () = get_ikind !ptrdiffType

Expand Down Expand Up @@ -444,6 +442,16 @@ and typeOffset basetyp =
| _ -> raise (TypeOfError Field_NonCompound)


(** {!Cil.mkCast} using our {!typeOf}. *)
let mkCast ~(e: exp) ~(newt: typ) =
let oldt =
try
typeOf e
with TypeOfError _ -> (* e might involve alloc variables, weird offsets, etc *)
Cil.voidType (* oldt is only used for avoiding duplicate cast, so this falls back to adding cast *)
in
Cil.mkCastT ~e ~oldt ~newt

let get_ikind_exp e = get_ikind (typeOf e)


Expand Down