[AArch64] Make IFUNC opt-in rather than opt-out. #171648

hvdijk · 2025-12-10T16:54:04Z

IFUNCs require loader support, so for arbitrary environments, the safe assumption is to assume that they are not supported.

In particular, aarch64-linux-pauthtest may be used with musl, and was wrongly detected as supporting IFUNCs.

hvdijk · 2025-12-10T16:56:12Z

cc @pcc, this changes #133533, does this look okay to you? And if so, do you think it needs a test? (Edit: of course I will update the existing test either way.)

github-actions · 2025-12-10T17:32:30Z

🐧 Linux x64 Test Results

187304 tests passed
4952 tests skipped

✅ The build succeeded and all tests passed.

github-actions · 2025-12-10T17:32:30Z

🪟 Windows x64 Test Results

128571 tests passed
2805 tests skipped

✅ The build succeeded and all tests passed.

llvmbot · 2025-12-10T17:33:47Z

@llvm/pr-subscribers-backend-aarch64

Author: Harald van Dijk (hvdijk)

Changes

IFUNCs require loader support, so for arbitrary environments, the safe assumption is to assume that they are not supported.

In particular, aarch64-linux-pauthtest may be used with musl, and was wrongly detected as supporting IFUNCs.

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

1 Files Affected:

(modified) llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp (+2-5)

diff --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index 25fc59e3fc258..daf20591b6731 100644
--- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -2402,11 +2402,8 @@ static bool targetSupportsIRelativeRelocation(const Triple &TT) {
   if (!TT.isOSBinFormatELF())
     return false;
 
-  // musl doesn't support IFUNCs.
-  if (TT.isMusl())
-    return false;
-
-  return true;
+  // IFUNCs are supported on glibc.
+  return TT.isGNUEnvironment();
 }
 
 // Emit an ifunc resolver that returns a signed pointer to the specified target,

hvdijk · 2025-12-10T17:53:44Z

Oh, I see a problem. From targetSupportsPAuthRelocation: "No released version of glibc supports PAuth relocations."

So basically, we need to disable PAuth relocations for glibc, but we also need to disable IFUNC for anything not glibc. What target is meant to be handled here?

Either way, even without this PR, #133533 looks like it has a problem, it results in a crash if the existing test is updated to use aarch64-linux-gnu.

pcc · 2025-12-10T18:10:56Z

Several other libcs support IFUNC, musl is the exception. Can we make it so that isMusl() or some other predicate returns true for pauthtest?

hvdijk · 2025-12-10T18:13:47Z

Several other libcs support IFUNC, musl is the exception.

I do not think musl is alone in not supporting it, I see no support for it in uclibc-ng either.

Can we make it so that isMusl() or some other predicate returns true for pauthtest?

I think that would be wrong, pauthtest does not imply any particular libc. Any triple that does not indicate what loader is used should be taken as suggesting a loader may be used that does not support IFUNC.

What are the other triples that do support IFUNCs? Searching a bit more, I find FreeBSD and bionic and have no issue adding these in this PR, are there more?

pcc · 2025-12-10T18:52:43Z

I do not think musl is alone in not supporting it, I see no support for it in uclibc-ng either.

Yeah, there are some others that don't support it.

What are the other triples that do support IFUNCs? Searching a bit more, I find FreeBSD and bionic and have no issue adding these in this PR, are there more?

I wanted to avoid this turning into a long list of libcs but maybe that would be better than the somewhat confusing predicate that I added in #133533. From a quick look:

Support: glibc, bionic, FreeBSD, DragonflyBSD, NetBSD
Do not support: musl, OpenBSD, uclibc-ng I guess

So how about we make the logic like this: if the libc supports IFUNC (according to the list above), use IFUNC, otherwise use PAuth relocations. That's still not 100% correct because we will end up using PAuth relocations even when we don't know that the target supports them, but that was the status quo before so maybe that's fine.

I think that would be wrong, pauthtest does not imply any particular libc. Any triple that does not indicate what loader is used should be taken as suggesting a loader may be used that does not support IFUNC.

Okay, I was under the impression that pauthtest referred to the musl-based system that was created for testing.

hvdijk · 2025-12-10T18:56:15Z

So how about we make the logic like this: if the libc supports IFUNC (according to the list above), use IFUNC, otherwise use PAuth relocations. That's still not 100% correct because we will end up using PAuth relocations even when we don't know that the target supports them, but that was the status quo before so maybe that's fine.

Thanks, I'll update this PR accordingly.

I've posted the details about the crash that I noticed earlier on #133533, but I think we can handle that separately from this PR.

Okay, I was under the impression that pauthtest referred to the musl-based system that was created for testing.

That is how we are using it, but we are doing that through config file overrides. What has been merged into LLVM does not make pauthtest imply musl, and the defaults do not work for musl. (Mind you, if we want to change it so that it does imply musl, personally I have no issue with that.)

hvdijk · 2025-12-10T22:47:44Z

Updated accordingly, does that look okay? I'm a little bit concerned that a global fix like isOSGlibc() shouldn't be included in a commit that's meant to be limited to AArch64. Edit: It turned out that that did have an unwanted effect on other targets. I've split this out into a larger NFC PR, #171734. I will rebase this one once that other one is ready (either in its current form or some other way).

llvm/include/llvm/TargetParser/Triple.h

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

hvdijk · 2025-12-11T04:24:11Z

The current test update no longer works now that targetSupportsPAuthRelocation is no longer there: it means aarch64-android no longer works as an alternative target to preserve the existing test exactly. It will need a bit more work on the test, but review of the rest of the PR is welcome.

IFUNCs require loader support, so for arbitrary environments, the safe assumption is to assume that they are not supported. In particular, aarch64-linux-pauthtest may be used with musl, and was wrongly detected as supporting IFUNCs.

hvdijk · 2025-12-11T16:41:04Z

I took out the removal of targetSupportsPAuthRelocation from this PR. It was a good suggestion to look at but in retrospect I don't think it's right, unless I am misunderstanding. For targets that support some PAuth relocations directly, and also support IFUNCs, we should still emit those PAuth relocations directly, that is what that targetSupportsPAuthRelocation check achieved before, I don't see how we can do that another way.

kovdan01

LGTM, thanks! This resolves the following linker errors which started to appear after #133533 when building libcxx with signed GOT enabled:

ld.lld: error: both AUTH and non-AUTH GOT entries for '_ZTVNSt3__113basic_istreamIcNS_11char_traitsIcEEEE' requested, but only one type of GOT entry per symbol is supported
ld.lld: error: both AUTH and non-AUTH GOT entries for '_ZTVNSt3__113basic_ostreamIcNS_11char_traitsIcEEEE' requested, but only one type of GOT entry per symbol is supported
ld.lld: error: both AUTH and non-AUTH GOT entries for '_ZTVNSt3__114basic_iostreamIcNS_11char_traitsIcEEEE' requested, but only one type of GOT entry per symbol is supported
ld.lld: error: both AUTH and non-AUTH GOT entries for '_ZTVNSt3__113basic_istreamIwNS_11char_traitsIwEEEE' requested, but only one type of GOT entry per symbol is supported
ld.lld: error: both AUTH and non-AUTH GOT entries for '_ZTVNSt3__113basic_ostreamIwNS_11char_traitsIwEEEE' requested, but only one type of GOT entry per symbol is supported
ld.lld: error: both AUTH and non-AUTH GOT entries for '_ZTVNSt3__114basic_ifstreamIcNS_11char_traitsIcEEEE' requested, but only one type of GOT entry per symbol is supported
ld.lld: error: both AUTH and non-AUTH GOT entries for '_ZTVNSt3__114basic_ofstreamIcNS_11char_traitsIcEEEE' requested, but only one type of GOT entry per symbol is supported

@pcc I would appreciate if you could take a look at this as soon as you can since your changes from #133533 unfortunately break pauthtest toolchain build with signed GOT enabled, and this PR resolves the issue.

kovdan01 · 2025-12-12T07:47:32Z

@hvdijk Could you please update the PR description adding info about the error introduced in #133533 which is fixed by this PR? See my comment above

hvdijk · 2025-12-12T14:18:45Z

Could you please update the PR description adding info about the error introduced in #133533 which is fixed by this PR?

I do mention already that this fixes aarch64-linux-pauthtest, but that particular error can still occur for other targets, because that is not specific to aarch64-linux-pauthtest, but to -fptrauth-elf-got. I am working on a separate PR fixing that.

asl · 2025-12-12T17:05:52Z

llvm/include/llvm/TargetParser/Triple.h

    return (getOS() == Triple::Linux || getOS() == Triple::KFreeBSD ||
            getOS() == Triple::Hurd) &&
-           !isAndroid() && !isMusl();
+           !isAndroid() && !isMusl() && getEnvironment() != Triple::PAuthTest;


Please add comment that pauthtest as of now implies not particular libc implementation

asl · 2025-12-12T17:08:04Z

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

 static bool targetSupportsPAuthRelocation(const Triple &TT,
                                          const MCExpr *Target,
                                          const MCExpr *DSExpr) {
  // No released version of glibc supports PAuth relocations.


The change above also implicitly changed the behavior here (as isOSGlibc() started to return false for pauthtest). In order to correctly represent the intended semantics we need to gate the libc check with environment check first as this also would protect us from potential future changes if / when pautest might imply a particular libc implementation.

Ideally the check whether pauth relocations are supported or not, should also be opt-in rather than opt-out, but we'd need to add some additional hooks for this, so probably not worth at the present time.

asl · 2025-12-12T17:09:07Z

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

                                          const MCExpr *Target,
                                          const MCExpr *DSExpr) {
  // No released version of glibc supports PAuth relocations.
  if (TT.isOSGlibc() || TT.isMusl())


Suggested change

if (TT.getEnvironment() != Triple::PAuthTest &&

(TT.isOSGlibc() || TT.isMusl()))

asl · 2025-12-12T17:15:44Z

llvm/test/CodeGen/AArch64/ptrauth-reloc.ll

 ; RUN: llc < ok.ll -mtriple arm64e-apple-darwin \
 ; RUN:   | FileCheck %s --check-prefix=CHECK-MACHO
-; RUN: llc < ok.ll -mtriple aarch64-elf -mattr=+pauth \
+; RUN: llc < ok.ll -mtriple aarch64-android -mattr=+pauth \


The tests should remain test elf triple.

hvdijk · 2025-12-12T17:22:29Z

This is not productive.

llvmbot added the backend:AArch64 label Dec 10, 2025

hvdijk mentioned this pull request Dec 10, 2025

CodeGen: Optionally emit PAuth relocations as IRELATIVE relocations. #133533

Merged

hvdijk force-pushed the aarch64-ifunc-opt-in branch from 0bb2d20 to 0184403 Compare December 10, 2025 22:45

pcc reviewed Dec 10, 2025

View reviewed changes

llvm/include/llvm/TargetParser/Triple.h Outdated Show resolved Hide resolved

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp Show resolved Hide resolved

hvdijk force-pushed the aarch64-ifunc-opt-in branch from 0184403 to 9458793 Compare December 11, 2025 04:05

[AArch64] Make IFUNC opt-in rather than opt-out.

d9f5db5

IFUNCs require loader support, so for arbitrary environments, the safe assumption is to assume that they are not supported. In particular, aarch64-linux-pauthtest may be used with musl, and was wrongly detected as supporting IFUNCs.

hvdijk force-pushed the aarch64-ifunc-opt-in branch from 9458793 to d9f5db5 Compare December 11, 2025 16:36

kovdan01 approved these changes Dec 12, 2025

View reviewed changes

asl requested changes Dec 12, 2025

View reviewed changes

hvdijk closed this Dec 12, 2025

hvdijk deleted the aarch64-ifunc-opt-in branch December 12, 2025 17:22


	if (TT.getEnvironment() != Triple::PAuthTest &&
	(TT.isOSGlibc() \|\| TT.isMusl()))

[AArch64] Make IFUNC opt-in rather than opt-out. #171648

[AArch64] Make IFUNC opt-in rather than opt-out. #171648

Conversation

hvdijk commented Dec 10, 2025

Uh oh!

hvdijk commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🐧 Linux x64 Test Results

Uh oh!

github-actions bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🪟 Windows x64 Test Results

Uh oh!

llvmbot commented Dec 10, 2025

Uh oh!

hvdijk commented Dec 10, 2025

Uh oh!

pcc commented Dec 10, 2025

Uh oh!

hvdijk commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pcc commented Dec 10, 2025

Uh oh!

hvdijk commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hvdijk commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hvdijk commented Dec 11, 2025

Uh oh!

hvdijk commented Dec 11, 2025

Uh oh!

kovdan01 left a comment

Choose a reason for hiding this comment

Uh oh!

kovdan01 commented Dec 12, 2025

Uh oh!

hvdijk commented Dec 12, 2025

Uh oh!

asl Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

asl Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asl Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

asl Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

hvdijk commented Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hvdijk commented Dec 10, 2025 •

edited

Loading

github-actions bot commented Dec 10, 2025 •

edited

Loading

github-actions bot commented Dec 10, 2025 •

edited

Loading

hvdijk commented Dec 10, 2025 •

edited

Loading

hvdijk commented Dec 10, 2025 •

edited

Loading

hvdijk commented Dec 10, 2025 •

edited

Loading

asl Dec 12, 2025 •

edited

Loading