Possible code generation issue on Windows ARM64

help
1

Hi folks,

I've been working on a project that uses imgui-rs and ran into an issue running my code on ARM64 Windows machines (target aarch64-pc-windows-msvc). At first I filed an issue in the imgui-rs repo, but the more I stare at it I think it may actually be a lower level issue. I can't reproduce the issue when targeting x86_64-pc-windows-msvc.

I've put together a minimal repro here: https://github.com/robmikh/imguiarm64repro/tree/sys

use imgui_sys::{
    igSetCurrentContext, igSetNextWindowPos, ImGuiCond_FirstUseEver, ImGuiContext, ImVec2,
};

fn main() {
    // Dummy ImGuiContext
    let mut bytes = vec![0u8; 1025];
    let context = bytes.as_mut_ptr() as *mut ImGuiContext;
    unsafe { igSetCurrentContext(context) };

    // When you step through igSetNextWindowPos and follow it to the C++ side, you can
    // see that pos, cond, and pivot have incorrect values. It may or not trigger the
    // assert in ImGui::SetNextWindowPos depending on what cond becomes. On my machine,
    // cond becomes 1025 (seemingly from vec size above) and triggers the assert.
    let pos = ImVec2 { x: 200.0, y: 300.0 };
    let cond = ImGuiCond_FirstUseEver; // 4
    let pivot = ImVec2::zero();
    unsafe { igSetNextWindowPos(pos, cond as i32, pivot) };
}

What happens is that when I call igSetNextWindowPos, the parameters seem to change when following the calls in the debugger. For example, given the following stack:

0:000> kc
 # Call Site
00 imguiarm64repro!ImGui::SetNextWindowPos
01 imguiarm64repro!igSetNextWindowPos
02 imguiarm64repro!imguiarm64repro::main

These are the locals in the last Rust frame (in particular, pay attention to cond):

0:000> dv
          pivot = struct imgui_sys::bindings::ImVec2
          bytes = { len=0x401 }
        context = 0x000001c7`ac10e8d0
            pos = struct imgui_sys::bindings::ImVec2
           cond = 4

However, in igSetNextWindowPos and ImGui::SetNextgWindowPos, the locals look like this:

0:000> dv
            pos = struct ImVec2
           cond = 0n1025
          pivot = struct ImVec2

From what I can tell, the C++ code expects cond to be in register w1 but it instead seems to be in w0:

    imguiarm64repro!igSetNextWindowPos:
00007ff6`ecb2d278 fd7bbda9 stp     fp, lr, [sp, #-0x30]!
00007ff6`ecb2d27c fd030091 mov     fp, sp
00007ff6`ecb2d280 e01300f9 str     x0, [sp, pos{.x(!!)} (sp+20h)]
00007ff6`ecb2d284 e11300b9 str     w1, [sp, cond (sp+10h)]
00007ff6`ecb2d288 e20f00f9 str     x2, [sp, pivot{.x(!!)} (sp+18h)]
00007ff6`ecb2d28c e2630091 add     x2, sp, #0x18
00007ff6`ecb2d290 e11340b9 ldr     w1, [sp, cond (sp+10h)]
00007ff6`ecb2d294 e0830091 add     x0, sp, #0x20
00007ff6`ecb2d298 4862fd97 bl      imguiarm64repro!ImGui::SetNextWindowPos (7ff6eca85bb8)
00007ff6`ecb2d29c 1f2003d5 nop     
00007ff6`ecb2d2a0 fd7bc3a8 ldp     fp, lr, [sp], #0x30
00007ff6`ecb2d2a4 c0035fd6 ret     

0:000> r
 x0=0000000000000004   x1=0000000000000401   x2=fffffffffffffff1   x3=000001c7ac10ec80
 x4=000001c7ac10ecd1   x5=000000c64a4ff290   x6=00000000000000f8   x7=000001c7ac10daf8
 x8=0000000000000000   x9=00007ff6ecbc0000  x10=000001c7ac10e8d0  x11=0000000000000401
x12=000001c7ac10daf0  x13=0000000000000040  x14=0000000000000000  x15=000000000000d000
x16=000097821d2b149f  x17=000000c64a3c9260  x18=000000c64a3c9000  x19=000000c64a4ff9a0
x20=00007ff6ecb5d4f0  x21=0000000000000000  x22=0000000000000000  x23=0000000000000000
x24=0000000000000000  x25=0000000000000000  x26=0000000000000000  x27=0000000000000000
x28=0000000000000000   fp=000000c64a4ff7c0   lr=00007ff6eca61dbc   sp=000000c64a4ff7c0
 pc=00007ff6ecb2d284  psr=80000000 N--- EL0
imguiarm64repro!igSetNextWindowPos+0xc:
00007ff6`ecb2d284 b90013e1 str         w1,[sp,#0x10]

Am I on the right track or have I overlooked something? I wanted to get a second opinion before I opened an issue in the rust-lang/rust repo.

Apologies for the mess of a post, I'm still wrapping my head around this.

EDIT: I'm using this version of rustc:
rustc 1.81.0 (eeb90cda1 2024-09-04)
binary: rustc
commit-hash: eeb90cda1969383f56a2637cbd3037bdf598841c
commit-date: 2024-09-04
host: aarch64-pc-windows-msvc
release: 1.81.0
LLVM version: 18.1.7

2

What are pivot and pos on the C++ side?

Any chance the context (bytes) is too small?

You cast cond as a signed 32 bit integer. Any chance that's declared as 64 bits? (I assume the Rust compiler would complain if it were.)

pub type ImGuiCond_ = cty::c_uint;
...that "u" implies cond is unsigned. I doubt that matters but it is suspicious. Any chance you're building with an out-of-date version of imgui_sys?

3

Both pos and pivot come out wrong as well (this comes from the ImGui::SetNextWindowPos frame):

0:000> dv
            pos = 0x000000c6`4a4ff7e0
           cond = 0n1025
          pivot = 0x000000c6`4a4ff7d8
              g = 0x000001c7`ac10e8d0
0:000> dx -r1 ((imguiarm64repro!ImVec2 *)0xc64a4ff7e0)
((imguiarm64repro!ImVec2 *)0xc64a4ff7e0)                 : 0xc64a4ff7e0 [Type: ImVec2 *]
    [+0x000] x                : 0.000000 [Type: float]
    [+0x004] y                : 0.000000 [Type: float]
0:000> dx -r1 ((imguiarm64repro!ImVec2 *)0xc64a4ff7d8)
((imguiarm64repro!ImVec2 *)0xc64a4ff7d8)                 : 0xc64a4ff7d8 [Type: ImVec2 *]
    [+0x000] x                : -nan [Type: float]
    [+0x004] y                : -nan [Type: float]

The ImGuiContext doesn't really come into play, I only do it because the first line of SetNextWindowPos dereferences a global. Just to be clear, I don't mess around with the context in my actual project where I first found this (nor do I use imgui-sys directly). I just did it here to keep the repro small. Here's the code in ImGui::SetNextWindowPos:

void ImGui::SetNextWindowPos(const ImVec2& pos, ImGuiCond cond, const ImVec2& pivot)
{
    ImGuiContext& g = *GImGui;
    IM_ASSERT(cond == 0 || ImIsPowerOfTwo(cond)); // Make sure the user doesn't attempt to combine multiple condition flags.
    g.NextWindowData.Flags |= ImGuiNextWindowDataFlags_HasPos;
    g.NextWindowData.PosVal = pos;
    g.NextWindowData.PosPivotVal = pivot;
    g.NextWindowData.PosCond = cond ? cond : ImGuiCond_Always;
}

In my full project, the assert is triggered because cond is some large value that isn't a power of two, despite passing in 4.

I was also a bit confused about the u32/i32 mismatch, but I don't think it comes to anything. There is a similar cast in imgui-rs:

pub fn begin(self) -> Option<WindowToken<'ui>> {
        if self.pos_cond != Condition::Never {
            unsafe {
                sys::igSetNextWindowPos(
                    self.pos.into(),
                    self.pos_cond as i32,
                    self.pos_pivot.into(),
                )
            };
        }
        // ... snip ...
}

This repro uses the latest imgui-sys (v0.12) and my full project uses the latest imgui-rs (v0.12).

4

It does if it's too small, igSetCurrentContext overruns it, leaving behind a corrupt stack frame (undefined behavior). Which is why I asked if it's reasonably sized.

Does this...

...require pos and pivot to be passed by reference? They are not in your examples.

Are you using bindgen? Did it mess up the pass-by-reference parameters?

5

Fair enough!

Ah, yeah this comes down to the structure of imgui-sys. It looks like the crate compiles cimgui, which wraps imgui with a C-friendly API. That C-friendly API is then what bindgen is run against. I don't call directly into ImGui::SetNextWindowPos, but instead I call into this wrapper:

CIMGUI_API void igSetNextWindowPos(const ImVec2 pos,ImGuiCond cond,const ImVec2 pivot)
{
    return ImGui::SetNextWindowPos(pos,cond,pivot);
}

This is the binding for that API:

extern "C" {
    pub fn igSetNextWindowPos(pos: ImVec2, cond: ImGuiCond, pivot: ImVec2);
}

PS: Happy anniversary on the site :slight_smile:

6

Oh yeah. Two days away. Cool. Thanks.

As far as I can tell that's defined here.

I believe this defines the calling convention for Windows + ARM64.

Presumably CIMGUI_API includes extern "C" when building the C wrapper and does not include that when bindgen runs on it. I don't see where that will make a difference in the calling convention. But, it is a potential difference which means it could be causing trouble.

If you can, without too much difficulty, I suggest creating your own replacement for igSetNextWindowPos that takes references instead of structs. I'm struggling to understand the rules the compiler uses when automatically switching to by-reference. Maybe the Rust folks also struggled with those rules and made a mistake. Explicitly passing by-reference should eliminate the possibility mentioned above and the possibility that automatic switch-to-by-reference is causing problems.

7

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.