[llvm] [MergeFunc] Preserve observable function pointer identity (PR #213604)
Gauarv Chaudhary via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 2 23:54:32 PDT 2026
https://github.com/ANAMASGARD created https://github.com/llvm/llvm-project/pull/213604
Fixes #213206
# Description
FunctionComparator currently treats references to the functions being compared as equivalent in every context. This is
unsound for MergeFunc, which may replace one implementation with a forwarding thunk to the other function.
As a result, code that observes a function’s address can be miscompiled. For example, comparing %p with @g inside g may
become a comparison with @f, even though @g remains a distinct function pointer.
This patch makes self-reference comparison context-sensitive:
- Ordinary function values are compared as normal global values.
- Self-references are equivalent only when used as corresponding CallBase callees.
- Call arguments remain ordinary observable values.
- Existing blockaddress comparison behavior is preserved.
- Recursive-function merging remains supported.
Added regression coverage for:
- The original llvm.assume miscompilation.
- Function self-references passed as arguments.
- Recursive self-call merging.
- Direct FunctionComparator behavior.
Testing:
- MergeFunc lit tests: 89/89 passed
- FunctionComparatorTest.*: 2/2 passed
- MergeFunctions.*: 4/4 passed
- git diff --check: passed
---
AI assistance was used in understanding the issue and fixing the bug .
>From d67332f5ca316674e7ba0120cabe8b057c8dec20 Mon Sep 17 00:00:00 2001
From: Gaurav Chaudhary <chaudharygaurav2004 at gmail.com>
Date: Mon, 3 Aug 2026 12:17:02 +0530
Subject: [PATCH] [MergeFunc] Preserve observable function pointer identity
Signed-off-by: Gaurav Chaudhary <chaudharygaurav2004 at gmail.com>
---
.../Transforms/Utils/FunctionComparator.h | 12 +++--
.../Transforms/Utils/FunctionComparator.cpp | 37 +++++++++-----
.../MergeFunc/recursive-self-reference.ll | 25 ++++++++++
.../Transforms/MergeFunc/self-reference.ll | 50 +++++++++++++++++++
.../Utils/FunctionComparatorTest.cpp | 46 +++++++++++++++++
5 files changed, 153 insertions(+), 17 deletions(-)
create mode 100644 llvm/test/Transforms/MergeFunc/recursive-self-reference.ll
create mode 100644 llvm/test/Transforms/MergeFunc/self-reference.ll
diff --git a/llvm/include/llvm/Transforms/Utils/FunctionComparator.h b/llvm/include/llvm/Transforms/Utils/FunctionComparator.h
index d765875864a89..0acf1e51d8cd1 100644
--- a/llvm/include/llvm/Transforms/Utils/FunctionComparator.h
+++ b/llvm/include/llvm/Transforms/Utils/FunctionComparator.h
@@ -101,6 +101,8 @@ class FunctionComparator {
LLVM_ABI int compare();
protected:
+ enum class ValueComparisonKind { Normal, CallTarget, BlockAddress };
+
/// Start the comparison.
void beginCompare() {
sn_mapL.clear();
@@ -226,9 +228,9 @@ class FunctionComparator {
/// return whether the numbers are equal. Numbers are assigned in the order
/// visited.
/// Comparison order:
- /// Stage 0: Value that is function itself is always greater then others.
- /// If left and right values are references to their functions, then
- /// they are equal.
+ /// Stage 0: In CallTarget or BlockAddress comparison mode, references to the
+ /// functions being compared are equal. In Normal comparison mode,
+ /// function values are compared like other global values.
/// Stage 1: Constants are greater than non-constants.
/// If both left and right are constants, then the result of
/// cmpConstants is used as cmpValues result.
@@ -240,7 +242,9 @@ class FunctionComparator {
/// then left value is greater.
/// In another words, we compare serial numbers, for more details
/// see comments for sn_mapL and sn_mapR.
- LLVM_ABI int cmpValues(const Value *L, const Value *R) const;
+ LLVM_ABI int cmpValues(
+ const Value *L, const Value *R,
+ ValueComparisonKind Kind = ValueComparisonKind::Normal) const;
/// Compare two Instructions for equivalence, similar to
/// Instruction::isSameOperationAs.
diff --git a/llvm/lib/Transforms/Utils/FunctionComparator.cpp b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
index c9cc62adc2604..c213bb06fed42 100644
--- a/llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ b/llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -478,7 +478,8 @@ int FunctionComparator::cmpConstants(const Constant *L,
case Value::BlockAddressVal: {
const BlockAddress *LBA = cast<BlockAddress>(L);
const BlockAddress *RBA = cast<BlockAddress>(R);
- if (int Res = cmpValues(LBA->getFunction(), RBA->getFunction()))
+ if (int Res = cmpValues(LBA->getFunction(), RBA->getFunction(),
+ ValueComparisonKind::BlockAddress))
return Res;
if (LBA->getFunction() == RBA->getFunction()) {
// They are BBs in the same function. Order by which comes first in the
@@ -892,17 +893,21 @@ int FunctionComparator::cmpInlineAsm(const InlineAsm *L,
/// this is the first time the values are seen, they're added to the mapping so
/// that we will detect mismatches on next use.
/// See comments in declaration for more details.
-int FunctionComparator::cmpValues(const Value *L, const Value *R) const {
- // Catch self-reference case.
- if (L == FnL) {
- if (R == FnR)
- return 0;
- return -1;
- }
- if (R == FnR) {
- if (L == FnL)
- return 0;
- return 1;
+int FunctionComparator::cmpValues(const Value *L, const Value *R,
+ ValueComparisonKind Kind) const {
+ // A self-reference is equivalent only in structural contexts where the
+ // reference remains valid after forwarding-thunk merging. Otherwise,
+ // merging the functions can change the observable function pointer value.
+ if (Kind != ValueComparisonKind::Normal) {
+ const Value *StrippedL = L->stripPointerCasts();
+ const Value *StrippedR = R->stripPointerCasts();
+ if (StrippedL == FnL) {
+ if (StrippedR == FnR)
+ return 0;
+ return -1;
+ }
+ if (StrippedR == FnR)
+ return 1;
}
const Constant *ConstL = dyn_cast<Constant>(L);
@@ -961,11 +966,17 @@ int FunctionComparator::cmpBasicBlocks(const BasicBlock *BBL,
return Res;
if (needToCmpOperands) {
assert(InstL->getNumOperands() == InstR->getNumOperands());
+ const auto *CBL = dyn_cast<CallBase>(InstL);
+ const auto *CBR = dyn_cast<CallBase>(InstR);
for (unsigned i = 0, e = InstL->getNumOperands(); i != e; ++i) {
Value *OpL = InstL->getOperand(i);
Value *OpR = InstR->getOperand(i);
- if (int Res = cmpValues(OpL, OpR))
+ ValueComparisonKind Kind = ValueComparisonKind::Normal;
+ if (CBL && CBR && CBL->isCallee(&InstL->getOperandUse(i)) &&
+ CBR->isCallee(&InstR->getOperandUse(i)))
+ Kind = ValueComparisonKind::CallTarget;
+ if (int Res = cmpValues(OpL, OpR, Kind))
return Res;
// cmpValues should ensure this is true.
assert(cmpTypes(OpL->getType(), OpR->getType()) == 0);
diff --git a/llvm/test/Transforms/MergeFunc/recursive-self-reference.ll b/llvm/test/Transforms/MergeFunc/recursive-self-reference.ll
new file mode 100644
index 0000000000000..f7680a8a096cb
--- /dev/null
+++ b/llvm/test/Transforms/MergeFunc/recursive-self-reference.ll
@@ -0,0 +1,25 @@
+; RUN: opt -S -passes=mergefunc < %s | FileCheck %s
+
+; Recursive call targets may still be considered equivalent.
+; CHECK-LABEL: define internal void @recursive_f(
+; CHECK: call void @recursive_f()
+; CHECK-NOT: @recursive_g
+; CHECK-LABEL: define i32 @main(
+; CHECK: call void @recursive_f()
+; CHECK: call void @recursive_f()
+
+define internal void @recursive_f() {
+ call void @recursive_f()
+ ret void
+}
+
+define internal void @recursive_g() {
+ call void @recursive_g()
+ ret void
+}
+
+define i32 @main() {
+ call void @recursive_f()
+ call void @recursive_g()
+ ret i32 0
+}
diff --git a/llvm/test/Transforms/MergeFunc/self-reference.ll b/llvm/test/Transforms/MergeFunc/self-reference.ll
new file mode 100644
index 0000000000000..2033be2073355
--- /dev/null
+++ b/llvm/test/Transforms/MergeFunc/self-reference.ll
@@ -0,0 +1,50 @@
+; RUN: opt -S -passes=mergefunc < %s | FileCheck %s
+
+; A function's address is observable when it is used as a value, so it must not
+; be replaced with the address of a forwarding thunk's target.
+; CHECK-LABEL: define void @f(
+; CHECK: icmp eq ptr {{.*}}, @f
+; CHECK: call void @llvm.assume
+; CHECK-LABEL: define void @g(
+; CHECK: icmp eq ptr {{.*}}, @g
+; CHECK-NOT: tail call void @f
+; CHECK: call void @llvm.assume
+
+define void @f(ptr %p) {
+ %cmp = icmp eq ptr %p, @f
+ call void @llvm.assume(i1 %cmp)
+ ret void
+}
+
+define void @g(ptr %p) {
+ %cmp = icmp eq ptr %p, @g
+ call void @llvm.assume(i1 %cmp)
+ ret void
+}
+
+; A self-reference passed as an argument is also observable and must remain a
+; normal value comparison rather than a call-target comparison.
+; CHECK-LABEL: define void @arg_f(
+; CHECK: call void @consume(ptr @arg_f)
+; CHECK-LABEL: define void @arg_g(
+; CHECK: call void @consume(ptr @arg_g)
+
+declare void @consume(ptr)
+
+define void @arg_f() {
+ call void @consume(ptr @arg_f)
+ ret void
+}
+
+define void @arg_g() {
+ call void @consume(ptr @arg_g)
+ ret void
+}
+
+define i32 @main() {
+ call void @f(ptr @f)
+ call void @g(ptr @g)
+ call void @arg_f()
+ call void @arg_g()
+ ret i32 0
+}
diff --git a/llvm/unittests/Transforms/Utils/FunctionComparatorTest.cpp b/llvm/unittests/Transforms/Utils/FunctionComparatorTest.cpp
index cd2b4e8046b2e..609c1ef036a28 100644
--- a/llvm/unittests/Transforms/Utils/FunctionComparatorTest.cpp
+++ b/llvm/unittests/Transforms/Utils/FunctionComparatorTest.cpp
@@ -6,12 +6,15 @@
//
//===----------------------------------------------------------------------===//
#include "llvm/Transforms/Utils/FunctionComparator.h"
+#include "llvm/AsmParser/Parser.h"
#include "llvm/IR/BasicBlock.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Module.h"
+#include "llvm/Support/SourceMgr.h"
#include "gtest/gtest.h"
+#include <memory>
using namespace llvm;
@@ -127,3 +130,46 @@ TEST(FunctionComparatorTest, TestAPI) {
EXPECT_EQ(Cmp.testCmpTypes(F1.T, F2.T), 0);
EXPECT_EQ(Cmp.testCmpPrimitives(), -4);
}
+
+TEST(FunctionComparatorTest, SelfReferenceComparisonContext) {
+ LLVMContext C;
+ SMDiagnostic Err;
+ std::unique_ptr<Module> M(parseAssemblyString(R"IR(
+ define void @f() {
+ call void @f()
+ ret void
+ }
+
+ define void @g() {
+ call void @g()
+ ret void
+ }
+
+ define void @observable_f(ptr %p) {
+ %cmp = icmp eq ptr %p, @observable_f
+ call void @llvm.assume(i1 %cmp)
+ ret void
+ }
+
+ define void @observable_g(ptr %p) {
+ %cmp = icmp eq ptr %p, @observable_g
+ call void @llvm.assume(i1 %cmp)
+ ret void
+ }
+
+ declare void @llvm.assume(i1)
+ )IR",
+ Err, C));
+ ASSERT_TRUE(M);
+
+ GlobalNumberState GN;
+ EXPECT_EQ(FunctionComparator(M->getFunction("f"), M->getFunction("g"), &GN)
+ .compare(),
+ 0);
+
+ GN.clear();
+ EXPECT_NE(FunctionComparator(M->getFunction("observable_f"),
+ M->getFunction("observable_g"), &GN)
+ .compare(),
+ 0);
+}
More information about the llvm-commits
mailing list