From 8d92d799c7fd1127f4fa004c26dca45e7eab8ed0 Mon Sep 17 00:00:00 2001 From: George Burgess IV Date: Fri, 24 Feb 2017 02:49:47 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@296076 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Type.h | 13 +++- lib/CodeGen/CGCall.cpp | 83 ++++++++++++---------- lib/Sema/SemaType.cpp | 5 ++ lib/Serialization/ASTReaderDecl.cpp | 7 +- test/CodeGenObjCXX/arc-attrs-abi.mm | 5 +- test/Modules/Inputs/overloadable-attrs/a.h | 12 ++++ test/Modules/overloadable-attrs.cpp | 3 + 7 files changed, 83 insertions(+), 45 deletions(-) diff --git a/include/clang/AST/Type.h b/include/clang/AST/Type.h index f88d0435b2..9c1d110067 100644 --- a/include/clang/AST/Type.h +++ b/include/clang/AST/Type.h @@ -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; diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp index 428098da26..e389e4224e 100644 --- a/lib/CodeGen/CGCall.cpp +++ b/lib/CodeGen/CGCall.cpp @@ -101,39 +101,64 @@ CodeGenTypes::arrangeFreeFunctionType(CanQual FTNP) { FTNP->getExtInfo(), {}, RequiredArgs(0)); } -/// Adds the formal parameters in FPT to the given prefix. If any parameter in +static void addExtParameterInfosForCall( + llvm::SmallVectorImpl ¶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 &prefix, SmallVectorImpl ¶mInfos, - CanQual 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 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()) + 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 &CGT, bool instanceMethod, 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::arrangeCXXStructorDeclaration(const CXXMethodDecl *MD, // 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 &ctx, const FunctionArgList &args) { return argTypes; } -static void addExtParameterInfosForCall( - llvm::SmallVectorImpl ¶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 getExtParameterInfosForCall(const FunctionProtoType *proto, unsigned prefixArgs, unsigned totalArgs) { diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 6300aa6f1a..4fe0cf337f 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -4488,6 +4488,11 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state, HasAnyInterestingExtParameterInfos = true; } + if (Param->hasAttr()) { + ExtParameterInfos[i] = ExtParameterInfos[i].withHasPassObjectSize(); + HasAnyInterestingExtParameterInfos = true; + } + ParamTys.push_back(ParamTy); } diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index 0f43e9fcf6..7971c30b48 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -2664,9 +2664,12 @@ static bool isSameTemplateParameterList(const TemplateParameterList *X, } /// 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 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(const FunctionDecl *A, 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; } diff --git a/test/CodeGenObjCXX/arc-attrs-abi.mm b/test/CodeGenObjCXX/arc-attrs-abi.mm index 22ef7a55d3..14bdf8be30 100644 --- a/test/CodeGenObjCXX/arc-attrs-abi.mm +++ b/test/CodeGenObjCXX/arc-attrs-abi.mm @@ -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 { diff --git a/test/Modules/Inputs/overloadable-attrs/a.h b/test/Modules/Inputs/overloadable-attrs/a.h index e7dabfbe58..1af769dad3 100644 --- a/test/Modules/Inputs/overloadable-attrs/a.h +++ b/test/Modules/Inputs/overloadable-attrs/a.h @@ -14,3 +14,15 @@ constexpr int fn4(int i) __attribute__((enable_if(i, ""))) { return 1; } 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; +} +} diff --git a/test/Modules/overloadable-attrs.cpp b/test/Modules/overloadable-attrs.cpp index ae9bd73f99..38c9558fe8 100644 --- a/test/Modules/overloadable-attrs.cpp +++ b/test/Modules/overloadable-attrs.cpp @@ -17,3 +17,6 @@ static_assert(enable_if_attrs::fn4(0) == 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, ""); -- 2.50.1