[gcc r13-5537] gccrs: backend: Add overflow checks to every arithmetic operation

[gcc r13-5537] gccrs: backend: Add overflow checks to every arithmetic operation

[Arthur Cohen]

https://gcc.gnu.org/g:9c87dc0afe059ce54280dd81ff773f910a6dffdf

commit r13-5537-g9c87dc0afe059ce54280dd81ff773f910a6dffdf
Author: Arthur Cohen <arthur.cohen@embecosm.com>
Date:   Fri Aug 26 08:44:02 2022 +0200

    gccrs: backend: Add overflow checks to every arithmetic operation
    
    gcc/rust/ChangeLog:
    
            * backend/rust-compile-expr.cc (CompileExpr::visit): Insert overflow
            checks logic.
            (CompileExpr::array_copied_expr): Insert overflow checks logic.
            * backend/rust-compile-item.cc (CompileItem::visit): Insert overflow
            checks logic.
            * backend/rust-compile-type.cc (TyTyResolveCompile::visit): Insert
            overflow checks logic.
            * rust-gcc.cc (Gcc_backend::arithmetic_or_logical_expression): Differentiate
            existing function from `arithmetic_or_logical_expression_checked`.
            This function does insert perform overflow checks.
            (Gcc_backend::arithmetic_or_logical_expression_checked): New
            function.
            (is_overflowing_expr): New function. Check if an expression is an
            overflowing one (ADD, SUB, MUL).
            (fetch_overflow_builtins): New function.
            * rust-backend.h: Declare `arithmetic_or_logical_expression_checked` in
            abstract `Backend` class.
    
    gcc/testsuite/ChangeLog:
    
            * rust/debug/win64-abi.rs: Fix assertion to take into account
            overflow builtins
            * rust/compile/torture/macro-issue1426.rs: Moved to...
            * rust/execute/torture/macro-issue1426.rs: ...here.
            * rust/execute/torture/overflow1.rs: New test.

Diff:
---
 gcc/rust/backend/rust-compile-expr.cc              |  55 ++++++--
 gcc/rust/backend/rust-compile-item.cc              |   6 +
 gcc/rust/backend/rust-compile-type.cc              |   4 +
 gcc/rust/rust-backend.h                            |  19 ++-
 gcc/rust/rust-gcc.cc                               | 140 ++++++++++++++++++---
 gcc/testsuite/rust/debug/win64-abi.rs              |   8 +-
 .../torture/macro-issue1426.rs                     |   9 +-
 gcc/testsuite/rust/execute/torture/overflow1.rs    |  20 +++
 8 files changed, 223 insertions(+), 38 deletions(-)

diff --git a/gcc/rust/backend/rust-compile-expr.cc b/gcc/rust/backend/rust-compile-expr.cc
index 9db14a238a7..ea146731cbe 100644
--- a/gcc/rust/backend/rust-compile-expr.cc
+++ b/gcc/rust/backend/rust-compile-expr.cc
@@ -26,6 +26,7 @@
 #include "rust-compile-block.h"
 #include "rust-compile-implitem.h"
 #include "rust-constexpr.h"
+#include "rust-gcc.h"
 
 #include "fold-const.h"
 #include "realmpfr.h"
@@ -146,9 +147,26 @@ CompileExpr::visit (HIR::ArithmeticOrLogicalExpr &expr)
       return;
     }
 
-  translated
-    = ctx->get_backend ()->arithmetic_or_logical_expression (op, lhs, rhs,
-							     expr.get_locus ());
+  if (ctx->in_fn () && !ctx->const_context_p ())
+    {
+      auto receiver_tmp = NULL_TREE;
+      auto receiver
+	= ctx->get_backend ()->temporary_variable (ctx->peek_fn ().fndecl,
+						   NULL_TREE, TREE_TYPE (lhs),
+						   lhs, true, expr.get_locus (),
+						   &receiver_tmp);
+      auto check
+	= ctx->get_backend ()->arithmetic_or_logical_expression_checked (
+	  op, lhs, rhs, expr.get_locus (), receiver);
+
+      ctx->add_statement (check);
+      translated = receiver->get_tree (expr.get_locus ());
+    }
+  else
+    {
+      translated = ctx->get_backend ()->arithmetic_or_logical_expression (
+	op, lhs, rhs, expr.get_locus ());
+    }
 }
 
 void
@@ -176,13 +194,27 @@ CompileExpr::visit (HIR::CompoundAssignmentExpr &expr)
       return;
     }
 
-  auto operator_expr
-    = ctx->get_backend ()->arithmetic_or_logical_expression (op, lhs, rhs,
-							     expr.get_locus ());
-  tree assignment
-    = ctx->get_backend ()->assignment_statement (lhs, operator_expr,
-						 expr.get_locus ());
-  ctx->add_statement (assignment);
+  if (ctx->in_fn () && !ctx->const_context_p ())
+    {
+      auto tmp = NULL_TREE;
+      auto receiver
+	= ctx->get_backend ()->temporary_variable (ctx->peek_fn ().fndecl,
+						   NULL_TREE, TREE_TYPE (lhs),
+						   lhs, true, expr.get_locus (),
+						   &tmp);
+      auto check
+	= ctx->get_backend ()->arithmetic_or_logical_expression_checked (
+	  op, lhs, rhs, expr.get_locus (), receiver);
+      ctx->add_statement (check);
+
+      translated = ctx->get_backend ()->assignment_statement (
+	lhs, receiver->get_tree (expr.get_locus ()), expr.get_locus ());
+    }
+  else
+    {
+      translated = ctx->get_backend ()->arithmetic_or_logical_expression (
+	op, lhs, rhs, expr.get_locus ());
+    }
 }
 
 void
@@ -2383,7 +2415,10 @@ CompileExpr::array_copied_expr (Location expr_locus,
       return error_mark_node;
     }
 
+  ctx->push_const_context ();
   tree capacity_expr = CompileExpr::Compile (elems.get_num_copies_expr (), ctx);
+  ctx->pop_const_context ();
+
   if (!TREE_CONSTANT (capacity_expr))
     {
       rust_error_at (expr_locus, "non const num copies %qT", array_type);
diff --git a/gcc/rust/backend/rust-compile-item.cc b/gcc/rust/backend/rust-compile-item.cc
index 6977ddea9cb..634b983a771 100644
--- a/gcc/rust/backend/rust-compile-item.cc
+++ b/gcc/rust/backend/rust-compile-item.cc
@@ -160,6 +160,9 @@ CompileItem::visit (HIR::Function &function)
     function.get_mappings ().get_nodeid (), &canonical_path);
   rust_assert (ok);
 
+  if (function.get_qualifiers ().is_const ())
+    ctx->push_const_context ();
+
   tree fndecl
     = compile_function (ctx, function.get_function_name (),
 			function.get_self_param (),
@@ -169,6 +172,9 @@ CompileItem::visit (HIR::Function &function)
 			function.get_definition ().get (), canonical_path,
 			fntype, function.has_function_return_type ());
   reference = address_expression (fndecl, ref_locus);
+
+  if (function.get_qualifiers ().is_const ())
+    ctx->pop_const_context ();
 }
 
 void
diff --git a/gcc/rust/backend/rust-compile-type.cc b/gcc/rust/backend/rust-compile-type.cc
index a81aed7d190..fe1b7ce95e3 100644
--- a/gcc/rust/backend/rust-compile-type.cc
+++ b/gcc/rust/backend/rust-compile-type.cc
@@ -370,7 +370,11 @@ TyTyResolveCompile::visit (const TyTy::ArrayType &type)
 {
   tree element_type
     = TyTyResolveCompile::compile (ctx, type.get_element_type ());
+
+  ctx->push_const_context ();
   tree capacity_expr = CompileExpr::Compile (&type.get_capacity_expr (), ctx);
+  ctx->pop_const_context ();
+
   tree folded_capacity_expr = fold_expr (capacity_expr);
 
   translated
diff --git a/gcc/rust/rust-backend.h b/gcc/rust/rust-backend.h
index a3d37325271..01c5fc40e8c 100644
--- a/gcc/rust/rust-backend.h
+++ b/gcc/rust/rust-backend.h
@@ -235,13 +235,24 @@ public:
   // Supported values of OP are enumerated in ArithmeticOrLogicalOperator.
   virtual tree arithmetic_or_logical_expression (ArithmeticOrLogicalOperator op,
 						 tree left, tree right,
-						 Location)
+						 Location loc)
+    = 0;
+
+  // Return an expression for the operation LEFT OP RIGHT.
+  // Supported values of OP are enumerated in ArithmeticOrLogicalOperator.
+  // This function adds overflow checking and returns a list of statements to
+  // add to the current function context. The `receiver` variable refers to the
+  // variable which will contain the result of that operation.
+  virtual tree
+  arithmetic_or_logical_expression_checked (ArithmeticOrLogicalOperator op,
+					    tree left, tree right, Location loc,
+					    Bvariable *receiver)
     = 0;
 
   // Return an expression for the operation LEFT OP RIGHT.
   // Supported values of OP are enumerated in ComparisonOperator.
   virtual tree comparison_expression (ComparisonOperator op, tree left,
-				      tree right, Location)
+				      tree right, Location loc)
     = 0;
 
   // Return an expression for the operation LEFT OP RIGHT.
@@ -416,8 +427,8 @@ public:
   // variable, and may not be very useful.  This function should
   // return a variable which can be referenced later and should set
   // *PSTATEMENT to a statement which initializes the variable.
-  virtual Bvariable *temporary_variable (tree, tree, tree, tree init,
-					 bool address_is_taken,
+  virtual Bvariable *temporary_variable (tree fndecl, tree bind_tree, tree type,
+					 tree init, bool address_is_taken,
 					 Location location, tree *pstatement)
     = 0;
 
diff --git a/gcc/rust/rust-gcc.cc b/gcc/rust/rust-gcc.cc
index 82b2d33977d..cf20b5b98e0 100644
--- a/gcc/rust/rust-gcc.cc
+++ b/gcc/rust/rust-gcc.cc
@@ -53,6 +53,7 @@
 #include "rust-gcc.h"
 
 #include "backend/rust-tree.h"
+#include "backend/rust-builtins.h"
 
 // Get the tree of a variable for use as an expression.  If this is a
 // zero-sized global, create an expression that refers to the decl but
@@ -206,6 +207,10 @@ public:
   tree arithmetic_or_logical_expression (ArithmeticOrLogicalOperator op,
 					 tree left, tree right, Location);
 
+  tree arithmetic_or_logical_expression_checked (ArithmeticOrLogicalOperator op,
+						 tree left, tree right,
+						 Location, Bvariable *receiver);
+
   tree comparison_expression (ComparisonOperator op, tree left, tree right,
 			      Location);
 
@@ -1381,25 +1386,26 @@ Gcc_backend::negation_expression (NegationOperator op, tree expr_tree,
   return new_tree;
 }
 
-// Return an expression for the arithmetic or logical operation LEFT OP RIGHT.
 tree
 Gcc_backend::arithmetic_or_logical_expression (ArithmeticOrLogicalOperator op,
-					       tree left_tree, tree right_tree,
+					       tree left, tree right,
 					       Location location)
 {
   /* Check if either expression is an error, in which case we return an error
      expression. */
-  if (left_tree == error_mark_node || right_tree == error_mark_node)
+  if (left == error_mark_node || right == error_mark_node)
     return error_mark_node;
 
   /* We need to determine if we're doing floating point arithmetics of integer
      arithmetics. */
-  bool floating_point = is_floating_point (left_tree);
+  bool floating_point = is_floating_point (left);
+  auto ret = NULL_TREE;
 
   /* For arithmetic or logical operators, the resulting type should be the same
      as the lhs operand. */
-  auto tree_type = TREE_TYPE (left_tree);
+  auto tree_type = TREE_TYPE (left);
   auto original_type = tree_type;
+  auto loc = location.gcc_location ();
   auto tree_code = operator_to_tree_code (op, floating_point);
 
   /* For floating point operations we may need to extend the precision of type.
@@ -1410,21 +1416,127 @@ Gcc_backend::arithmetic_or_logical_expression (ArithmeticOrLogicalOperator op,
       extended_type = excess_precision_type (tree_type);
       if (extended_type != NULL_TREE)
 	{
-	  left_tree = convert (extended_type, left_tree);
-	  right_tree = convert (extended_type, right_tree);
+	  left = convert (extended_type, left);
+	  right = convert (extended_type, right);
 	  tree_type = extended_type;
 	}
     }
 
-  /* Construct a new tree and build an expression from it. */
-  auto new_tree = fold_build2_loc (location.gcc_location (), tree_code,
-				   tree_type, left_tree, right_tree);
-  TREE_CONSTANT (new_tree)
-    = TREE_CONSTANT (left_tree) && TREE_CONSTANT (right_tree);
+  ret = fold_build2_loc (loc, tree_code, tree_type, left, right);
+  TREE_CONSTANT (ret) = TREE_CONSTANT (left) & TREE_CONSTANT (right);
 
+  // TODO: How do we handle floating point?
   if (floating_point && extended_type != NULL_TREE)
-    new_tree = convert (original_type, new_tree);
-  return new_tree;
+    ret = convert (original_type, ret);
+
+  return ret;
+}
+
+static bool
+is_overflowing_expr (ArithmeticOrLogicalOperator op)
+{
+  switch (op)
+    {
+    case ArithmeticOrLogicalOperator::ADD:
+    case ArithmeticOrLogicalOperator::SUBTRACT:
+    case ArithmeticOrLogicalOperator::MULTIPLY:
+      return true;
+    default:
+      return false;
+    }
+}
+
+static std::pair<tree, tree>
+fetch_overflow_builtins (ArithmeticOrLogicalOperator op)
+{
+  auto builtin_ctx = Rust::Compile::BuiltinsContext::get ();
+
+  auto builtin = NULL_TREE;
+  auto abort = NULL_TREE;
+
+  switch (op)
+    {
+    case ArithmeticOrLogicalOperator::ADD:
+      builtin_ctx.lookup_simple_builtin ("add_overflow", &builtin);
+      break;
+    case ArithmeticOrLogicalOperator::SUBTRACT:
+      builtin_ctx.lookup_simple_builtin ("sub_overflow", &builtin);
+      break;
+    case ArithmeticOrLogicalOperator::MULTIPLY:
+      builtin_ctx.lookup_simple_builtin ("mul_overflow", &builtin);
+      break;
+    default:
+      gcc_unreachable ();
+      break;
+    };
+
+  builtin_ctx.lookup_simple_builtin ("abort", &abort);
+
+  rust_assert (abort);
+  rust_assert (builtin);
+
+  // FIXME: ARTHUR: This is really ugly. The builtin context should take care of
+  // that
+  TREE_SIDE_EFFECTS (abort) = 1;
+  TREE_READONLY (abort) = 0;
+
+  // FIXME: ARTHUR: Same here. Remove these!
+  TREE_SIDE_EFFECTS (builtin) = 1;
+  TREE_READONLY (builtin) = 0;
+
+  return {abort, builtin};
+}
+
+// Return an expression for the arithmetic or logical operation LEFT OP RIGHT
+// with overflow checking when possible
+tree
+Gcc_backend::arithmetic_or_logical_expression_checked (
+  ArithmeticOrLogicalOperator op, tree left, tree right, Location location,
+  Bvariable *receiver_var)
+{
+  /* Check if either expression is an error, in which case we return an error
+     expression. */
+  if (left == error_mark_node || right == error_mark_node)
+    return error_mark_node;
+
+  auto loc = location.gcc_location ();
+
+  // FIXME: Add `if (!debug_mode)`
+  // No overflow checks for floating point operations or divisions. In that
+  // case, simply assign the result of the operation to the receiver variable
+  if (is_floating_point (left) || !is_overflowing_expr (op))
+    return assignment_statement (
+      receiver_var->get_tree (location),
+      arithmetic_or_logical_expression (op, left, right, location), location);
+
+  auto receiver = receiver_var->get_tree (location);
+  TREE_ADDRESSABLE (receiver) = 1;
+  auto result_ref = build_fold_addr_expr_loc (loc, receiver);
+
+  auto builtins = fetch_overflow_builtins (op);
+  auto abort = builtins.first;
+  auto builtin = builtins.second;
+
+  auto abort_call = build_call_expr_loc (loc, abort, 0);
+
+  // FIXME: ARTHUR: Is that needed?
+  TREE_SIDE_EFFECTS (abort_call) = 1;
+  TREE_READONLY (abort_call) = 0;
+
+  auto builtin_call
+    = build_call_expr_loc (loc, builtin, 3, left, right, result_ref);
+  auto overflow_check
+    = build2_loc (loc, EQ_EXPR, boolean_type_node, builtin_call,
+		  boolean_constant_expression (true));
+
+  auto if_block = build3_loc (loc, COND_EXPR, void_type_node, overflow_check,
+			      abort_call, NULL_TREE);
+
+  // FIXME: ARTHUR: Needed?
+  TREE_SIDE_EFFECTS (if_block) = 1;
+  TREE_READONLY (if_block) = 0;
+
+  return if_block;
 }
 
 // Return an expression for the comparison operation LEFT OP RIGHT.
diff --git a/gcc/testsuite/rust/debug/win64-abi.rs b/gcc/testsuite/rust/debug/win64-abi.rs
index b2b08cd5114..4e02906a195 100644
--- a/gcc/testsuite/rust/debug/win64-abi.rs
+++ b/gcc/testsuite/rust/debug/win64-abi.rs
@@ -1,11 +1,11 @@
 // { dg-do compile { target { x86_64-*-* } } }
 // { dg-options "-gdwarf-5 -dA -w -O1 -m64" }
-pub extern "win64" fn square(num: i32) -> i32 {
-    num * num
+pub extern "win64" fn id(num: i32) -> i32 {
+    num
 }
 
 fn main() {
     // MS ABI dictates that the first argument is ecx instead of edi from the sysv world
-    // { dg-final { scan-assembler "%ecx, %ecx" } }
-    square(1);
+    // { dg-final { scan-assembler "%ecx, %eax" } }
+    id(1);
 }
diff --git a/gcc/testsuite/rust/compile/torture/macro-issue1426.rs b/gcc/testsuite/rust/execute/torture/macro-issue1426.rs
similarity index 68%
rename from gcc/testsuite/rust/compile/torture/macro-issue1426.rs
rename to gcc/testsuite/rust/execute/torture/macro-issue1426.rs
index 1b558cfa83d..4fb6a29c8c1 100644
--- a/gcc/testsuite/rust/compile/torture/macro-issue1426.rs
+++ b/gcc/testsuite/rust/execute/torture/macro-issue1426.rs
@@ -1,5 +1,3 @@
-// { dg-additional-options -fdump-tree-ccp1-raw }
-
 macro_rules! stmt {
     ($s:stmt) => {
         $s
@@ -22,11 +20,10 @@ pub fn test() -> i32 {
         let e = 5,
         let f = b + c + d + e
     );
+
     f
-    // { dg-final { scan-tree-dump-times {gimple_return <14>} 1 ccp1 { target __OPTIMIZE__ } } }
 }
 
-fn main() {
-    let _ = test();
+fn main() -> i32 {
+    test() - 14
 }
-
diff --git a/gcc/testsuite/rust/execute/torture/overflow1.rs b/gcc/testsuite/rust/execute/torture/overflow1.rs
new file mode 100644
index 00000000000..57a0824ce0a
--- /dev/null
+++ b/gcc/testsuite/rust/execute/torture/overflow1.rs
@@ -0,0 +1,20 @@
+// { dg-shouldfail "i8 overflow" }
+// { dg-options "-fdump-tree-original" }
+
+fn five() -> i8 {
+    5
+}
+
+extern "C" {
+    fn printf(fmt: *const i8, ...);
+}
+
+fn main() {
+    let a = 127i8;
+    let b = five();
+
+    // { dg-final { scan-tree-dump ADD_OVERFLOW original } }
+    let c = a + b;
+
+    unsafe { printf("%d\n\0" as *const str as *const i8, c) }
+}