From: stephan Date: Mon, 12 Dec 2022 18:42:39 +0000 (+0000) Subject: Revert part of [9386d6f63468] because the new automatic function pointer binding... X-Git-Tag: version-3.41.0~259 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=33ce0b3ff34530a47e928fbce053c76db83ab3d2;p=thirdparty%2Fsqlite.git Revert part of [9386d6f63468] because the new automatic function pointer binding cannot properly track per-context function mappings when the context is more complex than a single context-type pointer. e.g. it is fine for sqlite3_trace_v2() but it breaks down with sqlite3_create_collation() because that one needs to use the collation name as part of the context key and we cannot sensibly do so with the current code. FossilOrigin-Name: 6cd21b79075367227b57bccf829cc7d4ccc7d7fbcfaed226b4c8e942ddae4eb6 --- diff --git a/ext/wasm/api/sqlite3-api-glue.js b/ext/wasm/api/sqlite3-api-glue.js index da17fcb98e..91b42cd937 100644 --- a/ext/wasm/api/sqlite3-api-glue.js +++ b/ext/wasm/api/sqlite3-api-glue.js @@ -141,7 +141,8 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ "sqlite3*", "int", new wasm.xWrap.FuncPtrAdapter({ name: 'xProgressHandler', signature: 'i(p)', - bindMode: 'singleton' + bindScope: 'context', + contextKey: (argIndex,argv)=>'sqlite3@'+argv[0] }), "*" ] ], @@ -178,7 +179,12 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ "sqlite3*", "string", "string", "string", "**", "**", "*", "*", "*"], ["sqlite3_total_changes", "int", "sqlite3*"], - ["sqlite3_trace_v2", "int", "sqlite3*", "int", "*", "*"], + ["sqlite3_trace_v2", "int", "sqlite3*", "int", + new wasm.xWrap.FuncPtrAdapter({ + name: 'sqlite3_trace_v2::callback', + signature: 'i(ippp)', + contextKey: (argIndex, argv)=>'sqlite3@'+argv[0] + }), "*"], ["sqlite3_txn_state", "int", ["sqlite3*","string"]], /* Note that sqlite3_uri_...() have very specific requirements for their first C-string arguments, so we cannot perform any value @@ -454,27 +460,44 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ if(1){/* Bindings for sqlite3_create_collation() */ + const __collationContextKey = (argIndex,argv)=>{ + return 'argv['+argIndex+']:sqlite3@'+argv[0]+ + ':'+((/*THIS IS WRONG. We can't sensibly use a converted-to-C-string + address here and don't have access to the JS string (IF ANY) + which the user passed in.*/ + ''+argv[1] + ).toLowerCase()); + }; const __ccv2 = wasm.xWrap( 'sqlite3_create_collation_v2', 'int', - 'sqlite3*','string','int','*', + 'sqlite3*','string','int','*','*','*' + /* int(*xCompare)(void*,int,const void*,int,const void*) */ + /* void(*xDestroy(void*) */ + ); + if(0){ + // Problem: we cannot, due to xWrap() arg-passing limitations, + // currently easily/efficiently get a per-collation distinct + // key for purposes of creating distinct FuncPtrAdapter contexts. new wasm.xWrap.FuncPtrAdapter({ /* int(*xCompare)(void*,int,const void*,int,const void*) */ name: 'xCompare', signature: 'i(pipip)', - bindMode: 'static' + bindScope: 'context', + contextKey: __collationContextKey }), new wasm.xWrap.FuncPtrAdapter({ /* void(*xDestroy(void*) */ name: 'xDestroy', signature: 'v(p)', - bindMode: 'static' + bindScope: 'context', + contextKey: __collationContextKey }) - ); + } /** Works exactly like C's sqlite3_create_collation_v2() except that: - 1) It permits its two function arguments to be JS functions, + 1) It accepts JS functions for its function-pointer arguments, for which it will install WASM-bound proxies. The bindings are "permanent," in that they will stay in the WASM environment until it shuts down unless the client somehow finds and removes @@ -494,10 +517,19 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ pDb, capi.SQLITE_FORMAT, "SQLITE_UTF8 is the only supported encoding." ); } - let rc; + let rc, pfCompare, pfDestroy; try{ - rc = __ccv2(pDb, zName, eTextRep, pArg, xCompare, xDestroy); + if(xCompare instanceof Function){ + pfCompare = wasm.installFunction(xCompare, 'i(pipip)'); + } + if(xDestroy instanceof Function){ + pfDestroy = wasm.installFunction(xDestroy, 'v(p)'); + } + rc = __ccv2(pDb, zName, eTextRep, pArg, + pfCompare || xCompare, pfDestroy || xDestroy); }catch(e){ + if(pfCompare) wasm.uninstallFunction(pfCompare); + if(pfDestroy) wasm.uninstallFunction(pfDestroy); rc = util.sqlite3_wasm_db_error(pDb, e); } return rc; @@ -516,7 +548,7 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ ["sqlite3*", "string:flexible", new wasm.xWrap.FuncPtrAdapter({ signature: 'i(pipp)', - bindMode: 'transient' + bindScope: 'transient' }), "*", "**"]); /* Documented in the api object's initializer. */ capi.sqlite3_exec = function f(pDb, sql, callback, pVoid, pErrMsg){ diff --git a/ext/wasm/common/whwasmutil.js b/ext/wasm/common/whwasmutil.js index 0885db112a..50f22c61e2 100644 --- a/ext/wasm/common/whwasmutil.js +++ b/ext/wasm/common/whwasmutil.js @@ -1434,77 +1434,155 @@ self.WhWasmUtilInstaller = function(target){ }); } - const __FuncPtrBindModes = ['transient','static','singleton']; /** - EXPERIMENTAL. + EXPERIMENTAL! DO NOT USE IN CLIENT CODE! An attempt at adding function pointer conversion support to xWrap(). This type is recognized by xWrap() as a proxy for - converting a JS function to a C-side function, either permanently - or only for the duration of a a single call into the C layer. + converting a JS function to a C-side function, either + permanently, for the duration of a single call into the C layer, + or semi-contextual, where it may keep track of a single binding + for a given context and uninstall the binding if it's replaced. + + Requires an options object with these properties: + + - name (optional): string describing the function binding. This + is solely for debugging and error-reporting purposes. If not + provided, an empty string is assumed. + + - signature: an function signature compatible with + jsFuncToWasm(). + + - bindScope (string): one of ('transient', 'context', + 'singleton'). Bind scopes are: + + - transient: it will convert JS functions to WASM only for the + duration of the xWrap()'d function call, using + scopedInstallFunction(). Before that call returns, the + WASM-side binding will be uninstalled. + + - singleton: holds one function-pointer binding for this + instance. If it's called with a different function pointer, + it uninstalls the previous one after converting the new + value. This is only useful for use with "global" functions + which do not rely on any state other than this function + pointer. If the being-converted function pointer is intended + to be mapped to some sort of state object (e.g. an sqlite3*) + then "context" (see below) is the proper mode. + + - context: similar to singleton mode but for a given "context", + where the context is a key provided by the user and possibly + dependent on a small amount of call-time context. This mode + is the default if bindScope is _not_ set but a property named + contextKey (described below) is. + + FIXME: the contextKey definition is only useful for very basic + contexts and breaks down with dynamic ones like + sqlite3_create_collation(). + + - contextKey (function): only used if bindScope is not set or is + 'context'. This function gets passed (argIndex,argv), where + argIndex is the index of this function pointer in its + _wrapping_ function's arguments and argv is the _current_ + being-xWrap()-processed args array. All arguments to the left + of argIndex will have been processed by xWrap() by the time + this is called. argv[argIndex] will be the value the user + passed in to the xWrap()'d function for the argument this + FuncPtrAdapter is mapped to. Arguments to the right of + argv[argIndex] will not yet have been converted before this is + called. The function must return a key which uniquely + identifies this function mapping context for _this_ + FuncPtrAdapter instance (other instances are not considered), + taking into account that C functions often take some sort of + state object as one or more of their arguments. As an example, + if the xWrap()'d function takes `(int,T*,functionPtr,X*)` and + this FuncPtrAdapter is the argv[2]nd arg, contextKey(2,argv) + might return 'T@'+argv[1], or even just argv[1]. Note, + however, that the (X*) argument will not yet have been + processed by the time this is called and should not be used as + part of that key. Similarly, C-string-type keys should not be + used as part of keys because they are normally transient in + this environment. + + The constructor only saves the above state for later, and does + not actually bind any functions. Its convertArg() methor is + called via xWrap() to perform any bindings. + + Caveats: + + - singleton is globally singleton. This type does not currently + have enough context to apply, e.g., a different singleton for + each (sqlite3*) db handle. */ - xArg.FuncPtrAdapter = class { - /** - Requires an options object with these properties: - - - signature: an function signature compatible with - jsFuncToWasm(). - - - bindMode (string): one of ('transient', 'static', - 'singleton'). If 'transient', it uses - scopedInstallFunction() for its function bindings, meaning - they're limited to the lifetime of a single xWrap()-induced - function call. If it's 'static', the binding is permanent, - lasting the life of the WASM environment. If it's 'singleton' - then this function remembers the last function it installed - and uninstalls it before installing any replacements on - subsequent calls. If it's passed the exact same JS function - to install later, it will re-use the existing binding. - - - name (optional): string describing the function binding. This - is solely for debugging and error-reporting purposes. If not - provided, an empty string is assumed. - - The constructor only saves the above state for later, and does - not actually bind any functions. Its convertArg() methor is - called via xWrap() to perform any bindings. - */ - constructor(opt){ - this.signature = opt.signature; - if(__FuncPtrBindModes.indexOf(opt.bindMode)<0){ - toss("Invalid options.bindMode ("+opt.bindMod+") for FuncPtrAdapter. "+ - "Expecting one of: ("+__FuncPtrBindModes.join(', ')+')'); - } - this.bindMode = opt.bindMode; - this.name = opt.name || ''; - this.singleton = ('singleton'===this.bindMode) ? [] : undefined; - //console.warn("FuncPtrAdapter()",this.signature,this.transient); + xArg.FuncPtrAdapter = function ctor(opt) { + if(!(this instanceof xArg.FuncPtrAdapter)){ + toss("FuncPtrAdapter can only be used as a constructor. Use 'new'."); + } + this.signature = opt.signature; + if(!opt.bindScope && (opt.contextKey instanceof Function)){ + opt.bindScope = 'context'; + }else if(ctor.bindScopes.indexOf(opt.bindScope)<0){ + toss("Invalid options.bindScope ("+opt.bindMod+") for FuncPtrAdapter. "+ + "Expecting one of: ("+ctor.bindScopes.join(', ')+')'); + } + this.bindScope = opt.bindScope; + this.name = opt.name || ''; + if(opt.contextKey) this.contextKey = opt.contextKey /*else inherit one*/; + this.isTransient = 'transient'===this.bindScope; + this.isContext = 'context'===this.bindScope; + if( ('singleton'===this.bindScope) ){ + this.singleton = []; + }else{ + this.singleton = undefined; } + //console.warn("FuncPtrAdapter()",opt,this); + }; + xArg.FuncPtrAdapter.bindScopes = [ + 'transient', 'context', 'singleton' + ]; + xArg.FuncPtrAdapter.prototype = { + contextKey: function(argIndex,argv){ + return this; + }, + contextMap: function(key){ + const cm = (this.__cmap || (this.__cmap = new Map)); + let rc = cm.get(key); + if(undefined===rc) cm.set(key, (rc = [])); + return rc; + }, /** Gets called via xWrap() to "convert" v to a WASM-bound function pointer. If v is one of (a pointer, null, undefined) then (v||0) is returned, otherwise v must be a Function, for which - xit creates (if needed) a WASM function binding and returns the - WASM pointer to that binding. It throws if passed an invalid - type. + it creates (if needed) a WASM function binding and returns the + WASM pointer to that binding. It will remember the binding for + at least the next call, to avoid recreating the function + unnecessarily. */ - convertArg(v){ + convertArg: function(v,argIndex,argv){ //console.warn("FuncPtrAdapter.convertArg()",this.signature,this.transient,v); + let pair = this.singleton; + if(!pair && this.isContext){ + pair = this.contextMap(this.contextKey(argIndex, argv)); + } + if(pair && pair[0]===v) return pair[1]; if(v instanceof Function){ - if(this.singleton && this.singleton[0]===v){ - return this.singleton[1]; - } - const fp = __installFunction(v, this.signature, 'transient'===this.bindMode); - if(this.singleton){ - if(this.singleton[1]){ - try{target.uninstallFunction(this.singleton[1])} + const fp = __installFunction(v, this.signature, this.isTransient); + if(pair){ + if(pair[1]){ + try{target.uninstallFunction(pair[1])} catch(e){/*ignored*/} } - this.singleton[0] = v; - this.singleton[1] = fp; + pair[0] = v; + pair[1] = fp; } return fp; }else if(target.isPtr(v) || null===v || undefined===v){ + if(pair && pair[1]){ + try{target.uninstallFunction(pair[1])} + catch(e){/*ignored*/} + pair[0] = pair[1] = (v || 0); + } return v || 0; }else{ throw new TypeError("Invalid FuncPtrAdapter argument type. "+ @@ -1513,7 +1591,7 @@ self.WhWasmUtilInstaller = function(target){ this.signature+"."); } } - }; + }/*FuncPtrAdapter.prototype*/; const __xArgAdapterCheck = (t)=>xArg.get(t) || toss("Argument adapter not found:",t); @@ -1521,7 +1599,7 @@ self.WhWasmUtilInstaller = function(target){ const __xResultAdapterCheck = (t)=>xResult.get(t) || toss("Result adapter not found:",t); - cache.xWrap.convertArg = (t,v)=>__xArgAdapterCheck(t)(v); + cache.xWrap.convertArg = (t,...args)=>__xArgAdapterCheck(t)(...args); cache.xWrap.convertResult = (t,v)=>(null===t ? v : (t ? __xResultAdapterCheck(t)(v) : undefined)); @@ -1686,7 +1764,7 @@ self.WhWasmUtilInstaller = function(target){ /*Verify the arg type conversions are valid...*/; if(undefined!==resultType && null!==resultType) __xResultAdapterCheck(resultType); for(const t of argTypes){ - if(t instanceof xArg.FuncPtrAdapter) xArg.set(t, (v)=>t.convertArg(v)); + if(t instanceof xArg.FuncPtrAdapter) xArg.set(t, (...args)=>t.convertArg(...args)); else __xArgAdapterCheck(t); } const cxw = cache.xWrap; @@ -1700,7 +1778,7 @@ self.WhWasmUtilInstaller = function(target){ if(args.length!==xf.length) __argcMismatch(fname, xf.length); const scope = target.scopedAllocPush(); try{ - for(const i in args) args[i] = cxw.convertArg(argTypes[i], args[i]); + for(const i in args) args[i] = cxw.convertArg(argTypes[i], args[i], i, args); return cxw.convertResult(resultType, xf.apply(null,args)); }finally{ target.scopedAllocPop(scope); diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js index 92e8ec0521..aa52d64079 100644 --- a/ext/wasm/tester1.c-pp.js +++ b/ext/wasm/tester1.c-pp.js @@ -1788,7 +1788,7 @@ self.sqlite3InitModule = sqlite3InitModule; .assert(args[0] === 'testvtab') .assert(args[1] === 'main') .assert(args[2] === 'testvtab'); - console.debug("xConnect() args =",args); + //console.debug("xConnect() args =",args); const rc = capi.sqlite3_declare_vtab( pDb, "CREATE TABLE ignored(a,b)" ); diff --git a/manifest b/manifest index 037d4b5536..3e29f49192 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\snew\slogging\scode\sSQLITE_NOTICE_RBU\sand\suse\sit\swhen\slogging\sfor\sthe\sspurious\serror\sthat\sRBU\sinjects\sinto\sSQLite\sas\spart\sof\sapplying\san\supdate. -D 2022-12-12T17:33:36.415 +C Revert\spart\sof\s[9386d6f63468]\sbecause\sthe\snew\sautomatic\sfunction\spointer\sbinding\scannot\sproperly\strack\sper-context\sfunction\smappings\swhen\sthe\scontext\sis\smore\scomplex\sthan\sa\ssingle\scontext-type\spointer.\se.g.\sit\sis\sfine\sfor\ssqlite3_trace_v2()\sbut\sit\sbreaks\sdown\swith\ssqlite3_create_collation()\sbecause\sthat\sone\sneeds\sto\suse\sthe\scollation\sname\sas\spart\sof\sthe\scontext\skey\sand\swe\scannot\ssensibly\sdo\sso\swith\sthe\scurrent\scode. +D 2022-12-12T18:42:39.215 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -503,7 +503,7 @@ F ext/wasm/api/post-js-footer.js cd0a8ec768501d9bd45d325ab0442037fb0e33d1f3b4f08 F ext/wasm/api/post-js-header.js 47b6b281f39ad59fa6e8b658308cd98ea292c286a68407b35ff3ed9cfd281a62 F ext/wasm/api/pre-js.c-pp.js b88499dc303c21fc3f55f2c364a0f814f587b60a95784303881169f9e91c1d5f F ext/wasm/api/sqlite3-api-cleanup.js 680d5ccfff54459db136a49b2199d9f879c8405d9c99af1dda0cc5e7c29056f4 -F ext/wasm/api/sqlite3-api-glue.js 5c3854480b600195edd7a9c98a9ea4f07f59cc6abe7ab2f19d30cdaa62f24b78 +F ext/wasm/api/sqlite3-api-glue.js 1ff6deb11bd192c13cbd247e333a4739e2303aad3d1a9dc68defced2d7393375 F ext/wasm/api/sqlite3-api-oo1.js 6d10849609231ccd46fa11b1d3fbbe0f45d9fe84c66a0b054601036540844300 F ext/wasm/api/sqlite3-api-prologue.js 39fbca8f25219c218d631433828ede53be8d518aa9f0da480758a3ea8abc1be8 F ext/wasm/api/sqlite3-api-worker1.js e94ba98e44afccfa482874cd9acb325883ade50ed1f9f9526beb9de1711f182f @@ -521,7 +521,7 @@ F ext/wasm/c-pp.c 92285f7bce67ed7b7020b40fde8ed0982c442b63dc33df9dfd4b658d4a6c07 F ext/wasm/common/SqliteTestUtil.js d8bf97ecb0705a2299765c8fc9e11b1a5ac7f10988bbf375a6558b7ca287067b F ext/wasm/common/emscripten.css 11bd104b6c0d597c67d40cc8ecc0a60dae2b965151e3b6a37fa5708bac3acd15 F ext/wasm/common/testing.css 0ff15602a3ab2bad8aef2c3bd120c7ee3fd1c2054ad2ace7e214187ae68d926f -F ext/wasm/common/whwasmutil.js 60531749126c3d505521b9e9a0adb468951e3a201ca65fdc8a37d20a6c1194e0 +F ext/wasm/common/whwasmutil.js 85bdcefc3ae4b550231da1f94b196e15458828d7652b1f61f86d20873ff915ed F ext/wasm/demo-123-worker.html a0b58d9caef098a626a1a1db567076fca4245e8d60ba94557ede8684350a81ed F ext/wasm/demo-123.html 8c70a412ce386bd3796534257935eb1e3ea5c581e5d5aea0490b8232e570a508 F ext/wasm/demo-123.js ebae30756585bca655b4ab2553ec9236a87c23ad24fc8652115dcedb06d28df6 @@ -555,7 +555,7 @@ F ext/wasm/test-opfs-vfs.html 1f2d672f3f3fce810dfd48a8d56914aba22e45c6834e262555 F ext/wasm/test-opfs-vfs.js 44363db07b2a20e73b0eb1808de4400ca71b703af718d0fa6d962f15e73bf2ac F ext/wasm/tester1-worker.html d43f3c131d88f10d00aff3e328fed13c858d674ea2ff1ff90225506137f85aa9 F ext/wasm/tester1.c-pp.html d34bef3d48e5cbc1c7c06882ad240fec49bf88f5f65696cc2c72c416933aa406 -F ext/wasm/tester1.c-pp.js 6f7fcaeedff2c94fd9ebfd37a665cc7797524868b70fce2854ecab60b3d74274 +F ext/wasm/tester1.c-pp.js ee609a41cc1aabc971a6514b5d1b155f5f15d092ee015f5d03a204880532e62d F ext/wasm/tests/opfs/concurrency/index.html 86d8ac435074d1e7007b91105f4897f368c165e8cecb6a9aa3d81f5cf5dcbe70 F ext/wasm/tests/opfs/concurrency/test.js a98016113eaf71e81ddbf71655aa29b0fed9a8b79a3cdd3620d1658eb1cc9a5d F ext/wasm/tests/opfs/concurrency/worker.js 0a8c1a3e6ebb38aabbee24f122693f1fb29d599948915c76906681bb7da1d3d2 @@ -2067,8 +2067,8 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 9386d6f634680b4e0fa5487c34c63acb29f0b7a6ae738b8f6164ad084a229b62 -R 11d1a6998924a37a55370025a25b1ff2 -U dan -Z 8aa12dbc9239c3322fc6c9c4ad75ec00 +P cd881d35150be7f28cc1ca1eca0e950b5a039bef61190fcae4f944ef0e91f234 +R 27f9155fc78c491f877b292e106a06a7 +U stephan +Z cb8c8da215323ce40b195861e9820bf0 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 026995470e..f74c1059fe 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -cd881d35150be7f28cc1ca1eca0e950b5a039bef61190fcae4f944ef0e91f234 \ No newline at end of file +6cd21b79075367227b57bccf829cc7d4ccc7d7fbcfaed226b4c8e942ddae4eb6 \ No newline at end of file