From patchwork Tue Jun 20 13:06:56 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Martin_Li=C5=A1ka?= X-Patchwork-Id: 778306 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3wsSmr0p3qz9s76 for ; Tue, 20 Jun 2017 23:07:22 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="tnyH65zB"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=G0PkmpdzqUcLw4xny 5NtOl4iuWzGEAKPny6i/BwDuIy990I5TeBQkp3hN7xeruXO8J0A5r9/m9aiuRHR+ fD73nNmH30qvjRrgoj7Y1Ec7lu+2gXywbOcCEw+A34eehnkLlMf2PQhgXL+3ETMb R7ztqYUY1rWlZ+3KHM3B3bqO7g= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=f9Gzx8P9xrDGE+n00oMdBuk MTJg=; b=tnyH65zBgGLeBiLAPQMySPNy3gUbP1RL+xJAxVbiXQ8zJYoX/fyjX05 xq/1GDCtnWcrBr/ona3H0wrN3mZEyVGaTICevn63kvXuAJ1qGbiYySvvmnWv7e1/ jY0/M0UMROUJo0sWmpEI+OD8dW9iluHfCXkT9nTVkNjE0iMr31iI= Received: (qmail 112142 invoked by alias); 20 Jun 2017 13:07:12 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 112054 invoked by uid 89); 20 Jun 2017 13:07:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 20 Jun 2017 13:06:59 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 87B65AC12; Tue, 20 Jun 2017 13:06:57 +0000 (UTC) Subject: Re: [PATCH] ASAN: handle addressable params (PR sanitize/81040). To: Jakub Jelinek Cc: GCC Patches References: <20170619141340.GP2123@tucnak> <20170620093204.GF2123@tucnak> From: =?UTF-8?Q?Martin_Li=c5=a1ka?= Message-ID: Date: Tue, 20 Jun 2017 15:06:56 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <20170620093204.GF2123@tucnak> X-IsSubscribed: yes On 06/20/2017 11:32 AM, Jakub Jelinek wrote: > On Tue, Jun 20, 2017 at 11:23:36AM +0200, Martin Liška wrote: >>> Then something needs to be done for debugging too. If it is without VTA, >>> then probably just having DECL_VALUE_EXPR is good enough, otherwise >>> (VTA) you probably don't want that (or can reset it at that point), but >>> instead emit after the initialization stmt a debug stmt that the variable >>> value now lives in a different var. Though ideally we want the debugger >>> to be able to also change the value of the var, that might be harder. >>> With DECL_VALUE_EXPR on the other side the debug info will be incorrect in >>> the prologue until it is assigned to the slot. >> >> Here I'm not sure about how to distinguish whether to build or not to build >> the debug statement. According to flag_var_tracking? > > More like if (target_for_debug_bind (arg)) > And if that is false, just make sure DECL_VALUE_EXPR is set to var. > >> You mean something like: >> g = gimple_build_debug_bind (arg, var, g); >> ? > > Well, there is no stmt, so the last argument would be just NULL. > >>> I don't understand the distinction. If you turn the original parm >>> for complex/vector DECL_GIMPLE_REG_P, you should need the exact same code >>> (but I think it would be better to use the default SSA_NAME of the PARM_DECL >>> if it is a gimple reg type, rather than use the PARM_DECL itself >>> and wait for update_ssa). >> >> Yes, the test-case /gcc/testsuite/g++.dg/asan/function-argument-3.C fails for me >> as one needs to have a temporary SSA name, otherwise: >> >> /home/marxin/Programming/gcc/gcc/testsuite/g++.dg/asan/function-argument-3.C:13:1: error: invalid rhs for gimple memory store >> foo (v4si arg) >> ^~~ >> arg >> >> arg >> >> # .MEM_4 = VDEF <.MEM_1(D)> >> arg = arg; >> during GIMPLE pass: sanopt >> >> If I see correctly the function in my test-case does not have default def SSA name for the parameter. >> Thus I guess I need to create a SSA name? > > I'd expect if you have DECL_GIMPLE_REG_P set on the PARM_DECL and > use the default def, you shouldn't run into this. > > Jakub > Good I fixed that in v2, that passes regression tests. Ale objections should be resolved in the version. Ready for trunk? Martin From ed5da705250c3015e964de8d23d1aa3d0056012a Mon Sep 17 00:00:00 2001 From: marxin Date: Wed, 14 Jun 2017 11:40:01 +0200 Subject: [PATCH] ASAN: handle addressable params (PR sanitize/81040). gcc/testsuite/ChangeLog: 2017-06-19 Martin Liska PR sanitize/81040 * g++.dg/asan/function-argument-1.C: New test. * g++.dg/asan/function-argument-2.C: New test. * g++.dg/asan/function-argument-3.C: New test. gcc/ChangeLog: 2017-06-19 Martin Liska PR sanitize/81040 * sanopt.c (rewrite_usage_of_param): New function. (sanitize_rewrite_addressable_params): Likewise. (pass_sanopt::execute): Call rewrite_usage_of_param. --- gcc/sanopt.c | 132 ++++++++++++++++++++++++ gcc/testsuite/g++.dg/asan/function-argument-1.C | 30 ++++++ gcc/testsuite/g++.dg/asan/function-argument-2.C | 24 +++++ gcc/testsuite/g++.dg/asan/function-argument-3.C | 27 +++++ 4 files changed, 213 insertions(+) create mode 100644 gcc/testsuite/g++.dg/asan/function-argument-1.C create mode 100644 gcc/testsuite/g++.dg/asan/function-argument-2.C create mode 100644 gcc/testsuite/g++.dg/asan/function-argument-3.C diff --git a/gcc/sanopt.c b/gcc/sanopt.c index 16bdba76042..077811b5b93 100644 --- a/gcc/sanopt.c +++ b/gcc/sanopt.c @@ -37,6 +37,12 @@ along with GCC; see the file COPYING3. If not see #include "gimple-ssa.h" #include "tree-phinodes.h" #include "ssa-iterators.h" +#include "gimplify.h" +#include "gimple-iterator.h" +#include "gimple-walk.h" +#include "cfghooks.h" +#include "tree-dfa.h" +#include "tree-ssa.h" /* This is used to carry information about basic blocks. It is attached to the AUX field of the standard CFG block. */ @@ -858,6 +864,129 @@ sanitize_asan_mark_poison (void) } } +/* Rewrite all usages of tree OP which is a PARM_DECL with a VAR_DECL + that is it's DECL_VALUE_EXPR. */ + +static tree +rewrite_usage_of_param (tree *op, int *walk_subtrees, void *) +{ + if (TREE_CODE (*op) == PARM_DECL && DECL_VALUE_EXPR (*op) != NULL_TREE) + { + *op = DECL_VALUE_EXPR (*op); + *walk_subtrees = 0; + } + + return NULL; +} + +/* For a given function FUN, rewrite all addressable parameters so that + a new automatic variable is introduced. Right after function entry + a parameter is assigned to the variable. */ + +static void +sanitize_rewrite_addressable_params (function *fun) +{ + gimple *g; + gimple_seq stmts = NULL; + auto_vec addressable_params; + + for (tree arg = DECL_ARGUMENTS (current_function_decl); + arg; arg = DECL_CHAIN (arg)) + { + if (TREE_ADDRESSABLE (arg) && !TREE_ADDRESSABLE (TREE_TYPE (arg))) + { + TREE_ADDRESSABLE (arg) = 0; + /* The parameter is no longer addressable. */ + tree type = TREE_TYPE (arg); + addressable_params.safe_push (arg); + + /* Create a new automatic variable. */ + tree var = build_decl (DECL_SOURCE_LOCATION (arg), + VAR_DECL, DECL_NAME (arg), type); + TREE_ADDRESSABLE (var) = 1; + DECL_ARTIFICIAL (var) = 1; + DECL_SEEN_IN_BIND_EXPR_P (var) = 0; + + gimple_add_tmp_var (var); + + if (dump_file) + fprintf (dump_file, + "Rewriting parameter whose address is taken: %s\n", + IDENTIFIER_POINTER (DECL_NAME (arg))); + + SET_DECL_VALUE_EXPR (arg, var); + + /* Assign value of parameter to newly created variable. */ + if ((TREE_CODE (type) == COMPLEX_TYPE + || TREE_CODE (type) == VECTOR_TYPE)) + { + /* We need to create a SSA name that will be used for the + assignment. */ + tree tmp; + if (DECL_GIMPLE_REG_P (arg)) + tmp = ssa_default_def (cfun, arg); + else + { + tmp = make_ssa_name (type); + g = gimple_build_assign (tmp, arg); + gimple_set_location (g, DECL_SOURCE_LOCATION (arg)); + gimple_seq_add_stmt (&stmts, g); + } + g = gimple_build_assign (var, tmp); + gimple_set_location (g, DECL_SOURCE_LOCATION (arg)); + gimple_seq_add_stmt (&stmts, g); + } + else + { + dump_function (0, cfun->decl); + g = gimple_build_assign (var, arg); + gimple_set_location (g, DECL_SOURCE_LOCATION (arg)); + gimple_seq_add_stmt (&stmts, g); + } + + if (target_for_debug_bind (arg)) + { + g = gimple_build_debug_bind (arg, var, NULL); + gimple_seq_add_stmt (&stmts, g); + } + } + } + + if (addressable_params.is_empty ()) + return; + + /* Replace all usages of PARM_DECLs with the newly + created variable VAR. */ + basic_block bb; + FOR_EACH_BB_FN (bb, fun) + { + gimple_stmt_iterator gsi; + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) + { + gimple *stmt = gsi_stmt (gsi); + gimple_stmt_iterator it = gsi_for_stmt (stmt); + walk_gimple_stmt (&it, NULL, rewrite_usage_of_param, NULL); + } + for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi)) + { + gphi *phi = dyn_cast (gsi_stmt (gsi)); + for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i) + { + hash_set visited_nodes; + walk_tree (gimple_phi_arg_def_ptr (phi, i), + rewrite_usage_of_param, NULL, &visited_nodes); + } + } + } + + /* Insert default assignments at the beginning of a function. */ + basic_block entry_bb = ENTRY_BLOCK_PTR_FOR_FN (fun); + entry_bb = split_edge (single_succ_edge (entry_bb)); + + gimple_stmt_iterator gsi = gsi_start_bb (entry_bb); + gsi_insert_seq_before (&gsi, stmts, GSI_NEW_STMT); +} + unsigned int pass_sanopt::execute (function *fun) { @@ -891,6 +1020,9 @@ pass_sanopt::execute (function *fun) sanitize_asan_mark_poison (); } + if (asan_sanitize_stack_p ()) + sanitize_rewrite_addressable_params (fun); + bool use_calls = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD; diff --git a/gcc/testsuite/g++.dg/asan/function-argument-1.C b/gcc/testsuite/g++.dg/asan/function-argument-1.C new file mode 100644 index 00000000000..148c4628316 --- /dev/null +++ b/gcc/testsuite/g++.dg/asan/function-argument-1.C @@ -0,0 +1,30 @@ +// { dg-do run } +// { dg-shouldfail "asan" } + +struct A +{ + int a[5]; +}; + +static __attribute__ ((noinline)) int +goo (A *a) +{ + int *ptr = &a->a[0]; + return *(volatile int *) (ptr - 1); +} + +__attribute__ ((noinline)) int +foo (A arg) +{ + return goo (&arg); +} + +int +main () +{ + return foo (A ()); +} + +// { dg-output "ERROR: AddressSanitizer: stack-buffer-underflow on address.*(\n|\r\n|\r)" } +// { dg-output "READ of size . at.*" } +// { dg-output ".*'arg' <== Memory access at offset \[0-9\]* underflows this variable.*" } diff --git a/gcc/testsuite/g++.dg/asan/function-argument-2.C b/gcc/testsuite/g++.dg/asan/function-argument-2.C new file mode 100644 index 00000000000..3a7c33bdaaa --- /dev/null +++ b/gcc/testsuite/g++.dg/asan/function-argument-2.C @@ -0,0 +1,24 @@ +// { dg-do run } +// { dg-shouldfail "asan" } + +static __attribute__ ((noinline)) int +goo (int *a) +{ + return *(volatile int *)a; +} + +__attribute__ ((noinline)) int +foo (char arg) +{ + return goo ((int *)&arg); +} + +int +main () +{ + return foo (12); +} + +// { dg-output "ERROR: AddressSanitizer: stack-buffer-overflow on address.*(\n|\r\n|\r)" } +// { dg-output "READ of size . at.*" } +// { dg-output ".*'arg' <== Memory access at offset \[0-9\]* partially overflows this variable.*" } diff --git a/gcc/testsuite/g++.dg/asan/function-argument-3.C b/gcc/testsuite/g++.dg/asan/function-argument-3.C new file mode 100644 index 00000000000..14617ba8425 --- /dev/null +++ b/gcc/testsuite/g++.dg/asan/function-argument-3.C @@ -0,0 +1,27 @@ +// { dg-do run } +// { dg-shouldfail "asan" } + +typedef int v4si __attribute__ ((vector_size (16))); + +static __attribute__ ((noinline)) int +goo (v4si *a) +{ + return (*(volatile v4si *) (a + 1))[2]; +} + +__attribute__ ((noinline)) int +foo (v4si arg) +{ + return goo (&arg); +} + +int +main () +{ + v4si v = {1,2,3,4}; + return foo (v); +} + +// { dg-output "ERROR: AddressSanitizer: stack-buffer-overflow on address.*(\n|\r\n|\r)" } +// { dg-output "READ of size . at.*" } +// { dg-output ".*'arg' <== Memory access at offset \[0-9\]* overflows this variable.*" } -- 2.13.1