public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/102926] New: TLS register value is spilled to the stack instead of reloaded from the system register
@ 2021-10-25 11:35 ardb at kernel dot org
2021-10-25 11:50 ` [Bug target/102926] " pinskia at gcc dot gnu.org
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: ardb at kernel dot org @ 2021-10-25 11:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102926
Bug ID: 102926
Summary: TLS register value is spilled to the stack instead of
reloaded from the system register
Product: gcc
Version: 12.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: ardb at kernel dot org
Target Milestone: ---
Target: ARM
The code below uses the hardware TLS register on ARM, and instead of reloading
its value directly from the system register, it spills its value to the stack.
This is suboptimal, and given that the TLS register is being proposed as an
alternative reference for the stack protector canary, it is also a security
concern, as an attacker that controls the stack may be able to control both
sides of the equation in the stack protector check occurring at the end of the
function.
Instead, I would expect any subsequent uses of the thread pointer to simply
issue the MRC again, which doesn't touch memory.
$ cat /tmp/spill.c
int foo(int);
int bar(void)
{
int *l = __builtin_thread_pointer();
return foo(l[0]) + l[1];
}
$ arm-linux-gnueabihf-gcc -o - -S /tmp/spill.c -O3 -mtp=cp15 -ffixed-r4
-ffixed-r5 -ffixed-r6 -ffixed-r7 -ffixed-r8 -ffixed-r9 -ffixed-r10
-fno-omit-frame-pointer
.cpu arm10tdmi
.arch armv5t
.fpu softvfp
.eabi_attribute 20, 1
.eabi_attribute 21, 1
.eabi_attribute 23, 3
.eabi_attribute 24, 1
.eabi_attribute 25, 1
.eabi_attribute 26, 2
.eabi_attribute 30, 2
.eabi_attribute 34, 0
.eabi_attribute 18, 4
.file "spill.c"
.text
.align 2
.global bar
.syntax unified
.arm
.type bar, %function
bar:
@ args = 0, pretend = 0, frame = 8
@ frame_needed = 1, uses_anonymous_args = 0
push {fp, lr}
add fp, sp, #4
sub sp, sp, #8
mrc p15, 0, r3, c13, c0, 3 @ load_tp_hard
ldr r0, [r3]
str r3, [fp, #-8]
bl foo
ldr r3, [fp, #-8]
ldr r3, [r3, #4]
add r0, r0, r3
sub sp, fp, #4
@ sp needed
pop {fp, pc}
.size bar, .-bar
.ident "GCC: (GNU) 12.0.0 20211024 (experimental)"
.section .note.GNU-stack,"",%progbits
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/102926] TLS register value is spilled to the stack instead of reloaded from the system register
2021-10-25 11:35 [Bug target/102926] New: TLS register value is spilled to the stack instead of reloaded from the system register ardb at kernel dot org
@ 2021-10-25 11:50 ` pinskia at gcc dot gnu.org
2021-10-25 12:02 ` rguenth at gcc dot gnu.org
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-10-25 11:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102926
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |missed-optimization
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I don't think it is a security risk really because all spills then can be
considered security risks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/102926] TLS register value is spilled to the stack instead of reloaded from the system register
2021-10-25 11:35 [Bug target/102926] New: TLS register value is spilled to the stack instead of reloaded from the system register ardb at kernel dot org
2021-10-25 11:50 ` [Bug target/102926] " pinskia at gcc dot gnu.org
@ 2021-10-25 12:02 ` rguenth at gcc dot gnu.org
2021-10-25 12:03 ` ardb at kernel dot org
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-10-25 12:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102926
--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I would also guess that load_tp_hard is slower and the large mnemonic suggests
a larger instruction (ok, but we're risc and thus fixed size instructions?)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/102926] TLS register value is spilled to the stack instead of reloaded from the system register
2021-10-25 11:35 [Bug target/102926] New: TLS register value is spilled to the stack instead of reloaded from the system register ardb at kernel dot org
2021-10-25 11:50 ` [Bug target/102926] " pinskia at gcc dot gnu.org
2021-10-25 12:02 ` rguenth at gcc dot gnu.org
@ 2021-10-25 12:03 ` ardb at kernel dot org
2021-10-25 12:12 ` pinskia at gcc dot gnu.org
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: ardb at kernel dot org @ 2021-10-25 12:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102926
--- Comment #3 from Ard Biesheuvel <ardb at kernel dot org> ---
(In reply to Andrew Pinski from comment #1)
> I don't think it is a security risk really because all spills then can be
> considered security risks.
I would argue that there is no consensus on that. In fact, there is elaborate
handling of the stack canary in the ARM backend, not only to prevent spilling
of the stack canary address or value, but even to ensure that the value is
cleared from all registers that carried it while performing the test or the
set.
In any case, regardless of whether this is a security issue or not (when/if my
TLS stack protector changes get accepted), spilling the TLS register value is
definitely unnecessary.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/102926] TLS register value is spilled to the stack instead of reloaded from the system register
2021-10-25 11:35 [Bug target/102926] New: TLS register value is spilled to the stack instead of reloaded from the system register ardb at kernel dot org
` (2 preceding siblings ...)
2021-10-25 12:03 ` ardb at kernel dot org
@ 2021-10-25 12:12 ` pinskia at gcc dot gnu.org
2021-10-25 14:46 ` ardb at kernel dot org
2021-10-25 15:24 ` rsandifo at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-10-25 12:12 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102926
--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
If anything it is a cost model issue in the back-end.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/102926] TLS register value is spilled to the stack instead of reloaded from the system register
2021-10-25 11:35 [Bug target/102926] New: TLS register value is spilled to the stack instead of reloaded from the system register ardb at kernel dot org
` (3 preceding siblings ...)
2021-10-25 12:12 ` pinskia at gcc dot gnu.org
@ 2021-10-25 14:46 ` ardb at kernel dot org
2021-10-25 15:24 ` rsandifo at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: ardb at kernel dot org @ 2021-10-25 14:46 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102926
--- Comment #5 from Ard Biesheuvel <ardb at kernel dot org> ---
Actually, I can reproduce the same behavior without the TLS register, where the
result of a MOVW/MOVT immediate assignment is also spilled to the stack.
int ll[2];
int foo(int);
int bar(void)
{
int *l = ll;
return foo(l[0]) + l[1];
}
will produce this for armv7-a (with [r4-r11] fixed)
push {fp, lr}
movw r3, #:lower16:.LANCHOR0
movt r3, #:upper16:.LANCHOR0
add fp, sp, #4
sub sp, sp, #8
ldr r0, [r3]
str r3, [fp, #-8]
bl foo
ldr r3, [fp, #-8]
ldr r3, [r3, #4]
add r0, r0, r3
sub sp, fp, #4
@ sp needed
pop {fp, pc}
whereas pre-v7 gives me
ldr r3, .L4
push {fp, lr}
add fp, sp, #4
ldr r0, [r3]
bl foo
ldr r3, .L4
ldr r3, [r3, #4]
add r0, r0, r3
pop {fp, pc}
i.e., the address is simply reloaded from the literal pool rather than spilled.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/102926] TLS register value is spilled to the stack instead of reloaded from the system register
2021-10-25 11:35 [Bug target/102926] New: TLS register value is spilled to the stack instead of reloaded from the system register ardb at kernel dot org
` (4 preceding siblings ...)
2021-10-25 14:46 ` ardb at kernel dot org
@ 2021-10-25 15:24 ` rsandifo at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: rsandifo at gcc dot gnu.org @ 2021-10-25 15:24 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102926
rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Ever confirmed|0 |1
Status|UNCONFIRMED |NEW
Last reconfirmed| |2021-10-25
CC| |rsandifo at gcc dot gnu.org
--- Comment #6 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
I guess the first step would be to wrap the rhs of:
(define_insn "load_tp_hard"
[(set (match_operand:SI 0 "register_operand" "=r")
(unspec:SI [(const_int 0)] UNSPEC_TLS))]
"TARGET_HARD_TP"
"mrc%?\\tp15, 0, %0, c13, c0, 3\\t@ load_tp_hard"
[(set_attr "predicable" "yes")
(set_attr "type" "mrs")]
)
in (const …), so that it can be rematerialised.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-25 15:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-25 11:35 [Bug target/102926] New: TLS register value is spilled to the stack instead of reloaded from the system register ardb at kernel dot org
2021-10-25 11:50 ` [Bug target/102926] " pinskia at gcc dot gnu.org
2021-10-25 12:02 ` rguenth at gcc dot gnu.org
2021-10-25 12:03 ` ardb at kernel dot org
2021-10-25 12:12 ` pinskia at gcc dot gnu.org
2021-10-25 14:46 ` ardb at kernel dot org
2021-10-25 15:24 ` rsandifo at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).