From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 101846 invoked by alias); 30 Mar 2015 17:43:22 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 101834 invoked by uid 89); 30 Mar 2015 17:43:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 30 Mar 2015 17:43:18 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t2UHhHct024196 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 30 Mar 2015 13:43:17 -0400 Received: from tucnak.zalov.cz (ovpn-116-58.ams2.redhat.com [10.36.116.58]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t2UHh5ib027403 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 30 Mar 2015 13:43:16 -0400 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.14.9/8.14.9) with ESMTP id t2UHh0GP012843; Mon, 30 Mar 2015 19:43:00 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.14.9/8.14.9/Submit) id t2UHgvPN012842; Mon, 30 Mar 2015 19:42:57 +0200 Date: Mon, 30 Mar 2015 17:43:00 -0000 From: Jakub Jelinek To: Marat Zakirov Cc: "gcc-patches@gcc.gnu.org" , Andrew Pinski , Kostya Serebryany , Dmitry Vyukov , Yury Gribov , Andrey Ryabinin Subject: Re: [PINGv4][PATCH] ASan on unaligned accesses Message-ID: <20150330174257.GJ1746@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <54F6BBA0.2010302@samsung.com> <550A662E.3080308@samsung.com> <5513ACCF.7030502@samsung.com> <20150326115013.GY1746@tucnak.redhat.com> <5513FB85.4030305@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5513FB85.4030305@samsung.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg01574.txt.bz2 On Thu, Mar 26, 2015 at 03:28:53PM +0300, Marat Zakirov wrote: > 2015-02-25 Marat Zakirov > > * asan.c (asan_emit_stack_protection): Support for misalign accesses. > (asan_expand_check_ifn): Likewise. > * params.def: New option asan-catch-misaligned. > * params.h: New param ASAN_CATCH_MISALIGNED. > * doc/invoke.texi: New asan param description. Can you please start by explaining the asan_emit_stack_protection changes? What is the problem there, what do you want to achieve etc.? Is it to support ABI violating stack pointer alignment, or something different? If so, the compiler knows (or should be aware) what the stack alignment is. The asan_expand_check_ifn change looks reasonable. Also, the changes regress code quality without the parameter, say on the testcase you've added at -O2 -fsanitize=address (no param used), on x86_64 the patch causes undesirable movl $0, 2147450880(%rbp) - movq $0, 2147450892(%rbp) + movl $0, 2147450892(%rbp) + movl $0, 2147450896(%rbp) change. > --- a/gcc/asan.c > +++ b/gcc/asan.c > @@ -1050,7 +1050,6 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, > rtx_code_label *lab; > rtx_insn *insns; > char buf[30]; > - unsigned char shadow_bytes[4]; Do you really need to do that and why? > HOST_WIDE_INT base_offset = offsets[length - 1]; > HOST_WIDE_INT base_align_bias = 0, offset, prev_offset; > HOST_WIDE_INT asan_frame_size = offsets[0] - base_offset; > @@ -1059,6 +1058,7 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, > unsigned char cur_shadow_byte = ASAN_STACK_MAGIC_LEFT; > tree str_cst, decl, id; > int use_after_return_class = -1; > + bool misalign = (flag_sanitize & SANITIZE_KERNEL_ADDRESS) || ASAN_CATCH_MISALIGNED; Too long line. > + vec shadow_mems; > + vec shadow_bytes; > + > + shadow_mems.create(0); > + shadow_bytes.create(0); 2x missing space before (. > + shadow_mems.safe_push(shadow_mem); Similarly. > + shadow_bytes.safe_push (cur_shadow_byte); > + shadow_bytes.safe_push (cur_shadow_byte); > + shadow_bytes.safe_push (cur_shadow_byte); > + shadow_mems.safe_push(shadow_mem); Similarly. > } > + for (unsigned i = 0; misalign && i < shadow_bytes.length () - 1; i++) > + if (shadow_bytes[i] == 0 && shadow_bytes[i + 1] > 0) > + shadow_bytes[i] = 8 + (shadow_bytes[i + 1] > 7 ? 0 : shadow_bytes[i + 1]); Too long line. > - prev_offset = base_offset; > - last_offset = base_offset; > - last_size = 0; > - for (l = length; l; l -= 2) > - { > - offset = base_offset + ((offsets[l - 1] - base_offset) > - & ~(ASAN_RED_ZONE_SIZE - HOST_WIDE_INT_1)); > - if (last_offset + last_size != offset) > - { > - shadow_mem = adjust_address (shadow_mem, VOIDmode, > - (last_offset - prev_offset) > - >> ASAN_SHADOW_SHIFT); > - prev_offset = last_offset; > - asan_clear_shadow (shadow_mem, last_size >> ASAN_SHADOW_SHIFT); > - last_offset = offset; > - last_size = 0; > - } > - last_size += base_offset + ((offsets[l - 2] - base_offset) > - & ~(ASAN_RED_ZONE_SIZE - HOST_WIDE_INT_1)) > - - offset; > - } > - if (last_size) > - { > - shadow_mem = adjust_address (shadow_mem, VOIDmode, > - (last_offset - prev_offset) > - >> ASAN_SHADOW_SHIFT); > - asan_clear_shadow (shadow_mem, last_size >> ASAN_SHADOW_SHIFT); > - } > + for (unsigned i = 0; i < shadow_mems.length (); i++) > + asan_clear_shadow (shadow_mems[i], 4); Bet this change causes the regression I've mentioned above. > @@ -2546,6 +2555,7 @@ asan_expand_check_ifn (gimple_stmt_iterator *iter, bool use_calls) > gimple g = gsi_stmt (*iter); > location_t loc = gimple_location (g); > > + bool misalign = (flag_sanitize & SANITIZE_KERNEL_ADDRESS) || ASAN_CATCH_MISALIGNED; Too long line. Jakub