From patchwork Fri Dec 7 14:52:48 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wilco Dijkstra X-Patchwork-Id: 1009500 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-491885-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="hzL0r3E1"; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=armh.onmicrosoft.com header.i=@armh.onmicrosoft.com header.b="Ft19o9RN"; 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 43BFnp4JJCz9s1c for ; Sat, 8 Dec 2018 01:53:02 +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 :to:cc:subject:date:message-id:content-type :content-transfer-encoding:mime-version; q=dns; s=default; b=G8F eq2rgJp2UxDZ4Lhh6q9k5J/iF3GQVFmirU1btA9spx7iy7hVQ7yQkDrbSSDu0jXs /K7DaNh2duTe9zmAsM2TuitFvbBAlWdMEhjy8UUmuaig92JcONCXb59ypvZz2/iX 6Qk7OGd0GlhP4TyX2UhEIhcv4EnmJQQ+ApOsR1xU= 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 :to:cc:subject:date:message-id:content-type :content-transfer-encoding:mime-version; s=default; bh=EFTh9OAX5 8JRpj6DlyxtgzkPuz0=; b=hzL0r3E1nGb5nrl8/oWekBXJalx38tYmqpXcPb/+I hVf1yebjV/mVAJ71brtFpvcoiynxIVBD3aipUSiMNZCK3jktRcZ6sVEjiQSlHtpa XjqHTyY9wuG2Qpp698NIC9+bbVyMwZFDdTQm1DUK73gM5oXVFM3kkCeY2YCi9hF7 90= Received: (qmail 11164 invoked by alias); 7 Dec 2018 14:52:55 -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 11144 invoked by uid 89); 7 Dec 2018 14:52:54 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_NUMSUBJECT, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:4315, setjmp, inspected, VOIDmode X-HELO: EUR04-HE1-obe.outbound.protection.outlook.com Received: from mail-eopbgr70041.outbound.protection.outlook.com (HELO EUR04-HE1-obe.outbound.protection.outlook.com) (40.107.7.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 07 Dec 2018 14:52:52 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=To2SHof3Nt7tjwiS9Bw7N4Jd+pcHozySkCA4yjiiOWA=; b=Ft19o9RNlVuHIB5AySLVS41xEwRrz5elXsWwH+l2RNIKgj4wjmHCcEI+vq/VkEyv76u073t2tnGEajThPc/25/s3dnUk1qcZhZew2LPbrn12PkPhkWfvhENqTZQvr1QjEhkAGRiCwcOft0yUed/Pcd0MpiGU4YUncxHoufYwZok= Received: from DB5PR08MB1030.eurprd08.prod.outlook.com (10.166.14.15) by DB5PR08MB1158.eurprd08.prod.outlook.com (10.166.174.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1404.21; Fri, 7 Dec 2018 14:52:48 +0000 Received: from DB5PR08MB1030.eurprd08.prod.outlook.com ([fe80::8c16:7604:c66c:8942]) by DB5PR08MB1030.eurprd08.prod.outlook.com ([fe80::8c16:7604:c66c:8942%6]) with mapi id 15.20.1404.021; Fri, 7 Dec 2018 14:52:48 +0000 From: Wilco Dijkstra To: GCC Patches CC: nd Subject: [PATCH v2] Fix PR64242 Date: Fri, 7 Dec 2018 14:52:48 +0000 Message-ID: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Wilco.Dijkstra@arm.com; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) MIME-Version: 1.0 Improve the fix for PR64242. Various optimizations can change a memory reference into a frame access. Given there are multiple virtual frame pointers which may be replaced by multiple hard frame pointers, there are no checks for writes to the various frame pointers. So updates to a frame pointer tends to generate incorrect code. Improve the previous fix to also add clobbers of several frame pointers and add a scheduling barrier. This should work in most cases until GCC supports a generic "don't optimize across this instruction" feature. Also improve testcase to handle alloca implementations which allocate too much, and increase stack sizes to avoid accidentally passing the test like reported by Jakub in comment #10 in the PR. Bootstrap OK. Testcase passes on AArch64 and x86-64. Inspected x86, Arm, Thumb-1 and Thumb-2 assembler which looks correct. ChangeLog: 2018-12-07 Wilco Dijkstra gcc/ PR middle-end/64242 * builtins.c (expand_builtin_longjmp): Add frame clobbers and schedule block. (expand_builtin_nonlocal_goto): Likewise. testsuite/ PR middle-end/64242 * gcc.c-torture/execute/pr64242.c: Update test. diff --git a/gcc/builtins.c b/gcc/builtins.c index 669e548706f537fa9a92c5f47f30fc3c6ee38176..2ef9c9afcc69fcb775dc6a6fff550025bdc76337 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -1138,15 +1138,20 @@ expand_builtin_longjmp (rtx buf_addr, rtx value) emit_insn (targetm.gen_nonlocal_goto (value, lab, stack, fp)); else { - lab = copy_to_reg (lab); - emit_clobber (gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode))); emit_clobber (gen_rtx_MEM (BLKmode, hard_frame_pointer_rtx)); + lab = copy_to_reg (lab); + /* Restore the frame pointer and stack pointer. We must use a temporary since the setjmp buffer may be a local. */ fp = copy_to_reg (fp); emit_stack_restore (SAVE_NONLOCAL, stack); + + /* Ensure the frame pointer move is not optimized. */ + emit_insn (gen_blockage ()); + emit_clobber (hard_frame_pointer_rtx); + emit_clobber (frame_pointer_rtx); emit_move_insn (hard_frame_pointer_rtx, fp); emit_use (hard_frame_pointer_rtx); @@ -1285,15 +1290,20 @@ expand_builtin_nonlocal_goto (tree exp) emit_insn (targetm.gen_nonlocal_goto (const0_rtx, r_label, r_sp, r_fp)); else { - r_label = copy_to_reg (r_label); - emit_clobber (gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (VOIDmode))); emit_clobber (gen_rtx_MEM (BLKmode, hard_frame_pointer_rtx)); + r_label = copy_to_reg (r_label); + /* Restore the frame pointer and stack pointer. We must use a temporary since the setjmp buffer may be a local. */ r_fp = copy_to_reg (r_fp); emit_stack_restore (SAVE_NONLOCAL, r_sp); + + /* Ensure the frame pointer move is not optimized. */ + emit_insn (gen_blockage ()); + emit_clobber (hard_frame_pointer_rtx); + emit_clobber (frame_pointer_rtx); emit_move_insn (hard_frame_pointer_rtx, r_fp); /* USE of hard_frame_pointer_rtx added for consistency; diff --git a/gcc/testsuite/gcc.c-torture/execute/pr64242.c b/gcc/testsuite/gcc.c-torture/execute/pr64242.c index 46a7b23d28d71604d141281c21fb0b77849b1b0d..e6139ede3f34d587ac53d04e286e5d75fd2ca76c 100644 --- a/gcc/testsuite/gcc.c-torture/execute/pr64242.c +++ b/gcc/testsuite/gcc.c-torture/execute/pr64242.c @@ -5,46 +5,31 @@ extern void abort (void); __attribute ((noinline)) void broken_longjmp (void *p) { - void *buf[5]; + void *buf[32]; __builtin_memcpy (buf, p, 5 * sizeof (void*)); /* Corrupts stack pointer... */ __builtin_longjmp (buf, 1); } -__attribute ((noipa)) __UINTPTR_TYPE__ -foo (void *p) -{ - return (__UINTPTR_TYPE__) p; -} - -__attribute ((noipa)) void -bar (void *p) -{ - asm volatile ("" : : "r" (p)); -} - volatile int x = 0; -void *volatile p; -void *volatile q; +char *volatile p; +char *volatile q; int main () { void *buf[5]; - struct __attribute__((aligned (32))) S { int a[4]; } s; - bar (&s); p = __builtin_alloca (x); + q = __builtin_alloca (x); if (!__builtin_setjmp (buf)) broken_longjmp (buf); + /* Compute expected next alloca offset - some targets don't align properly + and allocate too much. */ + p = q + (q - p); + /* Fails if stack pointer corrupted. */ - q = __builtin_alloca (x); - if (foo (p) < foo (q)) - { - if (foo (q) - foo (p) >= 1024) - abort (); - } - else if (foo (p) - foo (q) >= 1024) + if (p != __builtin_alloca (x)) abort (); return 0;