diff --git a/.github/workflows/build-natives.yaml b/.github/workflows/build-natives.yaml index 06f816d..e625138 100644 --- a/.github/workflows/build-natives.yaml +++ b/.github/workflows/build-natives.yaml @@ -111,6 +111,31 @@ jobs: ls -la build/nativeLibs/win32-x86-64/ ls -la build/nativeLibs/win32-arm64/ + # Regression guard for issue #401: WinTray.dll must stay AVX-free so it does + # not crash with EXCEPTION_ILLEGAL_INSTRUCTION on pre-2011 CPUs (no AVX). + # AVX/AVX2 are VEX-encoded; their mnemonics start with 'v' (vpxor, vmovdqu, + # vzeroupper, vxorps, ...). No legacy x86-64 instruction MSVC emits for C code + # starts with 'v', so any such mnemonic in .text means AVX leaked in. + - name: Verify x64 WinTray.dll is AVX-free (issue #401) + shell: pwsh + run: | + $vswhere = "${env:ProgramFiles(x86)}\Microsoft Visual Studio\Installer\vswhere.exe" + $vsPath = & $vswhere -latest -products * -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationPath + if (-not $vsPath) { throw "Visual Studio installation not found" } + $dumpbin = Get-ChildItem -Path "$vsPath\VC\Tools\MSVC" -Recurse -Filter dumpbin.exe | + Where-Object { $_.FullName -match '\\Hostx64\\x64\\' } | Select-Object -First 1 + if (-not $dumpbin) { throw "dumpbin.exe not found" } + Write-Host "Using $($dumpbin.FullName)" + $disasm = & $dumpbin.FullName /DISASM:NOBYTES "build/nativeLibs/win32-x86-64/WinTray.dll" + $avx = $disasm | Select-String -Pattern '^\s+[0-9A-Fa-f]+:\s+v[a-z]' | + Where-Object { $_.Line -notmatch '\bv(err|erw)\b' } + if ($avx) { + Write-Host "AVX instructions found in WinTray.dll (x64):" + $avx | Select-Object -First 20 | ForEach-Object { Write-Host $_.Line.Trim() } + throw "WinTray.dll (x64) contains AVX instructions; it would crash on CPUs without AVX (issue #401). Do not add /arch:AVX* to CMakeLists." + } + Write-Host "OK: no AVX instructions found in x64 WinTray.dll" + - name: Upload Windows x64 library uses: actions/upload-artifact@v4 with: diff --git a/build.gradle.kts b/build.gradle.kts index 79eed11..df5d11e 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -52,6 +52,9 @@ kotlin { implementation(libs.nucleus.core.runtime) implementation(libs.nucleus.darkmode.detector) } + jvmTest.dependencies { + implementation(kotlin("test")) + } } } diff --git a/src/jvmMain/kotlin/com/kdroid/composetray/utils/JarResourceExtractor.kt b/src/jvmMain/kotlin/com/kdroid/composetray/utils/JarResourceExtractor.kt index 3a4246a..a13cd41 100644 --- a/src/jvmMain/kotlin/com/kdroid/composetray/utils/JarResourceExtractor.kt +++ b/src/jvmMain/kotlin/com/kdroid/composetray/utils/JarResourceExtractor.kt @@ -88,10 +88,10 @@ internal fun extractToTempIfDifferent(jarPath: String): File? { } // Extension to calculate SHA-256 of a file -private fun File.sha256(): String = inputStream().use { it.sha256() } +internal fun File.sha256(): String = inputStream().use { it.sha256() } // Extension to calculate SHA-256 of an InputStream -private fun InputStream.sha256(): String { +internal fun InputStream.sha256(): String { val digest = MessageDigest.getInstance("SHA-256") val buffer = ByteArray(1024) var bytesRead: Int diff --git a/src/jvmMain/kotlin/com/kdroid/composetray/utils/NativeLibraryLoader.kt b/src/jvmMain/kotlin/com/kdroid/composetray/utils/NativeLibraryLoader.kt index 42ff051..2225805 100644 --- a/src/jvmMain/kotlin/com/kdroid/composetray/utils/NativeLibraryLoader.kt +++ b/src/jvmMain/kotlin/com/kdroid/composetray/utils/NativeLibraryLoader.kt @@ -59,9 +59,13 @@ internal object NativeLibraryLoader { cacheDir.mkdirs() val cachedFile = File(cacheDir, fileName) - // Validate cache: re-extract if size differs or file is missing - val resourceSize = resourceUrl.openConnection().contentLengthLong - if (cachedFile.exists() && cachedFile.length() == resourceSize) { + // Validate the cache by content hash, not by size. Two builds of the same + // source can produce libraries of identical byte size but different code + // (e.g. the AVX vs SSE2 build of WinTray.dll — both 23552 bytes — see #401). + // A size-only check would keep serving a stale cached library after the user + // upgrades, so the original crash survives the fix. Compare hashes instead. + val resourceHash = resourceUrl.openStream().use { it.sha256() } + if (isCacheUpToDate(cachedFile, resourceHash)) { return cachedFile } @@ -80,6 +84,16 @@ internal object NativeLibraryLoader { return cachedFile } + /** + * A cached library is valid only if it exists and its content hash matches the + * resource currently on the classpath. Content-based (not size-based) so that an + * upgraded library replaces a same-sized stale one (see #401). + */ + internal fun isCacheUpToDate( + cachedFile: File, + expectedSha256: String, + ): Boolean = cachedFile.exists() && cachedFile.sha256() == expectedSha256 + private fun resolveCacheDir(platform: String): File { val os = System.getProperty("os.name")?.lowercase() ?: "" val base = diff --git a/src/jvmTest/kotlin/com/kdroid/composetray/utils/NativeLibraryLoaderCacheTest.kt b/src/jvmTest/kotlin/com/kdroid/composetray/utils/NativeLibraryLoaderCacheTest.kt new file mode 100644 index 0000000..000a0e9 --- /dev/null +++ b/src/jvmTest/kotlin/com/kdroid/composetray/utils/NativeLibraryLoaderCacheTest.kt @@ -0,0 +1,58 @@ +package com.kdroid.composetray.utils + +import java.io.File +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * Regression test for issue #401. + * + * The AVX/SSE2 builds of WinTray.dll are byte-for-byte the same size (23552 bytes) + * but differ in content. The old loader validated its persistent cache by size only, + * so after upgrading the user kept running the stale AVX library and the crash + * survived the fix. The cache must be validated by content hash instead. + */ +class NativeLibraryLoaderCacheTest { + private fun tempFileWith(bytes: ByteArray): File = + File.createTempFile("composetray-cache-test", ".bin").apply { + deleteOnExit() + writeBytes(bytes) + } + + @Test + fun `stale cache of identical size but different content is rejected`() { + // Two payloads with the SAME size but DIFFERENT content — the exact + // shape of the AVX vs SSE2 WinTray.dll pair. + val cachedBytes = ByteArray(23552) { 0x01 } + val resourceBytes = ByteArray(23552) { 0x02 } + require(cachedBytes.size == resourceBytes.size) + + val cachedFile = tempFileWith(cachedBytes) + val resourceHash = resourceBytes.inputStream().use { it.sha256() } + + // A size-only check would have accepted this stale file; the hash check rejects it. + assertFalse( + NativeLibraryLoader.isCacheUpToDate(cachedFile, resourceHash), + "Same-size but different-content cache must be treated as stale and re-extracted", + ) + } + + @Test + fun `matching content is served from cache`() { + val bytes = ByteArray(23552) { (it % 256).toByte() } + val cachedFile = tempFileWith(bytes) + val resourceHash = bytes.inputStream().use { it.sha256() } + + assertTrue( + NativeLibraryLoader.isCacheUpToDate(cachedFile, resourceHash), + "Identical content must be reused from cache", + ) + } + + @Test + fun `missing cache file is not up to date`() { + val missing = File.createTempFile("composetray-cache-test", ".bin").apply { delete() } + assertFalse(NativeLibraryLoader.isCacheUpToDate(missing, "anyhash")) + } +} diff --git a/src/native/windows/CMakeLists.txt b/src/native/windows/CMakeLists.txt index a908e78..1428fce 100644 --- a/src/native/windows/CMakeLists.txt +++ b/src/native/windows/CMakeLists.txt @@ -17,6 +17,12 @@ endif() if(CMAKE_GENERATOR_PLATFORM STREQUAL "x64" OR CMAKE_GENERATOR_PLATFORM STREQUAL "") set(TARGET_ARCH "x64") set(OUTPUT_DIR "${BASE_OUTPUT_DIR}/win32-x86-64") + # Do NOT add /arch:AVX, /arch:AVX2 or higher here. The default x64 baseline is + # SSE2, which every x86-64 CPU supports. Enabling AVX makes MSVC emit + # VEX-encoded instructions (e.g. VPXOR) that raise EXCEPTION_ILLEGAL_INSTRUCTION + # on pre-2011 CPUs without AVX (Nehalem Xeon, Core 2 Duo, ...). See issue #401. + # The CI build disassembles the produced DLL and fails if any AVX instruction + # is found, so a reintroduced /arch flag will be caught. elseif(CMAKE_GENERATOR_PLATFORM STREQUAL "ARM64") set(TARGET_ARCH "ARM64") set(OUTPUT_DIR "${BASE_OUTPUT_DIR}/win32-arm64")