From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x633.google.com (mail-pl1-x633.google.com [IPv6:2607:f8b0:4864:20::633]) by sourceware.org (Postfix) with ESMTPS id A98143858409 for ; Fri, 19 Jan 2024 06:24:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A98143858409 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A98143858409 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::633 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705645460; cv=none; b=o2er/23oasabNHc7g+Sd0vDPVi1kLrP91jGmOLqSWW06R7O59e7vR/fj0GitmAFRZs0eouPS9iaLmuzvrxH7zshzt9CaDx9clzjrYxXhVkpizvzVWlyMIKiarSIGtrFEmQBQeM1RdYoS+isWD+z1CeLRepjsY50VU9gBmTfz9pk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705645460; c=relaxed/simple; bh=/XvP4pM8O1qd7TFve+kDf+XpQm7vBOrq8lRsJqt7sAk=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=eQC3JdGsn3czKHjKmUE1c1vZbMQWh+Ab4qG3/tRZKmHOgfO6zaQQm9uBSwU2sbE+AC7059hQAo+cuLo4BasUVi2ygLwowsdrhDuX11Ub2POrlpYNuH7y/DbrXfJo5NzHwoecQBmVhf5pcJwL7zHrOmtNR58SOZGDb2aMQEnspYo= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x633.google.com with SMTP id d9443c01a7336-1d5d736cdcdso2980855ad.1 for ; Thu, 18 Jan 2024 22:24:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1705645456; x=1706250256; darn=gcc.gnu.org; h=mime-version:user-agent:message-id:in-reply-to:date:references :organization:subject:cc:to:from:from:to:cc:subject:date:message-id :reply-to; bh=RZe2Z2TKpH7iEJ2LglibxkTfhcWFNirj4uhW2l8IYio=; b=fly3V3lmFtP1dXtUvjxCj42dpa1mgIrL4fzB08ksjix25kCfKucqbolZQA4mD8IAPp O0A5mbRe/0oPf62Nrr0BSBgzDNgmMX+rxmloJx5fBqzGO/P4Pp8xTqaNlduce4oFph0q 1iW75FlYLRJqGMQDN0Rmjc+h9gMqh5mPgoa9UPj2i7P5Nsy3Oryf0DHpIMq8vCKic+n7 YKvV1NwaBjy8EXTIgU2pex7six2Mu+u4W1YujdLzElg/YZTnodTDYuMGn9+g5VC/G6Ef FHK6TGmIk68AApmFJO+t3yvpzQqhVchKeGjAEA2mhmAUi4yptKBF91Z7vmxPPnUEL9ts olng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705645456; x=1706250256; h=mime-version:user-agent:message-id:in-reply-to:date:references :organization:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=RZe2Z2TKpH7iEJ2LglibxkTfhcWFNirj4uhW2l8IYio=; b=YKFKGTwcAmr/Twjc5iWMjCiDIHDX6EI5DbUfw8Hlhk2m151Ss539O7RNjKvapTAgEa 6gkluMRv4Rg89jFV7rDXZPmGi9RpZf/+sQTfcsnaixVclvZzJ80LtNsK3z8/6uVbmrd2 m/wXyH0J1hUAN3MvZj9Cm3Z6+KFyD/uEBUccbGwtP8vmmlTOU8jqVoOkNqTg9ePNL1Ua Eg5Ds+lMYw5yNKytDE10/J5SlsCQWHFJIFrF6R73tFm4ATd6wLob8UNTpEwNKX1mh31S OZLNIv/v0BjlXQlCyxYDzCg4GwqQJCeelVnyunfnz36gyOs/LR9+SKd/zqHPkDKM9jXE bEcA== X-Gm-Message-State: AOJu0YzaWv8CPwvj7iBEO69fc/aC+k8QBSx9FJEDdw5duYouM5C80YZO beNDA3TypC6HiQtFSQZ1RNbhG5maDyhj1paM3CmhVdGbjPwMFRHkY8mxiR+mJw== X-Google-Smtp-Source: AGHT+IHOvHB23sXroc+mrpVTpQHNUIqqSICIZUorzcuTeKUFyudV8WP5ftHmuUGTot91Il7zkiswtg== X-Received: by 2002:a17:903:41d2:b0:1d6:e9d1:4ec1 with SMTP id u18-20020a17090341d200b001d6e9d14ec1mr2183833ple.56.1705645456528; Thu, 18 Jan 2024 22:24:16 -0800 (PST) Received: from free.home ([2804:7f1:218b:29b:c0bf:c44c:c08c:7420]) by smtp.gmail.com with ESMTPSA id b6-20020a170902ed0600b001d7121173ecsm1374864pld.199.2024.01.18.22.24.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jan 2024 22:24:16 -0800 (PST) Received: from livre (livre.home [172.31.160.2]) by free.home (8.15.2/8.15.2) with ESMTPS id 40J6NW6M075587 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 19 Jan 2024 03:23:32 -0300 From: Alexandre Oliva To: "Kewen.Lin" Cc: David Edelsohn , GCC Patches , Richard Biener , Jakub Jelinek , Peter Bergner , Segher Boessenkool , Jeff Law , Richard Sandiford Subject: Re: [PATCH] strub: Only unbias stack point for SPARC_STACK_BOUNDARY_HACK [PR113100] Organization: Free thinker, does not speak for AdaCore References: <91d2c107-0168-791b-b5fa-de21c2345f84@linux.ibm.com> Date: Fri, 19 Jan 2024 03:23:32 -0300 In-Reply-To: (Kewen Lin's message of "Thu, 18 Jan 2024 14:28:17 +0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.84 X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_QUOTING autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Jan 18, 2024, "Kewen.Lin" wrote: > Not sure if I missed something in the testing, could you > kindly double check if those test cases started to fail from r14-6275 on your > env? My guess is that they started to fail when David attempted to bypass the strub tests by changing the dg proc that detects strub support. The tests then detected the mismatch between the result of the proc and the expected errors when strub is disabled properly. Here's the cleanup patch I promised. Sorry it took so long, I was hitting bootstrap errors even on a pristine tree yesterday. Regstrapped on x86_64-linux-gnu. This patch restores the status on sparc-sun-solaris from before Kewen's patch, while leaving all other ports unchanged. Ok to install? strub: introduce STACK_ADDRESS_OFFSET Since STACK_POINTER_OFFSET is not necessarily at the boundary between caller- and callee-owned stack, as desired by __builtin_stack_address(), and using it as if it were or not causes problems, introduce a new macro so that ports can define it suitably, without modifying STACK_POINTER_OFFSET. for gcc/ChangeLog PR middle-end/112917 PR middle-end/113100 * builtins.cc (expand_builtin_stack_address): Use STACK_ADDRESS_OFFSET. * doc/extend.texi (__builtin_stack_address): Adjust. * config/sparc/sparc.h (STACK_ADDRESS_OFFSET): Define. * doc/tm.texi.in (STACK_ADDRESS_OFFSET): Document. * doc/tm.texi.in: Rebuilt. --- gcc/builtins.cc | 5 ++--- gcc/config/sparc/sparc.h | 7 +++++++ gcc/doc/extend.texi | 2 +- gcc/doc/tm.texi | 29 +++++++++++++++++++++++++++++ gcc/doc/tm.texi.in | 29 +++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 4 deletions(-) diff --git a/gcc/builtins.cc b/gcc/builtins.cc index 09f2354f1144b..37df7dcda0a0e 100644 --- a/gcc/builtins.cc +++ b/gcc/builtins.cc @@ -5450,7 +5450,7 @@ expand_builtin_stack_address () rtx ret = convert_to_mode (ptr_mode, copy_to_reg (stack_pointer_rtx), STACK_UNSIGNED); -#ifdef SPARC_STACK_BOUNDARY_HACK +#ifdef STACK_ADDRESS_OFFSET /* Unbias the stack pointer, bringing it to the boundary between the stack area claimed by the active function calling this builtin, and stack ranges that could get clobbered if it called another @@ -5477,8 +5477,7 @@ expand_builtin_stack_address () (caller) function's active area as well, whereas those pushed or allocated temporarily for a call are regarded as part of the callee's stack range, rather than the caller's. */ - if (SPARC_STACK_BOUNDARY_HACK) - ret = plus_constant (ptr_mode, ret, STACK_POINTER_OFFSET); + ret = plus_constant (ptr_mode, ret, STACK_ADDRESS_OFFSET); #endif return force_reg (ptr_mode, ret); diff --git a/gcc/config/sparc/sparc.h b/gcc/config/sparc/sparc.h index fc064a92c22d9..fb074808d30d4 100644 --- a/gcc/config/sparc/sparc.h +++ b/gcc/config/sparc/sparc.h @@ -734,6 +734,13 @@ along with GCC; see the file COPYING3. If not see parameter regs. */ #define STACK_POINTER_OFFSET (FIRST_PARM_OFFSET(0) + SPARC_STACK_BIAS) +/* Unbias the stack pointer if needed, and move past the register save area, + that is never in use while a function is active, so that it is regarded as a + callee save area rather than as part of the function's own stack area. This + enables __strub_leave() to do a better job of clearing the stack frame of a + previously-called sibling. */ +#define STACK_ADDRESS_OFFSET STACK_POINTER_OFFSET + /* Base register for access to local variables of the function. */ #define HARD_FRAME_POINTER_REGNUM 30 diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 0bc586d120e76..00d8aa390cc5e 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -12791,7 +12791,7 @@ situations. @deftypefn {Built-in Function} {void *} __builtin_stack_address () This function returns the stack pointer register, offset by -@code{STACK_POINTER_OFFSET}. +@code{STACK_ADDRESS_OFFSET} if that's defined. Conceptually, the returned address returned by this built-in function is the boundary between the stack area allocated for use by its caller, and diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 69ae63c77de6e..c8b8b126b2424 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -3456,6 +3456,35 @@ or type, otherwise return false. The default implementation always returns true. @end deftypefn +@defmac STACK_ADDRESS_OFFSET +Offset from the stack pointer register to the boundary address between +the stack area claimed by an active function, and stack ranges that +could get clobbered if it called another function. It should NOT +encompass any stack red zone, that is used in leaf functions. + +This value is added to the stack pointer register to compute the address +returned by @code{__builtin_stack_address}, and this is its only use. +If this macro is not defined, no offset is added. Defining it like +@code{STACK_POINTER_OFFSET} may be appropriate for many machines, but +not all. + +On SPARC, for example, the register save area is *not* considered active +or used by the active function, but rather as akin to the area in which +call-preserved registers are saved by callees, so the stack address is +above that area, even though the (unbiased) stack pointer points below +it. This enables @code{__strub_leave} to clear what would otherwise +overlap with its own register save area. + +On PowerPC, @code{STACK_POINTER_OFFSET} also reserves space for a save +area, but that area is used by the caller rather than the callee, so the +boundary address is below it. + +If the address is computed too high or too low, parts of a stack range +that should be scrubbed may be left unscrubbed, scrubbing may corrupt +active portions of the stack frame, and stack ranges may be +doubly-scrubbed by caller and callee. +@end defmac + @defmac TARGET_STRUB_USE_DYNAMIC_ARRAY If defined to nonzero, @code{__strub_leave} will allocate a dynamic array covering the stack range that needs scrubbing before clearing it. diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 21343d4d1bf2f..658e1e63371e5 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -2688,6 +2688,35 @@ may reduce the size of debug information on some ports. @hook TARGET_HAVE_STRUB_SUPPORT_FOR +@defmac STACK_ADDRESS_OFFSET +Offset from the stack pointer register to the boundary address between +the stack area claimed by an active function, and stack ranges that +could get clobbered if it called another function. It should NOT +encompass any stack red zone, that is used in leaf functions. + +This value is added to the stack pointer register to compute the address +returned by @code{__builtin_stack_address}, and this is its only use. +If this macro is not defined, no offset is added. Defining it like +@code{STACK_POINTER_OFFSET} may be appropriate for many machines, but +not all. + +On SPARC, for example, the register save area is *not* considered active +or used by the active function, but rather as akin to the area in which +call-preserved registers are saved by callees, so the stack address is +above that area, even though the (unbiased) stack pointer points below +it. This enables @code{__strub_leave} to clear what would otherwise +overlap with its own register save area. + +On PowerPC, @code{STACK_POINTER_OFFSET} also reserves space for a save +area, but that area is used by the caller rather than the callee, so the +boundary address is below it. + +If the address is computed too high or too low, parts of a stack range +that should be scrubbed may be left unscrubbed, scrubbing may corrupt +active portions of the stack frame, and stack ranges may be +doubly-scrubbed by caller and callee. +@end defmac + @defmac TARGET_STRUB_USE_DYNAMIC_ARRAY If defined to nonzero, @code{__strub_leave} will allocate a dynamic array covering the stack range that needs scrubbing before clearing it. -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer More tolerance and less prejudice are key for inclusion and diversity Excluding neuro-others for not behaving ""normal"" is *not* inclusive