Bug 52813 - %rsp in clobber list is silently ignored
Summary: %rsp in clobber list is silently ignored
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: inline-asm (show other bugs)
Version: 4.6.1
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL: https://gcc.gnu.org/ml/gcc-patches/20...
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-03-31 22:42 UTC by Josh Haberman
Modified: 2019-04-18 14:14 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-07-28 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Josh Haberman 2012-03-31 22:42:02 UTC
The following test program crashes even though I correctly listed %rsp as clobbered:

--

int main() {
  asm volatile ("movq $0, %%rsp" : : : "%rsp");
  return 0;
}

--

I would prefer gcc to error out in this case instead of silently ignoring my instruction.
Comment 1 Uroš Bizjak 2012-04-01 09:21:09 UTC
The compiler doesn't analyse asm string.
Comment 2 Josh Haberman 2012-04-01 15:54:27 UTC
I don't expect the compiler to analyze the asm string.  I expect the compiler to respect my clobber list.

I told GCC that I would clobber %rsp.  Any other register that I put in the clobber list causes GCC to save that register to the stack or to another register before the asm and restore it from the stack/register after the asm.  For example:

--

#include <stdlib.h>
int main() {
  int x = rand();
  asm volatile ("movq $0, %%rax" : : : "%rax");
  return x;
}

$ gcc -Wall -O3 -fomit-frame-pointer -c -o test.o test.c
$ objdump -d -r -M intel test.o
test.o:     file format elf64-x86-64


Disassembly of section .text.startup:

0000000000000000 <main>:
   0:	48 83 ec 08          	sub    rsp,0x8
   4:	e8 00 00 00 00       	call   9 <main+0x9>
			5: R_X86_64_PC32	rand-0x4
   9:	89 c2                	mov    edx,eax
   b:	48 c7 c0 00 00 00 00 	mov    rax,0x0
  12:	89 d0                	mov    eax,edx
  14:	48 83 c4 08          	add    rsp,0x8
  18:	c3                   	ret

--

Notice that it saved eax to edx before my asm and restored it afterwards.  This works for every register except %rsp, which is silently ignored if you try to list it in the clobber list.  This is a bug.
Comment 3 Uroš Bizjak 2012-04-01 18:58:27 UTC
(In reply to comment #2)
> I don't expect the compiler to analyze the asm string.  I expect the compiler
> to respect my clobber list.
> 
> I told GCC that I would clobber %rsp.  Any other register that I put in the
> clobber list causes GCC to save that register to the stack or to another
> register before the asm and restore it from the stack/register after the asm. 
> For example:

%rsp is considered a "fixed" register, used for fixed purposes all throughout
the compiled code and are therefore not available for general allocation.

So, save %rsp at the beginning of your asm code and restore it at the end.
Comment 4 Josh Haberman 2012-04-01 19:23:14 UTC
I understand that GCC may not be able to save/restore %rsp like it does other registers.  But if that's the case, GCC should throw an error if the user puts %rsp in the clobber list, instead of silently ignoring it.  Otherwise how is the user supposed to know that %rsp will not be saved except through trial and error?
Comment 5 Ralph Corderoy 2012-05-28 23:06:51 UTC
The examples clearly show the problem and it bites me here.  Please change the status to confirmed.
Comment 6 Eric Gallager 2017-07-28 14:28:21 UTC
(In reply to Ralph Corderoy from comment #5)
> The examples clearly show the problem and it bites me here.  Please change
> the status to confirmed.

OK.
Comment 7 Dimitar Dimitrov 2018-12-09 12:53:49 UTC
Suggested fix has been posted for review: https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00532.html
Comment 8 Richard Sandiford 2019-01-15 16:47:27 UTC
Author: rsandifo
Date: Tue Jan 15 16:46:54 2019
New Revision: 267941

URL: https://gcc.gnu.org/viewcvs?rev=267941&root=gcc&view=rev
Log:
PR inline-asm/52813 revisited

The original patch for this PR made it an error to list the stack
pointer in the clobber list of an inline asm.  However, the general
feeling seemed to be that going straight to a hard error was too harsh,
since there's quite a bit of existing code that has the clobber.

This patch implements the compromise discussed on IRC of making it
a -Wdeprecated warning instead.

2019-01-15  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	PR inline-asm/52813
	* doc/extend.texi: Document that listing the stack pointer in the
	clobber list of an asm is a deprecated feature.
	* common.opt (Wdeprecated): Moved from c-family/c.opt.
	* cfgexpand.c (asm_clobber_reg_is_valid): Issue a -Wdeprecated
	warning instead of an error for clobbers of the stack pointer.
	Add a note explaining why.

gcc/c-family/
	PR inline-asm/52813
	* c.opt (Wdeprecated): Move documentation and variable to common.opt.

gcc/d/
	PR inline-asm/52813
	* lang.opt (Wdeprecated): Reference common.opt instead of c.opt.

gcc/testsuite/
	PR inline-asm/52813
	* gcc.target/i386/pr52813.c (test1): Turn the diagnostic into a
	-Wdeprecated warning and expect a following note:.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c.opt
    trunk/gcc/cfgexpand.c
    trunk/gcc/common.opt
    trunk/gcc/d/ChangeLog
    trunk/gcc/d/lang.opt
    trunk/gcc/doc/extend.texi
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/i386/pr52813.c
Comment 9 Christophe Lyon 2019-01-18 09:58:13 UTC
Author: clyon
Date: Fri Jan 18 09:57:41 2019
New Revision: 268066

URL: https://gcc.gnu.org/viewcvs?rev=268066&root=gcc&view=rev
Log:
[ARM][testsuite] follow-up to PR target/52813 and target/11807 fix.

2019-01-18  Christophe Lyon  <christophe.lyon@linaro.org>

	* gcc.target/arm/pr77904.c: Add dg-warning for sp clobber.


Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/arm/pr77904.c
Comment 10 Eric Gallager 2019-04-18 04:40:42 UTC
(In reply to rsandifo@gcc.gnu.org from comment #8)
> Author: rsandifo
> Date: Tue Jan 15 16:46:54 2019
> New Revision: 267941
> 
> URL: https://gcc.gnu.org/viewcvs?rev=267941&root=gcc&view=rev
> Log:
> PR inline-asm/52813 revisited
> 
> The original patch for this PR made it an error to list the stack
> pointer in the clobber list of an inline asm.  However, the general
> feeling seemed to be that going straight to a hard error was too harsh,
> since there's quite a bit of existing code that has the clobber.
> 
> This patch implements the compromise discussed on IRC of making it
> a -Wdeprecated warning instead.
> 
> 2019-01-15  Richard Sandiford  <richard.sandiford@arm.com>
> 
> gcc/
> 	PR inline-asm/52813
> 	* doc/extend.texi: Document that listing the stack pointer in the
> 	clobber list of an asm is a deprecated feature.
> 	* common.opt (Wdeprecated): Moved from c-family/c.opt.
> 	* cfgexpand.c (asm_clobber_reg_is_valid): Issue a -Wdeprecated
> 	warning instead of an error for clobbers of the stack pointer.
> 	Add a note explaining why.
> 
> gcc/c-family/
> 	PR inline-asm/52813
> 	* c.opt (Wdeprecated): Move documentation and variable to common.opt.
> 
> gcc/d/
> 	PR inline-asm/52813
> 	* lang.opt (Wdeprecated): Reference common.opt instead of c.opt.
> 
> gcc/testsuite/
> 	PR inline-asm/52813
> 	* gcc.target/i386/pr52813.c (test1): Turn the diagnostic into a
> 	-Wdeprecated warning and expect a following note:.
> 
> Modified:
>     trunk/gcc/ChangeLog
>     trunk/gcc/c-family/ChangeLog
>     trunk/gcc/c-family/c.opt
>     trunk/gcc/cfgexpand.c
>     trunk/gcc/common.opt
>     trunk/gcc/d/ChangeLog
>     trunk/gcc/d/lang.opt
>     trunk/gcc/doc/extend.texi
>     trunk/gcc/testsuite/ChangeLog
>     trunk/gcc/testsuite/gcc.target/i386/pr52813.c

(In reply to Christophe Lyon from comment #9)
> Author: clyon
> Date: Fri Jan 18 09:57:41 2019
> New Revision: 268066
> 
> URL: https://gcc.gnu.org/viewcvs?rev=268066&root=gcc&view=rev
> Log:
> [ARM][testsuite] follow-up to PR target/52813 and target/11807 fix.
> 
> 2019-01-18  Christophe Lyon  <christophe.lyon@linaro.org>
> 
> 	* gcc.target/arm/pr77904.c: Add dg-warning for sp clobber.
> 
> 
> Modified:
>     trunk/gcc/testsuite/ChangeLog
>     trunk/gcc/testsuite/gcc.target/arm/pr77904.c

So after these 2, has this been fixed now?
Comment 11 Christophe Lyon 2019-04-18 06:45:20 UTC
(In reply to Eric Gallager from comment #10)
> So after these 2, has this been fixed now?
OK for me.
Comment 12 Eric Gallager 2019-04-18 14:14:12 UTC
(In reply to Christophe Lyon from comment #11)
> (In reply to Eric Gallager from comment #10)
> > So after these 2, has this been fixed now?
> OK for me.

OK.