-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[AArch64] Null check TargetStreamer before emitting .variant_pcs #138924
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
base: main
Are you sure you want to change the base?
Conversation
This was causing crashes on Mach-O targets as we don't construct a TargetStreamer for that object format. Other uses of the TargetStreamer (TS) appear to be limited to ELF and COFF platforms (where it is none null, so don't need changing).
@llvm/pr-subscribers-backend-aarch64 Author: Benjamin Maxwell (MacDue) ChangesThis was causing crashes on Mach-O targets as we don't construct a TargetStreamer for that object format. Other uses of the TargetStreamer (TS) appear to be limited to ELF and COFF platforms (where it is none null, so don't need changing). Full diff: https://github.com/llvm/llvm-project/pull/138924.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index 870df4c387ca4..1166a78bf1088 100644
--- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -1372,7 +1372,8 @@ void AArch64AsmPrinter::emitFunctionEntryLabel() {
MF->getInfo<AArch64FunctionInfo>()->isSVECC()) {
auto *TS =
static_cast<AArch64TargetStreamer *>(OutStreamer->getTargetStreamer());
- TS->emitDirectiveVariantPCS(CurrentFnSym);
+ if (TS)
+ TS->emitDirectiveVariantPCS(CurrentFnSym);
}
AsmPrinter::emitFunctionEntryLabel();
diff --git a/llvm/test/CodeGen/AArch64/variant-pcs.ll b/llvm/test/CodeGen/AArch64/variant-pcs.ll
index 49c504177358e..0c995b5b0e8ef 100644
--- a/llvm/test/CodeGen/AArch64/variant-pcs.ll
+++ b/llvm/test/CodeGen/AArch64/variant-pcs.ll
@@ -2,6 +2,9 @@
; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve -filetype=obj -o - %s \
; RUN: | llvm-readobj --symbols - | FileCheck %s --check-prefix=CHECK-OBJ
+; Check we don't crash when using a Mach-O object format.
+; RUN: llc -mtriple=arm64-apple-macosx15.0.0 -mattr=+sve -filetype=obj -o /dev/null %s
+
define i32 @base_pcs() {
; CHECK-ASM-LABEL: base_pcs:
; CHECK-ASM-NOT: .variant_pcs
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
auto *TS = | ||
static_cast<AArch64TargetStreamer *>(OutStreamer->getTargetStreamer()); | ||
TS->emitDirectiveVariantPCS(CurrentFnSym); | ||
if (auto *TS = static_cast<AArch64TargetStreamer *>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this again, I think we should only emit the variant PCS attribute when the target's triple is ELF (TM.getTargetTriple().isOSBinFormatELF()
) because this is specifically an ELF attribute. This means you can remove this condition, because for ELF targets it should never return a nullptr.
Curious: Why is it that the Mach-O format does not get a |
This was causing crashes on Mach-O targets as we don't construct a TargetStreamer for that object format. Other uses of the TargetStreamer (TS) appear to be limited to ELF and COFF platforms (where it is none null, so don't need changing).