Handle validation of uxtheme.dll undocumented exported functions in ARM64 builds

[hmartinez82]

This is required for #1045 to continue.

For reference, here are the disassemblies:
AllowDarkModeForWindow():

00007FFB16C32450 D503237F             pacibsp  
00007FFB16C32454 A9BE53F3             stp         x19,x20,[sp,#-0x20]!  
00007FFB16C32458 F9000BF5             str         x21,[sp,#0x10]  
00007FFB16C3245C A9BF7BFD             stp         fp,lr,[sp,#-0x10]!  
00007FFB16C32460 910003FD             mov         fp,sp  
00007FFB16C32464 53001C33             uxtb        w19,w1  
00007FFB16C32468 D29523C1             mov         x1,#0xA91E  
00007FFB16C3246C AA0003F5             mov         x21,x0  
00007FFB16C32470 9400F226             bl          #GetPropW (07FFB16C6ED08h)
...

AllowDarkModeForWindowWithTelemetryId():

00007FFB16C33210 D503237F             pacibsp  
00007FFB16C33214 A9BE53F3             stp         x19,x20,[sp,#-0x20]!  
00007FFB16C33218 F9000BF5             str         x21,[sp,#0x10]  
00007FFB16C3321C A9BF7BFD             stp         fp,lr,[sp,#-0x10]!  
00007FFB16C33220 910003FD             mov         fp,sp  
00007FFB16C33224 53001C34             uxtb        w20,w1  
00007FFB16C33228 D29523C1             mov         x1,#0xA91E  
00007FFB16C3322C AA0003F5             mov         x21,x0  
00007FFB16C33230 9400EEB6             bl          #GetPropW (07FFB16C6ED08h)
...

SetPreferredAppMode():

00007FFB16C33E10 D503237F             pacibsp  
00007FFB16C33E14 F81F0FF3             str         x19,[sp,#-0x10]!  
00007FFB16C33E18 A9BF7BFD             stp         fp,lr,[sp,#-0x10]!  
00007FFB16C33E1C 910003FD             mov         fp,sp  
00007FFB16C33E20 D00007E8             adrp        x8,wil::details::g_enabledStateManager+40h (07FFB16D31000h)  
00007FFB16C33E24 B94BD913             ldr         w19,[x8,#0xBD8]  
00007FFB16C33E28 2A0003E1             mov         w1,w0  
00007FFB16C33E2C 912F6100             add         x0,x8,#0xBD8  
00007FFB16C33E30 97FF7910             bl          _InterlockedExchange (07FFB16C12270h)  
00007FFB16C33E34 2A1303E0             mov         w0,w19  
00007FFB16C33E38 A8C17BFD             ldp         fp,lr,[sp],#0x10  
00007FFB16C33E3C F84107F3             ldr         x19,[sp],#0x10  
00007FFB16C33E40 D50323FF             autibsp  
00007FFB16C33E44 D65F03C0             ret  
00007FFB16C33E48 B0000090             adrp        xip0,GetBufferedPaintBitsEx+7320h (07FFB16C44000h)
...

[SyntevoAlex]

The code makes sense to me.
But I'm judging blindly without testing, let's wait until it can be smoke tested.

[hmartinez82]

How do you want to smoke test? :) these changes don't build standalone. They need @chirontt changes to be built. I thought the idea was for this to be merged so he can merge his

[SyntevoAlex]

He will merge return FALSE, then your branch is rebased on top and tesred.

[chirontt]

@hmartinez82 I've temporarily merged these changes of os_custom.c file to my PR and attempted to 'build' it, but the MSVC compiler complains about reinterpret_cast:

[INFO]      [exec] Microsoft (R) C/C++ Optimizing Compiler Version 19.38.33135 for ARM64
[INFO]      [exec] Copyright (C) Microsoft Corporation.  All rights reserved.
[INFO]      [exec]
[INFO]      [exec]      cl -O1 /WX /W4 -DNDEBUG -DUNICODE -D_UNICODE /c   -DJNI64  -DSWT_VERSION=4964 -DSWT_REVISION=7 -DUSE_ASSEMBLER  /I"\apps\jdk-17.0.10+7_arm64\include" /I"\apps\jdk-17.0.10+7_arm64\include\win32" /I. os_custom.c
[INFO]      [exec] os_custom.c
[INFO]      [exec] os_custom.c(83): error C2065: 'reinterpret_cast': undeclared identifier

[hmartinez82]

omg, it's C code, not C++ . I'll fix it in a moment 😅

[hmartinez82]

@chirontt I've updated this PR with the new changes. They are the same, just using C casting instead of C++ ones.

[chirontt]

Yes, it now compiles properly and the SWT binaries are generated for Arm64:

@SyntevoAlex how do we smoke test it, i.e. maybe not about the os_custom.c file itself, but about the generated *.dll modules? Do we have some C/Java tests for them already in the repo?

[SyntevoAlex]

Snippet Bug562043_DarkTableNoHover.java should have dark scrollbars (expand some Tree item for scrollbar to appear).

But also set display.setData("org.eclipse.swt.internal.win32.useShellTitleColoring", true); and check that Shell's titlebar is dark.

[hmartinez82]

@SyntevoAlex I can't test this. It will have to be @chirontt. I'll close this since he's incorporating it in his pull request.

I can't test this because I don't knwk to build anything that uses maven, :( I don't know how

[chirontt]

@hmartinez82 you should wait for someone (@SyntevoAlex ?) to approve this PR and merge it to master, to claim credit for your excellent work. I don't plan to incorporate it into my PR. My plan is to remove this file from my PR.

[hmartinez82]

Ok, understood. It seems that they want you to merge yours first returning false then I'll rebsse this. 👍 Sent with Shortwave <https://www.shortwave.com?utm_medium=email&utm_content=signature&utm_source=aGVybmFuLmMubWFydGluZXpAZ21haWwuY29t>

…

On Thu Feb 15, 2024, 03:50 AM GMT, Tue Ton ***@***.***> wrote: @hmartinez82 <https://github.com/hmartinez82> you should wait for someone ***@***.*** <https://github.com/SyntevoAlex> ?) to approve this PR and merge it to master, to claim credit for your excellent work. I don't plan to incorporate it into my PR. My plan is to remove this file from my PR. — Reply to this email directly, view it on GitHub <#1048 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIMVGFLW26OU6MCXJXUL23YTWAXXAVCNFSM6AAAAABDHY4HKOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBVGMZDANJXGU>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[chirontt]

Snippet Bug562043_DarkTableNoHover.java should have dark scrollbars (expand some Tree item for scrollbar to appear).

But also set display.setData("org.eclipse.swt.internal.win32.useShellTitleColoring", true); and check that Shell's titlebar is dark.

Yes, it works on my Arm64 box. I've built the org.eclipse.swt.win32.win32.aarch64-3.125.0-SNAPSHOT.jar which embeds the swt*.dll for Arm64, then run the Bug562043_DarkTableNoHover.java and it shows the dark scrollbars nicely, but with the lighted window title bar:

Then, as you suggested, I add the following statement to the Java code:

		display.setData("org.eclipse.swt.internal.win32.useShellTitleColoring", true);

and I get a window with dark title bar:

and out of curiosity, I comment out this existing statement:

		display.setData("org.eclipse.swt.internal.win32.useDarkModeExplorerTheme", true);

then I get a window with both lighted title bar and scrollbars:

[chirontt]

Further more, I've merged this PR to my Arm64 fork of this repo and built the complete Eclipse SDK, via the aggregator, and here's how it looks in Arm64:

For comparison, in previous build I used the return FALSE; statements in the os_custom.c for Arm64, and here's how it used to look:

I think my testing is enough to confirm the working of this PR for Arm64.

[SyntevoAlex]

Thanks for the detailed testing!

Yes, it can be merged, then.

Please squash commits into one nice commit.

Previous build failed for some weird reason, when you squash and push it will rebuild, hopefully well this time.

[iloveeclipse]

Yes, it can be merged, then.

Please note, we are between M3 and RC1 time frame. Any merge requires extra review & PM approval.
https://www.eclipse.org/lists/platform-releng-dev/msg39974.html

[SyntevoAlex]

Yes, I was checking mails right now... It says the feature freeze is on Feb 21 only?

Either way, I suggest to merge this PR, because it's under #elif defined(_M_ARM64) and therefore code changes may only affect arm64 build, which is not currently present.

@iloveeclipse please clarify what to do.

[github-actions]

Test Results

   299 files  ±0     299 suites  ±0   5m 58s ⏱️ +22s
 4 100 tests ±0   4 092 ✅ ±0   8 💤 ±0  0 ❌ ±0 
12 212 runs  ±0  12 137 ✅ ±0  75 💤 ±0  0 ❌ ±0 

Results for commit 8a19fbe. ± Comparison against base commit 36c3b19.

♻️ This comment has been updated with latest results.

[hmartinez82]

Rebased / squashed as requested

[iloveeclipse]

please clarify what to do.

Please add your "approving" review on this PR.
Looking on code, I agree it shouldn't harm so OK to merge for RC1.

[SyntevoAlex]

Looks good to me!

[SyntevoAlex]

The branch built successfully.
I added my approval.

[SyntevoAlex]

Everything looks good to me, I suggest to merge.

[SyntevoAlex]

@iloveeclipse so, should I merge? Or approval from PM is still needed? Who is PM?
Also, oculd you please clarify if freeze is already in effect or only since Feb 21?

[merks]

If everyone agrees it's very safe good to go, then it also has my +1 as a PMC member.

[SyntevoAlex]

OK, merged.
@hmartinez82 thanks for your contribution!
It was a pleasure to see someone who's not scared of disasm :)

[hmartinez82]

@SyntevoAlex If you want to give me a gift. I wouldn't mind a local build of SmartGit for Windows on ARM64. You can even give it to me without the Java Runtime. I'll use Microsoft's Java 17 build for ARM64 :)

[tmssngr]

@hmartinez82 At the moment Windows/ARM has low priority for us. Please vote for https://smartgit.userecho.com/communities/1/topics/1416-windows-arm-support

[chirontt]

@hmartinez82 Bad news: with recent Windows update to my Arm64 box, like this build:

these validation code no longer works, i.e. I get lighted scrollbars rather than dark ones. I guess Microsoft has updated/changed the uxtheme.dll file, so we may need your magical disassembly touch again :)

[hmartinez82]

Ok. I have the same version. I'll see what I can do.

@chirontt the function that changed is SetPreferredAppMode(). The other are still being validated. I'll create a PR soon.

[hmartinez82]

@chirontt I'll create the PR but you'll have to test it. I don't even know if the ordianal 135 is still SetPreferredAppMode(), but here is the disassembly:

00007FFA2A1D3E10 D503237F             pacibsp  
00007FFA2A1D3E14 F81F0FF3             str         x19,[sp,#-0x10]!  
00007FFA2A1D3E18 A9BF7BFD             stp         fp,lr,[sp,#-0x10]!  
00007FFA2A1D3E1C 910003FD             mov         fp,sp  
00007FFA2A1D3E20 B00007E8             adrp        x8,wil::details::g_enabledStateManager+40h (07FFA2A2D0000h)  
00007FFA2A1D3E24 B94BB913             ldr         w19,[x8,#0xBB8]  
00007FFA2A1D3E28 2A0003E1             mov         w1,w0  
00007FFA2A1D3E2C 912EE100             add         x0,x8,#0xBB8  
00007FFA2A1D3E30 97FF7910             bl          _InterlockedExchange (07FFA2A1B2270h)  
00007FFA2A1D3E34 2A1303E0             mov         w0,w19  
00007FFA2A1D3E38 A8C17BFD             ldp         fp,lr,[sp],#0x10  
00007FFA2A1D3E3C F84107F3             ldr         x19,[sp],#0x10  
00007FFA2A1D3E40 D50323FF             autibsp  
00007FFA2A1D3E44 D65F03C0             ret

[hmartinez82]

@chirontt @SyntevoAlex #1172