[Bug middle-end/112917] New: Most strub execution tests FAIL on SPARC

[Bug middle-end/112917] New: Most strub execution tests FAIL on SPARC

[ro at gcc dot gnu.org]

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

            Bug ID: 112917
           Summary: Most strub execution tests FAIL on SPARC
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ro at gcc dot gnu.org
                CC: aoliva at gcc dot gnu.org, ebotcazou at gcc dot gnu.org
  Target Milestone: ---
            Target: sparc-sun-solaris2.11

Most of the new strub execution tests FAIL on Solaris/SPARC (both 32 and
64-bit):

FAIL: c-c++-common/strub-defer-O1.c  -std=gnu++14 execution test
FAIL: c-c++-common/strub-defer-O1.c  -std=gnu++17 execution test
FAIL: c-c++-common/strub-defer-O1.c  -std=gnu++20 execution test
FAIL: c-c++-common/strub-defer-O1.c  -std=gnu++98 execution test

32-bit

FAIL: c-c++-common/strub-defer-O2.c  -std=gnu++14 execution test
FAIL: c-c++-common/strub-defer-O2.c  -std=gnu++17 execution test
FAIL: c-c++-common/strub-defer-O2.c  -std=gnu++20 execution test
FAIL: c-c++-common/strub-defer-O2.c  -std=gnu++98 execution test

32 and 64-bit

FAIL: c-c++-common/strub-defer-O3.c  -std=gnu++14 execution test
FAIL: c-c++-common/strub-defer-O3.c  -std=gnu++17 execution test
FAIL: c-c++-common/strub-defer-O3.c  -std=gnu++20 execution test
FAIL: c-c++-common/strub-defer-O3.c  -std=gnu++98 execution test
FAIL: c-c++-common/strub-defer-Os.c  -std=gnu++14 execution test
FAIL: c-c++-common/strub-defer-Os.c  -std=gnu++17 execution test
FAIL: c-c++-common/strub-defer-Os.c  -std=gnu++20 execution test
FAIL: c-c++-common/strub-defer-Os.c  -std=gnu++98 execution test

64-bit

FAIL: c-c++-common/torture/strub-run1.c   -O1  execution test
FAIL: c-c++-common/torture/strub-run1.c   -O2  execution test
FAIL: c-c++-common/torture/strub-run1.c   -O2 -flto  execution test
FAIL: c-c++-common/torture/strub-run1.c   -O2 -flto -flto-partition=none 
execution test
FAIL: c-c++-common/torture/strub-run1.c   -Os  execution test
FAIL: c-c++-common/torture/strub-run3.c   -O0  execution test
FAIL: c-c++-common/torture/strub-run3.c   -O1  execution test
FAIL: c-c++-common/torture/strub-run3.c   -O2  execution test
FAIL: c-c++-common/torture/strub-run3.c   -O2 -flto  execution test
FAIL: c-c++-common/torture/strub-run3.c   -O2 -flto -flto-partition=none 
execution test
FAIL: c-c++-common/torture/strub-run3.c   -Os  execution test
FAIL: c-c++-common/torture/strub-run4.c   -O0  execution test
FAIL: c-c++-common/torture/strub-run4.c   -O1  execution test
FAIL: c-c++-common/torture/strub-run4.c   -O2  execution test
FAIL: c-c++-common/torture/strub-run4.c   -O2 -flto  execution test
FAIL: c-c++-common/torture/strub-run4.c   -O2 -flto -flto-partition=none 
execution test
FAIL: c-c++-common/torture/strub-run4.c   -Os  execution test
FAIL: c-c++-common/torture/strub-run4c.c   -O0  execution test
FAIL: c-c++-common/torture/strub-run4c.c   -O1  execution test
FAIL: c-c++-common/torture/strub-run4c.c   -O2  execution test
FAIL: c-c++-common/torture/strub-run4c.c   -O2 -flto  execution test
FAIL: c-c++-common/torture/strub-run4c.c   -O2 -flto -flto-partition=none 
execution test
FAIL: c-c++-common/torture/strub-run4c.c   -Os  execution test
FAIL: c-c++-common/torture/strub-run4d.c   -O0  execution test
FAIL: c-c++-common/torture/strub-run4d.c   -O1  execution test
FAIL: c-c++-common/torture/strub-run4d.c   -O2  execution test
FAIL: c-c++-common/torture/strub-run4d.c   -O2 -flto  execution test
FAIL: c-c++-common/torture/strub-run4d.c   -O2 -flto -flto-partition=none 
execution test
FAIL: c-c++-common/torture/strub-run4d.c   -Os  execution test
FAIL: c-c++-common/torture/strub-run4i.c   -O0  execution test
FAIL: c-c++-common/torture/strub-run4i.c   -O1  execution test
FAIL: c-c++-common/torture/strub-run4i.c   -O2  execution test
FAIL: c-c++-common/torture/strub-run4i.c   -O2 -flto  execution test
FAIL: c-c++-common/torture/strub-run4i.c   -O2 -flto -flto-partition=none 
execution test
FAIL: c-c++-common/torture/strub-run4i.c   -Os  execution test

32 and 64-bit

I've run two of the tests under gdb:

Starting program:
/var/gcc/regression/master/11.4-gcc-gas/build/gcc/testsuite/gcc/strub-defer-O1.exe 
[...]
Thread 2 received signal SIGABRT, Aborted.
[Switching to Thread 1 (LWP 1)]
0xfec394ec in __lwp_sigqueue () from /lib/libc.so.1
(gdb) bt
#0  0xfec394ec in __lwp_sigqueue () from /lib/libc.so.1
#1  0xfeb770e8 in raise () from /lib/libc.so.1
#2  0xfeb485c0 in abort () from /lib/libc.so.1
#3  0x00010e5c in deferred_at_calls (.strub.watermark_ptr=<optimized out>)
    at
/vol/gcc/src/hg/master/local/gcc/testsuite/c-c++-common/strub-defer-O3.c:83
#4  0x00010f60 in main ()
    at
/vol/gcc/src/hg/master/local/gcc/testsuite/c-c++-common/strub-defer-O3.c:106

Starting program:
/var/gcc/regression/master/11.4-gcc-gas/build/gcc/testsuite/gcc/strub-run1.exe 
[...]
Thread 2 received signal SIGABRT, Aborted.
[Switching to Thread 1 (LWP 1)]
0xfec394ec in __lwp_sigqueue () from /lib/libc.so.1
(gdb) bt
#0  0xfec394ec in __lwp_sigqueue () from /lib/libc.so.1
#1  0xfeb770e8 in raise () from /lib/libc.so.1
#2  0xfeb485c0 in abort () from /lib/libc.so.1
#3  0x00010f80 in main ()
    at
/vol/gcc/src/hg/master/local/gcc/testsuite/c-c++-common/torture/strub-run1.c:92

The documentation is incomplete AFAICT: while invoke.texi describes the
-fstrub=
options, it barely tell what this is all about.  Seems to me like
implementation
detail without a high-level overview.

[Bug middle-end/112917] Most strub execution tests FAIL on SPARC

[ro at gcc dot gnu.org]

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

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0

[Bug middle-end/112917] Most strub execution tests FAIL on SPARC

[aoliva at gcc dot gnu.org]

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

Alexandre Oliva <aoliva at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |aoliva at gcc dot gnu.org
   Last reconfirmed|                            |2023-12-09

--- Comment #1 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
Hello, Rainer,

The bulk of the documentation about strub is not at the options, that are
indeed mostly developer-oriented implementation details, but in the attribute,
referenced from the first of the strub options.

It's about stack scrubbing, and the execution tests all follow roughly the same
pattern: there's a body that gets a certain string onto the stack, a check
before scrubbing that the string is there, and a check after scrubbing that the
string is no longer there.

I'm afraid I haven't had access to sparc hardware in a very long time; the
sparc machines in the compile farm that I used before have long been down, and
the solaris ones there aren't letting me in for some reason.

I've looked at the asm output for the tests, and I see nothing particularly
wrong.  What's probably happening is that the test_string, stored in the s
buffer within leak_string(), is getting into the stack range that, when the
deferred_at_calls calls strub_leave, is used by strub_leave itself, so it
doesn't get cleared.  sparc is quite stack hungry in this regard, and ISTM
that, if the register window doesn't get flushed, that range won't be
overwritten at all, and the copy of test_string will remain there.

If this theory is correct, this is a severe vulnerability in the stack
scrubbing implementation on sparc.  I'd envisioned overwriting some fixed stack
range after an out-of-line strub_update (hand-coded assembly tail-called from
strub_update could accomplish this), to catch just this kind of situation, but
strub_update has been so lean in stack use that this didn't seem necessary. 
Now, for sparc, this seems to be essential.

Could you please help me confirm this theory?

Since it is likely that GDB would cause register window flushes that wouldn't
occur in normal execution, inspecting the stack range would be little use, but
checking the addresses would likely confirm it.

Here's a debug session (on x86_64) that I'd appreciate if you could mirror on
sparc.  

gdb strub-defer-O1
b 32
run
p /x &s[7]
b strub.c:103
continue
p /x base
p /x end

If the theory is wrong, &s[7] will be between base and end, but if it's
correct, it will be above end, and increasing PAD enough in the testcase would
likely be enough to move the string out of the register window saving part of
__strub_leave's frame, and make this (and other tests that define PAD) pass,
confirming what we need for a proper fix.

Thanks,

[Bug middle-end/112917] Most strub execution tests FAIL on SPARC

[aoliva at gcc dot gnu.org]

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

--- Comment #2 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
Nevermind, I've managed to log into the cfarm machines running solaris/sparc.

[Bug middle-end/112917] Most strub execution tests FAIL on SPARC

[ro at CeBiTec dot Uni-Bielefeld.DE]

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

--- Comment #3 from ro at CeBiTec dot Uni-Bielefeld.DE <ro at CeBiTec dot Uni-Bielefeld.DE> ---
> --- Comment #2 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
> Nevermind, I've managed to log into the cfarm machines running solaris/sparc.

Good: while the Solaris 11.3/SPARC system (cfarm211) seems to have
worked fine for me, the Solaris 11.4 one (cfarm106) is off for 4 months.
I hope to add new Solaris 11.4/SPARC (and x86) cfarm hosts myself early
next year.

[Bug middle-end/112917] Most strub execution tests FAIL on SPARC

[aoliva at gcc dot gnu.org]

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

--- Comment #4 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
Ok, I understand the issues now.

The problem on sparc32 is indeed the large register save area that
__strub_leave allocates, that overlaps with stack space it's expected to scrub,
and that thus doesn't get scrubbed.  There's no inherent reason for
__strub_leave to allocate a stack frame, the only reason it does is because of
-fPIC.  Compiling strub.c with -fno-PIC on sparc solves the problem, without
any ill effects on libgcc_s.so (no global variables, no function calls), so I'm
probably going with it.  I have a patch that gets all strub tests to pass.

The remaining problem on sparc64 is the bias in the stack pointer: the used
stack space is usually outside the range delimited by the biased stack
pointers.  My plan to fix this is to modify __builtin_stack_address to return
the unbiased address, as long as __builtin_frame_address is unbiased, or
otherwise introduce target-specified unbiasing for them where the strub
machinery currently uses the stack-pointing (biased) registers.

[Bug middle-end/112917] Most strub execution tests FAIL on SPARC

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alexandre Oliva <aoliva@gcc.gnu.org>:

https://gcc.gnu.org/g:9fa35dbb901b11d31a897cc88c9258e7cd35b899

commit r14-6736-g9fa35dbb901b11d31a897cc88c9258e7cd35b899
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Thu Dec 14 03:21:37 2023 -0300

    strub: sparc: omit frame in strub_leave [PR112917]

    If we allow __strub_leave to allocate a frame on sparc, it will
    overlap with a lot of the stack range we're supposed to scrub, because
    of the large fixed-size outgoing args and register save area.
    Unfortunately, setting up the PIC register seems to prevent the frame
    pointer from being omitted.

    Since the strub runtime doesn't issue calls or use global variables,
    at least on sparc, disabling PIC to compile strub.c seems to do the
    right thing.


    for  libgcc/ChangeLog

            PR middle-end/112917
            * config.host (sparc, sparc64): Enable...
            * config/sparc/t-sparc: ... this new fragment.

[Bug middle-end/112917] Most strub execution tests FAIL on SPARC

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alexandre Oliva <aoliva@gcc.gnu.org>:

https://gcc.gnu.org/g:4e0a467302fea56d63b7a6d17f99c0f388960dc7

commit r14-6737-g4e0a467302fea56d63b7a6d17f99c0f388960dc7
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Thu Dec 14 04:50:45 2023 -0300

    strub: sparc64: unbias the stack address [PR112917]

    The stack pointer is biased by 2047 bytes on sparc64, so the range it
    delimits is way off.  Unbias the addresses returned by
    __builtin_stack_address (), so that the strub builtins, inlined or
    not, can function correctly.  I've considered introducing a new target
    macro, but using STACK_POINTER_OFFSET seems safe, and it enables the
    register save areas to be scrubbed as well.

    Because of the large fixed-size outgoing args area next to the
    register save area on sparc, we still need __strub_leave to not
    allocate its own frame, otherwise it won't be able to clear part of
    the frame it should.


    for  gcc/ChangeLog

            PR middle-end/112917
            * builtins.cc (expand_bultin_stack_address): Add
            STACK_POINTER_OFFSET.
            * doc/extend.texi (__builtin_stack_address): Adjust.

[Bug middle-end/112917] Most strub execution tests FAIL on SPARC

[aoliva at gcc dot gnu.org]

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

Alexandre Oliva <aoliva at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
Fixed

[Bug middle-end/112917] Most strub execution tests FAIL on SPARC

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alexandre Oliva <aoliva@gcc.gnu.org>:

https://gcc.gnu.org/g:320fb976e933e8892af905e68de65492568f2a49

commit r14-8642-g320fb976e933e8892af905e68de65492568f2a49
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Wed Jan 31 00:13:36 2024 -0300

    0From: Alexandre Oliva <oliva@adacore.com>

    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: Rebuilt.