Stack pointer modifications in asm are currently not flagged in
crtl->sp_is_unchanging due to RTX pointer comparison in
notice_stack_pointer_modification_1. Pointer comparison does not detect
that "(reg/v:DI 7 sp [ current_stack_pointer ])" and "(reg/f:DI 7 sp)"
RTXes actually all correspond to the same stack pointer register.
Due to the above omission, the compiler does not detect that asm RTX
manipulates stack pointer in the following construct:
--cut here--
register unsigned long current_stack_pointer asm ("%rsp");
#define ASM_CALL_CONSTRAINT "+r" (current_stack_pointer)
asm volatile ("pushfq; push %1; pop %0; popfq"
: "=r" (y), ASM_CALL_CONSTRAINT
: "e" (-1)
: "memory");
--cut here--
which corresponds to the following RTX:
(insn:TI 17 15 3 2 (parallel [
(set (reg:DI 0 ax [orig:114 y ] [114])
(asm_operands/v:DI ("pushfq; push %1; pop %0; popfq") ("=r") 0 [
(const_int -1 [0xffffffffffffffff])
(reg/v:DI 7 sp [ current_stack_pointer ])
]
[
(asm_input:SI ("e") pr117359.c:23)
(asm_input:DI ("1") pr117359.c:23)
]
[] pr117359.c:23))
(set (reg/v:DI 7 sp [ current_stack_pointer ])
(asm_operands/v:DI ("pushfq; push %1; pop %0; popfq") ("=r") 1 [
(const_int -1 [0xffffffffffffffff])
(reg/v:DI 7 sp [ current_stack_pointer ])
]
[
(asm_input:SI ("e") pr117359.c:23)
(asm_input:DI ("1") pr117359.c:23)
]
[] pr117359.c:23))
(clobber (mem:BLK (scratch) [0 A8]))
(clobber (reg:CC 17 flags))
]) "pr117359.c":23:3 -1
(expr_list:REG_UNUSED (reg:CC 17 flags)
(expr_list:REG_UNUSED (reg:DI 0 ax [orig:114 y ] [114])
(nil))))
The discussion in the PR117359 (and its dependent PR117312) suggested
adding "rsp" to the list of clobbered registers instead. However in this
case the compiler warns with:
warning: listing the stack pointer register ‘rsp’ in a clobber list is
deprecated [-Wdeprecated]
23 | asm volatile ("pushfq; push %1; pop %0; popfq"
| ^~~
note: the value of the stack pointer after an ‘asm’ statement must be
the same as it was before the statement
But in the above case, "(reg/f:DI 7 sp)" is indeed passed to
notice_stack_pointer_modification_1, which satisfied pointer comparison,
crtl->sp_is_unchanging is cleared and consequently redzone for x86
target is not constructed.
The original source however does *not* clobber the stack pointer, its value
after asm statement is still the same as it was before the statement
(as is requested in the note), so the clobber would be misleading. The asm
does change the memory location that depends on the stack pointer, but to
quote Jakub from PR117312c#19 regarding "memory" and "rsp" clobbers:
--q--
"memory" clobber is IMO about possibly changing any user var in memory
behind the back of the compiler, not about changing whatever compiler
internals stored somewhere on the stack in stack slots that don't have
address taken and could escape to the inline asm.
So, compiler doesn't need to assume the inline asm could have say changed
the saved frame pointer or return address on the stack, or stuff temporarily
pushed into the red zone, stack canary, pseudos pushed temporarily to stack
during reload, ...
Because there is no way the compiler can cope with that being changed,
reloading is exactly what the compiler does if say inline asm clobbers
a register in which something that needs to live across the inline asm
is stored.
--/q--
x86 enables redzone based on the crtl->sp_is_unchanging in:
if (ix86_using_red_zone ()
&& crtl->sp_is_unchanging
&& crtl->is_leaf
&& !ix86_pc_thunk_call_expanded
&& !ix86_current_function_calls_tls_descriptor)
and when a builtin function that manipulates stack is used
(e.g. __builtin_ia32_readeflags_u64 that pushes eflags to the stack
and pops the value from the stack int a register),
notice_stack_pointer_modification_1 detects a write to a stack pointer
and clears crtl->sp_is_unchanging.
Based on the above discussion, the patch improves the detection of
stack pointer RTX in notice_stack_pointer_modification_1 to set
crtl->sp_is_unchanging even when the stack pointer is declared in
the source and used as the output of the asm.
PR middle-end/117359
gcc/ChangeLog:
* stack-ptr-mod.cc (notice_stack_pointer_modification_1):
Use rtx_equal_p to check x for stack_pointer_rtx.
gcc/testsuite/ChangeLog:
* gcc.target/i386/pr117359.c: New test.
Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
OK for mainline and backports?
Thanks,
Uros.
diff --git a/gcc/stack-ptr-mod.cc b/gcc/stack-ptr-mod.cc
index ba38de54bb1..9a205c58707 100644
--- a/gcc/stack-ptr-mod.cc
+++ b/gcc/stack-ptr-mod.cc
@@ -34,13 +34,13 @@ static void
notice_stack_pointer_modification_1 (rtx x, const_rtx pat ATTRIBUTE_UNUSED,
void *data ATTRIBUTE_UNUSED)
{
- if (x == stack_pointer_rtx
+ if (rtx_equal_p (x, stack_pointer_rtx)
/* The stack pointer is only modified indirectly as the result
of a push until later. See the comments in rtl.texi
regarding Embedded Side-Effects on Addresses. */
|| (MEM_P (x)
&& GET_RTX_CLASS (GET_CODE (XEXP (x, 0))) == RTX_AUTOINC
- && XEXP (XEXP (x, 0), 0) == stack_pointer_rtx))
+ && rtx_equal_p (XEXP (XEXP (x, 0), 0), stack_pointer_rtx)))
crtl->sp_is_unchanging = 0;
}
diff --git a/gcc/testsuite/gcc.target/i386/pr117359.c
b/gcc/testsuite/gcc.target/i386/pr117359.c
new file mode 100644
index 00000000000..7c996d611db
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr117359.c
@@ -0,0 +1,44 @@
+/* PR middle-end/117359 */
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2 -mred-zone" } */
+
+register unsigned long current_stack_pointer asm ("%rsp");
+#define ASM_CALL_CONSTRAINT "+r" (current_stack_pointer)
+
+#define BLOBS 16 /* 64 bytes */
+
+unsigned long
+__attribute__((noipa))
+asm_clobbers_redzone (void)
+{
+ unsigned int blob[BLOBS];
+ unsigned long y, sum = 0;
+
+ /* Write some data to an array on the stack */
+ /* gcc puts this at [-72..-8](%rsp) */
+ for (int i = 0; i < BLOBS; i++)
+ blob[i] = 0x55555555;
+
+ /* Clobbers [-16..-1](%rsp) */
+ asm volatile ("pushfq; push %1; pop %0; popfq"
+ : "=r" (y), ASM_CALL_CONSTRAINT
+ : "e" (-1));
+
+ /* Make sure the data on the stack is used */
+ for (int i = 0; i < BLOBS; i++)
+ sum += blob[i];
+
+ return sum;
+}
+
+int main ()
+{
+ unsigned long a;
+
+ a = asm_clobbers_redzone ();
+
+ if (a != 0x555555550UL)
+ __builtin_abort();
+
+ return 0;
+}
