[Verifier] Assert gc_relocate always return a pointer type

[• chenli]

Summary

Add an assertion in verifier.cpp to make sure gc_relocate relocate a gc pointer, and its return type has the same address space with the relocated pointer.

[sanjoy]

I think we should assert that the address spaces of the source and destination pointers match. Also, please add a test case.

[pgavlin]

+1 to Sanjoy's comments. Thanks for doing this.

[• chenli]

Add an assert for address space and testcase.

[pgavlin]

Please add a test case for the first assert as well (i.e. a case where relocate is returning a non-pointer type).

[• chenli]

In D9695#172229, @pgavlin wrote:

Please add a test case for the first assert as well (i.e. a case where relocate is returning a non-pointer type).

Will do it now

[• chenli]

In D9695#172229, @pgavlin wrote:

Please add a test case for the first assert as well (i.e. a case where relocate is returning a non-pointer type).

Hi Pat,

After some digging, I found that the first assert is actually unnecessary.
gc_relocate is defined to return a pointer type in intrinsics.td:

def int_experimental_gc_relocate : Intrinsic<[llvm_anyptr_ty],
                                [llvm_i32_ty, llvm_i32_ty, llvm_i32_ty]>;

and verifier would verify it through VerifyIntrinsicType(). I think I will just modify the first assert to check the relocated value must be pointer type.

I will add two more tests. One to assert VerifyIntrinsicType() fails when gc_relocate's return type is not pointer type. Another one to assert relocated value is pointer type.

Does this sound good to you?

[pgavlin]

That sounds fine to me. Thanks for digging in a bit more!

[• chenli]

Add test cases as Pat requested.

[pgavlin]

LGTM. Thanks for doing this.

lib/IR/Verifier.cpp

Assert(GCParamArgsStart <= BaseIndex && BaseIndex < GCParamArgsEnd,
            "'gc parameters' section of the statepoint call",
            &CI);
     Assert(GCParamArgsStart <= DerivedIndex && DerivedIndex < GCParamArgsEnd,
            "gc.relocate: statepoint derived index doesn't fall within the "
            "'gc parameters' section of the statepoint call",
            &CI);
 
     // gc_relocate does not need to be the same type as the relocated pointer.
-    // It can casted to the correct type later if it's desired
+    // It can casted to the correct type later if it's desired. However, it
+    // must be a pointer type
+    Assert(CI.getType()->isPointerTy(),
+     "gc.relocate: relocating a pointer should always return a pointer", &CI);
+
     break;
   }
   };
 }
 
 /// \brief Carefully grab the subprogram from a local scope.
 ///
 /// This carefully grabs the subprogram from a local scope, avoiding the