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.

llvm-svn: 296076

clang/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;

clang/lib/CodeGen/CGCall.cpp

@@ -101,39 +101,64 @@ CodeGenTypes::arrangeFreeFunctionType(CanQual<FunctionNoProtoType> FTNP) {
                                  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> &paramInfos,
+                                        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> &paramInfos,
-                                 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 &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<FunctionProtoType::ExtParameterInfo> &paramInfos,
-                                        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) {

clang/lib/Sema/SemaType.cpp

@@ -4488,6 +4488,11 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
             HasAnyInterestingExtParameterInfos = true;
           }
 
+          if (Param->hasAttr<PassObjectSizeAttr>()) {
+            ExtParameterInfos[i] = ExtParameterInfos[i].withHasPassObjectSize();
+            HasAnyInterestingExtParameterInfos = true;
+          }
+
           ParamTys.push_back(ParamTy);
         }
 

clang/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<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(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;
 }
 

clang/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 {

clang/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;
+}
+}

clang/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, "");