Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARM32: Incorrect split passing of promoted structs with padding #57064

Closed
jakobbotsch opened this issue Aug 9, 2021 · 5 comments · Fixed by #57279
Closed

ARM32: Incorrect split passing of promoted structs with padding #57064

jakobbotsch opened this issue Aug 9, 2021 · 5 comments · Fixed by #57279
Assignees
Labels
arch-arm32 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@jakobbotsch
Copy link
Member

jakobbotsch commented Aug 9, 2021

EDIT: See the example below

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Aug 9, 2021
@jakobbotsch jakobbotsch added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI arch-arm32 labels Aug 9, 2021
@jakobbotsch
Copy link
Member Author

Not sure if the same problem, but also needs the virtual call. This one seemingly reads stack garbage.

// Generated by Fuzzlyn v1.2 on 2021-08-09 03:54:19
// Run on .NET 6.0.0-dev on Arm Linux
// Seed: 3925960003105271109
// Reduced from 45.3 KiB to 0.6 KiB in 00:01:09
// Debug: Outputs 0
// Release: Outputs 4059889664
struct S0
{
    public uint F0;
    public ushort F1;
    public uint F2;
}

public class Program
{
    static IRT s_rt;
    static sbyte s_18;
    static ushort s_20;
    static S0 s_27;
    public static void Main()
    {
        s_rt = new RT();
        var vr4 = new ushort[]{0};
        bool vr8 = s_20 == s_18;
        var vr6 = new bool[]{true};
        var vr7 = M31();
        M3(vr4, vr8, vr6, vr7, 0);
    }

    static void M3(ushort[] arg0, bool arg1, bool[] arg2, S0 arg3, uint arg4)
    {
        s_rt.Write(arg3.F2);
    }

    static S0 M31()
    {
        s_27.F0 = 0;
        return new S0();
    }
}

public interface IRT { void Write<T>(T val); }
public class RT : IRT
{
    public void Write<T>(T val)
    {
        System.Console.WriteLine(val);
    }
}

@jakobbotsch jakobbotsch self-assigned this Aug 9, 2021
@jakobbotsch jakobbotsch removed the untriaged New issue has not been triaged by the area owner label Aug 9, 2021
@jakobbotsch
Copy link
Member Author

I'll look at this.

@JulieLeeMSFT JulieLeeMSFT added this to the 6.0.0 milestone Aug 9, 2021
@jakobbotsch
Copy link
Member Author

The first one is fixed by #57076, however the second one looks to be different, so I'm looking at that one.

Seems to be a problem with how the args are stored in the out area. We generate:

000078  A805           add     r0, sp, 20       // [V03 loc3]
00007A  F64A 5311      movw    r3, 0xad11
00007E  F2CF 535C      movt    r3, 0xf55c
000082  4798           blx     r3               // Program:M31():S0
000084  2300           movs    r3, 0
000086  9302           str     r3, [sp+0x08]    // [V04 OutArgs+0x08]
000088  9B05           ldr     r3, [sp+0x14]    // [V06 tmp2]
00008A  F8BD 0018      ldrh    r0, [sp+0x18]    // [V07 tmp3]
00008E  9907           ldr     r1, [sp+0x1c]    // [V08 tmp4]
000090  F8AD 0000      strh    r0, [sp] // [V04 OutArgs]
000094  F8CD 1002      str     r1, [sp+0x02]    // [V04 OutArgs+0x02]
000098  4620           mov     r0, r4
00009A  4629           mov     r1, r5
00009C  4632           mov     r2, r6
00009E  F64A 5E05      movw    lr, 0xad05
0000A2  F2CF 5E5C      movt    lr, 0xf55c
0000A6  47F0           blx     lr               // Program:M3(System.UInt16[],bool,System.Boolean[],S0,int)

The first field of the struct goes into r3, the second goes on the stack at OutArgs+0, but then the last field goes on the stack at OutArgs+2, when it should probably be put at OutArgs+4.

@jakobbotsch jakobbotsch changed the title ARM32: Invalid result with interface call ARM32: Incorrect split passing of promoted structs with padding Aug 12, 2021
jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Aug 12, 2021
We were writing the stack part to the outgoing stack area by a loop
incrementing the offset by the written size in each iteration. However,
it is possible due to padding and optimization that the written size is
smaller than the delta to the next field. Instead, use the offset of
each field minus the offset of the first stack field to find the offset
in the outgoing arg area.

Fix dotnet#57064
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 12, 2021
@JulieLeeMSFT
Copy link
Member

CC @dotnet/jit-contrib Please do code review.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 14, 2021
jakobbotsch added a commit that referenced this issue Aug 14, 2021
We were writing the stack part to the outgoing stack area by a loop
incrementing the offset by the written size in each iteration. However,
it is possible due to padding and optimization that the written size is
smaller than the delta to the next field. Instead, use the offset of
each field minus the offset of the first stack field to find the offset
in the outgoing arg area.

Fix #57064
@ghost ghost locked as resolved and limited conversation to collaborators Sep 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm32 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants