r353181 - [opaque pointer types] Fix the CallInfo passed to EmitCall in some
James Y Knight via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 5 08:05:51 PST 2019
Author: jyknight
Date: Tue Feb 5 08:05:50 2019
New Revision: 353181
URL: http://llvm.org/viewvc/llvm-project?rev=353181&view=rev
Log:
[opaque pointer types] Fix the CallInfo passed to EmitCall in some
edge cases.
Currently, EmitCall emits a call instruction with a function type
derived from the pointee-type of the callee. This *should* be the same
as the type created from the CallInfo parameter, but in some cases an
incorrect CallInfo was being passed.
All of these fixes were discovered by the addition of the assert in
EmitCall which verifies that the passed-in CallInfo matches the
Callee's function type.
As far as I know, these issues caused no bugs at the moment, as the
correct types were ultimately being emitted. But, some would become
problematic when pointee types are removed.
List of fixes:
* arrangeCXXConstructorCall was passing an incorrect value for the
number of Required args, when calling an inheriting constructor
where the inherited constructor is variadic. (The inheriting
constructor doesn't actually get passed any of the user's args, but
the code was calculating it as if it did).
* arrangeFreeFunctionLikeCall was not including the count of the
pass_object_size arguments in the count of required args.
* OpenCL uses other address spaces for the "this" pointer. However,
commonEmitCXXMemberOrOperatorCall was not annotating the address
space on the "this" argument of the call.
* Destructor calls were being created with EmitCXXMemberOrOperatorCall
instead of EmitCXXDestructorCall in a few places. This was a problem
because the calling convention sometimes has destructors returning
"this" rather than void, and the latter function knows about that,
and sets up the types properly (through calling
arrangeCXXStructorDeclaration), while the former does not.
* generateObjCGetterBody: the 'objc_getProperty' function returns type
'id', but was being called as if it returned the particular
property's type. (That is of course the *dynamic* return type, and
there's a downcast immediately after.)
* OpenMP user-defined reduction functions (#pragma omp declare
reduction) can be called with a subclass of the declared type. In
such case, the call was being setup as if the function had been
actually declared to take the subtype, rather than the base type.
Differential Revision: https://reviews.llvm.org/D57664
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/CodeGen/CodeGenTypes.h
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/CodeGenObjC/getter-property-mismatch.m
Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=353181&r1=353180&r2=353181&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Tue Feb 5 08:05:50 2019
@@ -67,10 +67,17 @@ unsigned CodeGenTypes::ClangCallConvToLL
}
/// Derives the 'this' type for codegen purposes, i.e. ignoring method CVR
-/// qualification.
-static CanQualType GetThisType(ASTContext &Context, const CXXRecordDecl *RD,
- const CXXMethodDecl *MD) {
- QualType RecTy = Context.getTagDeclType(RD)->getCanonicalTypeInternal();
+/// qualification. Either or both of RD and MD may be null. A null RD indicates
+/// that there is no meaningful 'this' type, and a null MD can occur when
+/// calling a method pointer.
+CanQualType CodeGenTypes::DeriveThisType(const CXXRecordDecl *RD,
+ const CXXMethodDecl *MD) {
+ QualType RecTy;
+ if (RD)
+ RecTy = Context.getTagDeclType(RD)->getCanonicalTypeInternal();
+ else
+ RecTy = Context.VoidTy;
+
if (MD)
RecTy = Context.getAddrSpaceQualType(RecTy, MD->getMethodQualifiers().getAddressSpace());
return Context.getPointerType(CanQualType::CreateUnsafe(RecTy));
@@ -235,7 +242,7 @@ static CallingConv getCallingConventionF
/// Arrange the argument and result information for a call to an
/// unknown C++ non-static member function of the given abstract type.
-/// (Zero value of RD means we don't have any meaningful "this" argument type,
+/// (A null RD means we don't have any meaningful "this" argument type,
/// so fall back to a generic pointer type).
/// The member function must be an ordinary function, i.e. not a
/// constructor or destructor.
@@ -246,10 +253,7 @@ CodeGenTypes::arrangeCXXMethodType(const
SmallVector<CanQualType, 16> argTypes;
// Add the 'this' pointer.
- if (RD)
- argTypes.push_back(GetThisType(Context, RD, MD));
- else
- argTypes.push_back(Context.VoidPtrTy);
+ argTypes.push_back(DeriveThisType(RD, MD));
return ::arrangeLLVMFunctionInfo(
*this, true, argTypes,
@@ -303,7 +307,7 @@ CodeGenTypes::arrangeCXXStructorDeclarat
SmallVector<CanQualType, 16> argTypes;
SmallVector<FunctionProtoType::ExtParameterInfo, 16> paramInfos;
- argTypes.push_back(GetThisType(Context, MD->getParent(), MD));
+ argTypes.push_back(DeriveThisType(MD->getParent(), MD));
bool PassParams = true;
@@ -403,8 +407,11 @@ CodeGenTypes::arrangeCXXConstructorCall(
unsigned TotalPrefixArgs = 1 + ExtraPrefixArgs;
CanQual<FunctionProtoType> FPT = GetFormalType(D);
- RequiredArgs Required =
- RequiredArgs::forPrototypePlus(FPT, TotalPrefixArgs + ExtraSuffixArgs);
+ RequiredArgs Required = PassProtoArgs
+ ? RequiredArgs::forPrototypePlus(
+ FPT, TotalPrefixArgs + ExtraSuffixArgs)
+ : RequiredArgs::All;
+
GlobalDecl GD(D, CtorKind);
CanQualType ResultType = TheCXXABI.HasThisReturn(GD)
? ArgTypes.front()
@@ -530,7 +537,7 @@ const CGFunctionInfo &
CodeGenTypes::arrangeUnprototypedMustTailThunk(const CXXMethodDecl *MD) {
assert(MD->isVirtual() && "only methods have thunks");
CanQual<FunctionProtoType> FTP = GetFormalType(MD);
- CanQualType ArgTys[] = { GetThisType(Context, MD->getParent(), MD) };
+ CanQualType ArgTys[] = {DeriveThisType(MD->getParent(), MD)};
return arrangeLLVMFunctionInfo(Context.VoidTy, /*instanceMethod=*/false,
/*chainCall=*/false, ArgTys,
FTP->getExtInfo(), {}, RequiredArgs(1));
@@ -544,7 +551,7 @@ CodeGenTypes::arrangeMSCtorClosure(const
CanQual<FunctionProtoType> FTP = GetFormalType(CD);
SmallVector<CanQualType, 2> ArgTys;
const CXXRecordDecl *RD = CD->getParent();
- ArgTys.push_back(GetThisType(Context, RD, CD));
+ ArgTys.push_back(DeriveThisType(RD, CD));
if (CT == Ctor_CopyingClosure)
ArgTys.push_back(*FTP->param_type_begin());
if (RD->getNumVBases() > 0)
@@ -577,7 +584,7 @@ arrangeFreeFunctionLikeCall(CodeGenTypes
// extra prefix plus the arguments in the prototype.
if (const FunctionProtoType *proto = dyn_cast<FunctionProtoType>(fnType)) {
if (proto->isVariadic())
- required = RequiredArgs(proto->getNumParams() + numExtraRequiredArgs);
+ required = RequiredArgs::forPrototypePlus(proto, numExtraRequiredArgs);
if (proto->hasExtParameterInfos())
addExtParameterInfosForCall(paramInfos, proto, numExtraRequiredArgs,
@@ -802,6 +809,8 @@ CGFunctionInfo *CGFunctionInfo::create(u
ArrayRef<CanQualType> argTypes,
RequiredArgs required) {
assert(paramInfos.empty() || paramInfos.size() == argTypes.size());
+ assert(!required.allowsOptionalArgs() ||
+ required.getNumRequiredArgs() <= argTypes.size());
void *buffer =
operator new(totalSizeToAlloc<ArgInfo, ExtParameterInfo>(
@@ -3818,6 +3827,20 @@ RValue CodeGenFunction::EmitCall(const C
llvm::FunctionType *IRFuncTy = Callee.getFunctionType();
+#ifndef NDEBUG
+ if (!(CallInfo.isVariadic() && CallInfo.getArgStruct())) {
+ // For an inalloca varargs function, we don't expect CallInfo to match the
+ // function pointer's type, because the inalloca struct a will have extra
+ // fields in it for the varargs parameters. Code later in this function
+ // bitcasts the function pointer to the type derived from CallInfo.
+ //
+ // In other cases, we assert that the types match up (until pointers stop
+ // having pointee types).
+ llvm::FunctionType *IRFuncTyFromInfo = getTypes().GetFunctionType(CallInfo);
+ assert(IRFuncTy == IRFuncTyFromInfo);
+ }
+#endif
+
// 1. Set up the arguments.
// If we're using inalloca, insert the allocation after the stack save.
Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=353181&r1=353180&r2=353181&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Tue Feb 5 08:05:50 2019
@@ -40,13 +40,11 @@ commonEmitCXXMemberOrOperatorCall(CodeGe
isa<CXXOperatorCallExpr>(CE));
assert(MD->isInstance() &&
"Trying to emit a member or operator call expr on a static method!");
- ASTContext &C = CGF.getContext();
// Push the this ptr.
const CXXRecordDecl *RD =
CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD);
- Args.add(RValue::get(This),
- RD ? C.getPointerType(C.getTypeDeclType(RD)) : C.VoidPtrTy);
+ Args.add(RValue::get(This), CGF.getTypes().DeriveThisType(RD, MD));
// If there is an implicit parameter (e.g. VTT), emit it.
if (ImplicitParam) {
@@ -326,9 +324,6 @@ RValue CodeGenFunction::EmitCXXMemberOrO
CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent()),
/*Alignment=*/CharUnits::Zero(), SkippedChecks);
- // FIXME: Uses of 'MD' past this point need to be audited. We may need to use
- // 'CalleeDecl' instead.
-
// C++ [class.virtual]p12:
// Explicit qualification with the scope operator (5.1) suppresses the
// virtual call mechanism.
@@ -337,7 +332,7 @@ RValue CodeGenFunction::EmitCXXMemberOrO
// because then we know what the type is.
bool UseVirtualCall = CanUseVirtualCall && !DevirtualizedMethod;
- if (const CXXDestructorDecl *Dtor = dyn_cast<CXXDestructorDecl>(MD)) {
+ if (const CXXDestructorDecl *Dtor = dyn_cast<CXXDestructorDecl>(CalleeDecl)) {
assert(CE->arg_begin() == CE->arg_end() &&
"Destructor shouldn't have explicit parameters");
assert(ReturnValue.isNull() && "Destructor shouldn't have return value");
@@ -347,26 +342,29 @@ RValue CodeGenFunction::EmitCXXMemberOrO
cast<CXXMemberCallExpr>(CE));
} else {
CGCallee Callee;
- if (getLangOpts().AppleKext && MD->isVirtual() && HasQualifier)
- Callee = BuildAppleKextVirtualCall(MD, Qualifier, Ty);
+ if (getLangOpts().AppleKext && Dtor->isVirtual() && HasQualifier)
+ Callee = BuildAppleKextVirtualCall(Dtor, Qualifier, Ty);
else if (!DevirtualizedMethod)
Callee = CGCallee::forDirect(
CGM.getAddrOfCXXStructor(Dtor, StructorType::Complete, FInfo, Ty),
GlobalDecl(Dtor, Dtor_Complete));
else {
- const CXXDestructorDecl *DDtor =
- cast<CXXDestructorDecl>(DevirtualizedMethod);
Callee = CGCallee::forDirect(
- CGM.GetAddrOfFunction(GlobalDecl(DDtor, Dtor_Complete), Ty),
- GlobalDecl(DDtor, Dtor_Complete));
+ CGM.GetAddrOfFunction(GlobalDecl(Dtor, Dtor_Complete), Ty),
+ GlobalDecl(Dtor, Dtor_Complete));
}
- EmitCXXMemberOrOperatorCall(
- CalleeDecl, Callee, ReturnValue, This.getPointer(),
- /*ImplicitParam=*/nullptr, QualType(), CE, nullptr);
+
+ EmitCXXDestructorCall(Dtor, Callee, This.getPointer(),
+ /*ImplicitParam=*/nullptr,
+ /*ImplicitParamTy=*/QualType(), nullptr,
+ getFromDtorType(Dtor_Complete));
}
return RValue::get(nullptr);
}
+ // FIXME: Uses of 'MD' past this point need to be audited. We may need to use
+ // 'CalleeDecl' instead.
+
CGCallee Callee;
if (const CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(MD)) {
Callee = CGCallee::forDirect(
Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=353181&r1=353180&r2=353181&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Tue Feb 5 08:05:50 2019
@@ -1051,9 +1051,9 @@ CodeGenFunction::generateObjCGetterBody(
// FIXME: We shouldn't need to get the function info here, the
// runtime already should have computed it to build the function.
llvm::CallBase *CallInstruction;
- RValue RV = EmitCall(
- getTypes().arrangeBuiltinFunctionCall(propType, args),
- callee, ReturnValueSlot(), args, &CallInstruction);
+ RValue RV = EmitCall(getTypes().arrangeBuiltinFunctionCall(
+ getContext().getObjCIdType(), args),
+ callee, ReturnValueSlot(), args, &CallInstruction);
if (llvm::CallInst *call = dyn_cast<llvm::CallInst>(CallInstruction))
call->setTailCall();
Modified: cfe/trunk/lib/CodeGen/CodeGenTypes.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenTypes.h?rev=353181&r1=353180&r2=353181&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenTypes.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenTypes.h Tue Feb 5 08:05:50 2019
@@ -182,6 +182,10 @@ public:
/// Convert clang calling convention to LLVM callilng convention.
unsigned ClangCallConvToLLVMCallConv(CallingConv CC);
+ /// Derives the 'this' type for codegen purposes, i.e. ignoring method CVR
+ /// qualification.
+ CanQualType DeriveThisType(const CXXRecordDecl *RD, const CXXMethodDecl *MD);
+
/// ConvertType - Convert type T into a llvm::Type.
llvm::Type *ConvertType(QualType T);
Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=353181&r1=353180&r2=353181&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Tue Feb 5 08:05:50 2019
@@ -1566,9 +1566,8 @@ void ItaniumCXXABI::EmitDestructorCall(C
Callee = CGCallee::forDirect(
CGM.getAddrOfCXXStructor(DD, getFromDtorType(Type)), GD);
- CGF.EmitCXXMemberOrOperatorCall(DD, Callee, ReturnValueSlot(),
- This.getPointer(), VTT, VTTTy,
- nullptr, nullptr);
+ CGF.EmitCXXDestructorCall(DD, Callee, This.getPointer(), VTT, VTTTy, nullptr,
+ getFromDtorType(Type));
}
void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT,
@@ -1766,9 +1765,8 @@ llvm::Value *ItaniumCXXABI::EmitVirtualD
CGCallee Callee =
CGCallee::forVirtual(CE, GlobalDecl(Dtor, DtorType), This, Ty);
- CGF.EmitCXXMemberOrOperatorCall(Dtor, Callee, ReturnValueSlot(),
- This.getPointer(), /*ImplicitParam=*/nullptr,
- QualType(), CE, nullptr);
+ CGF.EmitCXXDestructorCall(Dtor, Callee, This.getPointer(), nullptr,
+ QualType(), nullptr, getFromDtorType(DtorType));
return nullptr;
}
Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=353181&r1=353180&r2=353181&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Feb 5 08:05:50 2019
@@ -10747,7 +10747,8 @@ buildDeclareReductionRef(Sema &SemaRef,
return D;
return nullptr;
}))
- return SemaRef.BuildDeclRefExpr(VD, Ty, VK_LValue, Loc);
+ return SemaRef.BuildDeclRefExpr(VD, VD->getType().getNonReferenceType(),
+ VK_LValue, Loc);
if (auto *VD = filterLookupForUDR<ValueDecl *>(
Lookups, [&SemaRef, Ty, Loc](ValueDecl *D) -> ValueDecl * {
if (!D->isInvalidDecl() &&
@@ -10765,7 +10766,8 @@ buildDeclareReductionRef(Sema &SemaRef,
/*DiagID=*/0) !=
Sema::AR_inaccessible) {
SemaRef.BuildBasePathArray(Paths, BasePath);
- return SemaRef.BuildDeclRefExpr(VD, Ty, VK_LValue, Loc);
+ return SemaRef.BuildDeclRefExpr(
+ VD, VD->getType().getNonReferenceType(), VK_LValue, Loc);
}
}
}
Modified: cfe/trunk/test/CodeGenObjC/getter-property-mismatch.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/getter-property-mismatch.m?rev=353181&r1=353180&r2=353181&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/getter-property-mismatch.m (original)
+++ cfe/trunk/test/CodeGenObjC/getter-property-mismatch.m Tue Feb 5 08:05:50 2019
@@ -15,6 +15,4 @@
// CHECK: [[CALL:%.*]] = tail call i8* @objc_getProperty
// CHECK: [[ONE:%.*]] = bitcast i8* [[CALL:%.*]] to [[T1:%.*]]*
-// CHECK: [[TWO:%.*]] = bitcast [[T1]]* [[ONE]] to [[T2:%.*]]*
-// CHECK: ret [[T2]]* [[TWO]]
-
+// CHECK: ret [[T1]]* [[ONE]]
More information about the cfe-commits
mailing list