Go: windows/arm: callback regression in upcoming 1.16

[zx2c4]

https://go-review.googlesource.com/c/go/+/258938 prevents windows/arm from working, with just a small TODO added to the code indicating that it's broken. That's a big problem. This issue is to track fixing that before 1.16 lands.

https://go-review.googlesource.com/c/go/+/269037 has additional cleanups of that code.

I've got an arm builder ready to test CLs that address this.

@cherrymui @aclements @alexbrainman @bradfitz

[zx2c4]

@gopherbot set tag release-blocker

[ALTree]

@zx2c4 If this actually only breaks windows/arm (and won't cause any issue on other platforms), I don't think it should be given release-blocker status.

windows/arm is not a first class port ("Broken builds block releases"). In fact, I would argue it's not even a second-class port: we don't have a builder at the moment (#38607: add a Windows ARM builder). windows/arm is basically in a "do we even support this" status.

We can still tentatively put this in the 1.16 milestone (which I did) but I don't think it'll block the 1.16 release.

[zx2c4]

Ooof. FWIW, real software is about to ship using windows/arm, because real hardware that uses arm on windows has been successfully commercialized. Hopefully we can get Go into shape for 1.16.

[zx2c4]

@aclements As discussed on that CL, I look forward to testing/polishing some things on the hardware. I thought I'd paste here the sample programs (from @rozmansi) that I've been using to test this:

C reference:

#include <windows.h>
#include <stdio.h>

VOID CALLBACK EventCallback(HWINEVENTHOOK hWinEventHook, DWORD event, HWND hwnd, LONG idObject, LONG idChild, DWORD idEventThread, DWORD dwmsEventTime)
{
    fprintf(stdout, "%p %x %p %x %x %x %x\n", hWinEventHook, event, hwnd, idObject, idChild, idEventThread, dwmsEventTime);
}

int main(void)
{
    HWINEVENTHOOK Hook = SetWinEventHook(EVENT_OBJECT_CREATE, EVENT_OBJECT_CREATE, NULL, EventCallback, 0, 0, WINEVENT_SKIPOWNPROCESS|WINEVENT_OUTOFCONTEXT);
    if (!Hook) {
        fprintf(stderr, "Error setting win event hook: 0x%x\n", GetLastError());
        return 1;
    }
    for (;;) {
        MSG Msg;
        if (GetMessageW(&Msg, 0, 0, 0)) {
            TranslateMessage(&Msg);
            DispatchMessageW(&Msg);
        }
    }
    UnhookWinEvent(Hook);
    return 0;
}

Go crasher:

package main

import (
    "fmt"
    "github.com/lxn/win"
)

func EventCallback(hWinEventHook win.HWINEVENTHOOK, event uint32, hwnd win.HWND, idObject int32, idChild int32, idEventThread uint32, dwmsEventTime uint32) uintptr {
    fmt.Printf("%x %x %x %x %x %x %x\n", hWinEventHook, event, hwnd, idObject, idChild, idEventThread, dwmsEventTime)
    return 0
}

func main() {
    hook, err := win.SetWinEventHook(win.EVENT_OBJECT_CREATE, win.EVENT_OBJECT_CREATE, 0, EventCallback, 0, 0, win.WINEVENT_SKIPOWNPROCESS|win.WINEVENT_OUTOFCONTEXT)
    defer win.UnhookWinEvent(hook)
    if err != nil {
        fmt.Printf("Error setting win event hook: %v\n", err)
        return
    }
    for {
        var msg win.MSG
        if m := win.GetMessage(&msg, 0, 0, 0); m != 0 {
            win.TranslateMessage(&msg)
            win.DispatchMessage(&msg)
        }
    }
}

With some minor differences in the format string, these should produce the same output on the console, which one can generate by clicking between various other windows on the system:

And here are some (working) pre-compiled binaries: tester.zip

[gopherbot]

Change https://golang.org/cl/270077 mentions this issue: runtime: simplify C -> Go callbacks on windows/arm

[networkimprov]

Just in case...

@gopherbot add release-blocker
@gopherbot add OS-Windows

[gopherbot]

Change https://golang.org/cl/271178 mentions this issue: runtime: support new callbackasm1 calling convention on windows/arm

[networkimprov]

Shouldn't regressions be release-blocker by default?

[ALTree]

Shouldn't regressions be release-blocker by default?

@networkimprov No, not at all. Serious regressions in first class ports, maybe. I already explained above that this is not the case.