From patchwork Tue Jan 30 04:48:38 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 867386 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-472242-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="hOa6JirM"; dkim-atps=neutral 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 3zVv6D6Fpzz9ryT for ; Tue, 30 Jan 2018 15:48:51 +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:from :cc:to:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=qm1PO5CBXLyka8xIWGmH61XcNO7PZcILAz3/e2sLaEnWiChQ7i tqcn1jagh82WjYPuuORQwR4kFvVFhDNsrA9Ao9t33Jby0i2fPw9PAjtKrKHh/Rad MDpDApCn5L+pqO07wB1V3wSFH4ABoxpBrVhJm660UiyMq0bKiXCkf5V88= 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:from :cc:to:subject:message-id:date:mime-version:content-type; s= default; bh=mAbtBxCHcIb/+n6YzwRMyJ+UeFo=; b=hOa6JirMtn9ar+hhoXYe T1TREo2Eq+54O3JzVLlBkFhdckinHSUcoY8c3wj7aF0uSXHVh+GAyAbkNwZukyUB 5WYUlng/jsSna1hrAnAIn5Fs9QqQikiy3TM/r6D6HkAyJig490X9qDIN2kAYxH91 QpjuDayKUkItTa0+PScFQsc= Received: (qmail 62907 invoked by alias); 30 Jan 2018 04:48:44 -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 62897 invoked by uid 89); 30 Jan 2018 04:48:43 -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_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=attacks, inclusive 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 ESMTP; Tue, 30 Jan 2018 04:48:42 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B35A2C057FAD; Tue, 30 Jan 2018 04:48:40 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-50.rdu2.redhat.com [10.10.112.50]) by smtp.corp.redhat.com (Postfix) with ESMTP id C84B16047B; Tue, 30 Jan 2018 04:48:39 +0000 (UTC) From: Jeff Law Cc: gcc-patches To: Uros Bizjak Subject: [PATCH][PR target/84064] Fix assertion with -fstack-clash-protection Message-ID: <2ee1ffc7-1583-c787-42d2-468890aeba86@redhat.com> Date: Mon, 29 Jan 2018 21:48:38 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 X-IsSubscribed: yes stack-clash-protection will sometimes force a prologue to use pushes to save registers. We try to limit how often that happens as it constrains options for generating efficient prologue sequences for common cases. We check the value of TO_ALLOCATE in ix86_compute_frame_layout and if it's greater than the probing interval, then we force the prologue to use pushes to save integer registers. That is conservatively correct and allows freedom in the most common cases. This is good. In expand_prologue we assert that the integer registers were saved via a push anytime we *might* generate a probe, even if the size of the allocated frame is too small to require explicit probing. Naturally, this conflicts with the code in ix86_compute_frame_layout that tries to avoid forcing pushes instead of moves. This patch moves the assertion inside expand_prologue down to the points where it actually needs to be true. Specifically when we're generating probes in a loop for -fstack-check or -fstack-clash-protection. [ Probing via calls to chkstk_ms is not affected by this change. ] Sorry to have to change this stuff for the 3rd time! Bootstrapped and regression tested on x86_64 and i686. Also verified that if stack-clash checking is enabled by default that both compilers will bootstrap. OK for the trunk? THanks, Jeff * i386.c (ix86_adjust_stack_and_probe_stack_clash): New argument INT_REGISTERS_SAVED. Check it prior to calling get_scratch_register_on_entry. (ix86_adjust_stack_and_probe): Similarly. (ix86_emit_probe_stack_range): Similarly. (ix86_expand_prologue): Corresponding changes. * gcc.target/i386/pr84064: New test. diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 3653ddd..fef34a1 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -12591,10 +12591,14 @@ release_scratch_register_on_entry (struct scratch_reg *sr) This differs from the next routine in that it tries hard to prevent attacks that jump the stack guard. Thus it is never allowed to allocate more than PROBE_INTERVAL bytes of stack space without a suitable - probe. */ + probe. + + INT_REGISTERS_SAVED is true if integer registers have already been + pushed on the stack. */ static void -ix86_adjust_stack_and_probe_stack_clash (const HOST_WIDE_INT size) +ix86_adjust_stack_and_probe_stack_clash (const HOST_WIDE_INT size, + const bool int_registers_saved) { struct machine_function *m = cfun->machine; @@ -12700,6 +12704,12 @@ ix86_adjust_stack_and_probe_stack_clash (const HOST_WIDE_INT size) } else { + /* We expect the GP registers to be saved when probes are used + as the probing sequences might need a scratch register and + the routine to allocate one assumes the integer registers + have already been saved. */ + gcc_assert (int_registers_saved); + struct scratch_reg sr; get_scratch_register_on_entry (&sr); @@ -12758,10 +12768,14 @@ ix86_adjust_stack_and_probe_stack_clash (const HOST_WIDE_INT size) emit_insn (gen_blockage ()); } -/* Emit code to adjust the stack pointer by SIZE bytes while probing it. */ +/* Emit code to adjust the stack pointer by SIZE bytes while probing it. + + INT_REGISTERS_SAVED is true if integer registers have already been + pushed on the stack. */ static void -ix86_adjust_stack_and_probe (const HOST_WIDE_INT size) +ix86_adjust_stack_and_probe (const HOST_WIDE_INT size, + const bool int_registers_saved) { /* We skip the probe for the first interval + a small dope of 4 words and probe that many bytes past the specified size to maintain a protection @@ -12822,6 +12836,12 @@ ix86_adjust_stack_and_probe (const HOST_WIDE_INT size) equality test for the loop condition. */ else { + /* We expect the GP registers to be saved when probes are used + as the probing sequences might need a scratch register and + the routine to allocate one assumes the integer registers + have already been saved. */ + gcc_assert (int_registers_saved); + HOST_WIDE_INT rounded_size; struct scratch_reg sr; @@ -12949,10 +12969,14 @@ output_adjust_stack_and_probe (rtx reg) } /* Emit code to probe a range of stack addresses from FIRST to FIRST+SIZE, - inclusive. These are offsets from the current stack pointer. */ + inclusive. These are offsets from the current stack pointer. + + INT_REGISTERS_SAVED is true if integer registers have already been + pushed on the stack. */ static void -ix86_emit_probe_stack_range (HOST_WIDE_INT first, HOST_WIDE_INT size) +ix86_emit_probe_stack_range (HOST_WIDE_INT first, HOST_WIDE_INT size, + const bool int_registers_saved) { /* See if we have a constant small number of probes to generate. If so, that's the easy case. The run-time loop is made up of 6 insns in the @@ -12980,6 +13004,12 @@ ix86_emit_probe_stack_range (HOST_WIDE_INT first, HOST_WIDE_INT size) equality test for the loop condition. */ else { + /* We expect the GP registers to be saved when probes are used + as the probing sequences might need a scratch register and + the routine to allocate one assumes the integer registers + have already been saved. */ + gcc_assert (int_registers_saved); + HOST_WIDE_INT rounded_size, last; struct scratch_reg sr; @@ -13733,15 +13763,10 @@ ix86_expand_prologue (void) && (flag_stack_check == STATIC_BUILTIN_STACK_CHECK || flag_stack_clash_protection)) { - /* We expect the GP registers to be saved when probes are used - as the probing sequences might need a scratch register and - the routine to allocate one assumes the integer registers - have already been saved. */ - gcc_assert (int_registers_saved); - if (flag_stack_clash_protection) { - ix86_adjust_stack_and_probe_stack_clash (allocate); + ix86_adjust_stack_and_probe_stack_clash (allocate, + int_registers_saved); allocate = 0; } else if (STACK_CHECK_MOVING_SP) @@ -13749,7 +13774,7 @@ ix86_expand_prologue (void) if (!(crtl->is_leaf && !cfun->calls_alloca && allocate <= get_probe_interval ())) { - ix86_adjust_stack_and_probe (allocate); + ix86_adjust_stack_and_probe (allocate, int_registers_saved); allocate = 0; } } @@ -13765,11 +13790,12 @@ ix86_expand_prologue (void) if (crtl->is_leaf && !cfun->calls_alloca) { if (size > get_probe_interval ()) - ix86_emit_probe_stack_range (0, size); + ix86_emit_probe_stack_range (0, size, int_registers_saved); } else ix86_emit_probe_stack_range (0, - size + get_stack_check_protect ()); + size + get_stack_check_protect (), + int_registers_saved); } else { @@ -13778,10 +13804,13 @@ ix86_expand_prologue (void) if (size > get_probe_interval () && size > get_stack_check_protect ()) ix86_emit_probe_stack_range (get_stack_check_protect (), - size - get_stack_check_protect ()); + (size + - get_stack_check_protect ()), + int_registers_saved); } else - ix86_emit_probe_stack_range (get_stack_check_protect (), size); + ix86_emit_probe_stack_range (get_stack_check_protect (), size, + int_registers_saved); } } } diff --git a/gcc/testsuite/gcc.target/i386/pr84064.c b/gcc/testsuite/gcc.target/i386/pr84064.c new file mode 100644 index 0000000..01f8d9e --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr84064.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -march=i686 -fstack-clash-protection" } */ +/* { dg-require-effective-target ia32 } */ + +void +f (void *p1, void *p2) +{ + __builtin_memcpy (p1, p2, 1000); +} +