Author: gbiv Date: Thu Feb 23 20:49:47 2017 New Revision: 296076 URL: http://llvm.org/viewvc/llvm-project?rev=296076&view=rev Log: Represent pass_object_size attrs in ExtParameterInfo
The goal of this is to fix a bug in modules where we'd merge FunctionDecls that differed in their pass_object_size attributes. Since we can overload on the presence of pass_object_size attributes, this behavior is incorrect. We don't represent `N` in `pass_object_size(N)` as part of ExtParameterInfo, since it's an error to overload solely on the value of N. This means that we have a bug if we have two modules that declare functions that differ only in their pass_object_size attrs, like so: // In module A, from a.h void foo(char *__attribute__((pass_object_size(0)))); // In module B, from b.h void foo(char *__attribute__((pass_object_size(1)))); // In module C, in main.c #include "a.h" #include "b.h" At the moment, we'll merge the foo decls, when we should instead emit a diagnostic about an invalid overload. We seem to have similar (silent) behavior if we overload only on the return type of `foo` instead; I'll try to find a good place to put a FIXME (or I'll just file a bug) soon. This patch also fixes a bug where we'd not output the proper extended parameter info for declarations with pass_object_size attrs. Modified: cfe/trunk/include/clang/AST/Type.h cfe/trunk/lib/CodeGen/CGCall.cpp cfe/trunk/lib/Sema/SemaType.cpp cfe/trunk/lib/Serialization/ASTReaderDecl.cpp cfe/trunk/test/CodeGenObjCXX/arc-attrs-abi.mm cfe/trunk/test/Modules/Inputs/overloadable-attrs/a.h cfe/trunk/test/Modules/overloadable-attrs.cpp Modified: cfe/trunk/include/clang/AST/Type.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=296076&r1=296075&r2=296076&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/Type.h (original) +++ cfe/trunk/include/clang/AST/Type.h Thu Feb 23 20:49:47 2017 @@ -3116,9 +3116,11 @@ public: class ExtParameterInfo { enum { ABIMask = 0x0F, - IsConsumed = 0x10 + IsConsumed = 0x10, + HasPassObjSize = 0x20, }; unsigned char Data; + public: ExtParameterInfo() : Data(0) {} @@ -3147,6 +3149,15 @@ public: return copy; } + bool hasPassObjectSize() const { + return Data & HasPassObjSize; + } + ExtParameterInfo withHasPassObjectSize() const { + ExtParameterInfo Copy = *this; + Copy.Data |= HasPassObjSize; + return Copy; + } + unsigned char getOpaqueValue() const { return Data; } static ExtParameterInfo getFromOpaqueValue(unsigned char data) { ExtParameterInfo result; Modified: cfe/trunk/lib/CodeGen/CGCall.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=296076&r1=296075&r2=296076&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGCall.cpp (original) +++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Feb 23 20:49:47 2017 @@ -101,39 +101,64 @@ CodeGenTypes::arrangeFreeFunctionType(Ca FTNP->getExtInfo(), {}, RequiredArgs(0)); } -/// Adds the formal parameters in FPT to the given prefix. If any parameter in +static void addExtParameterInfosForCall( + llvm::SmallVectorImpl<FunctionProtoType::ExtParameterInfo> ¶mInfos, + const FunctionProtoType *proto, + unsigned prefixArgs, + unsigned totalArgs) { + assert(proto->hasExtParameterInfos()); + assert(paramInfos.size() <= prefixArgs); + assert(proto->getNumParams() + prefixArgs <= totalArgs); + + paramInfos.reserve(totalArgs); + + // Add default infos for any prefix args that don't already have infos. + paramInfos.resize(prefixArgs); + + // Add infos for the prototype. + for (const auto &ParamInfo : proto->getExtParameterInfos()) { + paramInfos.push_back(ParamInfo); + // pass_object_size params have no parameter info. + if (ParamInfo.hasPassObjectSize()) + paramInfos.emplace_back(); + } + + assert(paramInfos.size() <= totalArgs && + "Did we forget to insert pass_object_size args?"); + // Add default infos for the variadic and/or suffix arguments. + paramInfos.resize(totalArgs); +} + +/// Adds the formal paramaters in FPT to the given prefix. If any parameter in /// FPT has pass_object_size attrs, then we'll add parameters for those, too. static void appendParameterTypes(const CodeGenTypes &CGT, SmallVectorImpl<CanQualType> &prefix, SmallVectorImpl<FunctionProtoType::ExtParameterInfo> ¶mInfos, - CanQual<FunctionProtoType> FPT, - const FunctionDecl *FD) { - // Fill out paramInfos. - if (FPT->hasExtParameterInfos() || !paramInfos.empty()) { - assert(paramInfos.size() <= prefix.size()); - auto protoParamInfos = FPT->getExtParameterInfos(); - paramInfos.reserve(prefix.size() + protoParamInfos.size()); - paramInfos.resize(prefix.size()); - paramInfos.append(protoParamInfos.begin(), protoParamInfos.end()); - } - - // Fast path: unknown target. - if (FD == nullptr) { + CanQual<FunctionProtoType> FPT) { + // Fast path: don't touch param info if we don't need to. + if (!FPT->hasExtParameterInfos()) { + assert(paramInfos.empty() && + "We have paramInfos, but the prototype doesn't?"); prefix.append(FPT->param_type_begin(), FPT->param_type_end()); return; } - // In the vast majority cases, we'll have precisely FPT->getNumParams() + unsigned PrefixSize = prefix.size(); + // In the vast majority of cases, we'll have precisely FPT->getNumParams() // parameters; the only thing that can change this is the presence of // pass_object_size. So, we preallocate for the common case. prefix.reserve(prefix.size() + FPT->getNumParams()); - assert(FD->getNumParams() == FPT->getNumParams()); + auto ExtInfos = FPT->getExtParameterInfos(); + assert(ExtInfos.size() == FPT->getNumParams()); for (unsigned I = 0, E = FPT->getNumParams(); I != E; ++I) { prefix.push_back(FPT->getParamType(I)); - if (FD->getParamDecl(I)->hasAttr<PassObjectSizeAttr>()) + if (ExtInfos[I].hasPassObjectSize()) prefix.push_back(CGT.getContext().getSizeType()); } + + addExtParameterInfosForCall(paramInfos, FPT.getTypePtr(), PrefixSize, + prefix.size()); } /// Arrange the LLVM function layout for a value of the given function @@ -147,7 +172,7 @@ arrangeLLVMFunctionInfo(CodeGenTypes &CG RequiredArgs Required = RequiredArgs::forPrototypePlus(FTP, prefix.size(), FD); // FIXME: Kill copy. - appendParameterTypes(CGT, prefix, paramInfos, FTP, FD); + appendParameterTypes(CGT, prefix, paramInfos, FTP); CanQualType resultType = FTP->getReturnType().getUnqualifiedType(); return CGT.arrangeLLVMFunctionInfo(resultType, instanceMethod, @@ -286,7 +311,7 @@ CodeGenTypes::arrangeCXXStructorDeclarat // Add the formal parameters. if (PassParams) - appendParameterTypes(*this, argTypes, paramInfos, FTP, MD); + appendParameterTypes(*this, argTypes, paramInfos, FTP); CGCXXABI::AddedStructorArgs AddedArgs = TheCXXABI.buildStructorSignature(MD, Type, argTypes); @@ -331,26 +356,6 @@ getArgTypesForDeclaration(ASTContext &ct return argTypes; } -static void addExtParameterInfosForCall( - llvm::SmallVectorImpl<FunctionProtoType::ExtParameterInfo> ¶mInfos, - const FunctionProtoType *proto, - unsigned prefixArgs, - unsigned totalArgs) { - assert(proto->hasExtParameterInfos()); - assert(paramInfos.size() <= prefixArgs); - assert(proto->getNumParams() + prefixArgs <= totalArgs); - - // Add default infos for any prefix args that don't already have infos. - paramInfos.resize(prefixArgs); - - // Add infos for the prototype. - auto protoInfos = proto->getExtParameterInfos(); - paramInfos.append(protoInfos.begin(), protoInfos.end()); - - // Add default infos for the variadic arguments. - paramInfos.resize(totalArgs); -} - static llvm::SmallVector<FunctionProtoType::ExtParameterInfo, 16> getExtParameterInfosForCall(const FunctionProtoType *proto, unsigned prefixArgs, unsigned totalArgs) { Modified: cfe/trunk/lib/Sema/SemaType.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=296076&r1=296075&r2=296076&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaType.cpp (original) +++ cfe/trunk/lib/Sema/SemaType.cpp Thu Feb 23 20:49:47 2017 @@ -4488,6 +4488,11 @@ static TypeSourceInfo *GetFullTypeForDec HasAnyInterestingExtParameterInfos = true; } + if (Param->hasAttr<PassObjectSizeAttr>()) { + ExtParameterInfos[i] = ExtParameterInfos[i].withHasPassObjectSize(); + HasAnyInterestingExtParameterInfos = true; + } + ParamTys.push_back(ParamTy); } Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=296076&r1=296075&r2=296076&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Thu Feb 23 20:49:47 2017 @@ -2664,9 +2664,12 @@ static bool isSameTemplateParameterList( } /// Determine whether the attributes we can overload on are identical for A and -/// B. Expects A and B to (otherwise) have the same type. +/// B. Will ignore any overloadable attrs represented in the type of A and B. static bool hasSameOverloadableAttrs(const FunctionDecl *A, const FunctionDecl *B) { + // Note that pass_object_size attributes are represented in the function's + // ExtParameterInfo, so we don't need to check them here. + SmallVector<const EnableIfAttr *, 4> AEnableIfs; // Since this is an equality check, we can ignore that enable_if attrs show up // in reverse order. @@ -2696,8 +2699,6 @@ static bool hasSameOverloadableAttrs(con return false; } - // FIXME: This doesn't currently consider pass_object_size attributes, since - // we aren't guaranteed that A and B have valid parameter lists yet. return true; } Modified: cfe/trunk/test/CodeGenObjCXX/arc-attrs-abi.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/arc-attrs-abi.mm?rev=296076&r1=296075&r2=296076&view=diff ============================================================================== --- cfe/trunk/test/CodeGenObjCXX/arc-attrs-abi.mm (original) +++ cfe/trunk/test/CodeGenObjCXX/arc-attrs-abi.mm Thu Feb 23 20:49:47 2017 @@ -5,14 +5,15 @@ // caused assertions to fire. Hence, minimal CHECKs. struct VirtualBase { - VirtualBase(__attribute__((ns_consumed)) id x); + VirtualBase(__attribute__((ns_consumed)) id x, + void * __attribute__((pass_object_size(0)))); }; struct WithVirtualBase : virtual VirtualBase { WithVirtualBase(__attribute__((ns_consumed)) id x); }; WithVirtualBase::WithVirtualBase(__attribute__((ns_consumed)) id x) - : VirtualBase(x) {} + : VirtualBase(x, (void *)0) {} struct VirtualBase2 { Modified: cfe/trunk/test/Modules/Inputs/overloadable-attrs/a.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/overloadable-attrs/a.h?rev=296076&r1=296075&r2=296076&view=diff ============================================================================== --- cfe/trunk/test/Modules/Inputs/overloadable-attrs/a.h (original) +++ cfe/trunk/test/Modules/Inputs/overloadable-attrs/a.h Thu Feb 23 20:49:47 2017 @@ -14,3 +14,15 @@ constexpr int fn4(int i) __attribute__(( constexpr int fn5(int i) __attribute__((enable_if(i, ""))) { return 1; } constexpr int fn5(int i) { return 0; } } + +namespace pass_object_size_attrs { +constexpr int fn1(void *const a __attribute__((pass_object_size(0)))) { + return 1; +} +constexpr int fn1(void *const a) { return 0; } + +constexpr int fn2(void *const a) { return 0; } +constexpr int fn2(void *const a __attribute__((pass_object_size(0)))) { + return 1; +} +} Modified: cfe/trunk/test/Modules/overloadable-attrs.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/overloadable-attrs.cpp?rev=296076&r1=296075&r2=296076&view=diff ============================================================================== --- cfe/trunk/test/Modules/overloadable-attrs.cpp (original) +++ cfe/trunk/test/Modules/overloadable-attrs.cpp Thu Feb 23 20:49:47 2017 @@ -17,3 +17,6 @@ static_assert(enable_if_attrs::fn4(0) == static_assert(enable_if_attrs::fn4(1) == 1, ""); static_assert(enable_if_attrs::fn5(0) == 0, ""); static_assert(enable_if_attrs::fn5(1) == 1, ""); + +static_assert(pass_object_size_attrs::fn1(nullptr) == 1, ""); +static_assert(pass_object_size_attrs::fn2(nullptr) == 1, ""); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits