Fix assert for ARM shuffle thunk

[BruceForstall]

Handle case where gap exists at the beginning of the stack
range.

Fix#13241

[BruceForstall]

This is a possible fix for #13241. It looks to me like the assert didn't handle the case of the "gap" at the beginning, even though the code did.

For the test case at #13241 (comment), the generated thunk is:

0511d0bc b570 push {r4-r6,lr}
0511d0be f8d0c010 ldr r12,[r0,#0x10]
0511d0c2 f20d0410 addw r4,sp,#0x10
0511d0c6 f20d0510 addw r5,sp,#0x10
0511d0ca 4608 mov r0,r1
0511d0cc 3420 adds r4,r4,#0x20 // gap
0511d0ce f8541b04 ldr r1,[r4],#4
0511d0d2 f8542b04 ldr r2,[r4],#4
0511d0d6 f8543b04 ldr r3,[r4],#4
0511d0da f8546b04 ldr r6,[r4],#4
0511d0de 3520 adds r5,r5,#0x20 // gap
0511d0e0 f8456b04 str r6,[r5],#4
0511d0e4 f8546b04 ldr r6,[r4],#4
0511d0e8 f8456b04 str r6,[r5],#4
0511d0ec f8546b04 ldr r6,[r4],#4
0511d0f0 f8456b04 str r6,[r5],#4
0511d0f4 f8cdc00c str r12,[sp,#0xC]
0511d0f8 bd70 pop {r4-r6,pc}

with the "// gap" lines being generated when the assertion was previously hitting.

Now, I don't know enough about shuffle thunks to know if the gaps are supposed to be there or not...

[BruceForstall]

@janvorli Who knows about (ARM) shuffle thunks?

cc @jakobbotsch

[jakobbotsch]

LGTM, I've ran 50000 callers in the pinvoke stress test without any mismatches so seems the existing code does handle it correctly as you suspected.
You can either reenable the stress test here or I can do it in #66553 after you merge this.

[BruceForstall]

You can either reenable the stress test here or I can do it in #66553 after you merge this.

@jakobbotsch I went ahead and merged; I'll let you re-enable the stress test in #66553

[BruceForstall]

Handle case where gap exists at the beginning of the stack
range.

Fix#13241

[BruceForstall]

This is a possible fix for #13241. It looks to me like the assert didn't handle the case of the "gap" at the beginning, even though the code did.

For the test case at #13241 (comment), the generated thunk is:

0511d0bc b570 push {r4-r6,lr}
0511d0be f8d0c010 ldr r12,[r0,#0x10]
0511d0c2 f20d0410 addw r4,sp,#0x10
0511d0c6 f20d0510 addw r5,sp,#0x10
0511d0ca 4608 mov r0,r1
0511d0cc 3420 adds r4,r4,#0x20 // gap
0511d0ce f8541b04 ldr r1,[r4],#4
0511d0d2 f8542b04 ldr r2,[r4],#4
0511d0d6 f8543b04 ldr r3,[r4],#4
0511d0da f8546b04 ldr r6,[r4],#4
0511d0de 3520 adds r5,r5,#0x20 // gap
0511d0e0 f8456b04 str r6,[r5],#4
0511d0e4 f8546b04 ldr r6,[r4],#4
0511d0e8 f8456b04 str r6,[r5],#4
0511d0ec f8546b04 ldr r6,[r4],#4
0511d0f0 f8456b04 str r6,[r5],#4
0511d0f4 f8cdc00c str r12,[sp,#0xC]
0511d0f8 bd70 pop {r4-r6,pc}

with the "// gap" lines being generated when the assertion was previously hitting.

Now, I don't know enough about shuffle thunks to know if the gaps are supposed to be there or not...

[BruceForstall]

@janvorli Who knows about (ARM) shuffle thunks?

cc @jakobbotsch

[jakobbotsch]

LGTM, I've ran 50000 callers in the pinvoke stress test without any mismatches so seems the existing code does handle it correctly as you suspected.
You can either reenable the stress test here or I can do it in #66553 after you merge this.

[BruceForstall]

You can either reenable the stress test here or I can do it in #66553 after you merge this.

@jakobbotsch I went ahead and merged; I'll let you re-enable the stress test in #66553

[BruceForstall]

Handle case where gap exists at the beginning of the stack
range.

Fix#13241

[BruceForstall]

This is a possible fix for #13241. It looks to me like the assert didn't handle the case of the "gap" at the beginning, even though the code did.

For the test case at #13241 (comment), the generated thunk is:

0511d0bc b570 push {r4-r6,lr}
0511d0be f8d0c010 ldr r12,[r0,#0x10]
0511d0c2 f20d0410 addw r4,sp,#0x10
0511d0c6 f20d0510 addw r5,sp,#0x10
0511d0ca 4608 mov r0,r1
0511d0cc 3420 adds r4,r4,#0x20 // gap
0511d0ce f8541b04 ldr r1,[r4],#4
0511d0d2 f8542b04 ldr r2,[r4],#4
0511d0d6 f8543b04 ldr r3,[r4],#4
0511d0da f8546b04 ldr r6,[r4],#4
0511d0de 3520 adds r5,r5,#0x20 // gap
0511d0e0 f8456b04 str r6,[r5],#4
0511d0e4 f8546b04 ldr r6,[r4],#4
0511d0e8 f8456b04 str r6,[r5],#4
0511d0ec f8546b04 ldr r6,[r4],#4
0511d0f0 f8456b04 str r6,[r5],#4
0511d0f4 f8cdc00c str r12,[sp,#0xC]
0511d0f8 bd70 pop {r4-r6,pc}

with the "// gap" lines being generated when the assertion was previously hitting.

Now, I don't know enough about shuffle thunks to know if the gaps are supposed to be there or not...

[BruceForstall]

@janvorli Who knows about (ARM) shuffle thunks?

cc @jakobbotsch

[jakobbotsch]

LGTM, I've ran 50000 callers in the pinvoke stress test without any mismatches so seems the existing code does handle it correctly as you suspected.
You can either reenable the stress test here or I can do it in #66553 after you merge this.

[BruceForstall]

You can either reenable the stress test here or I can do it in #66553 after you merge this.

@jakobbotsch I went ahead and merged; I'll let you re-enable the stress test in #66553

[BruceForstall]

Handle case where gap exists at the beginning of the stack
range.

Fix#13241

[BruceForstall]

This is a possible fix for #13241. It looks to me like the assert didn't handle the case of the "gap" at the beginning, even though the code did.

For the test case at #13241 (comment), the generated thunk is:

0511d0bc b570 push {r4-r6,lr}
0511d0be f8d0c010 ldr r12,[r0,#0x10]
0511d0c2 f20d0410 addw r4,sp,#0x10
0511d0c6 f20d0510 addw r5,sp,#0x10
0511d0ca 4608 mov r0,r1
0511d0cc 3420 adds r4,r4,#0x20 // gap
0511d0ce f8541b04 ldr r1,[r4],#4
0511d0d2 f8542b04 ldr r2,[r4],#4
0511d0d6 f8543b04 ldr r3,[r4],#4
0511d0da f8546b04 ldr r6,[r4],#4
0511d0de 3520 adds r5,r5,#0x20 // gap
0511d0e0 f8456b04 str r6,[r5],#4
0511d0e4 f8546b04 ldr r6,[r4],#4
0511d0e8 f8456b04 str r6,[r5],#4
0511d0ec f8546b04 ldr r6,[r4],#4
0511d0f0 f8456b04 str r6,[r5],#4
0511d0f4 f8cdc00c str r12,[sp,#0xC]
0511d0f8 bd70 pop {r4-r6,pc}

with the "// gap" lines being generated when the assertion was previously hitting.

Now, I don't know enough about shuffle thunks to know if the gaps are supposed to be there or not...

[BruceForstall]

@janvorli Who knows about (ARM) shuffle thunks?

cc @jakobbotsch

[jakobbotsch]

LGTM, I've ran 50000 callers in the pinvoke stress test without any mismatches so seems the existing code does handle it correctly as you suspected.
You can either reenable the stress test here or I can do it in #66553 after you merge this.

[BruceForstall]

You can either reenable the stress test here or I can do it in #66553 after you merge this.

@jakobbotsch I went ahead and merged; I'll let you re-enable the stress test in #66553

[BruceForstall]

Handle case where gap exists at the beginning of the stack
range.

Fix#13241

[BruceForstall]

This is a possible fix for #13241. It looks to me like the assert didn't handle the case of the "gap" at the beginning, even though the code did.

For the test case at #13241 (comment), the generated thunk is:

0511d0bc b570 push {r4-r6,lr}
0511d0be f8d0c010 ldr r12,[r0,#0x10]
0511d0c2 f20d0410 addw r4,sp,#0x10
0511d0c6 f20d0510 addw r5,sp,#0x10
0511d0ca 4608 mov r0,r1
0511d0cc 3420 adds r4,r4,#0x20 // gap
0511d0ce f8541b04 ldr r1,[r4],#4
0511d0d2 f8542b04 ldr r2,[r4],#4
0511d0d6 f8543b04 ldr r3,[r4],#4
0511d0da f8546b04 ldr r6,[r4],#4
0511d0de 3520 adds r5,r5,#0x20 // gap
0511d0e0 f8456b04 str r6,[r5],#4
0511d0e4 f8546b04 ldr r6,[r4],#4
0511d0e8 f8456b04 str r6,[r5],#4
0511d0ec f8546b04 ldr r6,[r4],#4
0511d0f0 f8456b04 str r6,[r5],#4
0511d0f4 f8cdc00c str r12,[sp,#0xC]
0511d0f8 bd70 pop {r4-r6,pc}

with the "// gap" lines being generated when the assertion was previously hitting.

Now, I don't know enough about shuffle thunks to know if the gaps are supposed to be there or not...

[BruceForstall]

@janvorli Who knows about (ARM) shuffle thunks?

cc @jakobbotsch

[jakobbotsch]

LGTM, I've ran 50000 callers in the pinvoke stress test without any mismatches so seems the existing code does handle it correctly as you suspected.
You can either reenable the stress test here or I can do it in #66553 after you merge this.

[BruceForstall]

You can either reenable the stress test here or I can do it in #66553 after you merge this.

@jakobbotsch I went ahead and merged; I'll let you re-enable the stress test in #66553

[BruceForstall]

Handle case where gap exists at the beginning of the stack
range.

Fix#13241

[BruceForstall]

This is a possible fix for #13241. It looks to me like the assert didn't handle the case of the "gap" at the beginning, even though the code did.

For the test case at #13241 (comment), the generated thunk is:

0511d0bc b570 push {r4-r6,lr}
0511d0be f8d0c010 ldr r12,[r0,#0x10]
0511d0c2 f20d0410 addw r4,sp,#0x10
0511d0c6 f20d0510 addw r5,sp,#0x10
0511d0ca 4608 mov r0,r1
0511d0cc 3420 adds r4,r4,#0x20 // gap
0511d0ce f8541b04 ldr r1,[r4],#4
0511d0d2 f8542b04 ldr r2,[r4],#4
0511d0d6 f8543b04 ldr r3,[r4],#4
0511d0da f8546b04 ldr r6,[r4],#4
0511d0de 3520 adds r5,r5,#0x20 // gap
0511d0e0 f8456b04 str r6,[r5],#4
0511d0e4 f8546b04 ldr r6,[r4],#4
0511d0e8 f8456b04 str r6,[r5],#4
0511d0ec f8546b04 ldr r6,[r4],#4
0511d0f0 f8456b04 str r6,[r5],#4
0511d0f4 f8cdc00c str r12,[sp,#0xC]
0511d0f8 bd70 pop {r4-r6,pc}

with the "// gap" lines being generated when the assertion was previously hitting.

Now, I don't know enough about shuffle thunks to know if the gaps are supposed to be there or not...

[BruceForstall]

@janvorli Who knows about (ARM) shuffle thunks?

cc @jakobbotsch

[jakobbotsch]

LGTM, I've ran 50000 callers in the pinvoke stress test without any mismatches so seems the existing code does handle it correctly as you suspected.
You can either reenable the stress test here or I can do it in #66553 after you merge this.

[BruceForstall]

You can either reenable the stress test here or I can do it in #66553 after you merge this.

@jakobbotsch I went ahead and merged; I'll let you re-enable the stress test in #66553

[BruceForstall]

Handle case where gap exists at the beginning of the stack
range.

Fix#13241

[BruceForstall]

This is a possible fix for #13241. It looks to me like the assert didn't handle the case of the "gap" at the beginning, even though the code did.

For the test case at #13241 (comment), the generated thunk is:

0511d0bc b570 push {r4-r6,lr}
0511d0be f8d0c010 ldr r12,[r0,#0x10]
0511d0c2 f20d0410 addw r4,sp,#0x10
0511d0c6 f20d0510 addw r5,sp,#0x10
0511d0ca 4608 mov r0,r1
0511d0cc 3420 adds r4,r4,#0x20 // gap
0511d0ce f8541b04 ldr r1,[r4],#4
0511d0d2 f8542b04 ldr r2,[r4],#4
0511d0d6 f8543b04 ldr r3,[r4],#4
0511d0da f8546b04 ldr r6,[r4],#4
0511d0de 3520 adds r5,r5,#0x20 // gap
0511d0e0 f8456b04 str r6,[r5],#4
0511d0e4 f8546b04 ldr r6,[r4],#4
0511d0e8 f8456b04 str r6,[r5],#4
0511d0ec f8546b04 ldr r6,[r4],#4
0511d0f0 f8456b04 str r6,[r5],#4
0511d0f4 f8cdc00c str r12,[sp,#0xC]
0511d0f8 bd70 pop {r4-r6,pc}

with the "// gap" lines being generated when the assertion was previously hitting.

Now, I don't know enough about shuffle thunks to know if the gaps are supposed to be there or not...

[BruceForstall]

@janvorli Who knows about (ARM) shuffle thunks?

cc @jakobbotsch

[jakobbotsch]

LGTM, I've ran 50000 callers in the pinvoke stress test without any mismatches so seems the existing code does handle it correctly as you suspected.
You can either reenable the stress test here or I can do it in #66553 after you merge this.

[BruceForstall]

You can either reenable the stress test here or I can do it in #66553 after you merge this.

@jakobbotsch I went ahead and merged; I'll let you re-enable the stress test in #66553

[BruceForstall]

Handle case where gap exists at the beginning of the stack
range.

Fix#13241

[BruceForstall]

This is a possible fix for #13241. It looks to me like the assert didn't handle the case of the "gap" at the beginning, even though the code did.

For the test case at #13241 (comment), the generated thunk is:

0511d0bc b570 push {r4-r6,lr}
0511d0be f8d0c010 ldr r12,[r0,#0x10]
0511d0c2 f20d0410 addw r4,sp,#0x10
0511d0c6 f20d0510 addw r5,sp,#0x10
0511d0ca 4608 mov r0,r1
0511d0cc 3420 adds r4,r4,#0x20 // gap
0511d0ce f8541b04 ldr r1,[r4],#4
0511d0d2 f8542b04 ldr r2,[r4],#4
0511d0d6 f8543b04 ldr r3,[r4],#4
0511d0da f8546b04 ldr r6,[r4],#4
0511d0de 3520 adds r5,r5,#0x20 // gap
0511d0e0 f8456b04 str r6,[r5],#4
0511d0e4 f8546b04 ldr r6,[r4],#4
0511d0e8 f8456b04 str r6,[r5],#4
0511d0ec f8546b04 ldr r6,[r4],#4
0511d0f0 f8456b04 str r6,[r5],#4
0511d0f4 f8cdc00c str r12,[sp,#0xC]
0511d0f8 bd70 pop {r4-r6,pc}

with the "// gap" lines being generated when the assertion was previously hitting.

Now, I don't know enough about shuffle thunks to know if the gaps are supposed to be there or not...

[BruceForstall]

@janvorli Who knows about (ARM) shuffle thunks?

cc @jakobbotsch

[jakobbotsch]

LGTM, I've ran 50000 callers in the pinvoke stress test without any mismatches so seems the existing code does handle it correctly as you suspected.
You can either reenable the stress test here or I can do it in #66553 after you merge this.

[BruceForstall]

You can either reenable the stress test here or I can do it in #66553 after you merge this.

@jakobbotsch I went ahead and merged; I'll let you re-enable the stress test in #66553