[win32][arm64] Support Dark Theme on newer Windows builds

[hmartinez82]

Continues #1048 #1045

Here's 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

The add x0,x8, instruction changed its parameter. So now the adrp x8,... instruction looks more stable because it uses a global variable (Using a global variable is also what is done when validating for the Intel x64 uxtheme.dll)

See https://developer.arm.com/documentation/ddi0602/2024-03/Base-Instructions/ADRP--Form-PC-relative-address-to-4KB-page-

• functionPtr[0x10] & 0x1F) == 0x08 checks that the destination register is x8
• (functionPtr[0x13] & 0x90) == 0x90 checks that the opcode is ADRP

[github-actions[bot]]

Test Results

   418 files  ±0     418 suites  ±0   7m 19s :stopwatch: -50s  4 121 tests ±0   4 113 :white_check_mark: ±0   8 :zzz: ±0  0 :x: ±0  16 313 runs  ±0  16 221 :white_check_mark: ±0  92 :zzz: ±0  0 :x: ±0 

Results for commit a0c4d795. ± Comparison against base commit b435cb8d.

:recycle: This comment has been updated with latest results.

[chirontt]

Thanks for your quick work! I now have the dark scrollbars again in my Arm64 box. Here are how I test it:

• merge this PR to my fork, and then locally rebuild the SWT native binaries by:

cd binaries\org.eclipse.swt.win32.win32.aarch64
del swt*.dll
mvn clean process-resources -Dnative=win32.win32.aarch64

The above mvn command would produce the updated SWT native (swt*.dll) files in the current directory.

• create the module jar file which would embed the above SWT natives:

mvn clean verify

and the org.eclipse.swt.win32.win32.aarch64-3.126.0-SNAPSHOT.jar file is generated in the target directory.

• manually run the Java test file Bug562043_DarkTableNoHover.java, using the above jar file in the classpath:

java -cp target\org.eclipse.swt.win32.win32.aarch64-3.126.0-SNAPSHOT.jar ^
..\..\tests\org.eclipse.swt.tests.win32\ManualTests\org\eclipse\swt\tests\win32\snippets\Bug562043_DarkTableNoHover.java

and the test program shows correct dark scrollbars in its window:

For comparison, here's the same window (with lighted scrollbars) before this fix:

[chirontt]

Further more, I've built the complete Eclipse SDK with this fix, from my aggregator fork, and here's its main window showing nicely dark scrollbars on my Arm64 box:

In previous builds before this fix, it was showing lighted scrollbars, which alerted me to the changed uxtheme.dll file due to recent Windows Arm64 update.

[HannesWell]

@niraj-modi or @SyntevoAlex could you please review this? I have no clue about the native code and no way to test it.

[SyntevoAlex]

The new test looks quite weak to me, because adrp x8, ??? is a rather common instruction which doesn't really identify the function we need for the dark theme.

I suggest that instead both offsets 0xBB8 and 0xBD8 are recognized (to recognize both windows versions)

[hmartinez82]

@SyntevoAlex ah good idea. I'll work on that tomorrow :)

[hmartinez82]

@chirontt can you try with the latest changes? I changed the title of the PR too

[SyntevoAlex]

It would be nice to also have code comments with Windows versions, like here:

https://github.com/eclipse-platform/eclipse.platform.swt/blob/1a35d6e03a426a86924516c347fdba21c02db1e8/bundles/org.eclipse.swt/Eclipse%20SWT%20PI/win32/library/os_custom.c#L124-L138

[SyntevoAlex]

^ this is just a suggestion, I will still approve without it.

[hmartinez82]

I don't have the old versions. @chirontt Can you find me the Windows builds before and after the original PR stopped working?

[chirontt]

Can you find me the Windows builds before and after the original PR stopped working?

I'm not quite sure I follow you here. It's not the Eclipse build that stops working; it's the recent (last month's) Windows Arm64 update that causes the problem due to changes in the uxtheme.dll in the Windows update. With your latest Windows version running in your Arm64 box, the lighted scrollbar problem appears, for any Eclipse WoA versions.

You can grab the latest build of the Eclipse SDK here especially the WoA zip package. Just unzip it and run the Eclipse IDE, put it in dark mode and you'll see the lighted scrollbars in the IDE. This latest build contains your original commit for the os_custom.c file.

To see if/how it works before, you need to restore your WoA box to before the current (last month's) Windows update, and test it with the same Eclipse SDK mentioned above. You'd see the dark scrollbars as intended.

[chirontt]

can you try with the latest changes? I changed the title of the PR too

I've tried the same steps as in my previous comment, and the scrollbars are still properly in dark mode, with your latest fix. Here's the screenshot of the Bug562043_DarkTableNoHover.java test:

[SyntevoAlex]

@chirontt Just to clarify:

• Before this PR, on your newest Windows, scrollbars are light
• With this PR, on your newest Windows, scrollbars are dark once again is that correct?

If yes, please run ver in your commandline and give us Windows version number (where it was broken and now once again fixed).

[chirontt]

* Before this PR, on your newest Windows, scrollbars are light

Yes.

* With this PR, on your newest Windows, scrollbars are dark once again
  is that correct?

Yes.

If yes, please run ver in your commandline and give us Windows version number (where it was broken and now once again fixed).

>ver

Microsoft Windows [Version 10.0.22631.3447]

[SyntevoAlex]

@hmartinez82 to clarify, I'm waiting for you. @chirontt already got the version numbers, see above.

[SyntevoAlex]

Please squash commits (because separating them does not convey any additional value here), you can use PR title as commit message. Then it can be merged.

[chirontt]

@SyntevoAlex , @hmartinez82 Sorry to sound like a broken record again, but the new Windows update this week (last Tuesday, to be precise) breaks this os_custom.c file again for WoA. The current WoA version on my box is now:

>ver

Microsoft Windows [Version 10.0.22631.3593]

and running the Bug562043_DarkTableNoHover.java test on it now shows the light scrollbars in dark mode:

I've checked the file date of uxtheme.dll and surely enough, it was updated last Tuesday:

>dir \Windows\System32\uxtheme.dll
 Volume in drive C has no label.
 Volume Serial Number is 7EBE-F3EB

 Directory of C:\Windows\System32

05/14/2024  08:01 PM         1,223,680 uxtheme.dll
               1 File(s)      1,223,680 bytes
               0 Dir(s)  96,054,374,400 bytes free

[hmartinez82]

The argument changed again, it's 0xBF8 now in SetPreferredAppMode()

00007FFD7EE92890 D503237F             pacibsp  
00007FFD7EE92894 F81F0FF3             str         x19,[sp,#-0x10]!  
00007FFD7EE92898 A9BF7BFD             stp         fp,lr,[sp,#-0x10]!  
00007FFD7EE9289C 910003FD             mov         fp,sp  
00007FFD7EE928A0 F00007E8             adrp        x8,wil::details::g_enabledStateManager+40h (07FFD7EF91000h)  
00007FFD7EE928A4 B94BF913             ldr         w19,[x8,#0xBF8]  
00007FFD7EE928A8 2A0003E1             mov         w1,w0  
00007FFD7EE928AC 912FE100             add         x0,x8,#0xBF8  
00007FFD7EE928B0 97FF7E70             bl          _InterlockedExchange (07FFD7EE72270h)  
00007FFD7EE928B4 2A1303E0             mov         w0,w19  
00007FFD7EE928B8 A8C17BFD             ldp         fp,lr,[sp],#0x10  
00007FFD7EE928BC F84107F3             ldr         x19,[sp],#0x10  
00007FFD7EE928C0 D50323FF             autibsp  
00007FFD7EE928C4 D65F03C0             ret

I think that chasing this value is not a good idea. It keeps changing, it seems very unstable across builds of uxtheme.dll. The line above, which deals with the global value has been stable across all the changes we've seen so far. The code detection for Intel is doing exactly that. See https://github.com/eclipse-platform/eclipse.platform.swt/blob/a39c32129cd733639e0ee68680b6ca974be1d645/bundles/org.eclipse.swt/Eclipse%20SWT%20PI/win32/library/os_custom.c#L233

What if I do what I did originally ? And also add a test for the location of the ret instruction? That's what's being done for Intel x64 too.

[hmartinez82]

@chirontt Try with the change from 1 min ago.

[chirontt]

@chirontt Try with the change from 1 min ago.

Yes, it works now, i.e. the scrollbars are now dark:

[SyntevoAlex]

The code detection for Intel is doing exactly that

This is not right, it checks a lot more: it checks that two instructions mention the SAME global variable.

https://github.com/eclipse-platform/eclipse.platform.swt/blob/a39c32129cd733639e0ee68680b6ca974be1d645/bundles/org.eclipse.swt/Eclipse%20SWT%20PI/win32/library/os_custom.c#L227-L230

[hmartinez82]

Given the three disassembles we've seen so far, what do you recommend?

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  

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

00007FFD7EE92890 D503237F             pacibsp  
00007FFD7EE92894 F81F0FF3             str         x19,[sp,#-0x10]!  
00007FFD7EE92898 A9BF7BFD             stp         fp,lr,[sp,#-0x10]!  
00007FFD7EE9289C 910003FD             mov         fp,sp  
00007FFD7EE928A0 F00007E8             adrp        x8,wil::details::g_enabledStateManager+40h (07FFD7EF91000h)  
00007FFD7EE928A4 B94BF913             ldr         w19,[x8,#0xBF8]  
00007FFD7EE928A8 2A0003E1             mov         w1,w0  
00007FFD7EE928AC 912FE100             add         x0,x8,#0xBF8  
00007FFD7EE928B0 97FF7E70             bl          _InterlockedExchange (07FFD7EE72270h)  
00007FFD7EE928B4 2A1303E0             mov         w0,w19  
00007FFD7EE928B8 A8C17BFD             ldp         fp,lr,[sp],#0x10  
00007FFD7EE928BC F84107F3             ldr         x19,[sp],#0x10  
00007FFD7EE928C0 D50323FF             autibsp  
00007FFD7EE928C4 D65F03C0             ret

[SyntevoAlex]

Here's the diff

D503237F pacibsp
F81F0FF3 str  x19,[sp,#-0x10]!  
A9BF7BFD stp  fp,lr,[sp,#-0x10]!  
910003FD mov  fp,sp  
D00007E8 adrp x8,wil::details::g_enabledStateManager+40h (07FFB16D31000h)  
B94BD913 ldr  w19,[x8,#0xBD8]  
2A0003E1 mov  w1,w0  
912F6100 add  x0,x8,#0xBD8  
97FF7910 bl   _InterlockedExchange (07FFB16C12270h)  
2A1303E0 mov  w0,w19  
A8C17BFD ldp  fp,lr,[sp],#0x10  
F84107F3 ldr  x19,[sp],#0x10  
D50323FF autibsp  
D65F03C0 ret  

........ .......
........ ...  ................
........ ...  ..................
........ ...  .....
........ .... .......................................... (07FFA2A2D0000h)
........ ...  ........#0xBB8.
........ ...  .....  
........ ...  ......#0xBB8  
........ ..   .................... (07FFA2A1B2270h)  
........ ...  ......  
........ ...  ................  
........ ...  ..............  
........ .......  
........ ...

........ .......  
........ ...  ................  
........ ...  ..................  
........ ...  .....  
........ .... .......................................... (07FFD7EF91000h)  
........ ...  ........#0xBF8.
........ ...  .....  
........ ...  ......#0xBF8  
........ ..   .................... (07FFD7EE72270h)  
........ ...  ......  
........ ...  ................  
........ ...  ..............  
........ .......  
........ ...

[SyntevoAlex]

It can be seen that only a few bytes in the function change. The rest can be verified.

[hmartinez82]

Ok. I'll add a check for the location of the InterlockExchange call (relative to the beginning of the function) instruction That, with the location of the adrp x8 instruction and the location of the ret instruction should be enough?

[SyntevoAlex]

I'll add a check for the location of the InterlockExchange call (relative to the beginning of the function)

Not sure what you mean.

To me, it makes sense to validate this part

ldr  w19,[x8,X1]
mov  w1,w0 
add  x0,x8,X2
bl   .................... ................  
mov  w0,w19

Where also X1 == X2

[hmartinez82]

@chirontt Can you give it a try with this latest change?

[chirontt]

@chirontt Can you give it a try with this latest change?

It's still working good, i.e. the scrollbars in WoA are still dark:

[SyntevoAlex]

In master now, thanks for your contribution. Today is the last day before the RC1 freeze, and the change only affects barely used arm64 platform, so it should be good.

[chirontt]

Thanks @SyntevoAlex , @hmartinez82 for this update. I hope this will keep things smooth for now, until MS decides to spoil the party again.

But I notice that tonight's I-build doesn't contain a recompile of the SWT natives due to this merge. @HannesWell does the automated recompile process require a version increment in order to trigger it?