Marge Bot pushed to branch wip/marge_bot_batch_merge_job at Glasgow Haskell Compiler / GHC Commits: c9b8465c by Cheng Shao at 2025-10-20T10:16:00-04:00 wasm: workaround WebKit bug in dyld This patch works around a WebKit bug and allows dyld to run on WebKit based platforms as well. See added note for detailed explanation. Co-authored-by: Codex - - - - - 97a92a6d by Julian Ospald at 2025-10-20T12:50:34-04:00 Improve error handling in 'getPackageArchives' When the library dirs in the package conf files are not set up correctly, the JS linker will happily ignore such packages and not link against them, although they're part of the link plan. Fixes #26383 - - - - - 36ab7344 by Sven Tennie at 2025-10-20T12:50:34-04:00 Align coding style Improve readability by using the same style for all constructor calls in this function. - - - - - c4e6e0df by Sven Tennie at 2025-10-20T12:50:34-04:00 Reduce complexity by removing joins with mempty ldArgs, cArgs and cppArgs are all `mempty`. Thus concatenating them adds nothing but some complexity while reading the code. - - - - - 8 changed files: - compiler/GHC/StgToJS/Linker/Linker.hs - hadrian/src/Rules/Gmp.hs - hadrian/src/Rules/Libffi.hs - hadrian/src/Settings/Builders/Cabal.hs - hadrian/src/Settings/Builders/Common.hs - hadrian/src/Settings/Builders/DeriveConstants.hs - hadrian/src/Settings/Builders/Hsc2Hs.hs - utils/jsffi/dyld.mjs Changes: ===================================== compiler/GHC/StgToJS/Linker/Linker.hs ===================================== @@ -2,6 +2,7 @@ {-# LANGUAGE TupleSections #-} {-# LANGUAGE LambdaCase #-} {-# LANGUAGE BlockArguments #-} +{-# LANGUAGE MultiWayIf #-} ----------------------------------------------------------------------------- -- | @@ -666,12 +667,19 @@ renderLinkerStats s = getPackageArchives :: StgToJSConfig -> UnitEnv -> [UnitId] -> IO [FilePath] -getPackageArchives cfg unit_env units = - filterM doesFileExist [ ST.unpack p "lib" ++ ST.unpack l ++ profSuff <.> "a" - | u <- units - , p <- getInstalledPackageLibDirs ue_state u - , l <- getInstalledPackageHsLibs ue_state u - ] +getPackageArchives cfg unit_env units = do + fmap concat $ forM units $ \u -> do + let archives = [ ST.unpack p "lib" ++ ST.unpack l ++ profSuff <.> "a" + | p <- getInstalledPackageLibDirs ue_state u + , l <- getInstalledPackageHsLibs ue_state u + ] + foundArchives <- filterM doesFileExist archives + if | not (null archives) + , null foundArchives + -> do + throwGhcExceptionIO (InstallationError $ "Could not find any library archives for unit-id: " <> (renderWithContext (csContext cfg) $ ppr u)) + | otherwise + -> pure foundArchives where ue_state = ue_homeUnitState unit_env ===================================== hadrian/src/Rules/Gmp.hs ===================================== @@ -11,7 +11,7 @@ import Target import Utilities import Hadrian.BuildPath import Hadrian.Expression -import Settings.Builders.Common (cArgs, getStagedCCFlags) +import Settings.Builders.Common (getStagedCCFlags) -- | Build in-tree GMP library objects (if GmpInTree flag is set) and return -- their paths. @@ -125,8 +125,7 @@ gmpRules = do cFlags <- interpretInContext ctx $ mconcat - [ cArgs - , getStagedCCFlags + [ getStagedCCFlags -- gmp symbols are only used by bignum logic in -- ghc-internal and shouldn't be exported by the -- ghc-internal shared library. ===================================== hadrian/src/Rules/Libffi.hs ===================================== @@ -130,17 +130,14 @@ fixLibffiMakefile top = configureEnvironment :: Stage -> Action [CmdOption] configureEnvironment stage = do context <- libffiContext stage - cFlags <- interpretInContext context $ mconcat - [ cArgs - , getStagedCCFlags ] - ldFlags <- interpretInContext context ldArgs + cFlags <- interpretInContext context getStagedCCFlags sequence [ builderEnvironment "CC" $ Cc CompileC stage , builderEnvironment "CXX" $ Cc CompileC stage - , builderEnvironment "AR" (Ar Unpack stage) + , builderEnvironment "AR" $ Ar Unpack stage , builderEnvironment "NM" Nm , builderEnvironment "RANLIB" Ranlib , return . AddEnv "CFLAGS" $ unwords cFlags ++ " -w" - , return . AddEnv "LDFLAGS" $ unwords ldFlags ++ " -w" ] + , return . AddEnv "LDFLAGS" $ "-w" ] -- Need the libffi archive and `trackAllow` all files in the build directory. -- See [Libffi indicating inputs]. ===================================== hadrian/src/Settings/Builders/Cabal.hs ===================================== @@ -188,18 +188,16 @@ configureArgs cFlags' ldFlags' = do values <- unwords <$> expr not (null values) ? arg ("--configure-option=" ++ key ++ "=" ++ values) - cFlags = mconcat [ remove ["-Werror"] cArgs - , getStagedCCFlags + cFlags = mconcat [ getStagedCCFlags -- See https://github.com/snowleopard/hadrian/issues/523 , arg $ "-iquote" , arg $ top -/- pkgPath pkg , cFlags' ] - ldFlags = ldArgs <> ldFlags' mconcat [ conf "CFLAGS" cFlags - , conf "LDFLAGS" ldFlags + , conf "LDFLAGS" ldFlags' , conf "--with-iconv-includes" $ arg =<< getSetting IconvIncludeDir , conf "--with-iconv-libraries" $ arg =<< getSetting IconvLibDir , conf "--with-gmp-includes" $ arg =<< getSetting GmpIncludeDir ===================================== hadrian/src/Settings/Builders/Common.hs ===================================== @@ -5,7 +5,7 @@ module Settings.Builders.Common ( module Oracles.Setting, module Settings, module UserSettings, - cIncludeArgs, ldArgs, cArgs, cppArgs, cWarnings, + cIncludeArgs, cWarnings, packageDatabaseArgs, bootPackageDatabaseArgs, getStagedCCFlags, wayCcArgs ) where @@ -38,15 +38,6 @@ cIncludeArgs = do , pure [ "-I" ++ pkgPath pkg -/- dir | dir <- incDirs ] , pure [ "-I" ++ unifyPath dir | dir <- depDirs ] ] -ldArgs :: Args -ldArgs = mempty - -cArgs :: Args -cArgs = mempty - -cppArgs :: Args -cppArgs = mempty - -- TODO: should be in a different file cWarnings :: Args cWarnings = mconcat ===================================== hadrian/src/Settings/Builders/DeriveConstants.hs ===================================== @@ -40,8 +40,7 @@ includeCcArgs :: Args includeCcArgs = do stage <- getStage rtsPath <- expr $ rtsBuildPath stage - mconcat [ cArgs - , cWarnings + mconcat [ cWarnings , prgFlags . ccProgram . tgtCCompiler <$> expr (targetStage Stage1) , queryTargetTarget tgtUnregisterised ? arg "-DUSE_MINIINTERPRETER" , arg "-Irts" ===================================== hadrian/src/Settings/Builders/Hsc2Hs.hs ===================================== @@ -50,7 +50,7 @@ getCFlags = do autogen <- expr $ autogenPath context let cabalMacros = autogen -/- "cabal_macros.h" expr $ need [cabalMacros] - mconcat [ remove ["-O"] (cArgs <> getStagedCCFlags) + mconcat [ remove ["-O"] getStagedCCFlags -- Either "-E" is not part of the configured cpp args, or we can't add those args to invocations of things like this -- ROMES:TODO: , prgFlags . cppProgram . tgtCPreprocessor <$> getStagedTargetConfig , cIncludeArgs @@ -64,6 +64,5 @@ getCFlags = do getLFlags :: Expr [String] getLFlags = mconcat [ prgFlags . ccLinkProgram . tgtCCompilerLink <$> getStagedTarget - , ldArgs , getContextData ldOpts , getContextData depLdOpts ] ===================================== utils/jsffi/dyld.mjs ===================================== @@ -670,7 +670,25 @@ class DyLD { // Wasm memory & table #memory = new WebAssembly.Memory({ initial: 1 }); + #table = new WebAssembly.Table({ element: "anyfunc", initial: 1 }); + // First free slot, might be invalid when it advances to #table.length + #tableFree = 1; + // See Note [The evil wasm table grower] + #tableGrowInstance = new WebAssembly.Instance( + new WebAssembly.Module( + new Uint8Array([ + 0, 97, 115, 109, 1, 0, 0, 0, 1, 6, 1, 96, 1, 127, 1, 127, 2, 35, 1, 3, + 101, 110, 118, 25, 95, 95, 105, 110, 100, 105, 114, 101, 99, 116, 95, + 102, 117, 110, 99, 116, 105, 111, 110, 95, 116, 97, 98, 108, 101, 1, + 112, 0, 0, 3, 2, 1, 0, 7, 31, 1, 27, 95, 95, 103, 104, 99, 95, 119, 97, + 115, 109, 95, 106, 115, 102, 102, 105, 95, 116, 97, 98, 108, 101, 95, + 103, 114, 111, 119, 0, 0, 10, 11, 1, 9, 0, 208, 112, 32, 0, 252, 15, 0, + 11, + ]) + ), + { env: { __indirect_function_table: this.#table } } + ); // __stack_pointer #sp = new WebAssembly.Global( @@ -715,6 +733,82 @@ class DyLD { // Global STG registers #regs = {}; + // Note [The evil wasm table grower] + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // We need to grow the wasm table as we load shared libraries in + // wasm dyld. We used to directly call the table.grow() JS API, + // which works as expected in Firefox/Chrome, but unfortunately, + // WebKit's implementation of the table.grow() JS API is broken: + // https://bugs.webkit.org/show_bug.cgi?id=290681, which means that + // the wasm dyld simply does not work in WebKit-based browsers like + // Safari. + // + // Now, one simple workaround would be to avoid growing the table at + // all: just allocate a huge table upfront (current limitation + // agreed by all vendors is 10000000). To avoid unnecessary space + // waste on non-WebKit platforms, we could additionally check + // navigator.userAgent against some regexes and only allocate + // fixed-length table when there's no blink/gecko mention. But this + // is fragile and gross, and it's better to stick to a uniform code + // path for all browsers. + // + // Fortunately, it turns out the table.grow wasm instruction work as + // expected in WebKit! So we can invoke a wasm function that grows + // the table for us. But don't open a champagne yet, where would + // that wasm function come from? It can't be put into RTS, or even + // libc.so, because loading those libraries would require growing + // the table in the first place! Or perhaps, reserve a table upfront + // that's just large enough to load RTS and then we can access that + // function for subsequent table grows? But then we need to + // experiment for a reasonable initial size, and add a magic number + // here, which is also fragile and gross and not future-proof! + // + // So this special wasm function needs to live in a single wasm + // module, which is loaded before we load anything else. The full + // source code for this module is: + // + // (module + // (type (func (param i32) (result i32))) + // (import "env" "__indirect_function_table" (table 0 funcref)) + // (export "__ghc_wasm_jsffi_table_grow" (func 0)) + // (func (type 0) (param i32) (result i32) + // ref.null func + // local.get 0 + // table.grow 0 + // ) + // ) + // + // This module is 103 bytes so that we can inline its blob in dyld, + // and use the usually discouraged synchronous + // WebAssembly.Instance/WebAssembly.Module constructors to load it. + // On non-WebKit platforms, growing tables this way would introduce + // a bit of extra JS/Wasm interop overhead, which can be amplified + // as we used to call table.grow(1, foo) for every GOT.func item. + // Therefore, unless we're about to exceed the hard limit of table + // size, we now grow the table exponentially, and use bump + // allocation to calculate the table index to be returned. + // Exponential growth is only implemented to minimize the JS/Wasm + // interop overhead when calling __ghc_wasm_jsffi_table_grow; + // V8/SpiderMonkey/WebKit already do their own exponential growth of + // the table's backing buffer in their table growth logic. + // + // Invariants: n >= 0; when v is non-null, n === 1 + #tableGrow(n, v) { + const prev_free = this.#tableFree; + if (prev_free + n > this.#table.length) { + const min_delta = prev_free + n - this.#table.length; + const delta = Math.max(min_delta, this.#table.length); + this.#tableGrowInstance.exports.__ghc_wasm_jsffi_table_grow( + this.#table.length + delta <= 10000000 ? delta : min_delta + ); + } + if (v) { + this.#table.set(prev_free, v); + } + this.#tableFree += n; + return prev_free; + } + constructor({ args, rpc }) { this.#rpc = rpc; @@ -878,7 +972,7 @@ class DyLD { // __memory_base & __table_base, different for each .so let memory_base; - let table_base = this.#table.grow(tableSize); + let table_base = this.#tableGrow(tableSize); console.assert(tableP2Align === 0); // libc.so is always the first one to be ever loaded and has VIP @@ -982,7 +1076,7 @@ class DyLD { if (this.exportFuncs[name]) { this.#gotFunc[name] = new WebAssembly.Global( { value: "i32", mutable: true }, - this.#table.grow(1, this.exportFuncs[name]) + this.#tableGrow(1, this.exportFuncs[name]) ); continue; } @@ -1033,7 +1127,7 @@ class DyLD { if (this.#gotFunc[k]) { const got = this.#gotFunc[k]; if (got.value === DyLD.#poison) { - const idx = this.#table.grow(1, v); + const idx = this.#tableGrow(1, v); got.value = idx; } else { this.#table.set(got.value, v); @@ -1103,7 +1197,7 @@ class DyLD { // Not in GOT.func yet, create the entry on demand if (this.exportFuncs[sym]) { console.assert(!this.#gotFunc[sym]); - const addr = this.#table.grow(1, this.exportFuncs[sym]); + const addr = this.#tableGrow(1, this.exportFuncs[sym]); this.#gotFunc[sym] = new WebAssembly.Global( { value: "i32", mutable: true }, addr View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/0d98f6fd494cbd34880c239af539a30... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/0d98f6fd494cbd34880c239af539a30... You're receiving this email because of your account on gitlab.haskell.org.