Fix the Rust ABI implementation

[MabezDev]

We should make sure that our ABI implementation performs identically to the llvm implementation, not including vaargs as Rust does not support these.

The current implementation is based on the clang abi impl from the first iteration of the llvm fork. It is currently not fully complete, and probably buggy.

[vmarkushin]

Here is information about rustc's type layouts debugging: https://www.ralfj.de/blog/2020/04/04/layout-debugging.html

[MabezDev]

The Rust ABI is wrong, it needs to be updated. Reference for how it should be done is here: https://github.com/espressif/llvm-project/blob/e9f57cdbcf3e0a63f395e683ccfaf7c4e6e1b093/clang/lib/CodeGen/TargetInfo.cpp#L11241

[ollpu]

Structs seem to be incorrectly split between registers and stack, when single arguments should be exclusively kept in one of them according to the ISA manual:

Each argument must be passed entirely in registers or entirely on the stack; an argument cannot be split with some words in registers and the remainder on the stack. If an argument does not fit entirely in the remaining unused registers, it is passed on the stack and those registers remain unused.

To demonstrate the disparity, C-code built as a custom component with esp-idf-sys (which to my understanding uses GCC):

struct Pair {
  int a, b;
};

void abi_test_5(int p1, int p2, int p3, int p4, struct Pair p5) {
    ESP_LOGW("abi_test_5", "p5.a=%d, p5.b=%d", p5.a, p5.b);
}

void abi_test_6(int p1, int p2, int p3, int p4, int p5, struct Pair p6) {
    ESP_LOGW("abi_test_6", "p6.a=%d, p6.b=%d", p6.a, p6.b);
}

Rust code:

use esp_idf_sys::c_types::c_int;

#[repr(C)]
struct Pair {
    a: c_int,
    b: c_int,
}

extern "C" {
    fn abi_test_5(p1: c_int, p2: c_int, p3: c_int, p4: c_int, p5: Pair);
    fn abi_test_6(p1: c_int, p2: c_int, p3: c_int, p4: c_int, p5: c_int, p6: Pair);
}

fn main() {
    unsafe {
        abi_test_5(0, 0, 0, 0, Pair { a: 1, b: 2 });
        abi_test_6(0, 0, 0, 0, 0, Pair { a: 1, b: 2 });
    }
}

outputs:

W (1335) abi_test_5: p5.a=1, p5.b=2
W (1335) abi_test_6: p6.a=2, p6.b=12288

[MabezDev]

@ollpu I'm working on this now, could you provide your test case as a repo?

[ollpu]

@MabezDev Here you go: https://github.com/ollpu/esp-rustc-abi-bug. Compiles, but I can't test it on hardware right now. The output happens to be this:

I (398) abi_test_5: p5.a=1, p5.b=2
I (398) abi_test_6: p6.a=2, p6.b=-1515870811

[MabezDev]

Thanks for the test cases @ollpu! I extended them a little more here: https://github.com/MabezDev/esp-rustc-abi-bug/. I think I have the ABI fixed up in 564a0ce.

I'll see if we can get a patch release of our compiler out soon. In the meantime, and if you are able, could you try out the branch above? It would require compiling the rustc, so I understand if you don't want to :D

[ollpu]

@MabezDev Looking good!

Before:

I (403) abi_test_5: p5.a=1, p5.b=2
I (403) abi_test_6: p6.a=2, p6.b=1073440000
I (413) abi_test_7: p7.a=1, p7.b=2
I (413) abi_test_8: p8.a=1, p8.b=2
I (423) abi_test_18: p18.a=1, p18.b=2
I (423) ret_abi_test_internal_4: a=1, b=2, c=3, d=4
I (433) ret_abi_test_internal_5: a=1, b=2, c=3, d=4, e=5
ret_abi_test_4: Return4 { a: 1, b: 2, c: 3, d: 4 }
ret_abi_test_5: Return5 { a: 1, b: 2, c: 3, d: 4, e: 5 }

After:

I (397) abi_test_5: p5.a=1, p5.b=2
I (397) abi_test_6: p6.a=1, p6.b=2
I (407) abi_test_7: p7.a=1, p7.b=2
I (407) abi_test_8: p8.a=1, p8.b=2
I (417) abi_test_18: p18.a=1, p18.b=2
I (417) ret_abi_test_internal_4: a=1, b=2, c=3, d=4
I (427) ret_abi_test_internal_5: a=1, b=2, c=3, d=4, e=5
ret_abi_test_4: Return4 { a: 1, b: 2, c: 3, d: 4 }
ret_abi_test_5: Return5 { a: 1, b: 2, c: 3, d: 4, e: 5 }

Edit: Ran the new tests on the old compiler as well. Looks like they don't break there.

[MabezDev]

Thanks, @ollpu! I've just pushed to the 1.63.0.1 release branch with this fix included. Builds should be available soon :)

[georgik]

Artifacts will become available here: https://github.com/esp-rs/rust-build/releases/tag/v1.63.0.1

[ollpu]

Heads up: We noticed that between updating from 1.63.0.0 to 1.63.0.1, std::thread::sleep (via esp-idf-sys) no longer works – it doesn't wait at all. I didn't have time to comb through the call tree yet, but this could be indicative of an ABI related miscompilation.