[PATCH] D17462: Fix a codegen bug for variadic functions with pass_object_size params

[George Burgess IV via cfe-commits]

george.burgess.iv created this revision.
george.burgess.iv added a reviewer: rsmith.
george.burgess.iv added subscribers: srhines, cfe-commits.

Currently, we get assertion failures/segfaults for variadic functions with `pass_object_size` params, e.g.:

```
void foo(void *const __attribute__((pass_object_size(0))) a, ...) {}
```

This is because we didn't consider the parameter injected by `pass_object_size` "required" when generating the `CGFunctionInfo` for `foo`. This patch teaches clang that said injected params are required.

(N.B. Only one user of `appendParameterTypes` is patched here because the other user, `arrangeCXXStructorDeclaration`, considers all args to be required).

http://reviews.llvm.org/D17462

Files:
  lib/CodeGen/CGCall.cpp
  test/CodeGen/pass-object-size.c

Index: test/CodeGen/pass-object-size.c
===================================================================
--- test/CodeGen/pass-object-size.c
+++ test/CodeGen/pass-object-size.c
@@ -351,3 +351,18 @@
   ObjectSize0(++p);
   ObjectSize0(p++);
 }
+
+// There was a bug where variadic functions with pass_object_size would cause
+// problems in the form of failed assertions.
+void my_sprintf(char *const c __attribute__((pass_object_size(0))), ...) {}
+
+// CHECK-LABEL: define void @test14
+void test14(char *c) {
+  // CHECK: @llvm.objectsize
+  // CHECK: call void (i8*, i64, ...) @my_sprintf
+  my_sprintf(c);
+
+  // CHECK: @llvm.objectsize
+  // CHECK: call void (i8*, i64, ...) @my_sprintf
+  my_sprintf(c, 1, 2, 3);
+}
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -96,27 +96,36 @@
 
 /// 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.
+///
+/// If NumSynthesizedParams is non-null, the number of pass_object_size
+/// parameters that we encounter will be stored in it.
 static void appendParameterTypes(const CodeGenTypes &CGT,
                                  SmallVectorImpl<CanQualType> &prefix,
                                  const CanQual<FunctionProtoType> &FPT,
-                                 const FunctionDecl *FD) {
-  // Fast path: unknown target.
-  if (FD == nullptr) {
+                                 const FunctionDecl *FD,
+                                 unsigned *NumSynthesizedParams = nullptr) {
+  unsigned NumPassObjSizeParams = 0;
+  if (!FD) {
+    // Fast path: unknown target.
     prefix.append(FPT->param_type_begin(), FPT->param_type_end());
-    return;
+  } else {
+    // 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());
+    for (unsigned I = 0, E = FPT->getNumParams(); I != E; ++I) {
+      prefix.push_back(FPT->getParamType(I));
+      if (FD->getParamDecl(I)->hasAttr<PassObjectSizeAttr>()) {
+        ++NumPassObjSizeParams;
+        prefix.push_back(CGT.getContext().getSizeType());
+      }
+    }
   }
 
-  // In the vast majority 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());
-  for (unsigned I = 0, E = FPT->getNumParams(); I != E; ++I) {
-    prefix.push_back(FPT->getParamType(I));
-    if (FD->getParamDecl(I)->hasAttr<PassObjectSizeAttr>())
-      prefix.push_back(CGT.getContext().getSizeType());
-  }
+  if (NumSynthesizedParams)
+    *NumSynthesizedParams = NumPassObjSizeParams;
 }
 
 /// Arrange the LLVM function layout for a value of the given function
@@ -126,13 +135,16 @@
                         SmallVectorImpl<CanQualType> &prefix,
                         CanQual<FunctionProtoType> FTP,
                         const FunctionDecl *FD) {
-  RequiredArgs required = RequiredArgs::forPrototypePlus(FTP, prefix.size());
+  unsigned StartParams = prefix.size();
+  unsigned SynthesizedParams;
   // FIXME: Kill copy.
-  appendParameterTypes(CGT, prefix, FTP, FD);
+  appendParameterTypes(CGT, prefix, FTP, FD, &SynthesizedParams);
+  RequiredArgs Required =
+      RequiredArgs::forPrototypePlus(FTP, StartParams + SynthesizedParams);
   CanQualType resultType = FTP->getReturnType().getUnqualifiedType();
   return CGT.arrangeLLVMFunctionInfo(resultType, instanceMethod,
                                      /*chainCall=*/false, prefix,
-                                     FTP->getExtInfo(), required);
+                                     FTP->getExtInfo(), Required);
 }
 
 /// Arrange the argument and result information for a value of the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D17462.48538.patch
Type: text/x-patch
Size: 4171 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160219/d8df25bd/attachment.bin>