Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix undefined behaviour when loading control file #687

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cargo-pgx/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
20 changes: 15 additions & 5 deletions cargo-pgx/src/command/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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(
Expand All @@ -384,20 +390,24 @@ 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() -> SqlGraphEntity> = lib
let symbol: libloading::os::unix::Symbol<
unsafe extern "Rust" fn() -> eyre::Result<ControlFile>,
> = 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 {
let symbol: libloading::os::unix::Symbol<unsafe extern "C" fn() -> SqlGraphEntity> =
let symbol: libloading::os::unix::Symbol<unsafe extern "Rust" fn() -> SqlGraphEntity> =
lib.get(symbol_to_call.as_bytes())
.expect(&format!("Couldn't call {:#?}", symbol_to_call));
let entity = symbol();
Expand Down
2 changes: 1 addition & 1 deletion pgx-utils/src/sql_entity_graph/aggregate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(),
Expand Down
4 changes: 2 additions & 2 deletions pgx-utils/src/sql_entity_graph/extension_sql/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion pgx-utils/src/sql_entity_graph/pg_extern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion pgx-utils/src/sql_entity_graph/pg_trigger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion pgx-utils/src/sql_entity_graph/postgres_enum/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion pgx-utils/src/sql_entity_graph/postgres_hash/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion pgx-utils/src/sql_entity_graph/postgres_ord/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion pgx-utils/src/sql_entity_graph/postgres_type/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion pgx-utils/src/sql_entity_graph/schema/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions pgx/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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;
Expand Down