Skip to content

Commit 717e753

Browse files
authored
[win][arm64ec] Handle empty function names (#151609)
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` 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.
1 parent 05b52ef commit 717e753

File tree

3 files changed

+31
-5
lines changed

3 files changed

+31
-5
lines changed

llvm/lib/IR/Mangler.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,9 @@ void llvm::emitLinkerFlagsForUsedCOFF(raw_ostream &OS, const GlobalValue *GV,
292292
}
293293

294294
std::optional<std::string> llvm::getArm64ECMangledFunctionName(StringRef Name) {
295+
assert(!Name.empty() &&
296+
"getArm64ECMangledFunctionName requires non-empty name");
297+
295298
if (Name[0] != '?') {
296299
// For non-C++ symbols, prefix the name with "#" unless it's already
297300
// mangled.

llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,14 @@ Function *AArch64Arm64ECCallLowering::buildEntryThunk(Function *F) {
597597
return Thunk;
598598
}
599599

600+
std::optional<std::string> getArm64ECMangledFunctionName(GlobalValue &GV) {
601+
if (!GV.hasName()) {
602+
GV.setName("__unnamed");
603+
}
604+
605+
return llvm::getArm64ECMangledFunctionName(GV.getName());
606+
}
607+
600608
// Builds the "guest exit thunk", a helper to call a function which may or may
601609
// not be an exit thunk. (We optimistically assume non-dllimport function
602610
// declarations refer to functions defined in AArch64 code; if the linker
@@ -608,7 +616,7 @@ Function *AArch64Arm64ECCallLowering::buildGuestExitThunk(Function *F) {
608616
getThunkType(F->getFunctionType(), F->getAttributes(),
609617
Arm64ECThunkType::GuestExit, NullThunkName, Arm64Ty, X64Ty,
610618
ArgTranslations);
611-
auto MangledName = getArm64ECMangledFunctionName(F->getName().str());
619+
auto MangledName = getArm64ECMangledFunctionName(*F);
612620
assert(MangledName && "Can't guest exit to function that's already native");
613621
std::string ThunkName = *MangledName;
614622
if (ThunkName[0] == '?' && ThunkName.find("@") != std::string::npos) {
@@ -790,7 +798,7 @@ bool AArch64Arm64ECCallLowering::runOnModule(Module &Mod) {
790798
if (!F)
791799
continue;
792800
if (std::optional<std::string> MangledName =
793-
getArm64ECMangledFunctionName(A.getName().str())) {
801+
getArm64ECMangledFunctionName(A)) {
794802
F->addMetadata("arm64ec_unmangled_name",
795803
*MDNode::get(M->getContext(),
796804
MDString::get(M->getContext(), A.getName())));
@@ -807,7 +815,7 @@ bool AArch64Arm64ECCallLowering::runOnModule(Module &Mod) {
807815
cast<GlobalValue>(F.getPersonalityFn()->stripPointerCasts());
808816
if (PersFn->getValueType() && PersFn->getValueType()->isFunctionTy()) {
809817
if (std::optional<std::string> MangledName =
810-
getArm64ECMangledFunctionName(PersFn->getName().str())) {
818+
getArm64ECMangledFunctionName(*PersFn)) {
811819
PersFn->setName(MangledName.value());
812820
}
813821
}
@@ -821,7 +829,7 @@ bool AArch64Arm64ECCallLowering::runOnModule(Module &Mod) {
821829
// Rename hybrid patchable functions and change callers to use a global
822830
// alias instead.
823831
if (std::optional<std::string> MangledName =
824-
getArm64ECMangledFunctionName(F.getName().str())) {
832+
getArm64ECMangledFunctionName(F)) {
825833
std::string OrigName(F.getName());
826834
F.setName(MangledName.value() + HybridPatchableTargetSuffix);
827835

@@ -927,7 +935,7 @@ bool AArch64Arm64ECCallLowering::processFunction(
927935
// FIXME: Handle functions with weak linkage?
928936
if (!F.hasLocalLinkage() || F.hasAddressTaken()) {
929937
if (std::optional<std::string> MangledName =
930-
getArm64ECMangledFunctionName(F.getName().str())) {
938+
getArm64ECMangledFunctionName(F)) {
931939
F.addMetadata("arm64ec_unmangled_name",
932940
*MDNode::get(M->getContext(),
933941
MDString::get(M->getContext(), F.getName())));
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
; RUN: llc -mtriple=arm64ec-pc-windows-msvc %s -o - | FileCheck %s
2+
3+
; Regression test: Arm64EC needs to look at the first character of a function
4+
; to decide if it will be mangled like a C or C++ function name, which caused
5+
; it to crash for empty function names.
6+
define void @""() {
7+
ret void
8+
}
9+
10+
define void @""() {
11+
ret void
12+
}
13+
14+
; CHECK: "#__unnamed":
15+
; CHECK: "#__unnamed.1":

0 commit comments

Comments
 (0)