Skip to content

[ARM] Honour -mno-movt in stack protector handling #109022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

ardbiesheuvel
Copy link
Contributor

When -mno-movt is passed to Clang, the ARM codegen correctly avoids movt/movw pairs to take the address of __stack_chk_guard in the stack protector code emitted into the function pro- and epilogues. However, the Thumb2 codegen fails to do so, and happily emits movw/movt pairs unless it is generating an ELF binary and the symbol might be in a different DSO. Let's incorporate a check for useMovt() in the logic here, so movt/movw are never emitted when -mno-movt is specified.

Suggestions welcome for how/where to add a test case for this.

@llvmbot
Copy link
Member

llvmbot commented Sep 17, 2024

@llvm/pr-subscribers-backend-arm

Author: Ard Biesheuvel (ardbiesheuvel)

Changes

When -mno-movt is passed to Clang, the ARM codegen correctly avoids movt/movw pairs to take the address of __stack_chk_guard in the stack protector code emitted into the function pro- and epilogues. However, the Thumb2 codegen fails to do so, and happily emits movw/movt pairs unless it is generating an ELF binary and the symbol might be in a different DSO. Let's incorporate a check for useMovt() in the logic here, so movt/movw are never emitted when -mno-movt is specified.

Suggestions welcome for how/where to add a test case for this.


Full diff: https://github.com/llvm/llvm-project/pull/109022.diff

1 Files Affected:

  • (modified) llvm/lib/Target/ARM/Thumb2InstrInfo.cpp (+2-1)
diff --git a/llvm/lib/Target/ARM/Thumb2InstrInfo.cpp b/llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
index d1e07b6703a5e6..ce6bc0681a5028 100644
--- a/llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
+++ b/llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
@@ -264,7 +264,8 @@ void Thumb2InstrInfo::expandLoadStackGuard(
   }
 
   const auto *GV = cast<GlobalValue>((*MI->memoperands_begin())->getValue());
-  if (MF.getSubtarget<ARMSubtarget>().isTargetELF() && !GV->isDSOLocal())
+  const ARMSubtarget &Subtarget = MF.getSubtarget<ARMSubtarget>();
+  if ((Subtarget.isTargetELF() && !GV->isDSOLocal()) || !Subtarget.useMovt())
     expandLoadStackGuardBase(MI, ARM::t2LDRLIT_ga_pcrel, ARM::t2LDRi12);
   else if (MF.getTarget().isPositionIndependent())
     expandLoadStackGuardBase(MI, ARM::t2MOV_ga_pcrel, ARM::t2LDRi12);

@MaskRay
Copy link
Member

MaskRay commented Sep 17, 2024

I refactored some AArch32 stack protector tests in 6ae7b73 and #70014 is the last change that modifies AArch32 stack protector. Add a test to one of the test files and utilize llc -mattr=+no-movt?

@ardbiesheuvel ardbiesheuvel force-pushed the arm-avoid-t2-movt-for-ssp branch from 96bcc47 to 537c04b Compare September 18, 2024 09:30
When -mno-movt is passed to Clang, the ARM codegen correctly avoids
movt/movw pairs for taking the address of __stack_chk_guard in the stack
protector code emitted into the function pro- and epilogues.

However, the Thumb2 codegen fails to do so, and happily emits movw/movt
pairs in the general case (i.e., unless it is generating an ELF binary
and the symbol might be in a different DSO).

Fix this by explicitly using a literal load when useMovt() yields FALSE.

Signed-off-by: Ard Biesheuvel <[email protected]>
@ardbiesheuvel ardbiesheuvel force-pushed the arm-avoid-t2-movt-for-ssp branch from 537c04b to 669fa6a Compare October 8, 2024 06:37
@kees kees merged commit 2e47b93 into llvm:main Oct 9, 2024
9 checks passed
@ardbiesheuvel ardbiesheuvel deleted the arm-avoid-t2-movt-for-ssp branch October 9, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants