From patchwork Tue Oct 21 13:26:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: max X-Patchwork-Id: 401495 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 AFD0614007B for ; Wed, 22 Oct 2014 00:27:41 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=S8+zuCKHiUPiNiuGw ckSXRJPkbaMOWYocqbcAqXR5DRf77s0ZaJImeBDwiYvWbYvhiBw4iT3kW2RlRkkU 39hvLLE9gajuZ4N98njHtvgbCSRqd5QR0sayYnv2+WSD4zNT+E+EfL5enKA4zngd 07nGWXLLmUpsCIwZrZFrAPrOuQ= 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=Kodq13ge52tS2g/DtKmc1l3 YI5A=; b=tafYh/7UAdllcucDjPkibPpbA9f/WCLNuWzlATxc6v/WV9rvHnvI9VD g0F1ANUukBmfBoLdJL2HTJHGL4sAL9g6Y66vkdlpoUoQiOsgPgS/WlU5sdyZSk6n tUGHMc4mQOuv3fxEJ9nJOc+RVm/CEuK2eDCscqXZpDi0e2oC77fA= Received: (qmail 6620 invoked by alias); 21 Oct 2014 13:26:22 -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 6590 invoked by uid 89); 21 Oct 2014 13:26:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mailout1.w1.samsung.com Received: from mailout1.w1.samsung.com (HELO mailout1.w1.samsung.com) (210.118.77.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (DES-CBC3-SHA encrypted) ESMTPS; Tue, 21 Oct 2014 13:26:17 +0000 Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout1.w1.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0NDS00505QSJ2340@mailout1.w1.samsung.com> for gcc-patches@gcc.gnu.org; Tue, 21 Oct 2014 14:29:07 +0100 (BST) Received: from eusync4.samsung.com ( [203.254.199.214]) by eucpsbgm1.samsung.com (EUCPMTA) with SMTP id 9B.60.04619.4FE56445; Tue, 21 Oct 2014 14:26:12 +0100 (BST) Received: from [106.109.128.119] by eusync4.samsung.com (Oracle Communications Messaging Server 7u4-24.01(7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTPA id <0NDS00DU2QNOH410@eusync4.samsung.com>; Tue, 21 Oct 2014 14:26:12 +0100 (BST) Message-id: <54465EF3.5060601@partner.samsung.com> Date: Tue, 21 Oct 2014 17:26:11 +0400 From: Maxim Ostapenko User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-version: 1.0 To: Jakub Jelinek , Yury Gribov Cc: GCC Patches , Slava Garbuzov , Maxim Ostapenko Subject: [PATCHv2] Don't expand string/memory builtins if ASan is enabled. References: <5441008A.2010706@partner.samsung.com> <54410170.9030100@partner.samsung.com> <20141017122420.GR10376@tucnak.redhat.com> <5441132D.8000500@samsung.com> <20141017130340.GW10376@tucnak.redhat.com> In-reply-to: <20141017130340.GW10376@tucnak.redhat.com> Content-type: multipart/mixed; boundary=------------060209020006080409060907 X-IsSubscribed: yes Hi, this is the second version of the patch. Here the major changes from the previous one: 1) Added a new intercepted_p parameter in get_mem_refs_of_builtin_call to decide whether builtin function should/shouldn't be instrumented. 2) Changed instrument_mem_region_access function. Now, we update asan_mem_ref_ht with (base, size_in_bytes), if we can determine access size during compile time. 3) Removed ASAN_CHECK_START_INSTRUMENTED and ASAN_CHECK_END_INSTRUMENTED from asan_check_flags since we don't instrument base and end of memory region with access size 1 anymore. 4) Specified builtins that shouldn't be expanded explicitly in gcc/builtins.c. Regtested / bootrapped on x86_64-unknown-linux-gnu. -Maxim On 10/17/2014 05:03 PM, Jakub Jelinek wrote: > On Fri, Oct 17, 2014 at 05:01:33PM +0400, Yury Gribov wrote: >> On 10/17/2014 04:24 PM, Jakub Jelinek wrote: >>>> +/* Returns TRUE if given FCODE corresponds to string or memory builtin function. >>>> + */ >>>> + >>>> +static inline bool >>>> +is_memory_builtin (enum built_in_function fcode) >>>> +{ >>>> + return fcode <= BUILT_IN_STRSTR && fcode >= BUILT_IN_BCMP; >>> This is too fragile and ugly. >>> IMHO you should list (supposedly not in a special inline, but directly >>> where you use it) in a switch all the builtins you don't want to expand. >> We already do this for BUILT_IN_ASAN_REPORT_LOAD1 ... BUILT_IN_ASAN_STOREN > I know, but it is still a coherent sent of builtins for very similar > purposes, many of them sorted by increasing size number. > >> but I agree that this one is more ugly. > The memops builtins are just random bag of them, it is expected many people > will add builtins into that range and outside of that range. > > Jakub > gcc/ChangeLog: 2014-10-21 Max Ostapenko * asan.c (asan_mem_ref_hasher::hash): Remove MEM_REF access size from hash value construction. Call iterative_hash_expr instead of explicit hash building. (asan_mem_ref_hasher::equal): Change condition. (has_mem_ref_been_instrumented): Likewise. (update_mem_ref_hash_table): Likewise. (maybe_update_mem_ref_hash_table): New function. (instrument_strlen_call): Removed. (get_mem_refs_of_builtin_call): Handle new parameter. (instrument_builtin_call): Call maybe_update_mem_ref_hash_table instead of instrument_mem_region_access if intercepted_p is true. (instrument_mem_region_access): Instrument only base with len instead of base and end with 1. (build_check_stmt): Remove start_instrumented and end_instrumented parameters. (enum asan_check_flags): Remove ASAN_CHECK_START_INSTRUMENTED and ASAN_CHECK_END_INSTRUMENTED. Change ASAN_CHECK_LAST. (asan_expand_check_ifn): Remove start_instrumented and end_instrumented. * builtins.c (expand_builtin): Don't expand string/memory builtin functions that have interceptors in libsanitizer if ASan is enabled. gcc/testsuite/ChangeLog: 2014-10-21 Max Ostapenko * c-c++-common/asan/no-redundant-instrumentation-1.c: Updated test. * c-c++-common/asan/no-redundant-instrumentation-4.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-5.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-6.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-7.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-8.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-2.c: Removed. * c-c++-common/asan/no-redundant-instrumentation-9.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-10.c: New test. * c-c++-common/asan/no-redundant-instrumentation-11.c: Likewise. diff --git a/gcc/asan.c b/gcc/asan.c index 2a61a82..a9eb9aa 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -253,9 +253,7 @@ enum asan_check_flags ASAN_CHECK_STORE = 1 << 0, ASAN_CHECK_SCALAR_ACCESS = 1 << 1, ASAN_CHECK_NON_ZERO_LEN = 1 << 2, - ASAN_CHECK_START_INSTRUMENTED = 1 << 3, - ASAN_CHECK_END_INSTRUMENTED = 1 << 4, - ASAN_CHECK_LAST + ASAN_CHECK_LAST = 1 << 3 }; /* Hashtable support for memory references used by gimple @@ -352,10 +350,7 @@ struct asan_mem_ref_hasher inline hashval_t asan_mem_ref_hasher::hash (const asan_mem_ref *mem_ref) { - inchash::hash hstate; - inchash::add_expr (mem_ref->start, hstate); - hstate.add_wide_int (mem_ref->access_size); - return hstate.end (); + return iterative_hash_expr (mem_ref->start, 0); } /* Compare two memory references. We accept the length of either @@ -365,8 +360,7 @@ inline bool asan_mem_ref_hasher::equal (const asan_mem_ref *m1, const asan_mem_ref *m2) { - return (m1->access_size == m2->access_size - && operand_equal_p (m1->start, m2->start, 0)); + return operand_equal_p (m1->start, m2->start, 0); } static hash_table *asan_mem_ref_ht; @@ -417,7 +411,8 @@ has_mem_ref_been_instrumented (tree ref, HOST_WIDE_INT access_size) asan_mem_ref r; asan_mem_ref_init (&r, ref, access_size); - return (get_mem_ref_hash_table ()->find (&r) != NULL); + asan_mem_ref *saved_ref = get_mem_ref_hash_table ()->find (&r); + return saved_ref && saved_ref->access_size >= access_size; } /* Return true iff the memory reference REF has been instrumented. */ @@ -434,19 +429,11 @@ has_mem_ref_been_instrumented (const asan_mem_ref *ref) static bool has_mem_ref_been_instrumented (const asan_mem_ref *ref, tree len) { - /* First let's see if the address of the beginning of REF has been - instrumented. */ - if (!has_mem_ref_been_instrumented (ref)) - return false; + HOST_WIDE_INT size_in_bytes + = tree_fits_shwi_p (len) ? tree_to_shwi (len) : -1; - if (len != 0) - { - /* Let's see if the end of the region has been instrumented. */ - if (!has_mem_ref_been_instrumented (asan_mem_ref_get_end (ref, len), - ref->access_size)) - return false; - } - return true; + return size_in_bytes != -1 + && has_mem_ref_been_instrumented (ref->start, size_in_bytes); } /* Set REF to the memory reference present in a gimple assignment @@ -492,7 +479,8 @@ get_mem_refs_of_builtin_call (const gimple call, asan_mem_ref *dst, tree *dst_len, bool *dst_is_store, - bool *dest_is_deref) + bool *dest_is_deref, + bool *intercepted_p) { gcc_checking_assert (gimple_call_builtin_p (call, BUILT_IN_NORMAL)); @@ -506,6 +494,7 @@ get_mem_refs_of_builtin_call (const gimple call, { /* (s, s, n) style memops. */ case BUILT_IN_BCMP: + *intercepted_p = false; case BUILT_IN_MEMCMP: source0 = gimple_call_arg (call, 0); source1 = gimple_call_arg (call, 1); @@ -514,18 +503,20 @@ get_mem_refs_of_builtin_call (const gimple call, /* (src, dest, n) style memops. */ case BUILT_IN_BCOPY: + *intercepted_p = false; source0 = gimple_call_arg (call, 0); dest = gimple_call_arg (call, 1); len = gimple_call_arg (call, 2); break; /* (dest, src, n) style memops. */ - case BUILT_IN_MEMCPY: case BUILT_IN_MEMCPY_CHK: - case BUILT_IN_MEMMOVE: + case BUILT_IN_MEMPCPY_CHK: case BUILT_IN_MEMMOVE_CHK: case BUILT_IN_MEMPCPY: - case BUILT_IN_MEMPCPY_CHK: + *intercepted_p = false; + case BUILT_IN_MEMCPY: + case BUILT_IN_MEMMOVE: dest = gimple_call_arg (call, 0); source0 = gimple_call_arg (call, 1); len = gimple_call_arg (call, 2); @@ -533,13 +524,15 @@ get_mem_refs_of_builtin_call (const gimple call, /* (dest, n) style memops. */ case BUILT_IN_BZERO: + *intercepted_p = false; dest = gimple_call_arg (call, 0); len = gimple_call_arg (call, 1); break; /* (dest, x, n) style memops*/ - case BUILT_IN_MEMSET: case BUILT_IN_MEMSET_CHK: + *intercepted_p = false; + case BUILT_IN_MEMSET: dest = gimple_call_arg (call, 0); len = gimple_call_arg (call, 2); break; @@ -759,6 +752,7 @@ get_mem_refs_of_builtin_call (const gimple call, gcc_unreachable (); access_size = int_size_in_bytes (TREE_TYPE (dest)); + *intercepted_p = false; } default: @@ -834,12 +828,12 @@ has_stmt_been_instrumented_p (gimple stmt) tree src0_len = NULL_TREE, src1_len = NULL_TREE, dest_len = NULL_TREE; bool src0_is_store = false, src1_is_store = false, - dest_is_store = false, dest_is_deref = false; + dest_is_store = false, dest_is_deref = false, intercepted_p = true; if (get_mem_refs_of_builtin_call (stmt, &src0, &src0_len, &src0_is_store, &src1, &src1_len, &src1_is_store, &dest, &dest_len, &dest_is_store, - &dest_is_deref)) + &dest_is_deref, &intercepted_p)) { if (src0.start != NULL_TREE && !has_mem_ref_been_instrumented (&src0, src0_len)) @@ -870,7 +864,7 @@ update_mem_ref_hash_table (tree ref, HOST_WIDE_INT access_size) asan_mem_ref_init (&r, ref, access_size); asan_mem_ref **slot = ht->find_slot (&r, INSERT); - if (*slot == NULL) + if (*slot == NULL || (*slot)->access_size < access_size) *slot = asan_mem_ref_new (ref, access_size); } @@ -1608,22 +1602,13 @@ static void build_check_stmt (location_t loc, tree base, tree len, HOST_WIDE_INT size_in_bytes, gimple_stmt_iterator *iter, bool is_non_zero_len, bool before_p, bool is_store, - bool is_scalar_access, unsigned int align = 0, - bool start_instrumented = false, - bool end_instrumented = false) + bool is_scalar_access, unsigned int align = 0) { gimple_stmt_iterator gsi = *iter; gimple g; gcc_assert (!(size_in_bytes > 0 && !is_non_zero_len)); - if (start_instrumented && end_instrumented) - { - if (!before_p) - gsi_next (iter); - return; - } - gsi = *iter; base = unshare_expr (base); @@ -1666,10 +1651,6 @@ build_check_stmt (location_t loc, tree base, tree len, flags |= ASAN_CHECK_NON_ZERO_LEN; if (is_scalar_access) flags |= ASAN_CHECK_SCALAR_ACCESS; - if (start_instrumented) - flags |= ASAN_CHECK_START_INSTRUMENTED; - if (end_instrumented) - flags |= ASAN_CHECK_END_INSTRUMENTED; g = gimple_build_call_internal (IFN_ASAN_CHECK, 4, build_int_cst (integer_type_node, flags), @@ -1784,6 +1765,22 @@ instrument_derefs (gimple_stmt_iterator *iter, tree t, } +/* Insert a memory reference into the hash table if access length + can be determined in compile time. */ + +static void +maybe_update_mem_ref_hash_table (tree base, tree len) +{ + if (!POINTER_TYPE_P (TREE_TYPE (base)) + || !INTEGRAL_TYPE_P (TREE_TYPE (len))) + return; + + HOST_WIDE_INT size_in_bytes = tree_fits_shwi_p (len) ? tree_to_shwi (len) : -1; + + if (size_in_bytes != -1) + update_mem_ref_hash_table (base, size_in_bytes); +} + /* Instrument an access to a contiguous memory region that starts at the address pointed to by BASE, over a length of LEN (expressed in the sizeof (*BASE) bytes). ITER points to the instruction before @@ -1802,97 +1799,20 @@ instrument_mem_region_access (tree base, tree len, || integer_zerop (len)) return; - /* If the beginning of the memory region has already been - instrumented, do not instrument it. */ - bool start_instrumented = has_mem_ref_been_instrumented (base, 1); - - /* If the end of the memory region has already been instrumented, do - not instrument it. */ - tree end = asan_mem_ref_get_end (base, len); - bool end_instrumented = has_mem_ref_been_instrumented (end, 1); - HOST_WIDE_INT size_in_bytes = tree_fits_shwi_p (len) ? tree_to_shwi (len) : -1; - build_check_stmt (location, base, len, size_in_bytes, iter, - /*is_non_zero_len*/size_in_bytes > 0, /*before_p*/true, - is_store, /*is_scalar_access*/false, /*align*/0, - start_instrumented, end_instrumented); - - update_mem_ref_hash_table (base, 1); - if (size_in_bytes != -1) - update_mem_ref_hash_table (end, 1); + if ((size_in_bytes == -1) + || !has_mem_ref_been_instrumented (base, size_in_bytes)) + { + build_check_stmt (location, base, len, size_in_bytes, iter, + /*is_non_zero_len*/size_in_bytes > 0, /*before_p*/true, + is_store, /*is_scalar_access*/false, /*align*/0); + } + maybe_update_mem_ref_hash_table (base, len); *iter = gsi_for_stmt (gsi_stmt (*iter)); } -/* Instrument the call (to the builtin strlen function) pointed to by - ITER. - - This function instruments the access to the first byte of the - argument, right before the call. After the call it instruments the - access to the last byte of the argument; it uses the result of the - call to deduce the offset of that last byte. - - Upon completion, iff the call has actually been instrumented, this - function returns TRUE and *ITER points to the statement logically - following the built-in strlen function call *ITER was initially - pointing to. Otherwise, the function returns FALSE and *ITER - remains unchanged. */ - -static bool -instrument_strlen_call (gimple_stmt_iterator *iter) -{ - gimple g; - gimple call = gsi_stmt (*iter); - gcc_assert (is_gimple_call (call)); - - tree callee = gimple_call_fndecl (call); - gcc_assert (is_builtin_fn (callee) - && DECL_BUILT_IN_CLASS (callee) == BUILT_IN_NORMAL - && DECL_FUNCTION_CODE (callee) == BUILT_IN_STRLEN); - - location_t loc = gimple_location (call); - - tree len = gimple_call_lhs (call); - if (len == NULL) - /* Some passes might clear the return value of the strlen call; - bail out in that case. Return FALSE as we are not advancing - *ITER. */ - return false; - gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (len))); - - len = maybe_cast_to_ptrmode (loc, len, iter, /*before_p*/false); - - tree str_arg = gimple_call_arg (call, 0); - bool start_instrumented = has_mem_ref_been_instrumented (str_arg, 1); - - tree cptr_type = build_pointer_type (char_type_node); - g = gimple_build_assign_with_ops (NOP_EXPR, - make_ssa_name (cptr_type, NULL), - str_arg, NULL); - gimple_set_location (g, loc); - gsi_insert_before (iter, g, GSI_SAME_STMT); - str_arg = gimple_assign_lhs (g); - - build_check_stmt (loc, str_arg, NULL_TREE, 1, iter, - /*is_non_zero_len*/true, /*before_p=*/true, - /*is_store=*/false, /*is_scalar_access*/true, /*align*/0, - start_instrumented, start_instrumented); - - g = gimple_build_assign_with_ops (POINTER_PLUS_EXPR, - make_ssa_name (cptr_type, NULL), - str_arg, - len); - gimple_set_location (g, loc); - gsi_insert_after (iter, g, GSI_NEW_STMT); - - build_check_stmt (loc, gimple_assign_lhs (g), NULL_TREE, 1, iter, - /*is_non_zero_len*/true, /*before_p=*/false, - /*is_store=*/false, /*is_scalar_access*/true, /*align*/0); - - return true; -} - /* Instrument the call to a built-in memory access function that is pointed to by the iterator ITER. @@ -1910,49 +1830,54 @@ instrument_builtin_call (gimple_stmt_iterator *iter) gcc_checking_assert (gimple_call_builtin_p (call, BUILT_IN_NORMAL)); - tree callee = gimple_call_fndecl (call); location_t loc = gimple_location (call); - if (DECL_FUNCTION_CODE (callee) == BUILT_IN_STRLEN) - iter_advanced_p = instrument_strlen_call (iter); - else - { - asan_mem_ref src0, src1, dest; - asan_mem_ref_init (&src0, NULL, 1); - asan_mem_ref_init (&src1, NULL, 1); - asan_mem_ref_init (&dest, NULL, 1); + asan_mem_ref src0, src1, dest; + asan_mem_ref_init (&src0, NULL, 1); + asan_mem_ref_init (&src1, NULL, 1); + asan_mem_ref_init (&dest, NULL, 1); - tree src0_len = NULL_TREE, src1_len = NULL_TREE, dest_len = NULL_TREE; - bool src0_is_store = false, src1_is_store = false, - dest_is_store = false, dest_is_deref = false; + tree src0_len = NULL_TREE, src1_len = NULL_TREE, dest_len = NULL_TREE; + bool src0_is_store = false, src1_is_store = false, dest_is_store = false, + dest_is_deref = false, intercepted_p = true; - if (get_mem_refs_of_builtin_call (call, - &src0, &src0_len, &src0_is_store, - &src1, &src1_len, &src1_is_store, - &dest, &dest_len, &dest_is_store, - &dest_is_deref)) + if (get_mem_refs_of_builtin_call (call, + &src0, &src0_len, &src0_is_store, + &src1, &src1_len, &src1_is_store, + &dest, &dest_len, &dest_is_store, + &dest_is_deref, &intercepted_p)) + { + if (dest_is_deref) { - if (dest_is_deref) - { - instrument_derefs (iter, dest.start, loc, dest_is_store); - gsi_next (iter); - iter_advanced_p = true; - } - else if (src0_len || src1_len || dest_len) - { - if (src0.start != NULL_TREE) - instrument_mem_region_access (src0.start, src0_len, - iter, loc, /*is_store=*/false); - if (src1.start != NULL_TREE) - instrument_mem_region_access (src1.start, src1_len, - iter, loc, /*is_store=*/false); - if (dest.start != NULL_TREE) - instrument_mem_region_access (dest.start, dest_len, - iter, loc, /*is_store=*/true); - *iter = gsi_for_stmt (call); - gsi_next (iter); - iter_advanced_p = true; - } + instrument_derefs (iter, dest.start, loc, dest_is_store); + gsi_next (iter); + iter_advanced_p = true; + } + else if (!intercepted_p + && (src0_len || src1_len || dest_len)) + { + if (src0.start != NULL_TREE) + instrument_mem_region_access (src0.start, src0_len, + iter, loc, /*is_store=*/false); + if (src1.start != NULL_TREE) + instrument_mem_region_access (src1.start, src1_len, + iter, loc, /*is_store=*/false); + if (dest.start != NULL_TREE) + instrument_mem_region_access (dest.start, dest_len, + iter, loc, /*is_store=*/true); + + *iter = gsi_for_stmt (call); + gsi_next (iter); + iter_advanced_p = true; + } + else + { + if (src0.start != NULL_TREE) + maybe_update_mem_ref_hash_table (src0.start, src0_len); + if (src1.start != NULL_TREE) + maybe_update_mem_ref_hash_table (src1.start, src1_len); + if (dest.start != NULL_TREE) + maybe_update_mem_ref_hash_table (dest.start, dest_len); } } return iter_advanced_p; @@ -2507,8 +2432,6 @@ asan_expand_check_ifn (gimple_stmt_iterator *iter, bool use_calls) bool is_scalar_access = (flags & ASAN_CHECK_SCALAR_ACCESS) != 0; bool is_store = (flags & ASAN_CHECK_STORE) != 0; bool is_non_zero_len = (flags & ASAN_CHECK_NON_ZERO_LEN) != 0; - bool start_instrumented = (flags & ASAN_CHECK_START_INSTRUMENTED) != 0; - bool end_instrumented = (flags & ASAN_CHECK_END_INSTRUMENTED) != 0; tree base = gimple_call_arg (g, 1); tree len = gimple_call_arg (g, 2); @@ -2613,46 +2536,42 @@ asan_expand_check_ifn (gimple_stmt_iterator *iter, bool use_calls) else { /* Slow path for 1, 2 and 4 byte accesses. */ - - if (!start_instrumented) + /* Test (shadow != 0) + & ((base_addr & 7) + (real_size_in_bytes - 1)) >= shadow). */ + tree shadow = build_shadow_mem_access (&gsi, loc, base_addr, + shadow_ptr_type); + gimple shadow_test = build_assign (NE_EXPR, shadow, 0); + gimple_seq seq = NULL; + gimple_seq_add_stmt (&seq, shadow_test); + /* Aligned (>= 8 bytes) can test just + (real_size_in_bytes - 1 >= shadow), as base_addr & 7 is known + to be 0. */ + if (align < 8) { - /* Test (shadow != 0) - & ((base_addr & 7) + (real_size_in_bytes - 1)) >= shadow). */ - tree shadow = build_shadow_mem_access (&gsi, loc, base_addr, - shadow_ptr_type); - gimple shadow_test = build_assign (NE_EXPR, shadow, 0); - gimple_seq seq = NULL; - gimple_seq_add_stmt (&seq, shadow_test); - /* Aligned (>= 8 bytes) can test just - (real_size_in_bytes - 1 >= shadow), as base_addr & 7 is known - to be 0. */ - if (align < 8) - { - gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, - base_addr, 7)); - gimple_seq_add_stmt (&seq, - build_type_cast (shadow_type, - gimple_seq_last (seq))); - if (real_size_in_bytes > 1) - gimple_seq_add_stmt (&seq, - build_assign (PLUS_EXPR, - gimple_seq_last (seq), - real_size_in_bytes - 1)); - t = gimple_assign_lhs (gimple_seq_last_stmt (seq)); - } - else - t = build_int_cst (shadow_type, real_size_in_bytes - 1); - gimple_seq_add_stmt (&seq, build_assign (GE_EXPR, t, shadow)); - gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, shadow_test, - gimple_seq_last (seq))); - t = gimple_assign_lhs (gimple_seq_last (seq)); - gimple_seq_set_location (seq, loc); - gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING); + gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, + base_addr, 7)); + gimple_seq_add_stmt (&seq, + build_type_cast (shadow_type, + gimple_seq_last (seq))); + if (real_size_in_bytes > 1) + gimple_seq_add_stmt (&seq, + build_assign (PLUS_EXPR, + gimple_seq_last (seq), + real_size_in_bytes - 1)); + t = gimple_assign_lhs (gimple_seq_last_stmt (seq)); } + else + t = build_int_cst (shadow_type, real_size_in_bytes - 1); + gimple_seq_add_stmt (&seq, build_assign (GE_EXPR, t, shadow)); + gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, shadow_test, + gimple_seq_last (seq))); + t = gimple_assign_lhs (gimple_seq_last (seq)); + gimple_seq_set_location (seq, loc); + gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING); /* For non-constant, misaligned or otherwise weird access sizes, - check first and last byte. */ - if (size_in_bytes == -1 && !end_instrumented) + check first and last byte. */ + if (size_in_bytes == -1) { g = gimple_build_assign_with_ops (MINUS_EXPR, make_ssa_name (pointer_sized_int_node, NULL), @@ -2683,9 +2602,8 @@ asan_expand_check_ifn (gimple_stmt_iterator *iter, bool use_calls) shadow)); gimple_seq_add_stmt (&seq, build_assign (BIT_AND_EXPR, shadow_test, gimple_seq_last (seq))); - if (!start_instrumented) - gimple_seq_add_stmt (&seq, build_assign (BIT_IOR_EXPR, t, - gimple_seq_last (seq))); + gimple_seq_add_stmt (&seq, build_assign (BIT_IOR_EXPR, t, + gimple_seq_last (seq))); t = gimple_assign_lhs (gimple_seq_last (seq)); gimple_seq_set_location (seq, loc); gsi_insert_seq_after (&gsi, seq, GSI_CONTINUE_LINKING); diff --git a/gcc/builtins.c b/gcc/builtins.c index 975f696..530b5a4 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -5762,6 +5762,37 @@ expand_builtin (tree exp, rtx target, rtx subtarget, enum machine_mode mode, enum machine_mode target_mode = TYPE_MODE (TREE_TYPE (exp)); int flags; + /* When ASan is enabled, we don't want to expand some memory/string + builtins and rely on libsanitizer's hooks. This allows us to avoid + redundant checks and be sure, that possible overflow will be detected + by ASan. */ + + if (flag_sanitize & SANITIZE_ADDRESS) + switch (fcode) + { + case BUILT_IN_INDEX: + case BUILT_IN_MEMCHR: + case BUILT_IN_MEMCMP: + case BUILT_IN_MEMCPY: + case BUILT_IN_MEMMOVE: + case BUILT_IN_MEMSET: + case BUILT_IN_STRCASECMP: + case BUILT_IN_STRCAT: + case BUILT_IN_STRCHR: + case BUILT_IN_STRCMP: + case BUILT_IN_STRCPY: + case BUILT_IN_STRDUP: + case BUILT_IN_STRLEN: + case BUILT_IN_STRNCASECMP: + case BUILT_IN_STRNCAT: + case BUILT_IN_STRNCMP: + case BUILT_IN_STRNCPY: + return expand_call (exp, target, ignore); + + default: + break; + } + if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD) return targetm.expand_builtin (exp, target, subtarget, mode, ignore); diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c index 028f8d7..baacb1e 100644 --- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c +++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c @@ -6,7 +6,7 @@ /* { dg-do compile } */ /* { dg-skip-if "" { *-*-* } { "*" } { "-O0" } } */ -extern char tab[4]; +extern char tab[6]; static int test0 () @@ -35,17 +35,10 @@ test1 (int i) the initialization. */ foo[i] = 1; - /*__builtin___asan_report_store_n called once here to instrument - the store to the memory region of tab. */ + /* Instrument tab memory region. */ __builtin_memset (tab, 3, sizeof (tab)); - /* There is no instrumentation for the two memset calls below. */ - __builtin_memset (tab, 4, sizeof (tab)); - __builtin_memset (tab, 5, sizeof (tab)); - - /* There is a call to __builtin___asan_report_store_n and a call - to __builtin___asan_report_load_n to instrument the store to - (subset of) the memory region of tab. */ + /* Instrument tab[1] with access size 3. */ __builtin_memcpy (&tab[1], foo + i, 3); /* This should not generate a __builtin___asan_report_load1 because @@ -63,6 +56,5 @@ main () } /* { dg-final { scan-tree-dump-times "__builtin___asan_report_store1" 3 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 2 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load" 1 "sanopt" } } */ +/* { dg-final { scan-tree-dump-not "__builtin___asan_report_load1" "sanopt" } } */ /* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-10.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-10.c new file mode 100644 index 0000000..24dfcfe --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-10.c @@ -0,0 +1,18 @@ +/* { dg-options "-fdump-tree-sanopt" } */ +/* { dg-do compile } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O0" } } */ + +extern __UINT32_TYPE__ a; + +void +foo () +{ + /* Instrument a with access size 3. */ + int d = __builtin_memcmp (&a, "123", 3); + /* This should generate a __builtin___asan_report_store4, because + the reference to a has been instrumented above with access size 3. */ + a = 1; +} + +/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store4" 1 "sanopt" } } */ +/* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-11.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-11.c new file mode 100644 index 0000000..4082f32 --- /dev/null +++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-11.c @@ -0,0 +1,20 @@ +/* { dg-options "-fdump-tree-sanopt" } */ +/* { dg-do compile } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O0" } } */ + +extern __UINT32_TYPE__ a; + +void +foo () +{ + /* Instrument a with access size 5. */ + int d = __builtin_memcmp (&a, "12345", 4); + /* This should not generate a __builtin___asan_report_store4 because + the reference to a has been already instrumented above with access + size 5. */ + a = 1; +} + +/* { dg-final { scan-tree-dump-not "& 7" "sanopt" } } */ +/* { dg-final { scan-tree-dump-not "__builtin___asan_report_store" "sanopt" } } */ +/* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c deleted file mode 100644 index a58411c..0000000 --- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c +++ /dev/null @@ -1,26 +0,0 @@ -/* This tests that when faced with two references to the same memory - location in the same basic block, the second reference should not - be instrumented by the Address Sanitizer. But in case of access to - overlapping regions we must be precise. */ - -/* { dg-options "-fdump-tree-sanopt" } */ -/* { dg-do compile } */ -/* { dg-skip-if "" { *-*-* } { "*" } { "-O0" } } */ - -int -main () -{ - char tab[5]; - - /* Here, we instrument the access at offset 0 and access at offset - 4. */ - __builtin_memset (tab, 1, sizeof (tab)); - /* We instrumented access at offset 0 above already, so only access - at offset 3 is instrumented. */ - __builtin_memset (tab, 1, 3); -} - -/* { dg-final { scan-tree-dump-times "& 7" 3 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 2 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report" 2 "sanopt" } } */ -/* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c index c3632aa..a613b92 100644 --- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c +++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c @@ -5,13 +5,13 @@ void foo (int *a, char *b, char *c) { - /* One check for c[0], one check for a[], one check for c, two checks for b. */ + /* One check for c[0], one check for a[]. */ __builtin_memmove (c, b, a[c[0]]); - /* For a total of 5 checks. */ + /* For a total of 2 checks. */ + int d = c[0] == 1; } -/* { dg-final { scan-tree-dump-times "& 7" 5 "sanopt" } } */ +/* { dg-final { scan-tree-dump-times "& 7" 2 "sanopt" } } */ /* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 1 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 1 "sanopt" } } */ +/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load4" 1 "sanopt" } } */ /* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c index 077ea34..f4ca603 100644 --- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c +++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c @@ -5,14 +5,12 @@ void foo (int *a, char *b, char *c) { - /* One check for b[0], one check for a[], 2 checks for c and one checks for b. */ - __builtin_memmove (c, b, a[b[0]]); - /* For a total of 5 checks. */ + /* One check for a[]. */ + __builtin_memmove (c, b, a[0]); + /* For a total of 1 checks. */ + int d = a[0] == 0; } -/* { dg-final { scan-tree-dump-times "& 7" 5 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "sanopt" } } */ +/* { dg-final { scan-tree-dump-times "& 7" 1 "sanopt" } } */ /* { dg-final { scan-tree-dump-times "__builtin___asan_report_load4" 1 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 1 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 1 "sanopt" } } */ /* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c index 6d87104..757f0ee 100644 --- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c +++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c @@ -5,16 +5,15 @@ void foo (int *a, char *b, char *c) { - /* One check for c[0], one check for a[], one check for c and 2 checks for b. */ + /* One check for c[0], one check for a[]. */ __builtin_memmove (c, b, a[c[0]]); - /* One check for a[], one check for c and one check for b. */ + /* One check for b[0], one check for a[]. */ __builtin_memmove (c, b, a[b[0]]); - /* For a total of 8 checks. */ + /* For a total of 4 checks. */ + int d = c[0] == b[0]; } -/* { dg-final { scan-tree-dump-times "& 7" 8 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "sanopt" } } */ +/* { dg-final { scan-tree-dump-times "& 7" 4 "sanopt" } } */ +/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 2 "sanopt" } } */ /* { dg-final { scan-tree-dump-times "__builtin___asan_report_load4" 2 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 2 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 2 "sanopt" } } */ /* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c index 5baa10d..0c50145 100644 --- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c +++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c @@ -2,26 +2,22 @@ /* { dg-do compile } */ /* { dg-skip-if "" { *-*-* } { "*" } { "-O0" } } */ -char e[200]; +char e[5]; -struct S +extern struct S { - char a[100]; - char b[100]; + int a; + char b; } s; int foo (int *a, char *b, char *c) { - /* 2 checks for s.a, 2 checks for e. */ - int d = __builtin_memcmp (s.a, e, 100); - /* One check for s.a and one check for e. */ - d += __builtin_memcmp (s.a, e, 200); - /* For a total of 6 checks. */ - return d; + int d = __builtin_memcmp (&s.a, e, 4); + /* No check because s.a was instrumented above with access size 4. */ + return s.a; } -/* { dg-final { scan-tree-dump-times "& 7" 6 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 4 "sanopt" } } */ -/* { dg-final { scan-tree-dump-not "__builtin___asan_report_store" "sanopt" } } */ +/* { dg-final { scan-tree-dump-not "& 7" "sanopt" } } */ +/* { dg-final { scan-tree-dump-not "__builtin___asan_report_load4" "sanopt" } } */ /* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c index 2a4c081..4eeedce 100644 --- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c +++ b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c @@ -5,16 +5,16 @@ char foo (int *a, char *b, char *c) { - /* One check for b[0], one check for a[], two checks for c and one check for b. */ + /* One check for b[0], one check for a[]. */ __builtin_memmove (c, b, a[b[0]]); + /* One check for c[0], one check for a[]. */ + __builtin_memmove (b, c, a[c[0]]); /* No checks here. */ return c[0] + b[0]; - /* For a total of 5 checks. */ + /* For a total of 4 checks. */ } -/* { dg-final { scan-tree-dump-times "& 7" 5 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 1 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load4" 1 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load_n" 1 "sanopt" } } */ -/* { dg-final { scan-tree-dump-times "__builtin___asan_report_store_n" 1 "sanopt" } } */ +/* { dg-final { scan-tree-dump-times "& 7" 4 "sanopt" } } */ +/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load1" 2 "sanopt" } } */ +/* { dg-final { scan-tree-dump-times "__builtin___asan_report_load4" 2 "sanopt" } } */ /* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-9.c b/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-9.c deleted file mode 100644 index 9449de5..0000000 --- a/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-9.c +++ /dev/null @@ -1,13 +0,0 @@ -/* { dg-options "-fdump-tree-sanopt" } */ -/* { dg-do compile } */ -/* { dg-skip-if "" { *-*-* } { "*" } { "-O0" } } */ - -__SIZE_TYPE__ -f (char *a) -{ - a[0] = '1'; - return __builtin_strlen (a); -} - -/* { dg-final { scan-tree-dump-times "__asan_report_load1" 1 "sanopt" } } */ -/* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git a/gcc/testsuite/c-c++-common/asan/strlen-overflow-1.c b/gcc/testsuite/c-c++-common/asan/strlen-overflow-1.c index f58f554..0f49286 100644 --- a/gcc/testsuite/c-c++-common/asan/strlen-overflow-1.c +++ b/gcc/testsuite/c-c++-common/asan/strlen-overflow-1.c @@ -9,14 +9,8 @@ char a[2] = "0"; #ifdef __cplusplus extern "C" #endif - -__attribute__((no_sanitize_address, noinline)) __SIZE_TYPE__ -strlen (const char *p) { - - __SIZE_TYPE__ n = 0; - for (; *p; ++n, ++p); - return n; -} +__SIZE_TYPE__ +strlen (const char *p); int main () { char *p = &a[0]; @@ -25,6 +19,6 @@ int main () { return __builtin_strlen (a); } -/* { dg-output "READ of size 1 at 0x\[0-9a-f\]+ thread T0.*(\n|\r\n|\r)" } */ -/* { dg-output " #0 0x\[0-9a-f\]+ (in _*main (\[^\n\r]*strlen-overflow-1.c:25|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ -/* { dg-output "\[^\n\r]*0x\[0-9a-f\]+ is located 1 bytes inside of global variable" } */ +/* { dg-output "READ of size 2 at 0x\[0-9a-f\]+ thread T0.*(\n|\r\n|\r)" } */ +/* { dg-output " #1 0x\[0-9a-f\]+ (in _*main (\[^\n\r]*strlen-overflow-1.c:19|\[^\n\r]*:0)|\[(\]).*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*0x\[0-9a-f\]+ is located 0 bytes to the right of global variable" } */