From afc9666e15d58662e087f33133d805dca57af496 Mon Sep 17 00:00:00 2001 From: Kevin Higgs Date: Thu, 7 Jan 2021 09:23:48 -0800 Subject: [PATCH] Fix SIL to HIL conversion for Exp.Var inside Lfield and Lindex (#1372) Summary: When accessing a field or array offset of a pure variable (`Exp.Var`) that does not resolve to an access expression, `HilExp.of_sil` will create an extraneous dereference that causes `HilExp.get_typ` to fail. This pull request wraps variables that are the bases of Lfield or Lindex expressions with AddressOf before they're dereferenced (this is already done for Lvar inside `AccessExpression.of_pvar`) and adds a couple of unit tests that make sure it behaves as expected. **More details on the bug:** Given the following code: ``` if (!event_obj->dict) ``` and SIL: ``` n$6=_fun_gdb::ref_ptr>::operator->(&event_obj:gdb::ref_ptr>&) [line 38, column 8]; n$7=*n$6.dict:_object* [line 38, column 8]; PRUNE(!n$7, true); [line 38, column 8]; ``` `operator->` has return type `event_object*`, but `n$6.dict` only has access to the type of the struct, `event_object`. `of_sil` [calls](https://github.com/facebook/infer/blob/9f98368e49d411941bb8e2a1e1839bed18cd625e/infer/src/absint/HilExp.ml#L567) `access_expr_of_lhs_exp` with that type, which [calls](https://github.com/facebook/infer/blob/9f98368e49d411941bb8e2a1e1839bed18cd625e/infer/src/absint/HilExp.ml#L498) `access_exprs_of_exp` (note that `add_deref` is always true). The Lfield case will then [recurse](https://github.com/facebook/infer/blob/9f98368e49d411941bb8e2a1e1839bed18cd625e/infer/src/absint/HilExp.ml#L469) to process the Exp.Var, and `AccessExpression.of_id` will return an `AccessPath.base` that is then [dereferenced](https://github.com/facebook/infer/blob/9f98368e49d411941bb8e2a1e1839bed18cd625e/infer/src/absint/HilExp.ml#L440). When resolving types, `get_typ` will find a non-pointer type wrapped by a `Dereference` and return [None](https://github.com/facebook/infer/blob/9f98368e49d411941bb8e2a1e1839bed18cd625e/infer/src/absint/HilExp.ml#L286). To fix this, we match what [of_pvar](https://github.com/facebook/infer/blob/9f98368e49d411941bb8e2a1e1839bed18cd625e/infer/src/absint/HilExp.ml#L295) does and wrap the base in an AddressOf, which is removed by the dereference. Pull Request resolved: https://github.com/facebook/infer/pull/1372 Reviewed By: ngorogiannis Differential Revision: D25803049 Pulled By: jvillard fbshipit-source-id: ceadc8cad --- infer/src/absint/HilExp.ml | 2 +- infer/src/inferunit.ml | 1 + infer/src/unit/HilExpTests.ml | 39 +++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 infer/src/unit/HilExpTests.ml diff --git a/infer/src/absint/HilExp.ml b/infer/src/absint/HilExp.ml index 55f1b91cb..2160aa3f7 100644 --- a/infer/src/absint/HilExp.ml +++ b/infer/src/absint/HilExp.ml @@ -435,7 +435,7 @@ and access_exprs_of_exp ~include_array_indexes ~f_resolve_id ~add_deref exp0 typ in add_accesses access_expr' :: acc | None -> - let access_expr = AccessExpression.of_id id typ in + let access_expr = AccessExpression.address_of_base (AccessExpression.base_of_id id typ) in let access_expr' = if add_deref then AccessExpression.dereference access_expr else access_expr in diff --git a/infer/src/inferunit.ml b/infer/src/inferunit.ml index ddb7e1e0e..c9985c6d4 100644 --- a/infer/src/inferunit.ml +++ b/infer/src/inferunit.ml @@ -34,6 +34,7 @@ let () = ; DifferentialTests.tests ; FileDiffTests.tests ; GradleTests.tests + ; HilExpTests.tests ; IListTests.tests ; JavaClassNameTests.tests ; JavaProfilerSamplesTest.tests diff --git a/infer/src/unit/HilExpTests.ml b/infer/src/unit/HilExpTests.ml new file mode 100644 index 000000000..c88461f4f --- /dev/null +++ b/infer/src/unit/HilExpTests.ml @@ -0,0 +1,39 @@ +(* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + *) +open! IStd +open OUnit2 + +let tests = + let struct_typ = Typ.mk_struct (Typ.Name.C.from_string "struct") in + let array_typ = Typ.mk_array StdTyp.int in + let fieldname = AccessPathTestUtils.make_fieldname "field" in + let var_id = Ident.create_normal (Ident.string_to_name "n") 0 in + let f_resolve_id _ = None in + let test_of_sil = + let var_field _ = + assert_equal ~printer:(Format.asprintf "%a" HilExp.pp) + (HilExp.of_sil ~include_array_indexes:false ~f_resolve_id ~add_deref:true + (Exp.Lfield (Exp.Var var_id, fieldname, struct_typ)) + StdTyp.void) + (HilExp.AccessExpression + (HilExp.AccessExpression.field_offset + (HilExp.AccessExpression.base (Var.of_id var_id, struct_typ)) + fieldname)) + in + let var_index _ = + assert_equal ~printer:(Format.asprintf "%a" HilExp.pp) + (HilExp.of_sil ~include_array_indexes:false ~f_resolve_id ~add_deref:true + (Exp.Lindex (Exp.Var var_id, Exp.zero)) + StdTyp.int) + (HilExp.AccessExpression + (HilExp.AccessExpression.array_offset + (HilExp.AccessExpression.base (Var.of_id var_id, array_typ)) + StdTyp.int None)) + in + "of_sil" >::: ["var_field" >:: var_field; "var_index" >:: var_index] + in + "hil_exp_suite" >::: [test_of_sil]