Merge pull request #2941 from weiznich/run_ci_with_asan_for_sqlite

Add a CI job to run sqlite tests using the address sanitizer

.github/workflows/ci.yml

@@ -367,15 +367,16 @@ jobs:
           args: --all -- --check
 
   sqlite_bundled:
-    name: Check sqlite bundled
+    name: Check sqlite bundled + Sqlite with asan
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
       - uses: actions-rs/toolchain@v1
         with:
-          toolchain: stable
+          toolchain: nightly
           profile: minimal
           override: true
+          components: rust-src
       - name: Cache cargo registry
         uses: actions/cache@v2
         with:
@@ -394,6 +395,20 @@ jobs:
           command: test
           args: --manifest-path diesel_cli/Cargo.toml --no-default-features --features "sqlite-bundled"
 
+      - name: Run diesel_tests with ASAN enabled
+        env:
+          RUSTFLAGS: -Zsanitizer=address
+          ASAN_OPTIONS: detect_stack_use_after_return=1
+        run: cargo -Z build-std test --manifest-path diesel_tests/Cargo.toml --no-default-features --features "sqlite libsqlite3-sys libsqlite3-sys/bundled libsqlite3-sys/with-asan" --target x86_64-unknown-linux-gnu
+
+      - name: Run diesel tests with ASAN enabled
+        env:
+          RUSTDOCFLAGS: -Zsanitizer=address
+          RUSTFLAGS: -Zsanitizer=address
+          ASAN_OPTIONS: detect_stack_use_after_return=1
+        run: cargo -Z build-std test --manifest-path diesel/Cargo.toml --no-default-features --features "sqlite extras libsqlite3-sys libsqlite3-sys/bundled libsqlite3-sys/with-asan" --target x86_64-unknown-linux-gnu
+
+
   minimal_rust_version:
     name: Check Minimal supported rust version (1.51.0)
     runs-on: ubuntu-latest

diesel/src/sqlite/connection/functions.rs

@@ -138,10 +138,10 @@ impl<'a> Drop for FunctionRow<'a> {
             if let PrivateSqliteRow::Duplicated { column_names, .. } =
                 DerefMut::deref_mut(RefCell::get_mut(args))
             {
-                if let Some(inner) = Rc::get_mut(column_names) {
+                if Rc::strong_count(column_names) == 1 {
                     // According the https://doc.rust-lang.org/std/mem/struct.ManuallyDrop.html#method.drop
                     // it's fine to just drop the values here
-                    unsafe { std::ptr::drop_in_place(inner as *mut _) }
+                    unsafe { std::ptr::drop_in_place(column_names as *mut _) }
                 }
             }
         }

diesel/src/sqlite/connection/raw.rs

@@ -113,7 +113,7 @@ impl RawConnection {
                 Some(run_custom_function::<F>),
                 None,
                 None,
-                Some(destroy_boxed::<F>),
+                Some(destroy_boxed::<CustomFunctionUserPtr<F>>),
             )
         };
 
@@ -172,13 +172,13 @@ impl RawConnection {
                 ffi::SQLITE_UTF8,
                 callback_fn as *mut _,
                 Some(run_collation_function::<F>),
-                Some(destroy_boxed::<F>),
+                Some(destroy_boxed::<CollationUserPtr<F>>),
             )
         };
 
         let result = Self::process_sql_function_result(result);
         if result.is_err() {
-            destroy_boxed::<F>(callback_fn as *mut _);
+            destroy_boxed::<CollationUserPtr<F>>(callback_fn as *mut _);
         }
         result
     }

diesel/src/sqlite/connection/sqlite_value.rs

@@ -86,7 +86,13 @@ impl<'a, 'b> SqliteValue<'a, 'b> {
         unsafe {
             let ptr = ffi::sqlite3_value_blob(self.value.as_ptr());
             let len = ffi::sqlite3_value_bytes(self.value.as_ptr());
-            slice::from_raw_parts(ptr as *const u8, len as usize)
+            if len == 0 {
+                // rusts std-lib has an debug_assert that prevents creating
+                // slices without elements from a pointer
+                &[]
+            } else {
+                slice::from_raw_parts(ptr as *const u8, len as usize)
+            }
         }
     }
 

diesel_tests/Cargo.toml

@@ -3,16 +3,10 @@ name = "diesel_tests"
 version = "0.1.0"
 authors = ["Sean Griffin <sean@seantheprogrammer.com>"]
 license = "MIT OR Apache-2.0"
-build = "build.rs"
 autotests = false
 autobenches = false
 edition = "2018"
 
-[build-dependencies]
-diesel = { path = "../diesel", default-features = false }
-diesel_migrations = { path = "../diesel_migrations" }
-dotenv = "0.15"
-
 [dependencies]
 assert_matches = "1.0.1"
 chrono = { version = "0.4.19", default-features = false, features = ["clock", "std"] }
@@ -25,6 +19,7 @@ serde_json = { version=">=0.9, <2.0" }
 ipnetwork = ">=0.12.2, <0.19.0"
 bigdecimal = ">= 0.0.13, < 0.4.0"
 rand = "0.7"
+libsqlite3-sys = { version = "0.23", optional = true }
 
 [features]
 default = []

diesel_tests/tests/schema/mod.rs

@@ -213,16 +213,44 @@ impl<'a> Drop for DropTable<'a> {
     }
 }
 
+#[cfg(feature = "sqlite")]
+const MIGRATIONS: diesel_migrations::EmbeddedMigrations =
+    diesel_migrations::embed_migrations!("../migrations/sqlite");
+
+#[cfg(feature = "postgres")]
+const MIGRATIONS: diesel_migrations::EmbeddedMigrations =
+    diesel_migrations::embed_migrations!("../migrations/postgresql");
+
+#[cfg(feature = "mysql")]
+const MIGRATIONS: diesel_migrations::EmbeddedMigrations =
+    diesel_migrations::embed_migrations!("../migrations/mysql");
+
 pub fn connection() -> TestConnection {
     let mut result = connection_without_transaction();
-    #[cfg(feature = "sqlite")]
-    result.execute("PRAGMA foreign_keys = ON").unwrap();
     result.begin_test_transaction().unwrap();
     result
 }
 
-#[cfg(feature = "postgres")]
 pub fn connection_without_transaction() -> TestConnection {
+    use diesel_migrations::MigrationHarness;
+    use std::sync::Once;
+    static MIGRATION_GUARD: Once = Once::new();
+
+    let mut result = backend_specific_connection();
+
+    if cfg!(feature = "sqlite") {
+        result.run_pending_migrations(MIGRATIONS).unwrap();
+    } else {
+        MIGRATION_GUARD.call_once(|| {
+            result.run_pending_migrations(MIGRATIONS).unwrap();
+        });
+    }
+
+    result
+}
+
+#[cfg(feature = "postgres")]
+pub fn backend_specific_connection() -> TestConnection {
     let connection_url = dotenv::var("PG_DATABASE_URL")
         .or_else(|_| dotenv::var("DATABASE_URL"))
         .expect("DATABASE_URL must be set in order to run tests");
@@ -236,19 +264,14 @@ pub fn connection_without_transaction() -> TestConnection {
 }
 
 #[cfg(feature = "sqlite")]
-const MIGRATIONS: diesel_migrations::EmbeddedMigrations =
-    diesel_migrations::embed_migrations!("../migrations/sqlite");
-
-#[cfg(feature = "sqlite")]
-pub fn connection_without_transaction() -> TestConnection {
-    use diesel_migrations::MigrationHarness;
-    let mut connection = SqliteConnection::establish(":memory:").unwrap();
-    connection.run_pending_migrations(MIGRATIONS).unwrap();
-    connection
+pub fn backend_specific_connection() -> TestConnection {
+    let mut conn = SqliteConnection::establish(":memory:").unwrap();
+    conn.execute("PRAGMA foreign_keys = ON").unwrap();
+    conn
 }
 
 #[cfg(feature = "mysql")]
-pub fn connection_without_transaction() -> TestConnection {
+pub fn backend_specific_connection() -> TestConnection {
     let connection_url = dotenv::var("MYSQL_DATABASE_URL")
         .or_else(|_| dotenv::var("DATABASE_URL"))
         .expect("DATABASE_URL must be set in order to run tests");