Skip to content

Commit

Permalink
fixes arguments in the call-return observation (#970)
Browse files Browse the repository at this point in the history
* fixes arguments in the call-return observation

When a call-return is filled in with the arguments we shouldn't
re-evaluate any actuals that are not `Out`, as `In` (obviously)
and `In Out` (not that obviously) argument actuals could be already
overwritten.

A deeper story. When the C language is detected and when we have a
prototype for a function, we classify arguments that are passed by
reference (i.e., those that are passed by a non-const referenced) as
`In Out`. However, when a value is passed by reference the reference
itself is passed by value.

The fun fact, or why we didn't notice it before. There are two places
where `call-return` observation is made, namely in the Primus
Interpreter and in the Primus Lisp Interpreter. The latter was always
doing the right thing - it was passing the input arguments, and
appended the output (as this PR is doing now for the former). Since,
our analyses were always focusing on external functions (such as
malloc, strcpy, strlen, etc) which were handled by the Primus Lisp
interpreter we never noticed anything fishy.

Thanks @Isweet for finding this issue up.

* updates testsuite

* properly init bap-veri sub sub module
  • Loading branch information
ivg authored Aug 13, 2019
1 parent c599519 commit aefa92b
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
4 changes: 2 additions & 2 deletions .travis_install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ export TESTS=false
if [ "$WITH_BUILD_CACHE" == "true" ]; then
export POST_INSTALL_HOOK='
rm -rf bap-veri
git clone https://github.com/BinaryAnalysisPlatform/bap-veri.git
opam pin add bap-veri bap-veri/ -y
git submodule update --init --recursive
opam pin add bap-veri testsuite/veri/bap-veri/ -y
OPAM_SWITCH=`opam config var switch`
mkdir -p $HOME/save_opam/$OPAM_SWITCH/lib/bap
cp -r $HOME/.opam/$OPAM_SWITCH/bin/ $HOME/save_opam/$OPAM_SWITCH/
Expand Down
21 changes: 12 additions & 9 deletions lib/bap_primus/bap_primus_interpreter.ml
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,10 @@ let sexp_of_concat ((x,y),r) = results r @@ sexps [
]

let sexp_of_ite ((cond, yes, no), r) = results r @@ sexps [
string_of_value cond;
string_of_value yes;
string_of_value no;
]
string_of_value cond;
string_of_value yes;
string_of_value no;
]

let binop,on_binop =
Observation.provide ~inspect:sexp_of_binop "binop"
Expand Down Expand Up @@ -524,15 +524,17 @@ module Make (Machine : Machine) = struct
let arg_def = term normal arg_t arg_def

let arg_use t = match Arg.intent t with
| None | Some (Out|Both) -> Arg.lhs t := Arg.rhs t
| Some Out -> Arg.lhs t := Arg.rhs t
| _ -> Machine.return ()

let arg_use = term normal arg_t arg_use

let get_arg t = Env.get (Arg.lhs t)
let get_args ~input sub =
Term.enum arg_t sub |>
Seq.filter ~f:(fun x -> not input || Arg.intent x <> Some Out) |>
Seq.filter ~f:(fun x -> match input with
| true -> Arg.intent x <> Some Out
| false -> Arg.intent x = Some Out) |>
Machine.Seq.map ~f:get_arg

let iter_args t f = Machine.Seq.iter (Term.enum arg_t t) ~f
Expand All @@ -542,11 +544,12 @@ module Make (Machine : Machine) = struct
| Some entry ->
let name = Sub.name t in
iter_args t arg_def >>= fun () ->
get_args ~input:true t >>| Seq.to_list >>= fun args ->
!!Linker.Trace.call_entered (name,args) >>= fun () ->
get_args ~input:true t >>| Seq.to_list_rev >>= fun inputs ->
!!Linker.Trace.call_entered (name,List.rev inputs) >>= fun () ->
blk entry >>= fun () ->
iter_args t arg_use >>= fun () ->
get_args ~input:false t >>| Seq.to_list >>= fun args ->
get_args ~input:false t >>| Seq.to_list >>= fun rets ->
let args = List.rev_append inputs rets in
!!Linker.Trace.call_returned (name,args)


Expand Down
2 changes: 1 addition & 1 deletion testsuite

0 comments on commit aefa92b

Please sign in to comment.