Segmentation fault in new extension loading API

[ignatz]

Hi folks,

I was enjoying the recent addition of extension loading. Everything worked smoothly with the simple "sqlite/ext/misc/uuid.c".

However, when building in release mode, I suddenly get a segmentation fault. I played with the different build profiles to further narrow it down. Even when building with debug presets and just bumping opt-level from zero to one, does it for me.

Throwing it in the debugger leads to

Thread 1 "libsql" received signal SIGSEGV, Segmentation fault.
0x00005555558d91fc in sqlite3_db_config (db=0x555555bb9c38, op=<optimized out>) at bundled/src/sqlite3.c:180105
bt
180105              *pRes = (db->flags & aFlagOp[i].mask)!=0;
(gdb) bt
#0  0x00005555558d91fc in sqlite3_db_config (db=0x555555bb9c38, op=<optimized out>) at bundled/src/sqlite3.c:180105
#1  0x0000555555628518 in libsql::local::connection::Connection::enable_load_extension (self=<optimized out>, onoff=false) at vendor/libsql/libsql/src/local/connection.rs:296
#2  libsql::local::impls::{impl#4}::enable_load_extension (self=<optimized out>, onoff=false) at vendor/libsql/libsql/src/local/impls.rs:70
#3  0x0000555555603066 in libsql::connection::Connection::load_extension_enable (self=<optimized out>) at vendor/libsql/libsql/src/connection.rs:142
#4  0x00005555555ea19b in libsql::main::{async_block#0} () at src/libsql.rs:12

or the entire function in question

/*
** Configuration settings for an individual database connection
*/
SQLITE_API int sqlite3_db_config(sqlite3 *db, int op, ...){
  va_list ap;
  int rc;

#ifdef SQLITE_ENABLE_API_ARMOR
  if( !sqlite3SafetyCheckOk(db) ) return SQLITE_MISUSE_BKPT;
#endif
  sqlite3_mutex_enter(db->mutex);
  va_start(ap, op);
  switch( op ){
    case SQLITE_DBCONFIG_MAINDBNAME: {
      /* IMP: R-06824-28531 */
      /* IMP: R-36257-52125 */
      db->aDb[0].zDbSName = va_arg(ap,char*);
      rc = SQLITE_OK;
      break;
    }
    case SQLITE_DBCONFIG_LOOKASIDE: {
      void *pBuf = va_arg(ap, void*); /* IMP: R-26835-10964 */
      int sz = va_arg(ap, int);       /* IMP: R-47871-25994 */
      int cnt = va_arg(ap, int);      /* IMP: R-04460-53386 */
      rc = setupLookaside(db, pBuf, sz, cnt);
      break;
    }
    default: {
      static const struct {
        int op;      /* The opcode */
        u32 mask;    /* Mask of the bit in sqlite3.flags to set/clear */
      } aFlagOp[] = {
        { SQLITE_DBCONFIG_ENABLE_FKEY,           SQLITE_ForeignKeys    },
        { SQLITE_DBCONFIG_ENABLE_TRIGGER,        SQLITE_EnableTrigger  },
        { SQLITE_DBCONFIG_ENABLE_VIEW,           SQLITE_EnableView     },
        { SQLITE_DBCONFIG_ENABLE_FTS3_TOKENIZER, SQLITE_Fts3Tokenizer  },
        { SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION, SQLITE_LoadExtension  },
        { SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE,      SQLITE_NoCkptOnClose  },
        { SQLITE_DBCONFIG_ENABLE_QPSG,           SQLITE_EnableQPSG     },
        { SQLITE_DBCONFIG_TRIGGER_EQP,           SQLITE_TriggerEQP     },
        { SQLITE_DBCONFIG_RESET_DATABASE,        SQLITE_ResetDatabase  },
        { SQLITE_DBCONFIG_DEFENSIVE,             SQLITE_Defensive      },
        { SQLITE_DBCONFIG_WRITABLE_SCHEMA,       SQLITE_WriteSchema|
                                                 SQLITE_NoSchemaError  },
        { SQLITE_DBCONFIG_LEGACY_ALTER_TABLE,    SQLITE_LegacyAlter    },
        { SQLITE_DBCONFIG_DQS_DDL,               SQLITE_DqsDDL         },
        { SQLITE_DBCONFIG_DQS_DML,               SQLITE_DqsDML         },
        { SQLITE_DBCONFIG_LEGACY_FILE_FORMAT,    SQLITE_LegacyFileFmt  },
        { SQLITE_DBCONFIG_TRUSTED_SCHEMA,        SQLITE_TrustedSchema  },
        { SQLITE_DBCONFIG_STMT_SCANSTATUS,       SQLITE_StmtScanStatus },
        { SQLITE_DBCONFIG_REVERSE_SCANORDER,     SQLITE_ReverseOrder   },
      };
      unsigned int i;
      rc = SQLITE_ERROR; /* IMP: R-42790-23372 */
      for(i=0; i<ArraySize(aFlagOp); i++){
        if( aFlagOp[i].op==op ){
          int onoff = va_arg(ap, int);
          int *pRes = va_arg(ap, int*);
          u64 oldFlags = db->flags;
          if( onoff>0 ){
            db->flags |= aFlagOp[i].mask;
          }else if( onoff==0 ){
            db->flags &= ~(u64)aFlagOp[i].mask;
          }
          if( oldFlags!=db->flags ){
            sqlite3ExpirePreparedStatements(db, 0);
          }
          if( pRes ){
            *pRes = (db->flags & aFlagOp[i].mask)!=0;
          }
          rc = SQLITE_OK;
          break;
        }
      }
      break;
    }
  }
  va_end(ap);
  sqlite3_mutex_leave(db->mutex);
  return rc;
}

I haven't looked more into it yet, since it's getting late but maybe there's something that sticks out to you folks.

[ignatz]

Still haven't gotten around to spend much more time on this but at least I wanted to share a minimal reproduction with you: https://github.com/ignatz/libsql_ext

[sivukhin]

Hi @ignatz!
Thanks for the reproduction example - this really helps and simplify process of debugging the issue!

I guess I found an issue in libsql code but fix is rather simple (although this is pretty nasty bug, I guess): #1477

BTW, it seems to me that libsql is already build with SQLITE_ENABLE_LOAD_EXTENSION=1 (see https://github.com/tursodatabase/libsql/blob/main/libsql-ffi/build.rs#L115). So I guess you should be able to load your extensions even when bug in load_extension_enable/load_extension_disable is here.

[ignatz]

👏

Is there any particular reason why you folks rely on sqlite3_db_config, it seems you're bindgening also the type-safe bindings:

libsql/libsql-ffi/bundled/bindings/bindgen.rs

Line 2096 in fbce426

pub fn sqlite3_enable_load_extension(

?

[sivukhin]

sqlite3_enable_load_extension works bit differently as it enables both C API and SQL function load_extension while sqlite3_db_config allow libsql to have more fine grained control over enabled features.

But actually I don't have any context here, so maybe @MarinPostma can comment on this is more details.