From 4ef01a1199e7478458d3ca1593978d932d3c2be8 Mon Sep 17 00:00:00 2001 From: Smitty Date: Tue, 13 Sep 2022 15:39:25 -0400 Subject: [PATCH 1/3] Fix undefined behaviour when loading control file --- cargo-pgx/src/command/schema.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cargo-pgx/src/command/schema.rs b/cargo-pgx/src/command/schema.rs index 77bbf0848..2c5f01d61 100644 --- a/cargo-pgx/src/command/schema.rs +++ b/cargo-pgx/src/command/schema.rs @@ -20,7 +20,7 @@ use object::Object; use once_cell::sync::OnceCell; use owo_colors::OwoColorize; use pgx_pg_config::{get_target_dir, PgConfig, Pgx}; -use pgx_utils::sql_entity_graph::{PgxSql, SqlGraphEntity}; +use pgx_utils::sql_entity_graph::{ControlFile, PgxSql, SqlGraphEntity}; use std::{ collections::HashSet, hash::{Hash, Hasher}, @@ -390,10 +390,14 @@ pub(crate) fn generate_schema( .expect(&format!("Couldn't call __pgx_sql_mappings")); sql_mapping = sql_mappings_symbol(); - let symbol: libloading::os::unix::Symbol SqlGraphEntity> = lib + let symbol: libloading::os::unix::Symbol< + unsafe extern "C" fn() -> eyre::Result, + > = lib .get("__pgx_marker".as_bytes()) .expect(&format!("Couldn't call __pgx_marker")); - let control_file_entity = symbol(); + let control_file_entity = SqlGraphEntity::ExtensionRoot( + symbol().expect("Failed to get control file information"), + ); entities.push(control_file_entity); for symbol_to_call in fns_to_call { From 75daa913eef5206316b93b7c444909602358216c Mon Sep 17 00:00:00 2001 From: Smitty Date: Wed, 14 Sep 2022 17:25:22 -0400 Subject: [PATCH 2/3] Use extern "Rust" when function types not FFI-safe Various functions used in schema generator were marked as extern "C" despite not having FFI-safe types (and not being useful to call from C code). --- cargo-pgx/src/command/schema.rs | 12 +++++++++--- pgx-utils/src/sql_entity_graph/aggregate/mod.rs | 2 +- pgx-utils/src/sql_entity_graph/extension_sql/mod.rs | 4 ++-- pgx-utils/src/sql_entity_graph/pg_extern/mod.rs | 2 +- pgx-utils/src/sql_entity_graph/pg_trigger/mod.rs | 2 +- pgx-utils/src/sql_entity_graph/postgres_enum/mod.rs | 2 +- pgx-utils/src/sql_entity_graph/postgres_hash/mod.rs | 2 +- pgx-utils/src/sql_entity_graph/postgres_ord/mod.rs | 2 +- pgx-utils/src/sql_entity_graph/postgres_type/mod.rs | 2 +- pgx-utils/src/sql_entity_graph/schema/mod.rs | 2 +- pgx/src/lib.rs | 6 ++++-- 11 files changed, 23 insertions(+), 15 deletions(-) diff --git a/cargo-pgx/src/command/schema.rs b/cargo-pgx/src/command/schema.rs index 2c5f01d61..50fe66117 100644 --- a/cargo-pgx/src/command/schema.rs +++ b/cargo-pgx/src/command/schema.rs @@ -367,7 +367,13 @@ pub(crate) fn generate_schema( let mut entities = Vec::default(); let sql_mapping; + #[rustfmt::skip] // explict extern "Rust" is more clear here unsafe { + // SAFETY: Calls foreign functions with the correct type signatures. + // Assumes that repr(Rust) enums are represented the same in this crate as in the external + // binary, which is the case in practice when the same compiler is used to compile the + // external crate. + POSTMASTER_LIBRARY .get_or_try_init(|| { libloading::os::unix::Library::open( @@ -384,14 +390,14 @@ pub(crate) fn generate_schema( .wrap_err_with(|| format!("Couldn't libload {}", lib_so.display()))?; let sql_mappings_symbol: libloading::os::unix::Symbol< - unsafe extern "C" fn() -> ::pgx_utils::sql_entity_graph::RustToSqlMapping, + unsafe extern "Rust" fn() -> ::pgx_utils::sql_entity_graph::RustToSqlMapping, > = lib .get("__pgx_sql_mappings".as_bytes()) .expect(&format!("Couldn't call __pgx_sql_mappings")); sql_mapping = sql_mappings_symbol(); let symbol: libloading::os::unix::Symbol< - unsafe extern "C" fn() -> eyre::Result, + unsafe extern "Rust" fn() -> eyre::Result, > = lib .get("__pgx_marker".as_bytes()) .expect(&format!("Couldn't call __pgx_marker")); @@ -401,7 +407,7 @@ pub(crate) fn generate_schema( entities.push(control_file_entity); for symbol_to_call in fns_to_call { - let symbol: libloading::os::unix::Symbol SqlGraphEntity> = + let symbol: libloading::os::unix::Symbol SqlGraphEntity> = lib.get(symbol_to_call.as_bytes()) .expect(&format!("Couldn't call {:#?}", symbol_to_call)); let entity = symbol(); diff --git a/pgx-utils/src/sql_entity_graph/aggregate/mod.rs b/pgx-utils/src/sql_entity_graph/aggregate/mod.rs index feceaedbe..19aae1b02 100644 --- a/pgx-utils/src/sql_entity_graph/aggregate/mod.rs +++ b/pgx-utils/src/sql_entity_graph/aggregate/mod.rs @@ -631,7 +631,7 @@ impl PgAggregate { let entity_item_fn: ItemFn = parse_quote! { #[no_mangle] #[doc(hidden)] - pub extern "C" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { + pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { let submission = ::pgx::utils::sql_entity_graph::PgAggregateEntity { full_path: ::core::any::type_name::<#target_ident>(), module_path: module_path!(), diff --git a/pgx-utils/src/sql_entity_graph/extension_sql/mod.rs b/pgx-utils/src/sql_entity_graph/extension_sql/mod.rs index f2d704972..ffa43c9ec 100644 --- a/pgx-utils/src/sql_entity_graph/extension_sql/mod.rs +++ b/pgx-utils/src/sql_entity_graph/extension_sql/mod.rs @@ -100,7 +100,7 @@ impl ToTokens for ExtensionSqlFile { let inv = quote! { #[no_mangle] #[doc(hidden)] - pub extern "C" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { + pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { extern crate alloc; use alloc::vec::Vec; use alloc::vec; @@ -207,7 +207,7 @@ impl ToTokens for ExtensionSql { ); let inv = quote! { #[no_mangle] - pub extern "C" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { + pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { extern crate alloc; use alloc::vec::Vec; use alloc::vec; diff --git a/pgx-utils/src/sql_entity_graph/pg_extern/mod.rs b/pgx-utils/src/sql_entity_graph/pg_extern/mod.rs index b4a2e357e..fd2631301 100644 --- a/pgx-utils/src/sql_entity_graph/pg_extern/mod.rs +++ b/pgx-utils/src/sql_entity_graph/pg_extern/mod.rs @@ -278,7 +278,7 @@ impl ToTokens for PgExtern { let inv = quote! { #[no_mangle] #[doc(hidden)] - pub extern "C" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { + pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { use core::any::TypeId; extern crate alloc; use alloc::vec::Vec; diff --git a/pgx-utils/src/sql_entity_graph/pg_trigger/mod.rs b/pgx-utils/src/sql_entity_graph/pg_trigger/mod.rs index beb08ded1..08da00996 100644 --- a/pgx-utils/src/sql_entity_graph/pg_trigger/mod.rs +++ b/pgx-utils/src/sql_entity_graph/pg_trigger/mod.rs @@ -66,7 +66,7 @@ impl PgTrigger { let tokens = quote! { #[no_mangle] #[doc(hidden)] - pub extern "C" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { + pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { use core::any::TypeId; extern crate alloc; use alloc::vec::Vec; diff --git a/pgx-utils/src/sql_entity_graph/postgres_enum/mod.rs b/pgx-utils/src/sql_entity_graph/postgres_enum/mod.rs index f4c7733f8..a4dbc6681 100644 --- a/pgx-utils/src/sql_entity_graph/postgres_enum/mod.rs +++ b/pgx-utils/src/sql_entity_graph/postgres_enum/mod.rs @@ -117,7 +117,7 @@ impl ToTokens for PostgresEnum { let inv = quote! { #[no_mangle] #[doc(hidden)] - pub extern "C" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { + pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { extern crate alloc; use alloc::vec::Vec; use alloc::vec; diff --git a/pgx-utils/src/sql_entity_graph/postgres_hash/mod.rs b/pgx-utils/src/sql_entity_graph/postgres_hash/mod.rs index 0bf3db40c..b00405e90 100644 --- a/pgx-utils/src/sql_entity_graph/postgres_hash/mod.rs +++ b/pgx-utils/src/sql_entity_graph/postgres_hash/mod.rs @@ -111,7 +111,7 @@ impl ToTokens for PostgresHash { let inv = quote! { #[no_mangle] #[doc(hidden)] - pub extern "C" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { + pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { use core::any::TypeId; extern crate alloc; use alloc::vec::Vec; diff --git a/pgx-utils/src/sql_entity_graph/postgres_ord/mod.rs b/pgx-utils/src/sql_entity_graph/postgres_ord/mod.rs index 1f8106043..fb5681ae9 100644 --- a/pgx-utils/src/sql_entity_graph/postgres_ord/mod.rs +++ b/pgx-utils/src/sql_entity_graph/postgres_ord/mod.rs @@ -111,7 +111,7 @@ impl ToTokens for PostgresOrd { let inv = quote! { #[no_mangle] #[doc(hidden)] - pub extern "C" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { + pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { use core::any::TypeId; extern crate alloc; use alloc::vec::Vec; diff --git a/pgx-utils/src/sql_entity_graph/postgres_type/mod.rs b/pgx-utils/src/sql_entity_graph/postgres_type/mod.rs index 386245484..27e2ee5ce 100644 --- a/pgx-utils/src/sql_entity_graph/postgres_type/mod.rs +++ b/pgx-utils/src/sql_entity_graph/postgres_type/mod.rs @@ -161,7 +161,7 @@ impl ToTokens for PostgresType { let inv = quote! { #[no_mangle] #[doc(hidden)] - pub extern "C" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { + pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { extern crate alloc; use alloc::vec::Vec; use alloc::vec; diff --git a/pgx-utils/src/sql_entity_graph/schema/mod.rs b/pgx-utils/src/sql_entity_graph/schema/mod.rs index 8a040b8ae..8bb8f8316 100644 --- a/pgx-utils/src/sql_entity_graph/schema/mod.rs +++ b/pgx-utils/src/sql_entity_graph/schema/mod.rs @@ -75,7 +75,7 @@ impl ToTokens for Schema { updated_content.push(syn::parse_quote! { #[no_mangle] #[doc(hidden)] - pub extern "C" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { + pub extern "Rust" fn #sql_graph_entity_fn_name() -> ::pgx::utils::sql_entity_graph::SqlGraphEntity { extern crate alloc; use alloc::vec::Vec; use alloc::vec; diff --git a/pgx/src/lib.rs b/pgx/src/lib.rs index 2788ffe1a..6e68fb66d 100644 --- a/pgx/src/lib.rs +++ b/pgx/src/lib.rs @@ -411,7 +411,8 @@ macro_rules! pg_sql_graph_magic { () => { #[no_mangle] #[doc(hidden)] - pub extern "C" fn __pgx_sql_mappings() -> ::pgx::utils::sql_entity_graph::RustToSqlMapping { + #[rustfmt::skip] // explict extern "Rust" is more clear here + pub extern "Rust" fn __pgx_sql_mappings() -> ::pgx::utils::sql_entity_graph::RustToSqlMapping { ::pgx::utils::sql_entity_graph::RustToSqlMapping { rust_type_id_to_sql: ::pgx::DEFAULT_RUST_TYPE_ID_TO_SQL.clone(), rust_source_to_sql: ::pgx::DEFAULT_RUST_SOURCE_TO_SQL.clone(), @@ -423,7 +424,8 @@ macro_rules! pg_sql_graph_magic { // A marker which must exist in the root of the extension. #[no_mangle] #[doc(hidden)] - pub extern "C" fn __pgx_marker( + #[rustfmt::skip] // explict extern "Rust" is more clear here + pub extern "Rust" fn __pgx_marker( ) -> ::pgx::utils::__reexports::eyre::Result<::pgx::utils::sql_entity_graph::ControlFile> { use ::core::convert::TryFrom; use ::pgx::utils::__reexports::eyre::WrapErr; From 9ee567e35d70aa997f027eb1f4297b3ba77c3669 Mon Sep 17 00:00:00 2001 From: Smitty Date: Wed, 14 Sep 2022 16:46:24 -0400 Subject: [PATCH 3/3] Document requirement for compiler to match --- cargo-pgx/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cargo-pgx/README.md b/cargo-pgx/README.md index 73f4009ca..aeb5566a2 100644 --- a/cargo-pgx/README.md +++ b/cargo-pgx/README.md @@ -14,7 +14,7 @@ Install via crates.io: $ cargo install --locked cargo-pgx ``` -As new versions of `pgx` are released, you'll want to make sure you run this command again to update it. +As new versions of `pgx` are released, you'll want to make sure you run this command again to update it. You should also reinstall `cargo-pgx` whenever you update `rustc` so that the same compiler is used to build `cargo-pgx` and your Postgres extensions. You can force `cargo` to reinstall an existing crate by passing `--force`. ## Usage