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
2025-07-29 08:01:23 +03:00 · 2022-12-13 08:25:28 +00:00
parent 7ca4312ff2
commit 30f50a2d34
6 changed files with 135 additions and 87 deletions
--- 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:
@ -519,17 +508,8 @@ 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);
+        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*/

--- 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);
--- a/ext/wasm/common/whwasmutil.js
+++ b/ext/wasm/common/whwasmutil.js
@ -1450,18 +1450,18 @@ self.WhWasmUtilInstaller = function(target){
       is solely for debugging and error-reporting purposes. If not
       provided, an empty string is assumed.

-     - signature: an function signature compatible with
+     - 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{
--- 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,(n1<n2?n1:n2));
        return rc ? rc : (n1 - n2);
      };
@ -2134,18 +2136,51 @@ self.sqlite3InitModule = sqlite3InitModule;
                                                0, myCmp, 0);
      this.db.checkRc(rc);
      rc = this.db.selectValue("select 'hi' = 'HI' collate mycollation");
-      T.assert(1===rc);
+      T.assert(1===rc).assert(1===collationCounter);
      rc = this.db.selectValue("select 'hii' = 'HI' collate mycollation");
-      T.assert(0===rc);
+      T.assert(0===rc).assert(2===collationCounter);
      rc = this.db.selectValue("select 'hi' = 'HIi' collate mycollation");
-      T.assert(0===rc);
+      T.assert(0===rc).assert(3===collationCounter);
      rc = capi.sqlite3_create_collation(this.db,"hi",capi.SQLITE_UTF8/*not enough args*/);
      T.assert(capi.SQLITE_MISUSE === rc);
      rc = capi.sqlite3_create_collation_v2(this.db,"hi",0/*wrong encoding*/,0,0,0);
      T.assert(capi.SQLITE_FORMAT === rc)
        .mustThrowMatching(()=>this.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(){
--- a/21
+++ b/21
@ -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.
--- a/manifest.uuid
+++ b/manifest.uuid
@ -1 +1 @@
-f0325359d5795237b79f90f21b42d7d942c7e918137cb0231d404365d3041e81
+0a60b7215e433f8c50027c70731b11e58d74c90ec5903e66ae42f9c98e40b044