[Bug c/108799] New: Improper deprecation diagnostic for rsp clobber

[Bug c/108799] New: Improper deprecation diagnostic for rsp clobber

[andrew.cooper3 at citrix dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108799

            Bug ID: 108799
           Summary: Improper deprecation diagnostic for rsp clobber
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: andrew.cooper3 at citrix dot com
  Target Milestone: ---

Originally from LKML. 
https://lore.kernel.org/lkml/Y9LfmQ%2Fr1%2FpEP+uv@biznet-home.integral.gnuweeb.org/

Slightly modified example: https://godbolt.org/z/xx76nEvKM

Given:
  static void clobber_redzone_buggy(void)
  {
      register unsigned long rsp asm("rsp");
      unsigned long fl;

      asm volatile ("pushf; popq %[fl]"
                    : [fl] "=r" (fl)
                       , "+r" (rsp)
                    :
                    : //"rsp"
      );
  }

  static void set_red_zone(long *mem, long val)
  {
          __asm__ volatile ("movq %[val], %[mem]"
                            : [mem] "=m" (*mem)
                            : [val] "r" (val));
  }

  static long get_red_zone(long *mem)
  {
          long ret;

          __asm__ volatile ("movq %[in], %[out]"
                            : [out] "=r" (ret)
                            : [in] "m" (*mem));
          return ret;
  }

  long a_leaf_func_with_red_zone(void)
  {
          long x;

          set_red_zone(&x, 1);
          clobber_redzone_buggy();
          /* The correct retval is 1 */
          return get_red_zone(&x);
  }

gcc generates:

  a_leaf_func_with_red_zone:
          movl    $1, %eax
          movq    %rax, -8(%rsp)
          pushf
          popq    %rax
          movq    -8(%rsp), %rax
          ret

which is buggy because the asm clobbers the same redzone slot as `x` occupies.

Swapping the "+r"(rsp) constraint for an explicit "rsp" clobber generates:

  a_leaf_func_with_red_zone:
          pushq   %rbp
          movl    $1, %eax
          movq    %rsp, %rbp
          subq    $16, %rsp
          movq    %rax, -8(%rbp)
          pushf
          popq    %rax
          movq    -8(%rbp), %rax
          leave
          ret

which seems to do the right thing.  It sets up a full stack frame and avoids
using the redzone.

However, doing so yields:

  warning: listing the stack pointer register 'rsp' in a clobber list is
deprecated [-Wdeprecated]
  note: the value of the stack pointer after an 'asm' statement must be the
same as it was before the statement

The note is incorrect.  For ABIs with a redzone, the requirement is stricter
than simply preserving the value of the stack pointer.

The warning suggests that there ought to be a different way to express "this
clobbers the redzone", but there doesn't appear to be any other way.  If this
is the case, why is it deprecated?

[Bug middle-end/108799] Improper deprecation diagnostic for rsp clobber

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108799

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |middle-end
           Keywords|                            |documentation

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I suspect:

      asm volatile ("pushf; popq %[fl]"
                    : [fl] "=r" (fl)
                       , "+r" (rsp)
                    :
                    : "memory"
      );

Will fix the issue in the inline-asm; this is just a documentation issue I
think.

[Bug middle-end/108799] Improper deprecation diagnostic for rsp clobber

[andrew.cooper3 at citrix dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108799

--- Comment #2 from Andrew Cooper <andrew.cooper3 at citrix dot com> ---
Adding a memory clobber doesn't make any difference that I can see, and I'm not
aware of any reason why it ought to make a difference.

I suppose that my real request here is to figure out what is the correct way to
indicate that the redzone is clobbered, and to document it.

[Bug middle-end/108799] Improper deprecation diagnostic for rsp clobber

[pskocik at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108799

Petr Skocik <pskocik at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pskocik at gmail dot com

--- Comment #3 from Petr Skocik <pskocik at gmail dot com> ---
Very good question. The deprecation of SP clobbers could use some explanation
if there are indeed good reasons for it. 

IMO, if listing the SP as a clobber both (1) forces a frame pointer with
frame-pointer-relative addressing of spills (and the frame pointer isn't
clobbered too) and (2) avoids the use of the red zone (and it absolutely should
continue to do both of these things in my opinion) then gcc shouldn't need to
care about redzone clobbers (as in the `pushf;pop` example) or even a wide
class of stack pointer changes (assembly-made stack allocation and frees) just
as long as no spills made by the compiler are clobbered (or opened to being
clobbered from signal handlers) by such head-of-the-stack manipulation. Even
with assembly-less standard C that uses VLAs or allocas, gcc cannot count on
being in control of the stack pointer anyway, so why be so fussy about it when
something as expert-oriented as inline assembly tries to manipulate it?