From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36600 invoked by alias); 14 Jun 2018 11:10:20 -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 36590 invoked by uid 89); 14 Jun 2018 11:10:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=strongly, radar, builtins.c, UD:builtins.c X-HELO: EUR01-VE1-obe.outbound.protection.outlook.com Received: from mail-ve1eur01on0058.outbound.protection.outlook.com (HELO EUR01-VE1-obe.outbound.protection.outlook.com) (104.47.1.58) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 14 Jun 2018 11:10:17 +0000 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Sudi.Das@arm.com; Received: from [10.2.206.246] (217.140.96.140) by AM5PR0801MB1715.eurprd08.prod.outlook.com (2603:10a6:203:3a::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.841.17; Thu, 14 Jun 2018 11:10:13 +0000 Subject: Re: [PATCH][AARCH64] PR target/84521 Fix frame pointer corruption with -fomit-frame-pointer with __builtin_setjmp To: Eric Botcazou Cc: gcc-patches@gcc.gnu.org, Jeff Law , nd , James Greenhalgh , Marcus Shawcroft , Richard Earnshaw , Wilco Dijkstra References: <4ee92fe8-3070-0129-59ad-40cbe0207822@arm.com> <5792268.7p7HWXWc8J@polaris> From: Sudakshina Das Message-ID: <613f1e3b-70ae-5144-62da-4f12d29a321a@arm.com> Date: Thu, 14 Jun 2018 11:10:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <5792268.7p7HWXWc8J@polaris> Content-Type: multipart/mixed; boundary="------------98348F4E2CD1637B421122B5" X-ClientProxiedBy: AM6PR0102CA0011.eurprd01.prod.exchangelabs.com (2603:10a6:209:14::24) To AM5PR0801MB1715.eurprd08.prod.outlook.com (2603:10a6:203:3a::21) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: f9b1eb9c-4394-4273-a217-08d5d1e766c2 X-MS-TrafficTypeDiagnostic: AM5PR0801MB1715: NoDisclaimer: True X-Exchange-Antispam-Report-Test: UriScan:(180628864354917)(22074186197030)(183786458502308); X-MS-Exchange-SenderADCheck: 1 X-Forefront-PRVS: 0703B549E4 Received-SPF: None (protection.outlook.com: arm.com does not designate permitted sender hosts) SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Jun 2018 11:10:13.2473 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: f9b1eb9c-4394-4273-a217-08d5d1e766c2 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0801MB1715 X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00830.txt.bz2 This is a multi-part message in MIME format. --------------98348F4E2CD1637B421122B5 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2236 Hi Eric On 07/06/18 16:33, Eric Botcazou wrote: >> Sorry this fell off my radar. I have reg-tested it on x86 and tried it >> on the sparc machine from the gcc farm but I think I couldn't finished >> the run and now its showing to he unreachable. > > The patch is a no-op for SPARC because it defines the nonlocal_goto pattern. > > But I would nevertheless strongly suggest _not_ fiddling with the generic code > like that and just defining the nonlocal_goto pattern for Aarch64 instead. > Thank you for the suggestion, I have edited the patch accordingly and defined the nonlocal_goto pattern for AArch64. This has also helped take care of the issue with __builtin_longjmp that Wilco had mentioned in his comment on the PR (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84521#c19). I have also modified the test case according to Wilco's comment to add an extra jump buffer. This test case passes with AArch64 but fails on x86 trunk as follows (It may fail on other targets as well): FAIL: gcc.c-torture/execute/pr84521.c -O1 execution test FAIL: gcc.c-torture/execute/pr84521.c -O2 execution test FAIL: gcc.c-torture/execute/pr84521.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions execution test FAIL: gcc.c-torture/execute/pr84521.c -O3 -g execution test FAIL: gcc.c-torture/execute/pr84521.c -Os execution test FAIL: gcc.c-torture/execute/pr84521.c -O2 -flto -fno-use-linker-plugin -flto-partition=none execution test FAIL: gcc.c-torture/execute/pr84521.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects execution test Testing: Bootstrapped and regtested on aarch64-none-linux-gnu. Is this ok for trunk? Sudi *** gcc/ChangeLog *** 2018-06-14 Sudakshina Das PR target/84521 * config/aarch64/aarch64.h (DONT_USE_BUILTIN_SETJMP): Update comment. * config/aarch64/aarch64.c (aarch64_needs_frame_chain): Add cfun->has_nonlocal_label to force frame chain. (aarch64_builtin_setjmp_frame_value): New. (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Define. * config/aarch64/aarch64.md (nonlocal_goto): New. *** gcc/testsuite/ChangeLog *** 2018-06-14 Sudakshina Das PR target/84521 * gcc.c-torture/execute/pr84521.c: New test. --------------98348F4E2CD1637B421122B5 Content-Type: text/x-patch; name="pr84521-new.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pr84521-new.diff" Content-length: 3996 diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 976f9af..f042def 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -474,7 +474,9 @@ extern unsigned aarch64_architecture_version; #define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, R4_REGNUM) #define EH_RETURN_HANDLER_RTX aarch64_eh_return_handler_rtx () -/* Don't use __builtin_setjmp until we've defined it. */ +/* Don't use __builtin_setjmp until we've defined it. + CAUTION: This macro is only used during exception unwinding. + Don't fall for its name. */ #undef DONT_USE_BUILTIN_SETJMP #define DONT_USE_BUILTIN_SETJMP 1 diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index bd0ac2f..95f7fe3 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -3998,7 +3998,7 @@ static bool aarch64_needs_frame_chain (void) { /* Force a frame chain for EH returns so the return address is at FP+8. */ - if (frame_pointer_needed || crtl->calls_eh_return) + if (frame_pointer_needed || crtl->calls_eh_return || cfun->has_nonlocal_label) return true; /* A leaf function cannot have calls or write LR. */ @@ -12213,6 +12213,13 @@ aarch64_expand_builtin_va_start (tree valist, rtx nextarg ATTRIBUTE_UNUSED) expand_expr (t, const0_rtx, VOIDmode, EXPAND_NORMAL); } +/* Implement TARGET_BUILTIN_SETJMP_FRAME_VALUE. */ +static rtx +aarch64_builtin_setjmp_frame_value (void) +{ + return hard_frame_pointer_rtx; +} + /* Implement TARGET_GIMPLIFY_VA_ARG_EXPR. */ static tree @@ -17829,6 +17836,9 @@ aarch64_run_selftests (void) #undef TARGET_FOLD_BUILTIN #define TARGET_FOLD_BUILTIN aarch64_fold_builtin +#undef TARGET_BUILTIN_SETJMP_FRAME_VALUE +#define TARGET_BUILTIN_SETJMP_FRAME_VALUE aarch64_builtin_setjmp_frame_value + #undef TARGET_FUNCTION_ARG #define TARGET_FUNCTION_ARG aarch64_function_arg diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 830f976..381fd83 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -6081,6 +6081,30 @@ DONE; }) +;; This is broadly similar to the builtins.c except that it uses +;; temporaries to load the incoming SP and FP. +(define_expand "nonlocal_goto" + [(use (match_operand 0 "general_operand")) + (use (match_operand 1 "general_operand")) + (use (match_operand 2 "general_operand")) + (use (match_operand 3 "general_operand"))] + "" +{ + rtx label_in = copy_to_reg (operands[1]); + rtx fp_in = copy_to_reg (operands[3]); + rtx sp_in = copy_to_reg (operands[2]); + + emit_move_insn (hard_frame_pointer_rtx, fp_in); + emit_stack_restore (SAVE_NONLOCAL, sp_in); + + emit_use (hard_frame_pointer_rtx); + emit_use (stack_pointer_rtx); + + emit_indirect_jump (label_in); + + DONE; +}) + ;; Helper for aarch64.c code. (define_expand "set_clobber_cc" [(parallel [(set (match_operand 0) diff --git a/gcc/testsuite/gcc.c-torture/execute/pr84521.c b/gcc/testsuite/gcc.c-torture/execute/pr84521.c new file mode 100644 index 0000000..c20ff5e --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr84521.c @@ -0,0 +1,53 @@ +/* { dg-require-effective-target indirect_jumps } */ + +#include +#include +#include + +jmp_buf buf; + +int uses_longjmp (void) +{ + jmp_buf buf2; + memcpy (buf2, buf, sizeof (buf)); + __builtin_longjmp (buf2, 1); +} + +int gl; +void after_longjmp (void) +{ + gl = 5; +} + +int +test_1 (int n) +{ + volatile int *p = alloca (n); + if (__builtin_setjmp (buf)) + { + after_longjmp (); + } + else + { + uses_longjmp (); + } + + return 0; +} + +int __attribute__ ((optimize ("no-omit-frame-pointer"))) +test_2 (int n) +{ + int i; + int *ptr = (int *)__builtin_alloca (sizeof (int) * n); + for (i = 0; i < n; i++) + ptr[i] = i; + test_1 (n); + return 0; +} + +int main (int argc, const char **argv) +{ + __builtin_memset (&buf, 0xaf, sizeof (buf)); + test_2 (100); +} --------------98348F4E2CD1637B421122B5--