* [Bug middle-end/112917] Most strub execution tests FAIL on SPARC
2023-12-08 10:28 [Bug middle-end/112917] New: Most strub execution tests FAIL on SPARC ro at gcc dot gnu.org
@ 2023-12-08 10:35 ` ro at gcc dot gnu.org
2023-12-09 5:45 ` aoliva at gcc dot gnu.org
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: ro at gcc dot gnu.org @ 2023-12-08 10:35 UTC (permalink / raw)
To: gcc-bugs
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
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/112917] Most strub execution tests FAIL on SPARC
2023-12-08 10:28 [Bug middle-end/112917] New: Most strub execution tests FAIL on SPARC ro at gcc dot gnu.org
2023-12-08 10:35 ` [Bug middle-end/112917] " ro at gcc dot gnu.org
@ 2023-12-09 5:45 ` aoliva at gcc dot gnu.org
2023-12-13 5:59 ` aoliva at gcc dot gnu.org
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: aoliva at gcc dot gnu.org @ 2023-12-09 5:45 UTC (permalink / raw)
To: gcc-bugs
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,
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/112917] Most strub execution tests FAIL on SPARC
2023-12-08 10:28 [Bug middle-end/112917] New: Most strub execution tests FAIL on SPARC ro at gcc dot gnu.org
2023-12-08 10:35 ` [Bug middle-end/112917] " ro at gcc dot gnu.org
2023-12-09 5:45 ` aoliva at gcc dot gnu.org
@ 2023-12-13 5:59 ` aoliva at gcc dot gnu.org
2023-12-13 9:11 ` ro at CeBiTec dot Uni-Bielefeld.DE
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: aoliva at gcc dot gnu.org @ 2023-12-13 5:59 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/112917] Most strub execution tests FAIL on SPARC
2023-12-08 10:28 [Bug middle-end/112917] New: Most strub execution tests FAIL on SPARC ro at gcc dot gnu.org
` (2 preceding siblings ...)
2023-12-13 5:59 ` aoliva at gcc dot gnu.org
@ 2023-12-13 9:11 ` ro at CeBiTec dot Uni-Bielefeld.DE
2023-12-13 11:32 ` aoliva at gcc dot gnu.org
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: ro at CeBiTec dot Uni-Bielefeld.DE @ 2023-12-13 9:11 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/112917] Most strub execution tests FAIL on SPARC
2023-12-08 10:28 [Bug middle-end/112917] New: Most strub execution tests FAIL on SPARC ro at gcc dot gnu.org
` (3 preceding siblings ...)
2023-12-13 9:11 ` ro at CeBiTec dot Uni-Bielefeld.DE
@ 2023-12-13 11:32 ` aoliva at gcc dot gnu.org
2023-12-20 8:19 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: aoliva at gcc dot gnu.org @ 2023-12-13 11:32 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/112917] Most strub execution tests FAIL on SPARC
2023-12-08 10:28 [Bug middle-end/112917] New: Most strub execution tests FAIL on SPARC ro at gcc dot gnu.org
` (4 preceding siblings ...)
2023-12-13 11:32 ` aoliva at gcc dot gnu.org
@ 2023-12-20 8:19 ` cvs-commit at gcc dot gnu.org
2023-12-20 8:19 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-12-20 8:19 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/112917] Most strub execution tests FAIL on SPARC
2023-12-08 10:28 [Bug middle-end/112917] New: Most strub execution tests FAIL on SPARC ro at gcc dot gnu.org
` (5 preceding siblings ...)
2023-12-20 8:19 ` cvs-commit at gcc dot gnu.org
@ 2023-12-20 8:19 ` cvs-commit at gcc dot gnu.org
2023-12-21 3:49 ` aoliva at gcc dot gnu.org
2024-01-31 3:24 ` cvs-commit at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-12-20 8:19 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/112917] Most strub execution tests FAIL on SPARC
2023-12-08 10:28 [Bug middle-end/112917] New: Most strub execution tests FAIL on SPARC ro at gcc dot gnu.org
` (6 preceding siblings ...)
2023-12-20 8:19 ` cvs-commit at gcc dot gnu.org
@ 2023-12-21 3:49 ` aoliva at gcc dot gnu.org
2024-01-31 3:24 ` cvs-commit at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: aoliva at gcc dot gnu.org @ 2023-12-21 3:49 UTC (permalink / raw)
To: gcc-bugs
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
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug middle-end/112917] Most strub execution tests FAIL on SPARC
2023-12-08 10:28 [Bug middle-end/112917] New: Most strub execution tests FAIL on SPARC ro at gcc dot gnu.org
` (7 preceding siblings ...)
2023-12-21 3:49 ` aoliva at gcc dot gnu.org
@ 2024-01-31 3:24 ` cvs-commit at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-01-31 3:24 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 10+ messages in thread