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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
