From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) by sourceware.org (Postfix) with ESMTPS id E0E773858031 for ; Tue, 31 Jan 2023 13:20:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E0E773858031 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-wm1-x336.google.com with SMTP id d4-20020a05600c3ac400b003db1de2aef0so10580244wms.2 for ; Tue, 31 Jan 2023 05:20:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=cInaeIzFprj9d9bclZsb7qZMwoozMqb3g9JXrlMoRJE=; b=NrjHm35C4iwUB01h2PiE4da3tbYX4l2rHjxTNVhu8yR2ejBeeKshFJVWeBAGp/Syro 4hHx820Gg7mSMiItozt6We5pgZA5owOvazZMtmIaqus3dOYvEj68bPQGEYrOQxsbvaAw wU4LZ0Ckkbqhrg/aZ2aoGhe3idsW9Vf7d03q73QnvS51kqZ/V/Z/IWcmu2ez/Dtf/HcI BeNzqupDKUQPw9eQiuhg8UpLorSUkHaggJIT6ZrGBkeg2aMhPRE0btt8wKFaTeKrGzdX 4C+cp02WpwOKLmfYOQ5qkjnpZvib/tLM2V+I+nK0B2Q+B1EIPalB1RoL4Nkjo436b+EO Rc5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=cInaeIzFprj9d9bclZsb7qZMwoozMqb3g9JXrlMoRJE=; b=NsKOlzbg/yfU2NeTA7B4wteUE4zgK4O1mEW8EVz60FDm7JFJ9mmpVVMcwrVHHDGGtM lz6PtQhtKC96MtG8m/yziZl+uio3ZHyUwxGohqYBeRpZnuU0Ot3BlX+kX+Ru/gx1jlPW IjC/8MH2ssSPNJ8mgCsPmfueSQ/86OVHd1F5anukedObx9iDOVErzsVzZie8t0tJ3n7G 7qRFZsXXuS3TPnwAX9UJezetwNQx/DKrF85DaQfrKb6STN+m3O60/eg5P9TJCca7P3Od OSO7CLCx3ezc58OnBGCTi1UDHcdLxkPz+l6ac6e9x1kCQIFnLz8f7E0kxepgCav48uN+ ieNQ== X-Gm-Message-State: AO0yUKWdIjva52qOejnHViVLmpJhNQ2amAGH3e3YaJ63Oj0wUqKCwqde PYWsJxU5WN2Afpm0eSKsgSLC X-Google-Smtp-Source: AK7set+diTbYia7aJOoXv80+zU98B6KeTy2PHbJOzrNu5/D9Z/jGc7BQNkkIkx/TECwu+U+JLowWJw== X-Received: by 2002:a05:600c:500d:b0:3dc:eaef:c1bb with SMTP id n13-20020a05600c500d00b003dceaefc1bbmr5414434wmr.35.1675171232561; Tue, 31 Jan 2023 05:20:32 -0800 (PST) Received: from platypus.lan ([2001:861:5e4c:3bb0:6424:328a:1734:3249]) by smtp.gmail.com with ESMTPSA id p7-20020a1c5447000000b003dc433355aasm12462649wmi.18.2023.01.31.05.20.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 31 Jan 2023 05:20:32 -0800 (PST) From: Arthur Cohen To: gcc-patches@gcc.gnu.org Cc: gcc-rust@gcc.gnu.org, Arthur Cohen Subject: [COMMITTED] gccrs: backend: Add overflow checks to every arithmetic operation Date: Tue, 31 Jan 2023 14:24:17 +0100 Message-Id: <20230131132417.661302-1-arthur.cohen@embecosm.com> X-Mailer: git-send-email 2.39.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-16.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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. Tested on x86_64-pc-linux-gnu, committed on master. --- 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 +- .../rust/execute/torture/overflow1.rs | 20 +++ 8 files changed, 223 insertions(+), 38 deletions(-) rename gcc/testsuite/rust/{compile => execute}/torture/macro-issue1426.rs (68%) create mode 100644 gcc/testsuite/rust/execute/torture/overflow1.rs 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 +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) } +} -- 2.39.1