[Verifier] Check that calls' arguments match the callee's byval/inalloca/preallocated

[aeubanks]

Summary

Previously we only checked this if the arguments had the attributes, but
not if the callee had the attributes.

As examples, there are a number of tests missing attributes on calls.

[dblaikie]

Though does this break bitcode backwards compatibility, or does the bitcode not have this ambiguity/duplication?

While the textual IR doesn't have backcompat issues - maybe it'd be easy enough for the textual IR parser to correct this issue by checking the declaration and attaching the right attribute value? Though not necessary by any means.

llvm/lib/IR/Verifier.cpp

3274–3277

Should this be in a separate patch? (looks not strictly related/required in this patch)

[aeubanks]

remove unrelated change
update some more tests

[aeubanks]

I think the bitcode and textual representation both allow this issue.
But based on the fact that so much of the checking for byval and related attributes is on the arguments rather than the parameters, IMO it's always been intended to have the attributes at least on the parameters. Users like Clang have always been properly emitting byval on the parameters (no check-clang tests regressed). This patch is just filling in a hole that should always have been filled in. But maybe someone else should chime in.

[dblaikie]

In D101725#2732577, @aeubanks wrote:

I think the bitcode and textual representation both allow this issue.
But based on the fact that so much of the checking for byval and related attributes is on the arguments rather than the parameters, IMO it's always been intended to have the attributes at least on the parameters. Users like Clang have always been properly emitting byval on the parameters (no check-clang tests regressed). This patch is just filling in a hole that should always have been filled in. But maybe someone else should chime in.

The argument is that this was always invalid, but a bug that it wasn't detected, so it doesn't need to be upgraded when reading IR, it's OK for that to fail the verifier)

Compared to an argument that it was valid, is now invalid, so needs to be upgraded to ensure bitcode compatibility?

Do you think it'd be especially complicated to auto-upgrade?

[nikic]

FWIW I think the that we don't auto-upgrade textual IR as a matter of general policy. The only exception that comes to mind are align annotations on load/store, and those are really more a matter of the annotation being optional in textual IR than a real auto-upgrade.

[aeubanks]

In D101725#2732620, @dblaikie wrote:

In D101725#2732577, @aeubanks wrote:

I think the bitcode and textual representation both allow this issue.
But based on the fact that so much of the checking for byval and related attributes is on the arguments rather than the parameters, IMO it's always been intended to have the attributes at least on the parameters. Users like Clang have always been properly emitting byval on the parameters (no check-clang tests regressed). This patch is just filling in a hole that should always have been filled in. But maybe someone else should chime in.

The argument is that this was always invalid, but a bug that it wasn't detected, so it doesn't need to be upgraded when reading IR, it's OK for that to fail the verifier)

Compared to an argument that it was valid, is now invalid, so needs to be upgraded to ensure bitcode compatibility?

Do you think it'd be especially complicated to auto-upgrade?

Yeah my argument is that it was always invalid.

I'm sure it's possible to auto-upgrade bitcode, but that would have non-zero cost for every call we read from bitcode. IMO it's not worth it if this was always supposed to be invalid.

Maybe we should ask llvm-dev for more opinions?

[rnk]

I need more time to read into this, but my first thought is, we can't do this. People can write code that does stuff like:

struct Byval { int x; } bv;
void takeByval(Byval);
void foo(bool isByRef, void (*fp)()) {
  if (isByRef) {  ((void(*)(ByVal*))fp)(&bv); }
  else { ((void(*)(ByVal))fp)(bv); }
}
void bar() {
  foo(false, &takeByval);
}

Inlining foo into bar and simplifying would lead to a verifier failure, no?

This is for example why the verifier doesn't check that calling conventions match:
https://llvm.org/docs/FAQ.html#why-does-instcombine-simplifycfg-turn-a-call-to-a-function-with-a-mismatched-calling-convention-into-unreachable-why-not-make-the-verifier-reject-it

[dblaikie]

In D101725#2734397, @aeubanks wrote:

In D101725#2732620, @dblaikie wrote:

In D101725#2732577, @aeubanks wrote:

I think the bitcode and textual representation both allow this issue.
But based on the fact that so much of the checking for byval and related attributes is on the arguments rather than the parameters, IMO it's always been intended to have the attributes at least on the parameters. Users like Clang have always been properly emitting byval on the parameters (no check-clang tests regressed). This patch is just filling in a hole that should always have been filled in. But maybe someone else should chime in.

The argument is that this was always invalid, but a bug that it wasn't detected, so it doesn't need to be upgraded when reading IR, it's OK for that to fail the verifier)

Compared to an argument that it was valid, is now invalid, so needs to be upgraded to ensure bitcode compatibility?

Do you think it'd be especially complicated to auto-upgrade?

Yeah my argument is that it was always invalid.

I'm sure it's possible to auto-upgrade bitcode, but that would have non-zero cost for every call we read from bitcode. IMO it's not worth it if this was always supposed to be invalid.

Maybe we should ask llvm-dev for more opinions?

Yeah - if it's not possible to make that sort of IR from the APIs - if it'd require hand-crafted textual IR, or really weird/novel bitcode creation (please confirm whether/how hard it would be to have this in bitcode - if simple use of the IRBuilder/IR creation APIs could result in this problematic representation) I'd probably be OK calling it an "accepts invalid" bug and declaring it invalid.

Pending @rnk's concerns too.

[aeubanks]

rnk's comment makes sense, abandoning in favor of D101806

llvm/lib/IR/Verifier.cpp

for (Type *ParamTy : FTy->params()) {
              "Function has metadata parameter but isn't an intrinsic", Call);
       Assert(!ParamTy->isTokenTy(),
              "Function has token parameter but isn't an intrinsic", Call);
     }
   }
 
   // Verify that indirect calls don't return tokens.
   if (!Call.getCalledFunction()) {
     Assert(!FTy->getReturnType()->isTokenTy(),
-           "Return type cannot be token for indirect call!");
+           "Return type cannot be token for indirect call!", Call);
     Assert(!FTy->getReturnType()->isX86_AMXTy(),
-           "Return type cannot be x86_amx for indirect call!");
+           "Return type cannot be x86_amx for indirect call!", Call);
   }
 
-  if (Function *F = Call.getCalledFunction())
-    if (Intrinsic::ID ID = (Intrinsic::ID)F->getIntrinsicID())
+  if (Function *F = Call.getCalledFunction()) {
+    if (Intrinsic::ID ID = F->getIntrinsicID())
       visitIntrinsicCall(ID, Call);
 
+    // Verify that arguments have matching attributes as the callee's
+    // parameters.
+    for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i) {
+      if (F->hasParamAttribute(i, Attribute::InAlloca)) {
+        Assert(Attrs.getParamAttributes(i).hasAttribute(Attribute::InAlloca),
+               "argument to inalloca parameter requires inalloca attribute",
+               Call);
+      }
+      if (F->hasParamAttribute(i, Attribute::ByVal)) {
+        Assert(Attrs.getParamAttributes(i).hasAttribute(Attribute::ByVal),
+               "argument to byval parameter requires byval attribute", Call);
+      }
+      if (F->hasParamAttribute(i, Attribute::Preallocated)) {
+        Assert(
+            Attrs.getParamAttributes(i).hasAttribute(Attribute::Preallocated),
+            "argument to preallocated parameter requires preallocated "
+            "attribute",
+            Call);
+      }
+    }
+  }
+
   // Verify that a callsite has at most one "deopt", at most one "funclet", at
   // most one "gc-transition", at most one "cfguardtarget",
   // and at most one "preallocated" operand bundle.
   bool FoundDeoptBundle = false, FoundFuncletBundle = false,
        FoundGCTransitionBundle = false, FoundCFGuardTargetBundle = false,
        FoundPreallocatedBundle = false, FoundGCLiveBundle = false,
        FoundAttachedCallBundle = false;
   for (unsigned i = 0, e = Call.getNumOperandBundles(); i < e; ++i) {

llvm/test/CodeGen/X86/movtopush.ll

 ; NORMAL:       pushl  $8
 ; NORMAL-NEXT:  pushl  $7
 ; NORMAL-NEXT:  pushl  $6
 ; NORMAL-NEXT:  pushl  $5
 ; NORMAL-NEXT: calll _good
 define void @test12() optsize {
 entry:
   %s = alloca %struct.s, align 4
-  call void @struct(%struct.s* %s, i32 2, i32 3, i32 4)
+  call void @struct(%struct.s* byval(%struct.s) %s, i32 2, i32 3, i32 4)
   call void @good(i32 5, i32 6, i32 7, i32 8)
-  call void @struct(%struct.s* %s, i32 10, i32 11, i32 12)
+  call void @struct(%struct.s* byval(%struct.s) %s, i32 10, i32 11, i32 12)
   ret void
 }
 
 ; But if the gains outweigh the overhead, we should do it
 ; NORMAL-LABEL: test12b:
 ; NORMAL: pushl    $4
 ; NORMAL-NEXT: pushl    $3
 ; NORMAL-NEXT: pushl    $2

 ; NORMAL=NEXT: pushl  $10
 ; NORMAL=NEXT: pushl  $9
 ; NORMAL=NEXT: calll  _good
 ; NORMAL=NEXT: addl  $16, %esp
 define void @test12b() optsize {
 entry:
   %s = alloca %struct.s, align 4
   call void @good(i32 1, i32 2, i32 3, i32 4)
-  call void @struct(%struct.s* %s, i32 6, i32 7, i32 8)
+  call void @struct(%struct.s* byval(%struct.s) %s, i32 6, i32 7, i32 8)
   call void @good(i32 9, i32 10, i32 11, i32 12)
   ret void
 }
 
 ; Make sure the add does not prevent folding loads into pushes.
 ; val1 and val2 will not be folded into pushes since they have
 ; an additional use, but val3 should be.
 ; NORMAL-LABEL: test13:

llvm/test/Transforms/ArgumentPromotion/dbg.ll

 ; CHECK-NEXT:    [[P_0_VAL:%.*]] = load i32, i32* [[P_0]], align 4, !dbg !5
 ; CHECK-NEXT:    [[P_1:%.*]] = getelementptr [[STRUCT_PAIR]], %struct.pair* [[P]], i32 0, i32 1, !dbg !5
 ; CHECK-NEXT:    [[P_1_VAL:%.*]] = load i32, i32* [[P_1]], align 4, !dbg !5
 ; CHECK-NEXT:    call void @test_byval(i32 [[P_0_VAL]], i32 [[P_1_VAL]]), !dbg !5
 ; CHECK-NEXT:    ret void
 ;
   call void @test(i32** %Y), !dbg !1
 
-  call void @test_byval(%struct.pair* %P), !dbg !6
+  call void @test_byval(%struct.pair* byval(%struct.pair) %P), !dbg !6
   ret void
 }
 
 
 !llvm.module.flags = !{!0}
 !llvm.dbg.cu = !{!3}
 
 !0 = !{i32 2, !"Debug Info Version", i32 3}
 !1 = !DILocation(line: 8, scope: !2)
 !2 = distinct !DISubprogram(name: "test", file: !5, line: 3, isLocal: true, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: false, unit: !3, scopeLine: 3, scope: null)
 !3 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, producer: "clang version 3.5.0 ", isOptimized: false, emissionKind: LineTablesOnly, file: !5)
 !5 = !DIFile(filename: "test.c", directory: "")
 !6 = !DILocation(line: 9, scope: !2)

llvm/test/Transforms/ArgumentPromotion/fp80.ll

 
 define void @run() {
 ; CHECK-LABEL: define {{[^@]+}}@run()
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TMP0:%.*]] = tail call i8 @UseLongDoubleUnsafely(%union.u* byval(%union.u) align 16 bitcast (%struct.s* @b to %union.u*))
 ; CHECK-NEXT:    [[DOT0:%.*]] = getelementptr [[UNION_U:%.*]], %union.u* bitcast (%struct.s* @b to %union.u*), i32 0, i32 0
 ; CHECK-NEXT:    [[DOT0_VAL:%.*]] = load x86_fp80, x86_fp80* [[DOT0]]
 ; CHECK-NEXT:    [[TMP1:%.*]] = tail call x86_fp80 @UseLongDoubleSafely(x86_fp80 [[DOT0_VAL]])
-; CHECK-NEXT:    [[TMP2:%.*]] = call i64 @AccessPaddingOfStruct(%struct.Foo* @a)
-; CHECK-NEXT:    [[TMP3:%.*]] = call i64 @CaptureAStruct(%struct.Foo* @a)
+; CHECK-NEXT:    [[TMP2:%.*]] = call i64 @AccessPaddingOfStruct(%struct.Foo* byval(%struct.Foo) @a)
+; CHECK-NEXT:    [[TMP3:%.*]] = call i64 @CaptureAStruct(%struct.Foo* byval(%struct.Foo) @a)
 ; CHECK-NEXT:    ret void
 ;
 entry:
   tail call i8 @UseLongDoubleUnsafely(%union.u* byval(%union.u) align 16 bitcast (%struct.s* @b to %union.u*))
   tail call x86_fp80 @UseLongDoubleSafely(%union.u* byval(%union.u) align 16 bitcast (%struct.s* @b to %union.u*))
-  call i64 @AccessPaddingOfStruct(%struct.Foo* @a)
-  call i64 @CaptureAStruct(%struct.Foo* @a)
+  call i64 @AccessPaddingOfStruct(%struct.Foo* byval(%struct.Foo) @a)
+  call i64 @CaptureAStruct(%struct.Foo* byval(%struct.Foo) @a)
   ret void
 }
 
 define internal i8 @UseLongDoubleUnsafely(%union.u* byval(%union.u) align 16 %arg) {
 ; CHECK-LABEL: define {{[^@]+}}@UseLongDoubleUnsafely
 ; CHECK-SAME: (%union.u* byval(%union.u) align 16 [[ARG:%.*]])
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[BITCAST:%.*]] = bitcast %union.u* [[ARG]] to %struct.s*

llvm/test/Transforms/Attributor/ArgumentPromotion/dbg.ll

 ; IS__CGSCC____-LABEL: define {{[^@]+}}@caller
 ; IS__CGSCC____-SAME: (i32** nocapture noundef nonnull readonly align 8 dereferenceable(8) [[Y:%.*]], %struct.pair* nocapture nofree readnone [[P:%.*]]) {
 ; IS__CGSCC____-NEXT:    call void @test(i32** nocapture noundef nonnull readonly align 8 dereferenceable(8) [[Y]]), !dbg [[DBG4:![0-9]+]]
 ; IS__CGSCC____-NEXT:    call void @test_byval(), !dbg [[DBG5:![0-9]+]]
 ; IS__CGSCC____-NEXT:    ret void
 ;
   call void @test(i32** %Y), !dbg !1
 
-  call void @test_byval(%struct.pair* %P), !dbg !6
+  call void @test_byval(%struct.pair* byval(%struct.pair) %P), !dbg !6
   ret void
 }
 
 
 !llvm.module.flags = !{!0}
 !llvm.dbg.cu = !{!3}
 
 !0 = !{i32 2, !"Debug Info Version", i32 3}

llvm/test/Transforms/Attributor/ArgumentPromotion/fp80.ll

 ; CHECK: @[[B:[a-zA-Z0-9_$"\\.-]+]] = internal global [[STRUCT_S:%.*]] { double 3.140000e+00, i16 9439, i8 25, [5 x i8] undef }, align 16
 ; CHECK: @[[A:[a-zA-Z0-9_$"\\.-]+]] = internal global [[STRUCT_FOO:%.*]] { i32 1, i64 2 }, align 8
 ;.
 define void @run() {
 ; IS________OPM: Function Attrs: nofree noreturn nosync nounwind readnone
 ; IS________OPM-LABEL: define {{[^@]+}}@run
 ; IS________OPM-SAME: () #[[ATTR0:[0-9]+]] {
 ; IS________OPM-NEXT:  entry:
-; IS________OPM-NEXT:    [[TMP0:%.*]] = call i64 @CaptureAStruct(%struct.Foo* nocapture nofree noundef nonnull readonly align 8 dereferenceable(16) @a) #[[ATTR0]]
+; IS________OPM-NEXT:    [[TMP0:%.*]] = call i64 @CaptureAStruct(%struct.Foo* nocapture nofree noundef nonnull readonly byval([[STRUCT_FOO:%.*]]) align 8 dereferenceable(16) @a) #[[ATTR0]]
 ; IS________OPM-NEXT:    unreachable
 ;
 ; IS__TUNIT_NPM: Function Attrs: nofree noreturn nosync nounwind readnone
 ; IS__TUNIT_NPM-LABEL: define {{[^@]+}}@run
 ; IS__TUNIT_NPM-SAME: () #[[ATTR0:[0-9]+]] {
 ; IS__TUNIT_NPM-NEXT:  entry:
 ; IS__TUNIT_NPM-NEXT:    [[A_CAST:%.*]] = bitcast %struct.Foo* @a to i32*
 ; IS__TUNIT_NPM-NEXT:    [[TMP0:%.*]] = load i32, i32* [[A_CAST]], align 8

 ; IS__CGSCC____-NEXT:    [[TMP0:%.*]] = load i32, i32* [[A_CAST]], align 8
 ; IS__CGSCC____-NEXT:    [[A_0_1:%.*]] = getelementptr [[STRUCT_FOO:%.*]], %struct.Foo* @a, i32 0, i32 1
 ; IS__CGSCC____-NEXT:    [[TMP1:%.*]] = load i64, i64* [[A_0_1]], align 8
 ; IS__CGSCC____-NEXT:    unreachable
 ;
 entry:
   tail call i8 @UseLongDoubleUnsafely(%union.u* byval(%union.u) align 16 bitcast (%struct.s* @b to %union.u*))
   tail call x86_fp80 @UseLongDoubleSafely(%union.u* byval(%union.u) align 16 bitcast (%struct.s* @b to %union.u*))
-  call i64 @AccessPaddingOfStruct(%struct.Foo* @a)
-  call i64 @CaptureAStruct(%struct.Foo* @a)
+  call i64 @AccessPaddingOfStruct(%struct.Foo* byval(%struct.Foo) @a)
+  call i64 @CaptureAStruct(%struct.Foo* byval(%struct.Foo) @a)
   ret void
 }
 
 define internal i8 @UseLongDoubleUnsafely(%union.u* byval(%union.u) align 16 %arg) {
 ; IS__CGSCC____: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
 ; IS__CGSCC____-LABEL: define {{[^@]+}}@UseLongDoubleUnsafely
 ; IS__CGSCC____-SAME: () #[[ATTR1:[0-9]+]] {
 ; IS__CGSCC____-NEXT:  entry:

llvm/test/Transforms/Attributor/readattrs.ll

 ;
   store i8 0, i8* %written
   ret void
 }
 
 define void @testbyval(i8* %read_only) {
 ; IS__TUNIT____-LABEL: define {{[^@]+}}@testbyval
 ; IS__TUNIT____-SAME: (i8* nocapture readonly [[READ_ONLY:%.*]]) {
-; IS__TUNIT____-NEXT:    call void @byval_not_readonly_1(i8* nocapture readonly [[READ_ONLY]]) #[[ATTR2]]
-; IS__TUNIT____-NEXT:    call void @byval_not_readnone_1(i8* noalias nocapture readnone [[READ_ONLY]])
+; IS__TUNIT____-NEXT:    call void @byval_not_readonly_1(i8* nocapture readonly byval(i8) [[READ_ONLY]]) #[[ATTR2]]
+; IS__TUNIT____-NEXT:    call void @byval_not_readnone_1(i8* noalias nocapture readnone byval(i8) [[READ_ONLY]])
 ; IS__TUNIT____-NEXT:    ret void
 ;
 ; IS__CGSCC____: Function Attrs: readonly
 ; IS__CGSCC____-LABEL: define {{[^@]+}}@testbyval
 ; IS__CGSCC____-SAME: (i8* nocapture noundef nonnull readonly dereferenceable(1) [[READ_ONLY:%.*]]) #[[ATTR2]] {
-; IS__CGSCC____-NEXT:    call void @byval_not_readonly_1(i8* noalias nocapture noundef nonnull readonly dereferenceable(1) [[READ_ONLY]]) #[[ATTR2]]
-; IS__CGSCC____-NEXT:    call void @byval_not_readnone_1(i8* noalias nocapture noundef nonnull readnone dereferenceable(1) [[READ_ONLY]])
+; IS__CGSCC____-NEXT:    call void @byval_not_readonly_1(i8* noalias nocapture noundef nonnull readonly byval(i8) dereferenceable(1) [[READ_ONLY]]) #[[ATTR2]]
+; IS__CGSCC____-NEXT:    call void @byval_not_readnone_1(i8* noalias nocapture noundef nonnull readnone byval(i8) dereferenceable(1) [[READ_ONLY]])
 ; IS__CGSCC____-NEXT:    ret void
 ;
-  call void @byval_not_readonly_1(i8* %read_only)
-  call void @byval_not_readonly_2(i8* %read_only)
-  call void @byval_not_readnone_1(i8* %read_only)
-  call void @byval_not_readnone_2(i8* %read_only)
-  call void @byval_no_fnarg(i8* %read_only)
+  call void @byval_not_readonly_1(i8* byval(i8) %read_only)
+  call void @byval_not_readonly_2(i8* byval(i8) %read_only)
+  call void @byval_not_readnone_1(i8* byval(i8) %read_only)
+  call void @byval_not_readnone_2(i8* byval(i8) %read_only)
+  call void @byval_no_fnarg(i8* byval(i8) %read_only)
   ret void
 }
 ;}
 
 declare i8* @maybe_returned_ptr(i8* readonly %ptr) readonly nounwind
 declare i8 @maybe_returned_val(i8* %ptr) readonly nounwind
 declare void @val_use(i8 %ptr) readonly nounwind
 

llvm/test/Transforms/Attributor/value-simplify.ll

 ; IS__CGSCC____-NEXT:    ret i32* [[A]]
 ;
   ret i32* %a
 }
 define i32* @complicated_args_inalloca(i32* %arg) {
 ; IS__TUNIT____: Function Attrs: nofree nosync nounwind readnone willreturn
 ; IS__TUNIT____-LABEL: define {{[^@]+}}@complicated_args_inalloca
 ; IS__TUNIT____-SAME: (i32* nofree readnone returned "no-capture-maybe-returned" [[ARG:%.*]]) #[[ATTR1]] {
-; IS__TUNIT____-NEXT:    [[CALL:%.*]] = call i32* @test_inalloca(i32* noalias nofree writeonly "no-capture-maybe-returned" [[ARG]]) #[[ATTR1]]
+; IS__TUNIT____-NEXT:    [[CALL:%.*]] = call i32* @test_inalloca(i32* noalias nofree writeonly inalloca(i32) "no-capture-maybe-returned" [[ARG]]) #[[ATTR1]]
 ; IS__TUNIT____-NEXT:    ret i32* [[CALL]]
 ;
 ; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
 ; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@complicated_args_inalloca
 ; IS__CGSCC_OPM-SAME: (i32* nofree noundef nonnull readnone returned dereferenceable(4) "no-capture-maybe-returned" [[ARG:%.*]]) #[[ATTR1]] {
-; IS__CGSCC_OPM-NEXT:    [[CALL:%.*]] = call i32* @test_inalloca(i32* noalias nofree noundef nonnull writeonly dereferenceable(4) "no-capture-maybe-returned" [[ARG]]) #[[ATTR5:[0-9]+]]
+; IS__CGSCC_OPM-NEXT:    [[CALL:%.*]] = call i32* @test_inalloca(i32* noalias nofree noundef nonnull writeonly inalloca(i32) dereferenceable(4) "no-capture-maybe-returned" [[ARG]]) #[[ATTR5:[0-9]+]]
 ; IS__CGSCC_OPM-NEXT:    ret i32* [[CALL]]
 ;
 ; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
 ; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@complicated_args_inalloca
 ; IS__CGSCC_NPM-SAME: (i32* nofree noundef nonnull readnone returned dereferenceable(4) "no-capture-maybe-returned" [[ARG:%.*]]) #[[ATTR1]] {
-; IS__CGSCC_NPM-NEXT:    [[CALL:%.*]] = call i32* @test_inalloca(i32* noalias nofree noundef nonnull writeonly dereferenceable(4) "no-capture-maybe-returned" [[ARG]]) #[[ATTR4:[0-9]+]]
+; IS__CGSCC_NPM-NEXT:    [[CALL:%.*]] = call i32* @test_inalloca(i32* noalias nofree noundef nonnull writeonly inalloca(i32) dereferenceable(4) "no-capture-maybe-returned" [[ARG]]) #[[ATTR4:[0-9]+]]
 ; IS__CGSCC_NPM-NEXT:    ret i32* [[CALL]]
 ;
-  %call = call i32* @test_inalloca(i32* %arg)
+  %call = call i32* @test_inalloca(i32* inalloca(i32) %arg)
   ret i32* %call
 }
 
 define internal i32* @test_preallocated(i32* preallocated(i32) %a) {
 ; IS__TUNIT____: Function Attrs: nofree nosync nounwind readnone willreturn
 ; IS__TUNIT____-LABEL: define {{[^@]+}}@test_preallocated
 ; IS__TUNIT____-SAME: (i32* noalias nofree noundef nonnull returned writeonly preallocated(i32) align 536870912 dereferenceable(4) "no-capture-maybe-returned" [[A:%.*]]) #[[ATTR1]] {
 ; IS__TUNIT____-NEXT:    ret i32* [[A]]

 ; IS__CGSCC_OPM-SAME: () #[[ATTR1]] {
 ; IS__CGSCC_OPM-NEXT:    ret void
 ;
 ; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind readonly willreturn
 ; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@complicated_args_byval
 ; IS__CGSCC_NPM-SAME: () #[[ATTR3:[0-9]+]] {
 ; IS__CGSCC_NPM-NEXT:    ret void
 ;
-  call void @test_byval(%struct.X* @S)
+  call void @test_byval(%struct.X* byval(%struct.X) @S)
   ret void
 }
 
 define internal i8*@test_byval2(%struct.X* byval(%struct.X) %a) {
 ; IS__TUNIT____: Function Attrs: nofree nosync nounwind readonly willreturn
 ; IS__TUNIT____-LABEL: define {{[^@]+}}@test_byval2
 ; IS__TUNIT____-SAME: () #[[ATTR3:[0-9]+]] {
 ; IS__TUNIT____-NEXT:    [[G0:%.*]] = getelementptr [[STRUCT_X:%.*]], %struct.X* @S, i32 0, i32 0

 ; IS__CGSCC_OPM-NEXT:    ret i8* [[C]]
 ;
 ; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind readonly willreturn
 ; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@complicated_args_byval2
 ; IS__CGSCC_NPM-SAME: () #[[ATTR3]] {
 ; IS__CGSCC_NPM-NEXT:    [[C:%.*]] = call i8* @test_byval2() #[[ATTR7:[0-9]+]]
 ; IS__CGSCC_NPM-NEXT:    ret i8* [[C]]
 ;
-  %c = call i8* @test_byval2(%struct.X* @S)
+  %c = call i8* @test_byval2(%struct.X* byval(%struct.X) @S)
   ret i8* %c
 }
 
 define void @fixpoint_changed(i32* %p) {
 ; IS__TUNIT_OPM: Function Attrs: argmemonly nofree nosync nounwind writeonly
 ; IS__TUNIT_OPM-LABEL: define {{[^@]+}}@fixpoint_changed
 ; IS__TUNIT_OPM-SAME: (i32* nocapture nofree writeonly [[P:%.*]]) #[[ATTR4:[0-9]+]] {
 ; IS__TUNIT_OPM-NEXT:  entry:

llvm/test/Transforms/TailCallElim/basic.ll

 
 %struct.foo = type { [10 x i32] }
 
 ; If an alloca is passed byval it is not a use of the alloca or an escape
 ; point, and both calls below can be marked tail.
 define void @test13() {
 ; CHECK-LABEL: @test13
 ; CHECK: tail call void @bar(%struct.foo* byval(%struct.foo) %f)
-; CHECK: tail call void @bar(%struct.foo* null)
+; CHECK: tail call void @bar(%struct.foo* byval(%struct.foo) null)
 entry:
   %f = alloca %struct.foo
   call void @bar(%struct.foo* byval(%struct.foo) %f)
-  call void @bar(%struct.foo* null)
+  call void @bar(%struct.foo* byval(%struct.foo) null)
   ret void
 }
 
 ; A call which passes a byval parameter using byval can be marked tail.
 define void @test14(%struct.foo* byval(%struct.foo) %f) {
 ; CHECK-LABEL: @test14
 ; CHECK: tail call void @bar
 entry:

llvm/test/Verifier/indirect-attrs.ll

+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
+
+declare void @inalloca(i32* inalloca(i32) %p)
+
+; CHECK: argument to inalloca parameter requires inalloca attribute
+define void @inalloca_not_on_parameter() {
+  %a = alloca inalloca i32
+  call void @inalloca(i32* %a)
+  ret void
+}
+
+declare void @byval(i32* byval(i32) %p)
+
+; CHECK: argument to byval parameter requires byval attribute
+define void @byval_not_on_parameter() {
+  %a = alloca i32
+  call void @byval(i32* %a)
+  ret void
+}
+
+declare token @llvm.call.preallocated.setup(i32)
+declare i8* @llvm.call.preallocated.arg(token, i32)
+declare void @preallocated(i32* preallocated(i32))
+
+; CHECK: argument to preallocated parameter requires preallocated attribute
+define void @preallocated_not_on_parameter() {
+    %cs = call token @llvm.call.preallocated.setup(i32 1)
+    %x = call i8* @llvm.call.preallocated.arg(token %cs, i32 0) preallocated(i32)
+    %y = bitcast i8* %x to i32*
+    call void @preallocated(i32* %y) ["preallocated"(token %cs)]
+    ret void
+}