[clang] [clang][bytecode] Classify function pointers as PT_Ptr (PR #135026)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 9 07:14:12 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Timm Baeder (tbaederr)
<details>
<summary>Changes</summary>
The Pointer class already has the capability to be a function pointer, but we still classifed function pointers as PT_FnPtr/FunctionPointer. This means when converting from a Pointer to a FunctionPointer, we lost the information of what the original Pointer pointed to.
---
Full diff: https://github.com/llvm/llvm-project/pull/135026.diff
8 Files Affected:
- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+5-3)
- (modified) clang/lib/AST/ByteCode/Context.cpp (+1-1)
- (modified) clang/lib/AST/ByteCode/Context.h (+1-4)
- (modified) clang/lib/AST/ByteCode/Interp.cpp (+12-7)
- (modified) clang/lib/AST/ByteCode/Interp.h (+22-21)
- (modified) clang/lib/AST/ByteCode/Pointer.cpp (+3)
- (modified) clang/lib/AST/ByteCode/Pointer.h (+2)
- (added) clang/test/AST/ByteCode/pointer-to-fnptr.cpp (+29)
``````````diff
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index e4f87d8b2af04..2d52ad047365b 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -463,6 +463,8 @@ bool Compiler<Emitter>::VisitCastExpr(const CastExpr *CE) {
if (!this->visit(SubExpr))
return false;
+ if (CE->getType()->isFunctionPointerType())
+ return true;
if (FromT == PT_Ptr)
return this->emitPtrPtrCast(SubExprTy->isVoidPointerType(), CE);
return true;
@@ -4921,10 +4923,10 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
} else if (!FuncDecl) {
const Expr *Callee = E->getCallee();
CalleeOffset =
- this->allocateLocalPrimitive(Callee, PT_FnPtr, /*IsConst=*/true);
+ this->allocateLocalPrimitive(Callee, PT_Ptr, /*IsConst=*/true);
if (!this->visit(Callee))
return false;
- if (!this->emitSetLocal(PT_FnPtr, *CalleeOffset, E))
+ if (!this->emitSetLocal(PT_Ptr, *CalleeOffset, E))
return false;
}
@@ -5011,7 +5013,7 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
if (!this->emitGetMemberPtrDecl(E))
return false;
} else {
- if (!this->emitGetLocal(PT_FnPtr, *CalleeOffset, E))
+ if (!this->emitGetLocal(PT_Ptr, *CalleeOffset, E))
return false;
}
if (!this->emitCallPtr(ArgSize, E, E))
diff --git a/clang/lib/AST/ByteCode/Context.cpp b/clang/lib/AST/ByteCode/Context.cpp
index 23f4c5a4fa4b7..431ccfcc821b1 100644
--- a/clang/lib/AST/ByteCode/Context.cpp
+++ b/clang/lib/AST/ByteCode/Context.cpp
@@ -189,7 +189,7 @@ std::optional<PrimType> Context::classify(QualType T) const {
if (T->isFunctionPointerType() || T->isFunctionReferenceType() ||
T->isFunctionType() || T->isBlockPointerType())
- return PT_FnPtr;
+ return PT_Ptr;
if (T->isPointerOrReferenceType() || T->isObjCObjectPointerType())
return PT_Ptr;
diff --git a/clang/lib/AST/ByteCode/Context.h b/clang/lib/AST/ByteCode/Context.h
index 8e142a0121ed3..8b542e6474780 100644
--- a/clang/lib/AST/ByteCode/Context.h
+++ b/clang/lib/AST/ByteCode/Context.h
@@ -78,11 +78,8 @@ class Context final {
/// Classifies an expression.
std::optional<PrimType> classify(const Expr *E) const {
assert(E);
- if (E->isGLValue()) {
- if (E->getType()->isFunctionType())
- return PT_FnPtr;
+ if (E->isGLValue())
return PT_Ptr;
- }
return classify(E->getType());
}
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 0acfe01a42410..4a300c02d007f 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -90,9 +90,6 @@ static bool BCP(InterpState &S, CodePtr &RealPC, int32_t Offset, PrimType PT) {
assert(S.Stk.size() == StackSizeBefore);
S.Stk.push<Integral<32, true>>(
Integral<32, true>::from(CheckBCPResult(S, Ptr)));
- } else if (PT == PT_FnPtr) {
- S.Stk.discard<FunctionPointer>();
- S.Stk.push<Integral<32, true>>(Integral<32, true>::from(0));
} else {
// Pop the result from the stack and return success.
TYPE_SWITCH(PT, S.Stk.pop<T>(););
@@ -318,6 +315,8 @@ bool CheckBCPResult(InterpState &S, const Pointer &Ptr) {
return false;
if (Ptr.isZero())
return true;
+ if (Ptr.isFunctionPointer())
+ return false;
if (Ptr.isIntegralPointer())
return true;
if (Ptr.isTypeidPointer())
@@ -1621,20 +1620,24 @@ bool CallBI(InterpState &S, CodePtr OpPC, const Function *Func,
bool CallPtr(InterpState &S, CodePtr OpPC, uint32_t ArgSize,
const CallExpr *CE) {
- const FunctionPointer &FuncPtr = S.Stk.pop<FunctionPointer>();
+ const Pointer &Ptr = S.Stk.pop<Pointer>();
- const Function *F = FuncPtr.getFunction();
- if (!F) {
+ if (Ptr.isZero()) {
const auto *E = cast<CallExpr>(S.Current->getExpr(OpPC));
S.FFDiag(E, diag::note_constexpr_null_callee)
<< const_cast<Expr *>(E->getCallee()) << E->getSourceRange();
return false;
}
- if (!FuncPtr.isValid() || !F->getDecl())
+ if (!Ptr.isFunctionPointer())
return Invalid(S, OpPC);
+ const FunctionPointer &FuncPtr = Ptr.asFunctionPointer();
+ const Function *F = FuncPtr.getFunction();
assert(F);
+ // Don't allow calling block pointers.
+ if (!F->getDecl())
+ return Invalid(S, OpPC);
// This happens when the call expression has been cast to
// something else, but we don't support that.
@@ -1774,6 +1777,8 @@ bool CheckPointerToIntegralCast(InterpState &S, CodePtr OpPC,
const Pointer &Ptr, unsigned BitWidth) {
if (Ptr.isDummy())
return false;
+ if (Ptr.isFunctionPointer())
+ return true;
const SourceInfo &E = S.Current->getSource(OpPC);
S.CCEDiag(E, diag::note_constexpr_invalid_cast)
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index ee69cea039990..e5813d6f7c982 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -979,20 +979,6 @@ bool CmpHelperEQ(InterpState &S, CodePtr OpPC, CompareFn Fn) {
return CmpHelper<T>(S, OpPC, Fn);
}
-/// Function pointers cannot be compared in an ordered way.
-template <>
-inline bool CmpHelper<FunctionPointer>(InterpState &S, CodePtr OpPC,
- CompareFn Fn) {
- const auto &RHS = S.Stk.pop<FunctionPointer>();
- const auto &LHS = S.Stk.pop<FunctionPointer>();
-
- const SourceInfo &Loc = S.Current->getSource(OpPC);
- S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_unspecified)
- << LHS.toDiagnosticString(S.getASTContext())
- << RHS.toDiagnosticString(S.getASTContext());
- return false;
-}
-
template <>
inline bool CmpHelperEQ<FunctionPointer>(InterpState &S, CodePtr OpPC,
CompareFn Fn) {
@@ -1019,18 +1005,27 @@ inline bool CmpHelper<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
const Pointer &RHS = S.Stk.pop<Pointer>();
const Pointer &LHS = S.Stk.pop<Pointer>();
+ // Function pointers cannot be compared in an ordered way.
+ if (LHS.isFunctionPointer() || RHS.isFunctionPointer()) {
+ const SourceInfo &Loc = S.Current->getSource(OpPC);
+ S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_unspecified)
+ << LHS.toDiagnosticString(S.getASTContext())
+ << RHS.toDiagnosticString(S.getASTContext());
+ return false;
+ }
+
if (!Pointer::hasSameBase(LHS, RHS)) {
const SourceInfo &Loc = S.Current->getSource(OpPC);
S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_unspecified)
<< LHS.toDiagnosticString(S.getASTContext())
<< RHS.toDiagnosticString(S.getASTContext());
return false;
- } else {
- unsigned VL = LHS.getByteOffset();
- unsigned VR = RHS.getByteOffset();
- S.Stk.push<BoolT>(BoolT::from(Fn(Compare(VL, VR))));
- return true;
}
+
+ unsigned VL = LHS.getByteOffset();
+ unsigned VR = RHS.getByteOffset();
+ S.Stk.push<BoolT>(BoolT::from(Fn(Compare(VL, VR))));
+ return true;
}
static inline bool IsOpaqueConstantCall(const CallExpr *E) {
@@ -1069,6 +1064,12 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
return false;
}
+ if (LHS.isFunctionPointer() && RHS.isFunctionPointer()) {
+ S.Stk.push<BoolT>(BoolT::from(Fn(Compare(LHS.getIntegerRepresentation(),
+ RHS.getIntegerRepresentation()))));
+ return true;
+ }
+
if (Pointer::hasSameBase(LHS, RHS)) {
if (LHS.inUnion() && RHS.inUnion()) {
// If the pointers point into a union, things are a little more
@@ -2787,7 +2788,7 @@ inline bool ArrayDecay(InterpState &S, CodePtr OpPC) {
inline bool GetFnPtr(InterpState &S, CodePtr OpPC, const Function *Func) {
assert(Func);
- S.Stk.push<FunctionPointer>(Func);
+ S.Stk.push<Pointer>(Func);
return true;
}
@@ -2822,7 +2823,7 @@ inline bool GetMemberPtrDecl(InterpState &S, CodePtr OpPC) {
const auto *FD = cast<FunctionDecl>(MP.getDecl());
const auto *Func = S.getContext().getOrCreateFunction(FD);
- S.Stk.push<FunctionPointer>(Func);
+ S.Stk.push<Pointer>(Func);
return true;
}
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index 5b522610a22f1..95a702bebf387 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -379,6 +379,9 @@ std::string Pointer::toDiagnosticString(const ASTContext &Ctx) const {
if (isIntegralPointer())
return (Twine("&(") + Twine(asIntPointer().Value + Offset) + ")").str();
+ if (isFunctionPointer())
+ return asFunctionPointer().toDiagnosticString(Ctx);
+
return toAPValue(Ctx).getAsString(Ctx, getType());
}
diff --git a/clang/lib/AST/ByteCode/Pointer.h b/clang/lib/AST/ByteCode/Pointer.h
index 64af5ed9b0a5d..f892cf1159b55 100644
--- a/clang/lib/AST/ByteCode/Pointer.h
+++ b/clang/lib/AST/ByteCode/Pointer.h
@@ -525,6 +525,8 @@ class Pointer {
}
bool isWeak() const {
+ if (isFunctionPointer())
+ return asFunctionPointer().isWeak();
if (!isBlockPointer())
return false;
diff --git a/clang/test/AST/ByteCode/pointer-to-fnptr.cpp b/clang/test/AST/ByteCode/pointer-to-fnptr.cpp
new file mode 100644
index 0000000000000..91fe782198a5f
--- /dev/null
+++ b/clang/test/AST/ByteCode/pointer-to-fnptr.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -o - %s -fexperimental-new-constant-interpreter | FileCheck %s
+
+template<typename T>
+struct Wrapper {
+ T *Val;
+
+ template<typename _Up>
+ constexpr Wrapper(_Up&& __u) {
+ T& __f = static_cast<_Up&&>(__u);
+ Val = &__f;
+ }
+ constexpr T& get() const { return *Val; }
+};
+
+void f(){}
+int main() {
+ auto W = Wrapper<decltype(f)>(f);
+
+ if (&W.get() != &f)
+ __builtin_abort();
+}
+
+/// We used to convert do the pointer->fnptr conversion
+/// by doing an integer conversion in between, which caused the
+/// %0 line to be:
+/// store ptr inttoptr (i64 138574454870464 to ptr), ptr %__f, align 8
+// CHECK: @_ZN7WrapperIFvvEEC2IRS0_EEOT_
+// CHECK: %0 = load ptr, ptr %__u.addr
``````````
</details>
https://github.com/llvm/llvm-project/pull/135026
More information about the cfe-commits
mailing list