-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[win][arm64ec] Handle empty function names #151609
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
Conversation
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-backend-aarch64 Author: Daniel Paoliello (dpaoliello) ChangesWhile testing Arm64EC, I observed that LLVM crashes when an empty function name is used. My original fix in #151409 was to raise an error, but this change now handles the empty name via To get this working, I had to create the Full diff: https://github.com/llvm/llvm-project/pull/151609.diff 7 Files Affected:
diff --git a/llvm/include/llvm/Target/TargetLoweringObjectFile.h b/llvm/include/llvm/Target/TargetLoweringObjectFile.h
index 397239b1685fb..9eebe26c45288 100644
--- a/llvm/include/llvm/Target/TargetLoweringObjectFile.h
+++ b/llvm/include/llvm/Target/TargetLoweringObjectFile.h
@@ -14,10 +14,12 @@
#ifndef LLVM_TARGET_TARGETLOWERINGOBJECTFILE_H
#define LLVM_TARGET_TARGETLOWERINGOBJECTFILE_H
+#include "llvm/IR/Mangler.h"
#include "llvm/MC/MCObjectFileInfo.h"
#include "llvm/MC/MCRegister.h"
#include "llvm/Support/Compiler.h"
#include <cstdint>
+#include <memory>
namespace llvm {
@@ -30,7 +32,6 @@ class GlobalObject;
class GlobalValue;
class MachineBasicBlock;
class MachineModuleInfo;
-class Mangler;
class MCContext;
class MCExpr;
class MCSection;
@@ -46,7 +47,7 @@ class DSOLocalEquivalent;
class LLVM_ABI TargetLoweringObjectFile : public MCObjectFileInfo {
/// Name-mangler for global names.
- Mangler *Mang = nullptr;
+ std::unique_ptr<Mangler> Mang = std::make_unique<Mangler>();
protected:
bool SupportIndirectSymViaGOTPCRel = false;
@@ -74,7 +75,7 @@ class LLVM_ABI TargetLoweringObjectFile : public MCObjectFileInfo {
TargetLoweringObjectFile(const TargetLoweringObjectFile &) = delete;
TargetLoweringObjectFile &
operator=(const TargetLoweringObjectFile &) = delete;
- virtual ~TargetLoweringObjectFile();
+ virtual ~TargetLoweringObjectFile() = default;
Mangler &getMangler() const { return *Mang; }
diff --git a/llvm/lib/IR/Mangler.cpp b/llvm/lib/IR/Mangler.cpp
index 010bd15e256dc..3931164750667 100644
--- a/llvm/lib/IR/Mangler.cpp
+++ b/llvm/lib/IR/Mangler.cpp
@@ -292,6 +292,8 @@ void llvm::emitLinkerFlagsForUsedCOFF(raw_ostream &OS, const GlobalValue *GV,
}
std::optional<std::string> llvm::getArm64ECMangledFunctionName(StringRef Name) {
+ assert(!Name.empty() &&
+ "getArm64ECMangledFunctionName requires non-empty name");
if (Name[0] != '?') {
// For non-C++ symbols, prefix the name with "#" unless it's already
// mangled.
diff --git a/llvm/lib/Target/AArch64/AArch64.h b/llvm/lib/Target/AArch64/AArch64.h
index 5496ebd495a55..07e373c351b6e 100644
--- a/llvm/lib/Target/AArch64/AArch64.h
+++ b/llvm/lib/Target/AArch64/AArch64.h
@@ -72,7 +72,8 @@ FunctionPass *createAArch64PostLegalizerLowering();
FunctionPass *createAArch64PostSelectOptimize();
FunctionPass *createAArch64StackTaggingPass(bool IsOptNone);
FunctionPass *createAArch64StackTaggingPreRAPass();
-ModulePass *createAArch64Arm64ECCallLoweringPass();
+ModulePass *createAArch64Arm64ECCallLoweringPass(const TargetMachine &,
+ Mangler &);
void initializeAArch64A53Fix835769Pass(PassRegistry&);
void initializeAArch64A57FPLoadBalancingPass(PassRegistry&);
diff --git a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
index 509cbb092705d..57c71595c06b4 100644
--- a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
@@ -62,7 +62,8 @@ struct ThunkArgInfo {
class AArch64Arm64ECCallLowering : public ModulePass {
public:
static char ID;
- AArch64Arm64ECCallLowering() : ModulePass(ID) {}
+ AArch64Arm64ECCallLowering(const TargetMachine &TM, Mangler &Mang)
+ : ModulePass(ID), TM(TM), Mangler(Mang) {}
Function *buildExitThunk(FunctionType *FnTy, AttributeList Attrs);
Function *buildEntryThunk(Function *F);
@@ -82,6 +83,8 @@ class AArch64Arm64ECCallLowering : public ModulePass {
Constant *GuardFnGlobal = nullptr;
Constant *DispatchFnGlobal = nullptr;
Module *M = nullptr;
+ Mangler &Mangler;
+ const TargetMachine &TM;
Type *PtrTy;
Type *I64Ty;
@@ -608,7 +611,11 @@ Function *AArch64Arm64ECCallLowering::buildGuestExitThunk(Function *F) {
getThunkType(F->getFunctionType(), F->getAttributes(),
Arm64ECThunkType::GuestExit, NullThunkName, Arm64Ty, X64Ty,
ArgTranslations);
- auto MangledName = getArm64ECMangledFunctionName(F->getName().str());
+ SmallVector<char> FunctionNameStore;
+ TM.getNameWithPrefix(FunctionNameStore, F, Mangler);
+ auto FunctionName =
+ StringRef(FunctionNameStore.data(), FunctionNameStore.size());
+ auto MangledName = getArm64ECMangledFunctionName(FunctionName);
assert(MangledName && "Can't guest exit to function that's already native");
std::string ThunkName = *MangledName;
if (ThunkName[0] == '?' && ThunkName.find("@") != std::string::npos) {
@@ -623,7 +630,7 @@ Function *AArch64Arm64ECCallLowering::buildGuestExitThunk(Function *F) {
GuestExit->addMetadata(
"arm64ec_unmangled_name",
*MDNode::get(M->getContext(),
- MDString::get(M->getContext(), F->getName())));
+ MDString::get(M->getContext(), FunctionName)));
GuestExit->setMetadata(
"arm64ec_ecmangled_name",
MDNode::get(M->getContext(),
@@ -820,9 +827,12 @@ bool AArch64Arm64ECCallLowering::runOnModule(Module &Mod) {
// Rename hybrid patchable functions and change callers to use a global
// alias instead.
+ SmallVector<char> FunctionNameStore;
+ TM.getNameWithPrefix(FunctionNameStore, &F, Mangler);
+ auto FunctionName =
+ StringRef(FunctionNameStore.data(), FunctionNameStore.size());
if (std::optional<std::string> MangledName =
- getArm64ECMangledFunctionName(F.getName().str())) {
- std::string OrigName(F.getName());
+ getArm64ECMangledFunctionName(FunctionName)) {
F.setName(MangledName.value() + HybridPatchableTargetSuffix);
// The unmangled symbol is a weak alias to an undefined symbol with the
@@ -831,8 +841,8 @@ bool AArch64Arm64ECCallLowering::runOnModule(Module &Mod) {
// can't represent that in IR, we create an alias to the target instead.
// The "EXP+" symbol is set as metadata, which is then used by
// emitGlobalAlias to emit the right alias.
- auto *A =
- GlobalAlias::create(GlobalValue::LinkOnceODRLinkage, OrigName, &F);
+ auto *A = GlobalAlias::create(GlobalValue::LinkOnceODRLinkage,
+ FunctionName, &F);
auto *AM = GlobalAlias::create(GlobalValue::LinkOnceODRLinkage,
MangledName.value(), &F);
F.replaceUsesWithIf(AM,
@@ -925,11 +935,15 @@ bool AArch64Arm64ECCallLowering::processFunction(
//
// FIXME: Handle functions with weak linkage?
if (!F.hasLocalLinkage() || F.hasAddressTaken()) {
+ SmallVector<char> FunctionNameStore;
+ TM.getNameWithPrefix(FunctionNameStore, &F, Mangler);
+ auto FunctionName =
+ StringRef(FunctionNameStore.data(), FunctionNameStore.size());
if (std::optional<std::string> MangledName =
- getArm64ECMangledFunctionName(F.getName().str())) {
+ getArm64ECMangledFunctionName(FunctionName)) {
F.addMetadata("arm64ec_unmangled_name",
*MDNode::get(M->getContext(),
- MDString::get(M->getContext(), F.getName())));
+ MDString::get(M->getContext(), FunctionName)));
if (F.hasComdat() && F.getComdat()->getName() == F.getName()) {
Comdat *MangledComdat = M->getOrInsertComdat(MangledName.value());
SmallVector<GlobalObject *> ComdatUsers =
@@ -994,6 +1008,7 @@ char AArch64Arm64ECCallLowering::ID = 0;
INITIALIZE_PASS(AArch64Arm64ECCallLowering, "Arm64ECCallLowering",
"AArch64Arm64ECCallLowering", false, false)
-ModulePass *llvm::createAArch64Arm64ECCallLoweringPass() {
- return new AArch64Arm64ECCallLowering;
+ModulePass *llvm::createAArch64Arm64ECCallLoweringPass(const TargetMachine &TM,
+ Mangler &Mang) {
+ return new AArch64Arm64ECCallLowering(TM, Mang);
}
diff --git a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
index 95eab16511e5a..4ceba1ad8b78a 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
@@ -676,7 +676,8 @@ void AArch64PassConfig::addIRPasses() {
// Add Control Flow Guard checks.
if (TM->getTargetTriple().isOSWindows()) {
if (TM->getTargetTriple().isWindowsArm64EC())
- addPass(createAArch64Arm64ECCallLoweringPass());
+ addPass(createAArch64Arm64ECCallLoweringPass(
+ *TM, TM->getObjFileLowering()->getMangler()));
else
addPass(createCFGuardCheckPass());
}
diff --git a/llvm/lib/Target/TargetLoweringObjectFile.cpp b/llvm/lib/Target/TargetLoweringObjectFile.cpp
index 9b03e85ca45bf..bbe9e76ee0072 100644
--- a/llvm/lib/Target/TargetLoweringObjectFile.cpp
+++ b/llvm/lib/Target/TargetLoweringObjectFile.cpp
@@ -40,8 +40,6 @@ using namespace llvm;
void TargetLoweringObjectFile::Initialize(MCContext &ctx,
const TargetMachine &TM) {
// `Initialize` can be called more than once.
- delete Mang;
- Mang = new Mangler();
initMCObjectFileInfo(ctx, TM.isPositionIndependent(),
TM.getCodeModel() == CodeModel::Large);
@@ -52,10 +50,6 @@ void TargetLoweringObjectFile::Initialize(MCContext &ctx,
this->TM = &TM;
}
-TargetLoweringObjectFile::~TargetLoweringObjectFile() {
- delete Mang;
-}
-
unsigned TargetLoweringObjectFile::getCallSiteEncoding() const {
// If target does not have LEB128 directives, we would need the
// call site encoding to be udata4 so that the alternative path
diff --git a/llvm/test/CodeGen/AArch64/arm64ec-empty-name.ll b/llvm/test/CodeGen/AArch64/arm64ec-empty-name.ll
new file mode 100644
index 0000000000000..0882eade0c831
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/arm64ec-empty-name.ll
@@ -0,0 +1,10 @@
+; RUN: llc -mtriple=arm64ec-pc-windows-msvc %s -o - | FileCheck %s
+
+; Regression test: Arm64EC needs to look at the first character of a function
+; to decide if it will be mangled like a C or C++ function name, which caused
+; it to crash for empty function names.
+define void @""() {
+ ret void
+}
+
+; CHECK: "#__unnamed_1":
|
@llvm/pr-subscribers-platform-windows Author: Daniel Paoliello (dpaoliello) ChangesWhile testing Arm64EC, I observed that LLVM crashes when an empty function name is used. My original fix in #151409 was to raise an error, but this change now handles the empty name via To get this working, I had to create the Full diff: https://github.com/llvm/llvm-project/pull/151609.diff 7 Files Affected:
diff --git a/llvm/include/llvm/Target/TargetLoweringObjectFile.h b/llvm/include/llvm/Target/TargetLoweringObjectFile.h
index 397239b1685fb..9eebe26c45288 100644
--- a/llvm/include/llvm/Target/TargetLoweringObjectFile.h
+++ b/llvm/include/llvm/Target/TargetLoweringObjectFile.h
@@ -14,10 +14,12 @@
#ifndef LLVM_TARGET_TARGETLOWERINGOBJECTFILE_H
#define LLVM_TARGET_TARGETLOWERINGOBJECTFILE_H
+#include "llvm/IR/Mangler.h"
#include "llvm/MC/MCObjectFileInfo.h"
#include "llvm/MC/MCRegister.h"
#include "llvm/Support/Compiler.h"
#include <cstdint>
+#include <memory>
namespace llvm {
@@ -30,7 +32,6 @@ class GlobalObject;
class GlobalValue;
class MachineBasicBlock;
class MachineModuleInfo;
-class Mangler;
class MCContext;
class MCExpr;
class MCSection;
@@ -46,7 +47,7 @@ class DSOLocalEquivalent;
class LLVM_ABI TargetLoweringObjectFile : public MCObjectFileInfo {
/// Name-mangler for global names.
- Mangler *Mang = nullptr;
+ std::unique_ptr<Mangler> Mang = std::make_unique<Mangler>();
protected:
bool SupportIndirectSymViaGOTPCRel = false;
@@ -74,7 +75,7 @@ class LLVM_ABI TargetLoweringObjectFile : public MCObjectFileInfo {
TargetLoweringObjectFile(const TargetLoweringObjectFile &) = delete;
TargetLoweringObjectFile &
operator=(const TargetLoweringObjectFile &) = delete;
- virtual ~TargetLoweringObjectFile();
+ virtual ~TargetLoweringObjectFile() = default;
Mangler &getMangler() const { return *Mang; }
diff --git a/llvm/lib/IR/Mangler.cpp b/llvm/lib/IR/Mangler.cpp
index 010bd15e256dc..3931164750667 100644
--- a/llvm/lib/IR/Mangler.cpp
+++ b/llvm/lib/IR/Mangler.cpp
@@ -292,6 +292,8 @@ void llvm::emitLinkerFlagsForUsedCOFF(raw_ostream &OS, const GlobalValue *GV,
}
std::optional<std::string> llvm::getArm64ECMangledFunctionName(StringRef Name) {
+ assert(!Name.empty() &&
+ "getArm64ECMangledFunctionName requires non-empty name");
if (Name[0] != '?') {
// For non-C++ symbols, prefix the name with "#" unless it's already
// mangled.
diff --git a/llvm/lib/Target/AArch64/AArch64.h b/llvm/lib/Target/AArch64/AArch64.h
index 5496ebd495a55..07e373c351b6e 100644
--- a/llvm/lib/Target/AArch64/AArch64.h
+++ b/llvm/lib/Target/AArch64/AArch64.h
@@ -72,7 +72,8 @@ FunctionPass *createAArch64PostLegalizerLowering();
FunctionPass *createAArch64PostSelectOptimize();
FunctionPass *createAArch64StackTaggingPass(bool IsOptNone);
FunctionPass *createAArch64StackTaggingPreRAPass();
-ModulePass *createAArch64Arm64ECCallLoweringPass();
+ModulePass *createAArch64Arm64ECCallLoweringPass(const TargetMachine &,
+ Mangler &);
void initializeAArch64A53Fix835769Pass(PassRegistry&);
void initializeAArch64A57FPLoadBalancingPass(PassRegistry&);
diff --git a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
index 509cbb092705d..57c71595c06b4 100644
--- a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
@@ -62,7 +62,8 @@ struct ThunkArgInfo {
class AArch64Arm64ECCallLowering : public ModulePass {
public:
static char ID;
- AArch64Arm64ECCallLowering() : ModulePass(ID) {}
+ AArch64Arm64ECCallLowering(const TargetMachine &TM, Mangler &Mang)
+ : ModulePass(ID), TM(TM), Mangler(Mang) {}
Function *buildExitThunk(FunctionType *FnTy, AttributeList Attrs);
Function *buildEntryThunk(Function *F);
@@ -82,6 +83,8 @@ class AArch64Arm64ECCallLowering : public ModulePass {
Constant *GuardFnGlobal = nullptr;
Constant *DispatchFnGlobal = nullptr;
Module *M = nullptr;
+ Mangler &Mangler;
+ const TargetMachine &TM;
Type *PtrTy;
Type *I64Ty;
@@ -608,7 +611,11 @@ Function *AArch64Arm64ECCallLowering::buildGuestExitThunk(Function *F) {
getThunkType(F->getFunctionType(), F->getAttributes(),
Arm64ECThunkType::GuestExit, NullThunkName, Arm64Ty, X64Ty,
ArgTranslations);
- auto MangledName = getArm64ECMangledFunctionName(F->getName().str());
+ SmallVector<char> FunctionNameStore;
+ TM.getNameWithPrefix(FunctionNameStore, F, Mangler);
+ auto FunctionName =
+ StringRef(FunctionNameStore.data(), FunctionNameStore.size());
+ auto MangledName = getArm64ECMangledFunctionName(FunctionName);
assert(MangledName && "Can't guest exit to function that's already native");
std::string ThunkName = *MangledName;
if (ThunkName[0] == '?' && ThunkName.find("@") != std::string::npos) {
@@ -623,7 +630,7 @@ Function *AArch64Arm64ECCallLowering::buildGuestExitThunk(Function *F) {
GuestExit->addMetadata(
"arm64ec_unmangled_name",
*MDNode::get(M->getContext(),
- MDString::get(M->getContext(), F->getName())));
+ MDString::get(M->getContext(), FunctionName)));
GuestExit->setMetadata(
"arm64ec_ecmangled_name",
MDNode::get(M->getContext(),
@@ -820,9 +827,12 @@ bool AArch64Arm64ECCallLowering::runOnModule(Module &Mod) {
// Rename hybrid patchable functions and change callers to use a global
// alias instead.
+ SmallVector<char> FunctionNameStore;
+ TM.getNameWithPrefix(FunctionNameStore, &F, Mangler);
+ auto FunctionName =
+ StringRef(FunctionNameStore.data(), FunctionNameStore.size());
if (std::optional<std::string> MangledName =
- getArm64ECMangledFunctionName(F.getName().str())) {
- std::string OrigName(F.getName());
+ getArm64ECMangledFunctionName(FunctionName)) {
F.setName(MangledName.value() + HybridPatchableTargetSuffix);
// The unmangled symbol is a weak alias to an undefined symbol with the
@@ -831,8 +841,8 @@ bool AArch64Arm64ECCallLowering::runOnModule(Module &Mod) {
// can't represent that in IR, we create an alias to the target instead.
// The "EXP+" symbol is set as metadata, which is then used by
// emitGlobalAlias to emit the right alias.
- auto *A =
- GlobalAlias::create(GlobalValue::LinkOnceODRLinkage, OrigName, &F);
+ auto *A = GlobalAlias::create(GlobalValue::LinkOnceODRLinkage,
+ FunctionName, &F);
auto *AM = GlobalAlias::create(GlobalValue::LinkOnceODRLinkage,
MangledName.value(), &F);
F.replaceUsesWithIf(AM,
@@ -925,11 +935,15 @@ bool AArch64Arm64ECCallLowering::processFunction(
//
// FIXME: Handle functions with weak linkage?
if (!F.hasLocalLinkage() || F.hasAddressTaken()) {
+ SmallVector<char> FunctionNameStore;
+ TM.getNameWithPrefix(FunctionNameStore, &F, Mangler);
+ auto FunctionName =
+ StringRef(FunctionNameStore.data(), FunctionNameStore.size());
if (std::optional<std::string> MangledName =
- getArm64ECMangledFunctionName(F.getName().str())) {
+ getArm64ECMangledFunctionName(FunctionName)) {
F.addMetadata("arm64ec_unmangled_name",
*MDNode::get(M->getContext(),
- MDString::get(M->getContext(), F.getName())));
+ MDString::get(M->getContext(), FunctionName)));
if (F.hasComdat() && F.getComdat()->getName() == F.getName()) {
Comdat *MangledComdat = M->getOrInsertComdat(MangledName.value());
SmallVector<GlobalObject *> ComdatUsers =
@@ -994,6 +1008,7 @@ char AArch64Arm64ECCallLowering::ID = 0;
INITIALIZE_PASS(AArch64Arm64ECCallLowering, "Arm64ECCallLowering",
"AArch64Arm64ECCallLowering", false, false)
-ModulePass *llvm::createAArch64Arm64ECCallLoweringPass() {
- return new AArch64Arm64ECCallLowering;
+ModulePass *llvm::createAArch64Arm64ECCallLoweringPass(const TargetMachine &TM,
+ Mangler &Mang) {
+ return new AArch64Arm64ECCallLowering(TM, Mang);
}
diff --git a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
index 95eab16511e5a..4ceba1ad8b78a 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
@@ -676,7 +676,8 @@ void AArch64PassConfig::addIRPasses() {
// Add Control Flow Guard checks.
if (TM->getTargetTriple().isOSWindows()) {
if (TM->getTargetTriple().isWindowsArm64EC())
- addPass(createAArch64Arm64ECCallLoweringPass());
+ addPass(createAArch64Arm64ECCallLoweringPass(
+ *TM, TM->getObjFileLowering()->getMangler()));
else
addPass(createCFGuardCheckPass());
}
diff --git a/llvm/lib/Target/TargetLoweringObjectFile.cpp b/llvm/lib/Target/TargetLoweringObjectFile.cpp
index 9b03e85ca45bf..bbe9e76ee0072 100644
--- a/llvm/lib/Target/TargetLoweringObjectFile.cpp
+++ b/llvm/lib/Target/TargetLoweringObjectFile.cpp
@@ -40,8 +40,6 @@ using namespace llvm;
void TargetLoweringObjectFile::Initialize(MCContext &ctx,
const TargetMachine &TM) {
// `Initialize` can be called more than once.
- delete Mang;
- Mang = new Mangler();
initMCObjectFileInfo(ctx, TM.isPositionIndependent(),
TM.getCodeModel() == CodeModel::Large);
@@ -52,10 +50,6 @@ void TargetLoweringObjectFile::Initialize(MCContext &ctx,
this->TM = &TM;
}
-TargetLoweringObjectFile::~TargetLoweringObjectFile() {
- delete Mang;
-}
-
unsigned TargetLoweringObjectFile::getCallSiteEncoding() const {
// If target does not have LEB128 directives, we would need the
// call site encoding to be udata4 so that the alternative path
diff --git a/llvm/test/CodeGen/AArch64/arm64ec-empty-name.ll b/llvm/test/CodeGen/AArch64/arm64ec-empty-name.ll
new file mode 100644
index 0000000000000..0882eade0c831
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/arm64ec-empty-name.ll
@@ -0,0 +1,10 @@
+; RUN: llc -mtriple=arm64ec-pc-windows-msvc %s -o - | FileCheck %s
+
+; Regression test: Arm64EC needs to look at the first character of a function
+; to decide if it will be mangled like a C or C++ function name, which caused
+; it to crash for empty function names.
+define void @""() {
+ ret void
+}
+
+; CHECK: "#__unnamed_1":
|
@@ -46,7 +47,7 @@ class DSOLocalEquivalent; | |||
|
|||
class LLVM_ABI TargetLoweringObjectFile : public MCObjectFileInfo { | |||
/// Name-mangler for global names. | |||
Mangler *Mang = nullptr; | |||
std::unique_ptr<Mangler> Mang = std::make_unique<Mangler>(); |
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.
The mangler has internal state: the AnonGlobalIDs map. Which means, if you modify any globals after the mangler is created, you invalidate the map, and end up creating a mess of globals with inconsistent names. I'm not sure how likely that is to happen in practice, but the less state we have in random maps, the better.
I think the simplest solution is to ignore the mangler, and just call F->setName("__unnamed");
in AArch64Arm64ECCallLowering. Which will automatically number the functions, and ensure there isn't any confusion about what name a function is supposed to have. It won't produce exactly the same names the mangler would, but I don't think that matters.
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.
It won't produce exactly the same names the mangler would, but I don't think that matters.
That was my worry, that we'd end up with the Arm64EC name and ordinary name being completely different. I suppose that doesn't really matter since the function is unnamed so there's no guarantee about naming in a different compilation so the anti-dependencies are moot?
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.
Right, you can't name an unnamed function from any other translation unit, so as long as the name is consistent within the translation unit, you won't have any issues.
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.
LGTM
While testing Arm64EC, I observed that LLVM crashes when an empty function name is used. My original fix in llvm#151409 was to raise an error, but this change now handles the empty name via `Mangler::getNameWithPrefix` (which assigns a name to the function). To get this working, I had to create the `Mangler` in `TargetLoweringObjectFile` early so it would be available to Arm64EC's lowering. There's no reason why `Mangler` is only created when `Initialize` is called (or re-created if it exists), and so I moved creation to the constructor and switched the raw pointer for a `unique_ptr` to avoid the explicit `delete` in the destructor.
While testing Arm64EC, I observed that LLVM crashes when an empty function name is used. My original fix in #151409 was to raise an error, but this change now handles the empty name via
Mangler::getNameWithPrefix
(which assigns a name to the function).To get this working, I had to create the
Mangler
inTargetLoweringObjectFile
early so it would be available to Arm64EC's lowering. There's no reason whyMangler
is only created whenInitialize
is called (or re-created if it exists), and so I moved creation to the constructor and switched the raw pointer for aunique_ptr
to avoid the explicitdelete
in the destructor.