llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang-codegen Author: Yanzuo Liu (zwuis) <details> <summary>Changes</summary> Before this patch, `typeid` operands are not evaluated if they are the most derived objects. Before #<!-- -->196544, only `typeid(non-reference-var)` is considered to be the most derived object, which doesn't have side effects obviously. So there was no miscompilation. Fixes #<!-- -->202051. --- Full diff: https://github.com/llvm/llvm-project/pull/208470.diff 4 Files Affected: - (modified) clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp (+10-9) - (modified) clang/lib/CodeGen/CGExprCXX.cpp (+12-10) - (added) clang/test/CIR/CodeGenCXX/typeid-most-derived.cpp (+134) - (modified) clang/test/CodeGenCXX/typeid-most-derived.cpp (+13) ``````````diff diff --git a/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp b/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp index df60005d77259..0f0c5e6be13e9 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp @@ -1916,9 +1916,8 @@ mlir::Value CIRGenFunction::emitDynamicCast(Address thisAddr, destCirTy, isRefCast, thisAddr); } -static mlir::Value emitCXXTypeidFromVTable(CIRGenFunction &cgf, const Expr *e, - mlir::Type typeInfoPtrTy, - bool hasNullCheck) { +static Address emitCXXTypeidOperand(CIRGenFunction &cgf, const Expr *e, + bool hasNullCheck) { Address thisPtr = cgf.emitLValue(e).getAddress(); QualType srcType = e->getType(); @@ -1940,7 +1939,7 @@ static mlir::Value emitCXXTypeidFromVTable(CIRGenFunction &cgf, const Expr *e, }); } - return cgf.cgm.getCXXABI().emitTypeid(cgf, srcType, thisPtr, typeInfoPtrTy); + return thisPtr; } mlir::Value CIRGenFunction::emitCXXTypeidExpr(const CXXTypeidExpr *e) { @@ -1958,11 +1957,13 @@ mlir::Value CIRGenFunction::emitCXXTypeidExpr(const CXXTypeidExpr *e) { // polymorphic class type, the result refers to a std::type_info object // representing the type of the most derived object (that is, the dynamic // type) to which the glvalue refers. - // If the operand is already most derived object, no need to look up vtable. - if (!e->isTypeOperand() && e->isPotentiallyEvaluated() && - !e->isMostDerived(getContext())) - return emitCXXTypeidFromVTable(*this, e->getExprOperand(), resultType, - e->hasNullCheck()); + if (!e->isTypeOperand() && e->isPotentiallyEvaluated()) { + Address thisPtr = + emitCXXTypeidOperand(*this, e->getExprOperand(), e->hasNullCheck()); + if (!e->isMostDerived(getContext())) + return cgm.getCXXABI().emitTypeid(*this, ty, thisPtr, resultType); + // If the operand is already most derived object, no need to look up vtable. + } auto typeInfo = cast<cir::GlobalViewAttr>(cgm.getAddrOfRTTIDescriptor(loc, ty)); diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index ebbc0addfed2c..ec4ae54354623 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -2173,9 +2173,8 @@ void CodeGenFunction::EmitCXXDeleteExpr(const CXXDeleteExpr *E) { } } -static llvm::Value *EmitTypeidFromVTable(CodeGenFunction &CGF, const Expr *E, - llvm::Type *StdTypeInfoPtrTy, - bool HasNullCheck) { +static Address EmitTypeidOperand(CodeGenFunction &CGF, const Expr *E, + bool HasNullCheck) { // Get the vtable pointer. Address ThisPtr = CGF.EmitLValue(E).getAddress(); @@ -2205,8 +2204,7 @@ static llvm::Value *EmitTypeidFromVTable(CodeGenFunction &CGF, const Expr *E, CGF.EmitBlock(EndBlock); } - return CGF.CGM.getCXXABI().EmitTypeid(CGF, SrcRecordTy, ThisPtr, - StdTypeInfoPtrTy); + return ThisPtr; } llvm::Value *CodeGenFunction::EmitCXXTypeidExpr(const CXXTypeidExpr *E) { @@ -2230,17 +2228,21 @@ llvm::Value *CodeGenFunction::EmitCXXTypeidExpr(const CXXTypeidExpr *E) { return MaybeASCast(TypeInfo); } + const Expr *Operand = E->getExprOperand(); + QualType OperandTy = Operand->getType(); + // C++ [expr.typeid]p2: // When typeid is applied to a glvalue expression whose type is a // polymorphic class type, the result refers to a std::type_info object // representing the type of the most derived object (that is, the dynamic // type) to which the glvalue refers. - // If the operand is already most derived object, no need to look up vtable. - if (E->isPotentiallyEvaluated() && !E->isMostDerived(getContext())) - return EmitTypeidFromVTable(*this, E->getExprOperand(), PtrTy, - E->hasNullCheck()); + if (E->isPotentiallyEvaluated()) { + Address ThisPtr = EmitTypeidOperand(*this, Operand, E->hasNullCheck()); + if (!E->isMostDerived(getContext())) + return CGM.getCXXABI().EmitTypeid(*this, OperandTy, ThisPtr, PtrTy); + // If the operand is already most derived object, no need to look up vtable. + } - QualType OperandTy = E->getExprOperand()->getType(); return MaybeASCast(CGM.GetAddrOfRTTIDescriptor(OperandTy)); } diff --git a/clang/test/CIR/CodeGenCXX/typeid-most-derived.cpp b/clang/test/CIR/CodeGenCXX/typeid-most-derived.cpp new file mode 100644 index 0000000000000..369a0e4e3ab7a --- /dev/null +++ b/clang/test/CIR/CodeGenCXX/typeid-most-derived.cpp @@ -0,0 +1,134 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir -mmlir --mlir-print-ir-before=cir-lowering-prepare %s -o %t.cir 2> %t-before.cir +// RUN: FileCheck %s --input-file=%t-before.cir --check-prefixes=CIR +// RUN: FileCheck %s --input-file=%t.cir --check-prefixes=CIR +// RUN: %clang_cc1 %s -triple %itanium_abi_triple -Wno-unused-value -std=c++11 -fclangir -emit-llvm -o - -std=c++11 | FileCheck %s --check-prefixes=CIR-TO-LLVM +// RUN: %clang_cc1 %s -triple %itanium_abi_triple -Wno-unused-value -std=c++11 -emit-llvm -o - -std=c++11 | FileCheck %s --check-prefixes=LLVM + +// FIXME: CIR-TO-LLVM output is incorrect. It should be same as LLVM output. + +namespace std { + class type_info {}; +} + +struct Base { + virtual int foo() { return 42; } + virtual ~Base(); +}; + +struct NonFinal : Base {}; +struct Final final : Base { + int foo() override { return 84; } +}; + +void func(); + +// Most derived +void base_by_value(Base b) { typeid(b); } +// CIR-LABEL: cir.func {{.*}}@_Z13base_by_value4Base +// CIR-NOT: cir.vtable.get_vptr +// CIR: cir.return +// LLVM-LABEL: define {{.*}}void @_Z13base_by_value4Base +// LLVM-NOT: getelementptr +// LLVM: ret void +// CIR-TO-LLVM-LABEL: define {{.*}}void @_Z13base_by_value4Base +// CIR-TO-LLVM-NOT: getelementptr +// CIR-TO-LLVM: ret void + +// Most derived +void final_ref(Final &f) { typeid(f); } +// CIR-LABEL: cir.func {{.*}}@_Z9final_refR5Final +// CIR-NOT: cir.vtable.get_vptr +// CIR: cir.return +// LLVM-LABEL: define {{.*}}void @_Z9final_refR5Final +// LLVM-NOT: getelementptr +// LLVM: ret void +// CIR-TO-LLVM-LABEL: define {{.*}}void @_Z9final_refR5Final +// CIR-TO-LLVM-NOT: getelementptr +// CIR-TO-LLVM: ret void + +// Most derived +void final_deref(Final *f) { typeid(*f); } +// CIR-LABEL: cir.func {{.*}}@_Z11final_derefP5Final +// CIR-NOT: cir.vtable.get_vptr +// CIR: cir.cmp +// CIR-NEXT: cir.if +// CIR-NOT: cir.vtable.get_vptr +// CIR: cir.return +// LLVM-LABEL: define {{.*}}void @_Z11final_derefP5Final +// LLVM-NOT: getelementptr +// LLVM: icmp eq {{.*}}, null +// LLVM-NEXT: br i1 +// LLVM-NOT: getelementptr +// LLVM: ret void +// CIR-TO-LLVM-LABEL: define {{.*}}void @_Z11final_derefP5Final +// CIR-TO-LLVM-NOT: getelementptr +// CIR-TO-LLVM: icmp eq {{.*}}, null +// CIR-TO-LLVM-NEXT: br i1 +// CIR-TO-LLVM-NOT: getelementptr +// CIR-TO-LLVM: ret void + +// Most derived +void should_evaluate(Final &f) { typeid(func(), f); } +// CIR-LABEL: cir.func {{.*}}@_Z15should_evaluateR5Final +// CIR-NOT: cir.vtable.get_vptr +// CIR: cir.call @_Z4funcv +// CIR-NOT: cir.vtable.get_vptr +// CIR: cir.return +// LLVM-LABEL: define {{.*}}void @_Z15should_evaluateR5Final +// LLVM-NOT: getelementptr +// LLVM: call void @_Z4funcv() +// LLVM-NOT: getelementptr +// LLVM: ret void +// CIR-TO-LLVM-LABEL: define {{.*}}void @_Z15should_evaluateR5Final +// CIR-TO-LLVM-NOT: getelementptr +// CIR-TO-LLVM: call void @_Z4funcv() +// CIR-TO-LLVM-NOT: getelementptr +// CIR-TO-LLVM: ret void + +// Not most derived +void base_ref(Base &b) { typeid(b); } +// CIR-LABEL: cir.func {{.*}}@_Z8base_refR4Base +// CIR : cir.vtable.get_vptr +// CIR: cir.return +// LLVM-LABEL: define {{.*}}void @_Z8base_refR4Base +// LLVM: getelementptr inbounds ptr, ptr +// LLVM: ret void +// CIR-TO-LLVM-LABEL: define {{.*}}void @_Z8base_refR4Base +// CIR-TO-LLVM: getelementptr ptr, ptr +// CIR-TO-LLVM: ret void + +// Not most derived +void base_deref(Base *b) { typeid(*b); } +// CIR-LABEL: cir.func {{.*}}@_Z10base_derefP4Base +// CIR : cir.vtable.get_vptr +// CIR: cir.return +// LLVM-LABEL: define {{.*}}void @_Z10base_derefP4Base +// LLVM: getelementptr inbounds ptr, ptr +// LLVM: ret void +// CIR-TO-LLVM-LABEL: define {{.*}}void @_Z10base_derefP4Base +// CIR-TO-LLVM-NOT: getelementptr +// CIR-TO-LLVM: ret void + +// Not most derived +void nonfinal_ref(NonFinal &d) { typeid(d); } +// CIR-LABEL: cir.func {{.*}}@_Z12nonfinal_refR8NonFinal +// CIR : cir.vtable.get_vptr +// CIR: cir.return +// LLVM-LABEL: define {{.*}}void @_Z12nonfinal_refR8NonFinal +// LLVM: getelementptr inbounds ptr, ptr +// LLVM: ret void +// CIR-TO-LLVM-LABEL: define {{.*}}void @_Z12nonfinal_refR8NonFinal +// CIR-TO-LLVM: getelementptr ptr, ptr +// CIR-TO-LLVM: ret void + +// Not most derived +void nonfinal_deref(NonFinal *d) { typeid(*d); } +// CIR-LABEL: cir.func {{.*}}@_Z14nonfinal_derefP8NonFinal +// CIR : cir.vtable.get_vptr +// CIR: cir.return +// LLVM-LABEL: define {{.*}}void @_Z14nonfinal_derefP8NonFinal +// LLVM: getelementptr inbounds ptr, ptr +// LLVM: ret void +// CIR-TO-LLVM-LABEL: define {{.*}}void @_Z14nonfinal_derefP8NonFinal +// CIR-TO-LLVM-NOT: getelementptr +// CIR-TO-LLVM: ret void diff --git a/clang/test/CodeGenCXX/typeid-most-derived.cpp b/clang/test/CodeGenCXX/typeid-most-derived.cpp index 2b6bb850ff415..99c743acfcaaf 100644 --- a/clang/test/CodeGenCXX/typeid-most-derived.cpp +++ b/clang/test/CodeGenCXX/typeid-most-derived.cpp @@ -14,6 +14,8 @@ struct Final final : Base { int foo() override { return 84; } }; +void func(); + // Most derived void base_by_value(Base b) { typeid(b); } // CHECK-LABEL: define {{.*}}void @_Z13base_by_value4Base @@ -30,6 +32,17 @@ void final_ref(Final &f) { typeid(f); } void final_deref(Final *f) { typeid(*f); } // CHECK-LABEL: define {{.*}}void @_Z11final_derefP5Final // CHECK-NOT: %vtable +// CHECK: icmp eq {{.*}}, null +// CHECK-NEXT: br i1 +// CHECK-NOT: %vtable +// CHECK: ret void + +// Most derived +void should_evaluate(Final &f) { typeid(func(), f); } +// CHECK-LABEL: define {{.*}}void @_Z15should_evaluateR5Final +// CHECK-NOT: %vtable +// CHECK: call void @_Z4funcv() +// CHECK-NOT: %vtable // CHECK: ret void // Not most derived `````````` </details> https://github.com/llvm/llvm-project/pull/208470 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
