From 30f50a2d34a0859cbd7d9424bd53eb6cd1306c3a Mon Sep 17 00:00:00 2001 From: stephan Date: Tue, 13 Dec 2022 08:25:28 +0000 Subject: [PATCH] Extend the sqlite3.wasm function pointer argument converter to be able to handle the "two-layered context" of sqlite3_create_collation() and friends and make use of FuncPtrAdapter to perform JS-to-WASM function conversion for them. FossilOrigin-Name: 0a60b7215e433f8c50027c70731b11e58d74c90ec5903e66ae42f9c98e40b044 --- ext/wasm/api/sqlite3-api-glue.js | 32 ++------ ext/wasm/api/sqlite3-api-oo1.js | 1 + ext/wasm/common/whwasmutil.js | 123 ++++++++++++++++++++----------- ext/wasm/tester1.c-pp.js | 43 ++++++++++- manifest | 21 +++--- manifest.uuid | 2 +- 6 files changed, 135 insertions(+), 87 deletions(-) diff --git a/ext/wasm/api/sqlite3-api-glue.js b/ext/wasm/api/sqlite3-api-glue.js index 91b42cd937..21dd34d136 100644 --- a/ext/wasm/api/sqlite3-api-glue.js +++ b/ext/wasm/api/sqlite3-api-glue.js @@ -462,22 +462,11 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ 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()); + ':'+wasm.cstrToJs(argv[1]).toLowerCase() }; const __ccv2 = wasm.xWrap( 'sqlite3_create_collation_v2', '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. + 'sqlite3*','string','int','*', new wasm.xWrap.FuncPtrAdapter({ /* int(*xCompare)(void*,int,const void*,int,const void*) */ name: 'xCompare', @@ -492,7 +481,7 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ bindScope: 'context', contextKey: __collationContextKey }) - } + ); /** Works exactly like C's sqlite3_create_collation_v2() except that: @@ -518,18 +507,9 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ ); } let rc, pfCompare, pfDestroy; - try{ - 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); + try{ + rc = __ccv2(pDb, zName, eTextRep, pArg, xCompare, xDestroy); }catch(e){ - if(pfCompare) wasm.uninstallFunction(pfCompare); - if(pfDestroy) wasm.uninstallFunction(pfDestroy); rc = util.sqlite3_wasm_db_error(pDb, e); } return rc; @@ -539,7 +519,7 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ return (5===arguments.length) ? capi.sqlite3_create_collation_v2(pDb,zName,eTextRep,pArg,xCompare,0) : __dbArgcMismatch(pDb, 'sqlite3_create_collation', 5); - } + }; }/*sqlite3_create_collation() and friends*/ diff --git a/ext/wasm/api/sqlite3-api-oo1.js b/ext/wasm/api/sqlite3-api-oo1.js index a593889a8e..d4191cf3fc 100644 --- a/ext/wasm/api/sqlite3-api-oo1.js +++ b/ext/wasm/api/sqlite3-api-oo1.js @@ -158,6 +158,7 @@ self.sqlite3ApiBootstrap.initializers.push(function(sqlite3){ let rc = capi.sqlite3_open_v2(fn, pPtr, oflags, vfsName || 0); pDb = wasm.peekPtr(pPtr); checkSqlite3Rc(pDb, rc); + capi.sqlite3_extended_result_codes(pDb, 1); if(flagsStr.indexOf('t')>=0){ capi.sqlite3_trace_v2(pDb, capi.SQLITE_TRACE_STMT, __dbTraceToConsole, 0); diff --git a/ext/wasm/common/whwasmutil.js b/ext/wasm/common/whwasmutil.js index 50f22c61e2..3d137eb008 100644 --- a/ext/wasm/common/whwasmutil.js +++ b/ext/wasm/common/whwasmutil.js @@ -1447,21 +1447,21 @@ self.WhWasmUtilInstaller = function(target){ 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. + is solely for debugging and error-reporting purposes. If not + provided, an empty string is assumed. - - signature: an function signature compatible with - jsFuncToWasm(). + - signature: a function signature string 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 + - '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 + - '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 @@ -1470,23 +1470,19 @@ self.WhWasmUtilInstaller = function(target){ 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. + - '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 + - contextKey (function): is 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 + still-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 @@ -1500,19 +1496,19 @@ self.WhWasmUtilInstaller = function(target){ 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. + part of that key because its pre-conversion data type might be + unpredictable. Similarly, care must be taken with C-string-type + arguments: those to the left in argv will, when this is called, + be WASM pointers, whereas those to the right might (and likely + do) have another data type. When using C-strings in keys, never + use their pointers in the key because most C-strings in this + constellation are transient. + + Yes, that ^^^ is a bit awkward, but it's what we have. The constructor only saves the above state for later, and does - not actually bind any functions. Its convertArg() methor is + not actually bind any functions. Its convertArg() method 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 = function ctor(opt) { if(!(this instanceof xArg.FuncPtrAdapter)){ @@ -1530,20 +1526,21 @@ self.WhWasmUtilInstaller = function(target){ 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; - } + if( ('singleton'===this.bindScope) ) this.singleton = []; + else this.singleton = undefined; //console.warn("FuncPtrAdapter()",opt,this); }; xArg.FuncPtrAdapter.bindScopes = [ 'transient', 'context', 'singleton' ]; xArg.FuncPtrAdapter.prototype = { + /* Dummy impl. Overwritten per-instance as needed. */ contextKey: function(argIndex,argv){ return this; }, + /* Returns this objects mapping for the given context key, in the + form of an an array, creating the mapping if needed. The key + may be anything suitable for use in a Map. */ contextMap: function(key){ const cm = (this.__cmap || (this.__cmap = new Map)); let rc = cm.get(key); @@ -1553,11 +1550,28 @@ self.WhWasmUtilInstaller = function(target){ /** 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 - 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. + (v||0) is returned and any earlier function installed by this + mapping _might_, depending on how it's bound, be + uninstalled. If v is not one of those types, it must be a + Function, for which it creates (if needed) a WASM function + binding and returns the WASM pointer to that binding. If this + instance is not in 'transient' mode, it will remember the + binding for at least the next call, to avoid recreating the + function binding unnecessarily. + + If it's passed a pointer(ish) value for v, it does _not_ + perform any function binding, so this object's bindMode is + irrelevant for such cases. + + argIndex is the argv index of _this_ argument in the + being-xWrap()'d call. argv is the current argument list + undergoing xWrap() argument conversion. argv entries to the + left of argIndex will have already undergone transformation and + those to the right will not have (they will have the values the + client-level code passed in, awaiting conversion). The RHS + indexes must never be relied upon for anything because their + types are indeterminate, whereas the LHS values will be + WASM-compatible values by the time this is called. */ convertArg: function(v,argIndex,argv){ //console.warn("FuncPtrAdapter.convertArg()",this.signature,this.transient,v); @@ -1569,6 +1583,7 @@ self.WhWasmUtilInstaller = function(target){ if(v instanceof Function){ const fp = __installFunction(v, this.signature, this.isTransient); if(pair){ + /* Replace existing stashed mapping */ if(pair[1]){ try{target.uninstallFunction(pair[1])} catch(e){/*ignored*/} @@ -1578,7 +1593,9 @@ self.WhWasmUtilInstaller = function(target){ } return fp; }else if(target.isPtr(v) || null===v || undefined===v){ - if(pair && pair[1]){ + if(pair && pair[1] && pair[1]!==v){ + /* uninstall stashed mapping and replace stashed mapping with v. */ + //console.warn("FuncPtrAdapter is uninstalling function", this.contextKey(argIndex,argv),v); try{target.uninstallFunction(pair[1])} catch(e){/*ignored*/} pair[0] = pair[1] = (v || 0); @@ -1586,11 +1603,12 @@ self.WhWasmUtilInstaller = function(target){ return v || 0; }else{ throw new TypeError("Invalid FuncPtrAdapter argument type. "+ - "Expecting "+(this.name ? this.name+' ' : '')+ + "Expecting a function pointer or a "+ + (this.name ? this.name+' ' : '')+ "function matching signature "+ this.signature+"."); } - } + }/*convertArg()*/ }/*FuncPtrAdapter.prototype*/; const __xArgAdapterCheck = @@ -1778,6 +1796,21 @@ self.WhWasmUtilInstaller = function(target){ if(args.length!==xf.length) __argcMismatch(fname, xf.length); const scope = target.scopedAllocPush(); try{ + /* + Maintenance reminder re. arguments passed to convertArgs(): + The public interface of argument adapters is that they take + ONE argument and return a (possibly) converted result for + it. The passing-on of arguments after the first is an + internal impl. detail for the sake of FuncPtrAdapter, and + not to be relied on or documented for other cases. The fact + that this is how FuncPtrAdapter.convertArgs() gets its 2nd+ + arguments, and how FuncPtrAdapter.contextKey() gets its + args, is also an implementation detail and subject to + change. i.e. the public interface of 1 argument is stable. + The fact that any arguments may be passed in after that one, + and what those arguments are, is _not_ part of the public + interface and is _not_ stable. + */ for(const i in args) args[i] = cxw.convertArg(argTypes[i], args[i], i, args); return cxw.convertResult(resultType, xf.apply(null,args)); }finally{ diff --git a/ext/wasm/tester1.c-pp.js b/ext/wasm/tester1.c-pp.js index aa52d64079..e31ca370f7 100644 --- a/ext/wasm/tester1.c-pp.js +++ b/ext/wasm/tester1.c-pp.js @@ -2125,8 +2125,10 @@ self.sqlite3InitModule = sqlite3InitModule; })/*custom vtab #2*/ //////////////////////////////////////////////////////////////////////// .t('Custom collation', function(sqlite3){ + let collationCounter = 0; let myCmp = function(pArg,n1,p1,n2,p2){ //int (*)(void*,int,const void*,int,const void*) + ++collationCounter; const rc = wasm.exports.sqlite3_strnicmp(p1,p2,(n1this.db.checkRc(rc), /SQLITE_UTF8 is the only supported encoding./); - }) + /* + We need to ensure that replacing that collation function does + the right thing. We don't have a handle to the underlying WASM + pointer from here, so cannot verify (without digging through + internal state) that the old one gets uninstalled, but we can + verify that a new one properly replaces it. (That said, + console.warn() output has shown that the uninstallation does + happen.) + */ + collationCounter = 0; + myCmp = function(pArg,n1,p1,n2,p2){ + --collationCounter; + return 0; + }; + rc = capi.sqlite3_create_collation_v2(this.db, "MYCOLLATION", capi.SQLITE_UTF8, + 0, myCmp, 0); + this.db.checkRc(rc); + rc = this.db.selectValue("select 'hi' = 'HI' collate mycollation"); + T.assert(rc>0).assert(-1===collationCounter); + rc = this.db.selectValue("select 'a' = 'b' collate mycollation"); + T.assert(rc>0).assert(-2===collationCounter); + rc = capi.sqlite3_create_collation_v2(this.db, "MYCOLLATION", capi.SQLITE_UTF8, + 0, null, 0); + this.db.checkRc(rc); + rc = 0; + try { + this.db.selectValue("select 'a' = 'b' collate mycollation"); + }catch(e){ + /* Why is e.resultCode not automatically an extended result + code? The DB() class enables those automatically. */ + rc = sqlite3.capi.sqlite3_extended_errcode(this.db); + } + T.assert(capi.SQLITE_ERROR_MISSING_COLLSEQ === rc); + })/*custom collation*/ //////////////////////////////////////////////////////////////////////// .t('Close db', function(){ diff --git a/manifest b/manifest index 71b7ff6eeb..249c79cfce 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Further\sfix\sfor\sticket\s[57c47526c34f01e8].\s\sIf\sa\ssubquery\shas\sa\sresult\sset\ncolumn\sof\sthe\sform\s"CAST(expr\sAS\sNUMERIC)"\sdo\snot\sgive\sthat\scolumn\sNUMERIC\naffinity.\s\sNUMERIC\saffinity\salways\sgoes\sto\san\sinteger\sif\sable,\sbut\sa\sCAST\nto\snumeric\sis\sa\sno-op\sif\sthe\sinput\sis\sa\snumber.\s\sSo\sthe\stwo\sare\snot\sequivalent. -D 2022-12-12T21:22:23.884 +C Extend\sthe\ssqlite3.wasm\sfunction\spointer\sargument\sconverter\sto\sbe\sable\sto\shandle\sthe\s"two-layered\scontext"\sof\ssqlite3_create_collation()\sand\sfriends\sand\smake\suse\sof\sFuncPtrAdapter\sto\sperform\sJS-to-WASM\sfunction\sconversion\sfor\sthem. +D 2022-12-13T08:25:28.590 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -503,8 +503,8 @@ 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 1ff6deb11bd192c13cbd247e333a4739e2303aad3d1a9dc68defced2d7393375 -F ext/wasm/api/sqlite3-api-oo1.js 6d10849609231ccd46fa11b1d3fbbe0f45d9fe84c66a0b054601036540844300 +F ext/wasm/api/sqlite3-api-glue.js 247e3777c921134e479129b54883cf1090d57093b23487917a7b29e151f4def3 +F ext/wasm/api/sqlite3-api-oo1.js f31a3b44489a71b5937048e373eed4e2e2112aab027ee29a12ea7a6b901c7beb F ext/wasm/api/sqlite3-api-prologue.js 39fbca8f25219c218d631433828ede53be8d518aa9f0da480758a3ea8abc1be8 F ext/wasm/api/sqlite3-api-worker1.js e94ba98e44afccfa482874cd9acb325883ade50ed1f9f9526beb9de1711f182f F ext/wasm/api/sqlite3-license-version-header.js a661182fc93fc2cf212dfd0b987f8e138a3ac98f850b1112e29b5fbdaecc87c3 @@ -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 85bdcefc3ae4b550231da1f94b196e15458828d7652b1f61f86d20873ff915ed +F ext/wasm/common/whwasmutil.js b4548b24c10e9cba674f110809693e5eb5722b59b15a939377e2c380e9458168 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 ee609a41cc1aabc971a6514b5d1b155f5f15d092ee015f5d03a204880532e62d +F ext/wasm/tester1.c-pp.js 3cb7433b2494407e174d1f503a7b549189750284dd71a1b4006408fd26e882b2 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,9 +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 6cd21b79075367227b57bccf829cc7d4ccc7d7fbcfaed226b4c8e942ddae4eb6 ece07d091c2ef3367a914187e0b6512c1f2390b8c34844536ad50e88c7e8c2f2 -R 8755cdaac46d5859a86e42b45318d91f -T +closed ece07d091c2ef3367a914187e0b6512c1f2390b8c34844536ad50e88c7e8c2f2 -U drh -Z 1f0ab14c8d40a872f59260c7ef73d610 +P f0325359d5795237b79f90f21b42d7d942c7e918137cb0231d404365d3041e81 +R 087816cc7e61136b9adc39c7ee021fe3 +U stephan +Z df587f398df78172294e1bc583a38771 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index ac3b975236..888f5c5ca6 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -f0325359d5795237b79f90f21b42d7d942c7e918137cb0231d404365d3041e81 \ No newline at end of file +0a60b7215e433f8c50027c70731b11e58d74c90ec5903e66ae42f9c98e40b044 \ No newline at end of file