From b7afa4727dcc718234e9147426b5c450d3d44845 Mon Sep 17 00:00:00 2001
From: Sam Blackshear <shb@fb.com>
Date: Thu, 11 May 2017 18:50:23 -0700
Subject: [PATCH] [hil] fix crash when translating C code that indexes string
 literals like arrays or does pointer arithmetic

Summary:
HIL had only been tested in Java, and it made some assumptions about what array expressions look like (the LHS is always resolvable to an access path) and assignments (the LHS is always an access path) that aren't true in C.
Fixed the code so we won't crash in this case.
Thanks to jeremydubreil for catching this.

Reviewed By: jeremydubreil

Differential Revision: D5047649

fbshipit-source-id: e8484f4
---
 infer/src/IR/HilExp.ml                        |  5 ++++
 infer/src/IR/HilInstr.ml                      | 25 +++++++++++++++++--
 .../codetoanalyze/cpp/quandary/arrays.cpp     | 18 +++++++++++++
 .../codetoanalyze/cpp/quandary/pointers.cpp   |  4 +++
 4 files changed, 50 insertions(+), 2 deletions(-)
 create mode 100644 infer/tests/codetoanalyze/cpp/quandary/arrays.cpp

diff --git a/infer/src/IR/HilExp.ml b/infer/src/IR/HilExp.ml
index 789be2b77..d1cff7c44 100644
--- a/infer/src/IR/HilExp.ml
+++ b/infer/src/IR/HilExp.ml
@@ -85,6 +85,11 @@ let rec of_sil ~f_resolve_id (exp : Exp.t) typ = match exp with
               AccessPath.base_of_pvar pvar typ, of_sil ~f_resolve_id value typ)
           closure.captured_vars in
       Closure (closure.name, environment)
+  | Lindex (Const (Cstr _), index_exp) ->
+      (* indexed string literal (e.g., "foo"[1]). represent this by introducing a dummy variable
+         for the string literal. if you actually need to see the value of the string literal in the
+         analysis, you should probably be using SIL *)
+      of_sil ~f_resolve_id (Exp.Lindex (Var (Ident.create_none ()), index_exp)) typ
   | Lvar _ | Lfield _ | Lindex _ ->
       match AccessPath.of_lhs_exp exp typ ~f_resolve_id with
       | Some access_path ->
diff --git a/infer/src/IR/HilInstr.ml b/infer/src/IR/HilInstr.ml
index c75af9003..65d526b7f 100644
--- a/infer/src/IR/HilInstr.ml
+++ b/infer/src/IR/HilInstr.ml
@@ -68,8 +68,29 @@ let of_sil ~f_resolve_id (instr : Sil.instr) =
   | Store (lhs_exp, typ, rhs_exp, loc) ->
       let lhs_access_path =
         match HilExp.of_sil ~f_resolve_id lhs_exp typ with
-        | AccessPath ap -> ap
-        | _ -> invalid_argf "Non-assignable LHS expression %a" Exp.pp lhs_exp in
+        | AccessPath ap ->
+            ap
+        | BinaryOperator (_, exp0, exp1) ->
+            (* pointer arithmetic. somewhere in one of the expressions, there should be at least
+               one pointer type represented as an access path. just use that access path and forget
+               about the arithmetic. if you need to model this more precisely, you should be using
+               SIL instead *)
+            begin
+              match HilExp.get_access_paths exp0 with
+              | ap :: _ ->
+                  ap
+              | [] ->
+                  begin
+                    match HilExp.get_access_paths exp1 with
+                    | ap :: _ ->
+                        ap
+                    | [] ->
+                        invalid_argf
+                          "Invalid pointer arithmetic expression %a used as LHS" Exp.pp lhs_exp
+                  end
+            end
+        | _ ->
+            invalid_argf "Non-assignable LHS expression %a" Exp.pp lhs_exp in
       Instr (Assign (lhs_access_path, HilExp.of_sil ~f_resolve_id rhs_exp typ, loc))
   | Call (ret_opt, call_exp, formals, loc, call_flags) ->
       let hil_ret = Option.map ~f:(fun (ret_id, ret_typ) -> Var.of_id ret_id, ret_typ) ret_opt in
diff --git a/infer/tests/codetoanalyze/cpp/quandary/arrays.cpp b/infer/tests/codetoanalyze/cpp/quandary/arrays.cpp
new file mode 100644
index 000000000..4278a7d76
--- /dev/null
+++ b/infer/tests/codetoanalyze/cpp/quandary/arrays.cpp
@@ -0,0 +1,18 @@
+/*
+ * Copyright (c) 2017 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+namespace arrays {
+
+// these examples used to crash the HIL conversion
+char index_of_literal_ok1() { return "foo"[1]; }
+
+char index_of_literal_ok2() { return "foo"[7]; }
+
+char index_of_literal_ok3(int i) { return "foo"[i]; }
+}
diff --git a/infer/tests/codetoanalyze/cpp/quandary/pointers.cpp b/infer/tests/codetoanalyze/cpp/quandary/pointers.cpp
index 098e6692f..287839759 100644
--- a/infer/tests/codetoanalyze/cpp/quandary/pointers.cpp
+++ b/infer/tests/codetoanalyze/cpp/quandary/pointers.cpp
@@ -64,4 +64,8 @@ void FP_reuse_pointer_as_local_ok(std::string* pointer) {
   reuse_pointer_as_local(pointer);
   __infer_taint_sink(*pointer);
 }
+
+void pointer_arithmetic_ok1(int* i) { *(i + 1) = 7; }
+
+void pointer_arithmetic_ok2(int* i) { *(2 + 7 + 5 + i + 1) = 7; }
 }