[OpaquePtr] Verify Opaque pointer in function parameter

[zequanwu]

Summary

Verifying opaque pointer as function parameter when using with byval, byref,
inalloca, preallocated.

[aeubanks]

can you add some negative tests for this? e.g. make sure you see the proper message for something like

call void @f(ptr inalloca(%somenotsizedtype) %a)

there are some negative verifier tests in llvm/test/Verifier, search RUN: not opt

llvm/lib/IR/Verifier.cpp

1869

we still need a Visited in the new code to account for recursive types

we can delete the checks below for isSized() and always use your new code, then only check that the pointee type and inalloca/etc type match for non-opaque pointers

[zequanwu]

Update.

[aeubanks]

llvm/lib/IR/Verifier.cpp

1869

we still should keep this check that for non-opaque pointers, the element type matches the attribute type, I only meant to make the check for sized common between opaque and non-opaque pointers (and we should keep all the tests that were deleted)

[zequanwu]

Update.

[aeubanks]

lgtm

llvm/lib/IR/Verifier.cpp

void Verifier::verifyParameterAttrs(AttributeSet Attrs, Type *Ty,
 
   AttrBuilder IncompatibleAttrs = AttributeFuncs::typeIncompatible(Ty);
   Assert(!AttrBuilder(Attrs).overlaps(IncompatibleAttrs),
          "Wrong types for attribute: " +
              AttributeSet::get(Context, IncompatibleAttrs).getAsString(),
          V);
 
   if (PointerType *PTy = dyn_cast<PointerType>(Ty)) {
-    SmallPtrSet<Type*, 4> Visited;
-    if (!PTy->getElementType()->isSized(&Visited)) {
-      Assert(!Attrs.hasAttribute(Attribute::ByVal) &&
-             !Attrs.hasAttribute(Attribute::ByRef) &&
-             !Attrs.hasAttribute(Attribute::InAlloca) &&
-             !Attrs.hasAttribute(Attribute::Preallocated),
-             "Attributes 'byval', 'byref', 'inalloca', and 'preallocated' do not "
-             "support unsized types!",
-             V);
+    if (Attrs.hasAttribute(Attribute::ByVal)) {
+      SmallPtrSet<Type *, 4> Visited;
+      Assert(Attrs.getByValType()->isSized(&Visited),
+             "Attribute 'byval' does not support unsized types!", V);
+    }
+    if (Attrs.hasAttribute(Attribute::ByRef)) {
+      SmallPtrSet<Type *, 4> Visited;
+      Assert(Attrs.getByRefType()->isSized(&Visited),
+             "Attribute 'byref' does not support unsized types!", V);
+    }
+    if (Attrs.hasAttribute(Attribute::InAlloca)) {
+      SmallPtrSet<Type *, 4> Visited;
+      Assert(Attrs.getInAllocaType()->isSized(&Visited),
+             "Attribute 'inalloca' does not support unsized types!", V);
+    }
+    if (Attrs.hasAttribute(Attribute::Preallocated)) {
+      SmallPtrSet<Type *, 4> Visited;
+      Assert(Attrs.getPreallocatedType()->isSized(&Visited),
+             "Attribute 'preallocated' does not support unsized types!", V);
     }
+    if (!PTy->isOpaque()) {
       if (!isa<PointerType>(PTy->getElementType()))
         Assert(!Attrs.hasAttribute(Attribute::SwiftError),
                "Attribute 'swifterror' only applies to parameters "
                "with pointer to pointer type!",
                V);
-
       if (Attrs.hasAttribute(Attribute::ByRef)) {
         Assert(Attrs.getByRefType() == PTy->getElementType(),
                "Attribute 'byref' type does not match parameter!", V);
       }
 
       if (Attrs.hasAttribute(Attribute::ByVal) && Attrs.getByValType()) {
         Assert(Attrs.getByValType() == PTy->getElementType(),
                "Attribute 'byval' type does not match parameter!", V);
       }
 
       if (Attrs.hasAttribute(Attribute::Preallocated)) {
         Assert(Attrs.getPreallocatedType() == PTy->getElementType(),
                "Attribute 'preallocated' type does not match parameter!", V);
       }
 
       if (Attrs.hasAttribute(Attribute::InAlloca)) {
         Assert(Attrs.getInAllocaType() == PTy->getElementType(),
                "Attribute 'inalloca' type does not match parameter!", V);
       }
+    }
   } else {
     Assert(!Attrs.hasAttribute(Attribute::ByVal),
            "Attribute 'byval' only applies to parameters with pointer type!",
            V);
     Assert(!Attrs.hasAttribute(Attribute::ByRef),
            "Attribute 'byref' only applies to parameters with pointer type!",
            V);
     Assert(!Attrs.hasAttribute(Attribute::SwiftError),

llvm/test/Assembler/invalid-byval-type3.ll

 ; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
 
-; CHECK: Attributes 'byval'{{.*}} do not support unsized types!
+; CHECK: Attribute 'byval' does not support unsized types!
 declare void @foo(void()* byval(void()))

llvm/test/Verifier/byref.ll

 ; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
 
 ; CHECK: Attribute 'byref' type does not match parameter!
 ; CHECK-NEXT: void (i32*)* @byref_mismatched_pointee_type0
 define void @byref_mismatched_pointee_type0(i32* byref(i8)) {
   ret void
 }
 
 ; CHECK: Attribute 'byref' type does not match parameter!
 ; CHECK-NEXT: void (i8*)* @byref_mismatched_pointee_type1
 define void @byref_mismatched_pointee_type1(i8* byref(i32)) {
   ret void
 }
 
 %opaque.ty = type opaque
 
-; CHECK: Attributes 'byval', 'byref', 'inalloca', and 'preallocated' do not support unsized types!
+; CHECK: Attribute 'byref' does not support unsized types!
 ; CHECK-NEXT: void (%opaque.ty*)* @byref_unsized
 define void @byref_unsized(%opaque.ty* byref(%opaque.ty)) {
   ret void
 }
 
 ; CHECK: Attributes 'byval', 'inalloca', 'preallocated', 'inreg', 'nest', 'byref', and 'sret' are incompatible!
 ; CHECK-NEXT: void (i32*)* @byref_byval
 define void @byref_byval(i32* byref(i32) byval(i32)) {

llvm/test/Verifier/inalloca1.ll

 
 declare void @d(i64* nest inalloca(i64) %p)
 ; CHECK: Attributes {{.*}} are incompatible
 
 declare void @e(i64* readonly inalloca(i64) %p)
 ; CHECK: Attributes {{.*}} are incompatible
 
 declare void @f(void ()* inalloca(void()) %p)
-; CHECK: do not support unsized types
+; CHECK: Attribute 'inalloca' does not support unsized types
 
 declare void @g(i32* inalloca(i32) %p, i32 %p2)
 ; CHECK: inalloca isn't on the last parameter!
 
 ; CHECK: Attribute 'inalloca' type does not match parameter!
 ; CHECK-NEXT: void (i32*)* @inalloca_mismatched_pointee_type0
 define void @inalloca_mismatched_pointee_type0(i32* inalloca(i8)) {
   ret void
 }
 
 ; CHECK: Wrong types for attribute:
 ; CHECK-NEXT: void (i8)* @inalloca_not_pointer
 define void @inalloca_not_pointer(i8 byref(i8)) {
   ret void
 }

llvm/test/Verifier/opaque-ptr-invalid.ll

+; RUN: not opt -verify < %s 2>&1 | FileCheck %s
+
+; CHECK: Attribute 'inalloca' does not support unsized types!
+; CHECK-NEXT: void (ptr)* @f
+define void @f(ptr inalloca(token)) {
+    ret void
+}