From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1643) id 247353851A9A; Wed, 31 Aug 2022 10:40:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 247353851A9A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1661942445; bh=hSDUxRZyJ96nxGnbJdg2r3ixuTjsYsr/gvY8d6KCaYI=; h=From:To:Subject:Date:From; b=Fg5zU4FvCTgXLn9jPwqG2FiUGE/qFXNr8fHZo+ljaJE/xQRvQVvsTSkdxcvSZeb/m WhbgXEPMSUyjHDpDrYqp9q0fOFgfRYc5xDTcNOOKo70pvWfQ5g3Gndoy6RGZGmDehz YyOag70lk4WPKYpuTEhZajl3JGlHo9HPvGHmu1+Y= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Thomas Schwinge To: gcc-cvs@gcc.gnu.org Subject: [gcc/devel/rust/master] backend: Add overflow checks to every arithmetic operation X-Act-Checkin: gcc X-Git-Author: Arthur Cohen X-Git-Refname: refs/heads/devel/rust/master X-Git-Oldrev: ff7d6bfed2e7bc08fba1cc456daf464e6176dd46 X-Git-Newrev: fc82b68cc1ee2778f2bfc53c5de47e5b36ac8b7c Message-Id: <20220831104045.247353851A9A@sourceware.org> Date: Wed, 31 Aug 2022 10:40:45 +0000 (GMT) List-Id: https://gcc.gnu.org/g:fc82b68cc1ee2778f2bfc53c5de47e5b36ac8b7c commit fc82b68cc1ee2778f2bfc53c5de47e5b36ac8b7c Author: Arthur Cohen Date: Fri Aug 26 08:44:02 2022 +0200 backend: Add overflow checks to every arithmetic operation 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 865ad250f2c..e13c08caae4 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 @@ -2378,7 +2410,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 ceba51c2d27..96c4e7fcf61 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 eced909673e..77d84748eb5 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 126283c1a54..cda264233b1 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 9c1bb5314bc..e5dc6dacc76 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); @@ -1380,25 +1385,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. @@ -1409,21 +1415,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 +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) } +}