Skip to content

Commit

Permalink
Fix undefined behaviour when loading control file (#687)
Browse files Browse the repository at this point in the history 
* Fix undefined behaviour when loading control file
* 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).

* Document requirement for compiler to match
  • Loading branch information
@syvb
syvb authored Sep 14, 2022
1 parent 8d9a1c3 commit bb393a1
Show file tree
Hide file tree
Showing 12 changed files with 30 additions and 18 deletions.
2 changes: 1 addition & 1 deletion cargo-pgx/README.md
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit bb393a1

Please sign in to comment.