From patchwork Tue Feb 27 08:40:55 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 86419 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 626E1385843E for ; Tue, 27 Feb 2024 08:41:50 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 0CC463858C56 for ; Tue, 27 Feb 2024 08:41:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0CC463858C56 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 0CC463858C56 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709023267; cv=none; b=Nfp8/1O/VcdoNse/JZPc19lxAR8SSXrHEw0x6eRzOC05EHTP+xwkm1CHCGtCer8kJBSHrO7hBOHTVIdOcyeY5haj5jB29M3DnDMuC1mwILjItyBgjvRIbMUEU4P8QGqH48IexkceM0AkF17wvpVR08i4sFR8efvjCk2mzLanqJs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709023267; c=relaxed/simple; bh=OeJwFGrsFBACJYol2JssW+76C7mzOHnfqBI3hy7zHYw=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=JAP6uHwW4kRARELQW1fuyAvNa1Hmev5wcYpAQFcwRbI+nfZZXvBHo7YYAZUrZm/zU+cXABue+tHbfGWyYd7oqkGCKQAXTVmDos/UZyOCQs6jZ4KIoAXbznknpZmVc0LBTsoxKhDVvYA6QAlkg5E/NQAyRfNii8dOVxnuRH7lUN0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1709023262; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=n9bxOq1v6fynnp8F5TOQbxXp1WCCG7IHnyz5y9FQ+no=; b=jFX3sdrx6/6tDzKZQLPi7fHpm8TNmVfaiEJvPw1lSP0XUvliHmaEYv35xFwA7vOET5m97H 2RfuqyzbalWOrzZSVQq24MA5KKjnz50ZW1bOTcIuX6SgOQAwNQ5DCyOqCHOLFXA2aj8nJB ZmTYQVxIl3gOqNpOXxrWGbHS9IJyLqU= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-481-vLVGAaVwN1Oe2OV2fHTS8Q-1; Tue, 27 Feb 2024 03:40:59 -0500 X-MC-Unique: vLVGAaVwN1Oe2OV2fHTS8Q-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id EA1C03C1E9CC; Tue, 27 Feb 2024 08:40:58 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.25]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8D52C492BC8; Tue, 27 Feb 2024 08:40:58 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 41R8euPR060387 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 27 Feb 2024 09:40:56 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 41R8etFe060386; Tue, 27 Feb 2024 09:40:55 +0100 Date: Tue, 27 Feb 2024 09:40:55 +0100 From: Jakub Jelinek To: Uros Bizjak , hjl.tools@gmail.com Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] i386: For noreturn functions save at least the bp register if it is used [PR114116] Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-3.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE, WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Jakub Jelinek Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Hi! As mentioned in the PR, on x86_64 currently a lot of ICEs end up with crashes in the unwinder like: during RTL pass: expand pr114044-2.c: In function ‘foo’: pr114044-2.c:5:3: internal compiler error: in expand_fn_using_insn, at internal-fn.cc:208 5 | __builtin_clzg (a); | ^~~~~~~~~~~~~~~~~~ 0x7d9246 expand_fn_using_insn ../../gcc/internal-fn.cc:208 pr114044-2.c:5:3: internal compiler error: Segmentation fault 0x1554262 crash_signal ../../gcc/toplev.cc:319 0x2b20320 x86_64_fallback_frame_state ./md-unwind-support.h:63 0x2b20320 uw_frame_state_for ../../../libgcc/unwind-dw2.c:1013 0x2b2165d _Unwind_Backtrace ../../../libgcc/unwind.inc:303 0x2acbd69 backtrace_full ../../libbacktrace/backtrace.c:127 0x2a32fa6 diagnostic_context::action_after_output(diagnostic_t) ../../gcc/diagnostic.cc:781 0x2a331bb diagnostic_action_after_output(diagnostic_context*, diagnostic_t) ../../gcc/diagnostic.h:1002 0x2a331bb diagnostic_context::report_diagnostic(diagnostic_info*) ../../gcc/diagnostic.cc:1633 0x2a33543 diagnostic_impl ../../gcc/diagnostic.cc:1767 0x2a33c26 internal_error(char const*, ...) ../../gcc/diagnostic.cc:2225 0xe232c8 fancy_abort(char const*, int, char const*) ../../gcc/diagnostic.cc:2336 0x7d9246 expand_fn_using_insn ../../gcc/internal-fn.cc:208 Segmentation fault (core dumped) The problem are the PR38534 r14-8470 changes which avoid saving call-saved registers in noreturn functions. If such functions ever touch the bp register but because of the r14-8470 changes don't save it in the prologue, the caller or any other function in the backtrace uses a frame pointer and the noreturn function or anything it calls directly or indirectly calls backtrace, then the unwinder crashes, because bp register contains some unrelated value, but in the frames which do use frame pointer CFA is based on the bp register. In theory this could happen with any other call-saved register, e.g. code written by hand in assembly with .cfi_* directives could use any other call-saved register as register into which store the CFA or something related to that, but in reality at least compiler generated code and usual assembly probably just making sure bp doesn't contain garbage could be enough for backtrace purposes. In the debugger of course it will not be enough, the values of the arguments etc. can be lost (if DW_CFA_undefined is emitted) or garbage. So, I think for noreturn function we should at least save the bp register if we use it. If user asks for it using no_callee_saved_registers attribute, let's honor what is asked for (but then it is up to the user to make sure e.g. backtrace isn't called from the function or anything it calls). As discussed in the PR, whether to save bp or not shouldn't be based on whether compiling with -g or -g0, because we don't want code generation changes without/with debugging, it would also break -fcompare-debug, and users can call backtrace(3), that doesn't use debug info, just unwind info, even backtrace_symbols{,_fd}(3) don't use debug info but just looks at dynamic symbol table. The patch also adds check for no_caller_saved_registers attribute in the implicit addition of not saving callee saved register in noreturn functions, because on I think __attribute__((no_caller_saved_registers, noreturn)) will otherwise error that no_caller_saved_registers and no_callee_saved_registers attributes are incompatible (but user didn't specify anything like that). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2024-02-27 Jakub Jelinek PR target/114116 * config/i386/i386.h (enum call_saved_registers_type): Add TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP enumerator. * config/i386/i386-options.cc (ix86_set_func_type): Remove has_no_callee_saved_registers variable, add no_callee_saved_registers instead, initialize it depending on whether it is no_callee_saved_registers function or not. Don't set it if no_caller_saved_registers attribute is present. Adjust users. * config/i386/i386.cc (ix86_function_ok_for_sibcall): Handle TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP like TYPE_NO_CALLEE_SAVED_REGISTERS. (ix86_save_reg): Handle TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP. * gcc.target/i386/pr38534-1.c: Allow push/pop of bp. * gcc.target/i386/pr38534-4.c: Likewise. * gcc.target/i386/pr38534-2.c: Likewise. * gcc.target/i386/pr38534-3.c: Likewise. * gcc.target/i386/pr114097-1.c: Likewise. * gcc.target/i386/stack-check-17.c: Expect no pop on ! ia32. Jakub --- gcc/config/i386/i386.h.jj 2024-01-27 22:59:53.121319813 +0100 +++ gcc/config/i386/i386.h 2024-02-26 16:33:14.644745362 +0100 @@ -2730,9 +2730,12 @@ enum call_saved_registers_type /* The current function is a function specified with the "interrupt" or "no_caller_saved_registers" attribute. */ TYPE_NO_CALLER_SAVED_REGISTERS, + /* The current function is a function specified with the + "no_callee_saved_registers" attribute. */ + TYPE_NO_CALLEE_SAVED_REGISTERS, /* The current function is a function specified with the "noreturn" - or "no_callee_saved_registers" attribute. */ - TYPE_NO_CALLEE_SAVED_REGISTERS + attribute. */ + TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP, }; enum queued_insn_type --- gcc/config/i386/i386-options.cc.jj 2024-02-26 16:29:07.471201524 +0100 +++ gcc/config/i386/i386-options.cc 2024-02-26 16:42:01.730358609 +0100 @@ -3384,7 +3384,9 @@ ix86_set_func_type (tree fndecl) { /* No need to save and restore callee-saved registers for a noreturn function with nothrow or compiled with -fno-exceptions unless when - compiling with -O0 or -Og. + compiling with -O0 or -Og. So that backtrace works for those at least + in most cases, save the bp register if it is used, because it often + is used in callers to compute CFA. NB: Can't use just TREE_THIS_VOLATILE to check if this is a noreturn function. The local-pure-const pass turns an interrupt function @@ -3394,15 +3396,20 @@ ix86_set_func_type (tree fndecl) function is marked with TREE_THIS_VOLATILE in the IR output, which leads to the incompatible attribute error in LTO1. Ignore the interrupt function in this case. */ - bool has_no_callee_saved_registers - = ((TREE_THIS_VOLATILE (fndecl) - && !lookup_attribute ("interrupt", - TYPE_ATTRIBUTES (TREE_TYPE (fndecl))) - && optimize - && !optimize_debug - && (TREE_NOTHROW (fndecl) || !flag_exceptions)) - || lookup_attribute ("no_callee_saved_registers", - TYPE_ATTRIBUTES (TREE_TYPE (fndecl)))); + enum call_saved_registers_type no_callee_saved_registers + = TYPE_DEFAULT_CALL_SAVED_REGISTERS; + if (lookup_attribute ("no_callee_saved_registers", + TYPE_ATTRIBUTES (TREE_TYPE (fndecl)))) + no_callee_saved_registers = TYPE_NO_CALLEE_SAVED_REGISTERS; + else if (TREE_THIS_VOLATILE (fndecl) + && optimize + && !optimize_debug + && (TREE_NOTHROW (fndecl) || !flag_exceptions) + && !lookup_attribute ("interrupt", + TYPE_ATTRIBUTES (TREE_TYPE (fndecl))) + && !lookup_attribute ("no_caller_saved_registers", + TYPE_ATTRIBUTES (TREE_TYPE (fndecl)))) + no_callee_saved_registers = TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP; if (cfun->machine->func_type == TYPE_UNKNOWN) { @@ -3413,7 +3420,7 @@ ix86_set_func_type (tree fndecl) error_at (DECL_SOURCE_LOCATION (fndecl), "interrupt and naked attributes are not compatible"); - if (has_no_callee_saved_registers) + if (no_callee_saved_registers) error_at (DECL_SOURCE_LOCATION (fndecl), "%qs and %qs attributes are not compatible", "interrupt", "no_callee_saved_registers"); @@ -3442,7 +3449,7 @@ ix86_set_func_type (tree fndecl) TYPE_ATTRIBUTES (TREE_TYPE (fndecl)))) cfun->machine->call_saved_registers = TYPE_NO_CALLER_SAVED_REGISTERS; - if (has_no_callee_saved_registers) + if (no_callee_saved_registers) { if (cfun->machine->call_saved_registers == TYPE_NO_CALLER_SAVED_REGISTERS) @@ -3451,7 +3458,7 @@ ix86_set_func_type (tree fndecl) "no_caller_saved_registers", "no_callee_saved_registers"); cfun->machine->call_saved_registers - = TYPE_NO_CALLEE_SAVED_REGISTERS; + = no_callee_saved_registers; } } } --- gcc/config/i386/i386.cc.jj 2024-02-26 11:12:36.275453513 +0100 +++ gcc/config/i386/i386.cc 2024-02-26 17:24:50.701602352 +0100 @@ -984,8 +984,9 @@ ix86_function_ok_for_sibcall (tree decl, /* Sibling call isn't OK if callee has no callee-saved registers and the calling function has callee-saved registers. */ - if ((cfun->machine->call_saved_registers - != TYPE_NO_CALLEE_SAVED_REGISTERS) + if (cfun->machine->call_saved_registers != TYPE_NO_CALLEE_SAVED_REGISTERS + && (cfun->machine->call_saved_registers + != TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP) && lookup_attribute ("no_callee_saved_registers", TYPE_ATTRIBUTES (type))) return false; @@ -6649,6 +6650,11 @@ ix86_save_reg (unsigned int regno, bool case TYPE_NO_CALLEE_SAVED_REGISTERS: return false; + + case TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP: + if (regno != HARD_FRAME_POINTER_REGNUM) + return false; + break; } if (regno == REAL_PIC_OFFSET_TABLE_REGNUM --- gcc/testsuite/gcc.target/i386/pr38534-1.c.jj 2024-02-01 16:00:37.443735635 +0100 +++ gcc/testsuite/gcc.target/i386/pr38534-1.c 2024-02-27 08:59:08.421878166 +0100 @@ -22,5 +22,5 @@ no_return_to_caller (void) while (1); } -/* { dg-final { scan-assembler-not "push" } } */ -/* { dg-final { scan-assembler-not "pop" } } */ +/* { dg-final { scan-assembler-not "push\[^\n\r\]*(?:\[abcd\]x|\[sd\]i|sp|r\[0-9\]|\[xyz\]mm)" } } */ +/* { dg-final { scan-assembler-not "pop\[^\n\r\]*(?:\[abcd\]x|\[sd\]i|sp|r\[0-9\]|\[xyz\]mm)" } } */ --- gcc/testsuite/gcc.target/i386/pr38534-4.c.jj 2024-02-01 16:00:37.443735635 +0100 +++ gcc/testsuite/gcc.target/i386/pr38534-4.c 2024-02-27 09:00:09.869021464 +0100 @@ -12,7 +12,7 @@ foo (fn_t bar) fn (); } -/* { dg-final { scan-assembler-not "push" } } */ -/* { dg-final { scan-assembler-not "pop" } } */ +/* { dg-final { scan-assembler-not "push\[^\n\r\]*(?:\[abcd\]x|\[sd\]i|sp|r\[0-9\]|\[xyz\]mm)" } } */ +/* { dg-final { scan-assembler-not "pop\[^\n\r\]*(?:\[abcd\]x|\[sd\]i|sp|r\[0-9\]|\[xyz\]mm)" } } */ /* { dg-final { scan-assembler-not "jmp" } } */ /* { dg-final { scan-assembler "call\[\\t \]+" } } */ --- gcc/testsuite/gcc.target/i386/pr38534-2.c.jj 2024-02-01 16:00:37.443735635 +0100 +++ gcc/testsuite/gcc.target/i386/pr38534-2.c 2024-02-27 08:59:35.723497526 +0100 @@ -12,7 +12,7 @@ foo (void) fn (); } -/* { dg-final { scan-assembler-not "push" } } */ -/* { dg-final { scan-assembler-not "pop" } } */ +/* { dg-final { scan-assembler-not "push\[^\n\r\]*(?:\[abcd\]x|\[sd\]i|sp|r\[0-9\]|\[xyz\]mm)" } } */ +/* { dg-final { scan-assembler-not "pop\[^\n\r\]*(?:\[abcd\]x|\[sd\]i|sp|r\[0-9\]|\[xyz\]mm)" } } */ /* { dg-final { scan-assembler-not "jmp\[\\t \]+_?bar" } } */ /* { dg-final { scan-assembler "call\[\\t \]+_?bar" } } */ --- gcc/testsuite/gcc.target/i386/pr38534-3.c.jj 2024-02-01 16:00:37.443735635 +0100 +++ gcc/testsuite/gcc.target/i386/pr38534-3.c 2024-02-27 08:59:49.669303090 +0100 @@ -13,7 +13,7 @@ foo (void) fn (); } -/* { dg-final { scan-assembler-not "push" } } */ -/* { dg-final { scan-assembler-not "pop" } } */ +/* { dg-final { scan-assembler-not "push\[^\n\r\]*(?:\[abcd\]x|\[sd\]i|sp|r\[0-9\]|\[xyz\]mm)" } } */ +/* { dg-final { scan-assembler-not "pop\[^\n\r\]*(?:\[abcd\]x|\[sd\]i|sp|r\[0-9\]|\[xyz\]mm)" } } */ /* { dg-final { scan-assembler-not "jmp" } } */ /* { dg-final { scan-assembler "call\[\\t \]+" } } */ --- gcc/testsuite/gcc.target/i386/pr114097-1.c.jj 2024-02-26 16:29:07.529200714 +0100 +++ gcc/testsuite/gcc.target/i386/pr114097-1.c 2024-02-27 09:01:37.876794453 +0100 @@ -22,5 +22,5 @@ no_return_to_caller (void) while (1); } -/* { dg-final { scan-assembler-not "push" } } */ -/* { dg-final { scan-assembler-not "pop" } } */ +/* { dg-final { scan-assembler-not "push\[^\n\r\]*(?:\[abcd\]x|\[sd\]i|sp|r\[0-9\]|\[xyz\]mm)" } } */ +/* { dg-final { scan-assembler-not "pop\[^\n\r\]*(?:\[abcd\]x|\[sd\]i|sp|r\[0-9\]|\[xyz\]mm)" } } */ --- gcc/testsuite/gcc.target/i386/stack-check-17.c.jj 2024-01-27 22:59:53.186318915 +0100 +++ gcc/testsuite/gcc.target/i386/stack-check-17.c 2024-02-27 09:06:10.805989259 +0100 @@ -32,5 +32,5 @@ f3 (void) register on ia32 for a noreturn function. */ /* { dg-final { scan-assembler-times "push\[ql\]" 1 { target { ! ia32 } } } } */ /* { dg-final { scan-assembler-times "push\[ql\]" 3 { target ia32 } } } */ -/* { dg-final { scan-assembler-times "pop" 1 } } */ - +/* { dg-final { scan-assembler-not "pop" { target { ! ia32 } } } } */ +/* { dg-final { scan-assembler-times "pop" 1 { target ia32 } } } */